[Fix] 64bitシフト演算を関数化(未定義動作の回避、仕様の明確化)

従来の64bitシフト関数には以下のような問題があった:

* s64b_RSHIFT() は、(V1<<(32-(N))) の部分で未定義動作が起こりうる。
  例えば V1 != 0 && N == 1 のとき、V1 が 31 回左シフトされてオーバーフローする。
  現状では敵を殴るだけでこれが発生するため、ubsan を用いたデバッグがしにくい。
* シフト量 0 を渡すと 32 回のシフトが発生して未定義動作となる。
* マクロで実装されているため型チェックが甘く、assert なども入れにくい。
* 引数の範囲についての仕様が書かれていない。

非マクロ関数を設けてこれらを解決する。
シフト対象は64bit非負整数、シフト量は [0,31] という仕様にする。
既存コードのシフト関数呼び出しを見る限り、これで要求は満たしているはず。

diff --git a/src/core/hp-mp-regenerator.c b/src/core/hp-mp-regenerator.c
index 3599956..20f7629 100644 (file)
--- a/src/core/hp-mp-regenerator.c
+++ b/src/core/hp-mp-regenerator.c
@@ -37,7 +37,7 @@ void regenhp(player_type *creature_ptr, int percent)
      */
     HIT_POINT new_chp = 0;
     u32b new_chp_frac = (creature_ptr->mhp * percent + PY_REGEN_HPBASE);
