Merge pull request #3157 from cremno/add-mrb_int_mul_overflow

add function for checked mrb_int multiplication

3 files changed, 45 insertions, 28 deletions

diff --git a/include/mruby/numeric.h b/include/mruby/numeric.h
index b16c299a4..e4086487e 100644
--- a/include/mruby/numeric.h
+++ b/include/mruby/numeric.h
@@ -62,6 +62,12 @@ mrb_int_sub_overflow(mrb_int minuend, mrb_int subtrahend, mrb_int *difference)
   return __builtin_sub_overflow(minuend, subtrahend, difference) || WBCHK(*difference);
 }
 
+static inline mrb_bool
+mrb_int_mul_overflow(mrb_int multiplier, mrb_int multiplicand, mrb_int *product)
+{
+  return __builtin_mul_overflow(multiplier, multiplicand, product) || WBCHK(*product);
+}
+
 #undef WBCHK
 
 #else
@@ -92,6 +98,35 @@ mrb_int_sub_overflow(mrb_int minuend, mrb_int subtrahend, mrb_int *difference)
   return !!(((x ^ z) & (~y ^ z)) & MRB_INT_OVERFLOW_MASK);
 }
 
+static inline mrb_bool
+mrb_int_mul_overflow(mrb_int multiplier, mrb_int multiplicand, mrb_int *product)
+{
+#if MRB_INT_BIT == 32
+  int64_t n = (int64_t)multiplier * multiplicand;
+  *product = (mrb_int)n;
+  return !FIXABLE(n);
+#else
+  if (multiplier > 0) {
+    if (multiplicand > 0) {
+      if (multiplier > MRB_INT_MAX / multiplicand) return TRUE;
+    }
+    else {
+      if (multiplicand < MRB_INT_MAX / multiplier) return TRUE;
+    }
+  }
+  else {
+    if (multiplicand > 0) {
+      if (multiplier < MRB_INT_MAX / multiplicand) return TRUE;
+    }
+    else {
+      if (multiplier != 0 && multiplicand < MRB_INT_MAX / multiplier) return TRUE;
+    }
+  }
+  *product = multiplier * multiplicand;
+  return FALSE;
+#endif
+}
+
 #undef MRB_INT_OVERFLOW_MASK
 #undef mrb_uint
 #undef MRB_UINT_MAKE
diff --git a/src/numeric.c b/src/numeric.c
index 7b49b29f7..85847284e 100644
--- a/src/numeric.c
+++ b/src/numeric.c
@@ -541,10 +541,6 @@ int_to_i(mrb_state *mrb, mrb_value num)
   return num;
 }
 
-/*tests if N*N would overflow*/
-#define SQRT_INT_MAX ((mrb_int)1<<((MRB_INT_BIT-1-MRB_FIXNUM_SHIFT)/2))
-#define FIT_SQRT_INT(n) (((n)<SQRT_INT_MAX)&&((n)>=-SQRT_INT_MAX))
-
 mrb_value
 mrb_fixnum_mul(mrb_state *mrb, mrb_value x, mrb_value y)
 {
@@ -552,18 +548,14 @@ mrb_fixnum_mul(mrb_state *mrb, mrb_value x, mrb_value y)
 
   a = mrb_fixnum(x);
   if (mrb_fixnum_p(y)) {
-    mrb_float c;
-    mrb_int b;
+    mrb_int b, c;
 
     if (a == 0) return x;
     b = mrb_fixnum(y);
-    if (FIT_SQRT_INT(a) && FIT_SQRT_INT(b))
-      return mrb_fixnum_value(a*b);
-    c = a * b;
-    if ((a != 0 && c/a != b) || !FIXABLE(c)) {
-      return mrb_float_value(mrb, (mrb_float)a*(mrb_float)b);
+    if (mrb_int_mul_overflow(a, b, &c)) {
+      return mrb_float_value(mrb, (mrb_float)a * (mrb_float)b);
     }
-    return mrb_fixnum_value((mrb_int)c);
+    return mrb_fixnum_value(c);
   }
   return mrb_float_value(mrb, (mrb_float)a * mrb_to_flo(mrb, y));
 }
diff --git a/src/vm.c b/src/vm.c
index fd2a9384c..6f8c5109f 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -1862,25 +1862,15 @@ RETRY_TRY_BLOCK:
       switch (TYPES2(mrb_type(regs[a]),mrb_type(regs[a+1]))) {
       case TYPES2(MRB_TT_FIXNUM,MRB_TT_FIXNUM):
         {
-          mrb_value z;
-
-          z = mrb_fixnum_mul(mrb, regs[a], regs[a+1]);
+          mrb_int x, y, z;
 
-          switch (mrb_type(z)) {
-          case MRB_TT_FIXNUM:
-            {
-              SET_INT_VALUE(regs[a], mrb_fixnum(z));
-            }
-            break;
-          case MRB_TT_FLOAT:
-            {
-              SET_FLOAT_VALUE(mrb, regs[a], mrb_float(z));
-            }
-            break;
-          default:
-            /* cannot happen */
+          x = mrb_fixnum(regs[a]);
+          y = mrb_fixnum(regs[a+1]);
+          if (mrb_int_mul_overflow(x, y, &z)) {
+            SET_FLOAT_VALUE(mrb, regs[a], (mrb_float)x * (mrb_float)y);
             break;
           }
+          SET_INT_VALUE(regs[a], z);
         }
         break;
       case TYPES2(MRB_TT_FIXNUM,MRB_TT_FLOAT):