Browse Source

Avoid integer overflow on fast 32-bit millisecond conversion.

Multiply-then-divide is more accurate, but it runs into trouble when
our input is above INT32_MAX/numerator.  So when our value is too
large, do divide-then-multiply instead.

Fixes part of bug 27139; bugfix on 0.3.4.1-alpha.
Nick Mathewson 5 years ago
parent
commit
f02e8b5944
1 changed files with 12 additions and 2 deletions
  1. 12 2
      src/common/compat_time.c

+ 12 - 2
src/common/compat_time.c

@@ -280,6 +280,7 @@ monotime_reset_ratchets_for_testing(void)
  */
 static struct mach_timebase_info mach_time_info;
 static struct mach_timebase_info mach_time_info_msec_cvt;
+static int32_t mach_time_msec_cvt_threshold;
 static int monotime_shift = 0;
 
 static void
@@ -304,6 +305,10 @@ monotime_init_internal(void)
     // denominator here to avoid multiple multiplies.
     mach_time_info_msec_cvt.numer = mach_time_info.numer * 2048;
     mach_time_info_msec_cvt.denom = mach_time_info.denom * 1953;
+    // For any value above this amount, we should divide before multiplying,
+    // to avoid overflow.  For a value below this, we should multiply
+    // before dividing, to improve accuracy.
+    mach_time_msec_cvt_threshold = INT32_MAX / mach_time_info_msec_cvt.numer;
   }
 }
 
@@ -366,8 +371,13 @@ monotime_coarse_diff_msec32_(const monotime_coarse_t *start,
   /* We already require in di_ops.c that right-shift performs a sign-extend. */
   const int32_t diff_microticks = (int32_t)(diff_ticks >> 20);
 
-  return (diff_microticks * mach_time_info_msec_cvt.numer) /
-    mach_time_info_msec_cvt.denom;
+  if (diff_microticks >= mach_time_msec_cvt_threshold) {
+    return (diff_microticks / mach_time_info_msec_cvt.denom) *
+      mach_time_info_msec_cvt.numer;
+  } else {
+    return (diff_microticks * mach_time_info_msec_cvt.numer) /
+      mach_time_info_msec_cvt.denom;
+  }
 }
 
 uint32_t