-    s64b_LSHIFT(new_chp, new_chp_frac, 16);
+    s64b_lshift(&new_chp, &new_chp_frac, 16);
     s64b_add(&(creature_ptr->chp), &(creature_ptr->chp_frac), new_chp, new_chp_frac);
     if (0 < s64b_cmp(creature_ptr->chp, creature_ptr->chp_frac, creature_ptr->mhp, 0)) {
         creature_ptr->chp = creature_ptr->mhp;
@@ -69,7 +69,7 @@ void regenmana(player_type *creature_ptr, MANA_POINT upkeep_factor, MANA_POINT r
     if (creature_ptr->csp > creature_ptr->msp) {
         s32b decay = 0;
         u32b decay_frac = (creature_ptr->msp * 32 * PY_REGEN_NORMAL + PY_REGEN_MNBASE);
-        s64b_LSHIFT(decay, decay_frac, 16);
+        s64b_lshift(&decay, &decay_frac, 16);
         s64b_sub(&(creature_ptr->csp), &(creature_ptr->csp_frac), decay, decay_frac);
         if (creature_ptr->csp < creature_ptr->msp) {
             creature_ptr->csp = creature_ptr->msp;
@@ -81,7 +81,7 @@ void regenmana(player_type *creature_ptr, MANA_POINT upkeep_factor, MANA_POINT r
     else if (regen_rate > 0) {
         MANA_POINT new_mana = 0;
         u32b new_mana_frac = (creature_ptr->msp * regen_rate / 100 + PY_REGEN_MNBASE);
-        s64b_LSHIFT(new_mana, new_mana_frac, 16);
+        s64b_lshift(&new_mana, &new_mana_frac, 16);
         s64b_add(&(creature_ptr->csp), &(creature_ptr->csp_frac), new_mana, new_mana_frac);
         if (creature_ptr->csp >= creature_ptr->msp) {
             creature_ptr->csp = creature_ptr->msp;
@@ -93,7 +93,7 @@ void regenmana(player_type *creature_ptr, MANA_POINT upkeep_factor, MANA_POINT r
     if (regen_rate < 0) {
         s32b reduce_mana = 0;
         u32b reduce_mana_frac = (creature_ptr->msp * (-1) * regen_rate / 100 + PY_REGEN_MNBASE);
-        s64b_LSHIFT(reduce_mana, reduce_mana_frac, 16);
+        s64b_lshift(&reduce_mana, &reduce_mana_frac, 16);
         s64b_sub(&(creature_ptr->csp), &(creature_ptr->csp_frac), reduce_mana, reduce_mana_frac);
         if (creature_ptr->csp < 0) {
             creature_ptr->csp = 0;

diff --git a/src/core/player-processor.c b/src/core/player-processor.c
index 6a3c6a3..3b71298 100644 (file)
--- a/src/core/player-processor.c
+++ b/src/core/player-processor.c
@@ -217,7 +217,7 @@ void process_player(player_type *creature_ptr)
     if (creature_ptr->action == ACTION_LEARN) {
         s32b cost = 0L;
         u32b cost_frac = (creature_ptr->msp + 30L) * 256L;
-        s64b_LSHIFT(cost, cost_frac, 16);
+        s64b_lshift(&cost, &cost_frac, 16);
         if (s64b_cmp(creature_ptr->csp, creature_ptr->csp_frac, cost, cost_frac) < 0) {
             creature_ptr->csp = 0;
             creature_ptr->csp_frac = 0;

diff --git a/src/monster/monster-status.c b/src/monster/monster-status.c
index 7d0f77e..27bdfec 100644 (file)
--- a/src/monster/monster-status.c
+++ b/src/monster/monster-status.c
@@ -184,7 +184,7 @@ static void get_exp_from_mon(player_type *target_ptr, HIT_POINT dam, monster_typ
 
         while (monnum_penarty--) {
             /* Divide by 4 */
-            s64b_RSHIFT(new_exp, new_exp_frac, 2);
+            s64b_rshift(&new_exp, &new_exp_frac, 2);
         }
     }
 

diff --git a/src/spell-realm/spells-song.c b/src/spell-realm/spells-song.c
index 8083b3b..9eb548f 100644 (file)
--- a/src/spell-realm/spells-song.c
+++ b/src/spell-realm/spells-song.c
@@ -37,7 +37,7 @@ void check_music(player_type *caster_ptr)
     MANA_POINT need_mana = mod_need_mana(caster_ptr, s_ptr->smana, spell, REALM_MUSIC);
     u32b need_mana_frac = 0;
 
-    s64b_RSHIFT(need_mana, need_mana_frac, 1);
+    s64b_rshift(&need_mana, &need_mana_frac, 1);
     if (s64b_cmp(caster_ptr->csp, caster_ptr->csp_frac, need_mana, need_mana_frac) < 0) {
         stop_singing(caster_ptr);
         return;

diff --git a/src/term/z-util.c b/src/term/z-util.c
index d1c424d..4304dbd 100644 (file)
--- a/src/term/z-util.c
+++ b/src/term/z-util.c
@@ -142,6 +142,22 @@ void core(concptr str)
 
 /*** 64-bit integer operations ***/
 
+void s64b_lshift(s32b* hi, u32b* lo, const int n)
+{
+       if (n == 0) return;
+
+       *hi = (s32b)((u32b)(*hi << n) | (*lo >> (32 - n)));
+       *lo <<= n;
+}
+
+void s64b_rshift(s32b* hi, u32b* lo, const int n)
+{
+       if (n == 0) return;
+
+       *lo = ((u32b)*hi << (32 - n)) | (*lo >> n);
+       *hi >>= n;
+}
+
 /* Add B to A */
 void s64b_add(s32b *A1, u32b *A2, s32b B1, u32b B2)
 {
@@ -229,7 +245,7 @@ void s64b_div(s32b *A1, u32b *A2, s32b B1, u32b B2)
         */
        while (s64b_cmp(A1val, A2val, B1, B2) == 1)
        {
-               s64b_LSHIFT(B1, B2, 1);
+               s64b_lshift(&B1, &B2, 1);
                bit++;
        }
 
@@ -246,7 +262,7 @@ void s64b_div(s32b *A1, u32b *A2, s32b B1, u32b B2)
                        s64b_sub(&A1val, &A2val, B1, B2);
                }
        
-               s64b_RSHIFT(B1, B2, 1);
+               s64b_rshift(&B1, &B2, 1);
                bit--;
        }
 
@@ -323,4 +339,4 @@ int mysqrt(int n)
                }
        }
        return kaeriti;
-}
\ No newline at end of file
+}

diff --git a/src/term/z-util.h b/src/term/z-util.h
index 07bdab5..a8bb474 100644 (file)
--- a/src/term/z-util.h
+++ b/src/term/z-util.h
@@ -52,8 +52,27 @@ extern void core(concptr str);
 
 
 /* 64-bit integer operations */
-#define s64b_LSHIFT(V1, V2, N) {V1 = (V1<<(N)) | (V2>>(32-(N))); V2 <<= (N);}
-#define s64b_RSHIFT(V1, V2, N) {V2 = (V1<<(32-(N))) | (V2>>(N)); V1 >>= (N);}
+
+/*!
+ * @brief 64bit非負整数を n 回左シフトする。
+ * @param hi 上位32bit。負であってはならない。
+ * @param lo 下位32bit。
+ * @param n  シフト量。[0,31] の範囲でなければならない。
+ *
+ * hi や n に範囲外の値を渡したり、オーバーフローした場合の動作は未定義。
+ */
+void s64b_lshift(s32b* hi, u32b* lo, int n);
+
+/*!
+ * @brief 64bit非負整数を n 回右シフトする。
+ * @param hi 上位32bit。負であってはならない。
+ * @param lo 下位32bit。
+ * @param n シフト量。[0,31] の範囲でなければならない。
+ *
+ * hi や n に範囲外の値を渡した場合の動作は未定義。
+ */
+void s64b_rshift(s32b* hi, u32b* lo, int n);
+
 extern void s64b_add(s32b *A1, u32b *A2, s32b B1, u32b B2);
 extern void s64b_sub(s32b *A1, u32b *A2, s32b B1, u32b B2);
 extern int s64b_cmp(s32b A1, u32b A2, s32b B1, u32b B2);