diff --git a/changes/bug27139 b/changes/bug27139 new file mode 100644 index 0000000000..0d1e3b4329 --- /dev/null +++ b/changes/bug27139 @@ -0,0 +1,14 @@ + o Minor bugfixes (32-bit OSX and iOS, timing): + - Fix an integer overflow bug in our optimized 32-bit millisecond- + difference algorithm for 32-bit Apple platforms. Previously, it + would overflow when calculating the difference between two times + more than 47 days apart. Fixes part of bug 27139; bugfix on + 0.3.4.1-alpha. + - Improve the precision of our 32-bit millisecond difference + algorithm for 32-bit Apple platforms. Fixes part of bug 27139; + bugfix on 0.3.4.1-alpha. + - Relax the tolerance on the mainloop/update_time_jumps test + when running on 32-bit Apple platforms. Fixes part of bug 27139; + bugfix on 0.3.4.1-alpha. + + diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 40847a8442..93b527def0 100644 --- a/src/common/compat_time.c +++ b/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 @@ -299,11 +300,15 @@ monotime_init_internal(void) } { // For converting ticks to milliseconds in a 32-bit-friendly way, we - // will first right-shift by 20, and then multiply by 20/19, since - // (1<<20) * 19/20 is about 1e6. We precompute a new numerate and + // will first right-shift by 20, and then multiply by 2048/1953, since + // (1<<20) * 1953/2048 is about 1e6. We precompute a new numerator and // denominator here to avoid multiple multiplies. - mach_time_info_msec_cvt.numer = mach_time_info.numer * 20; - mach_time_info_msec_cvt.denom = mach_time_info.denom * 19; + 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 diff --git a/src/common/compat_time.h b/src/common/compat_time.h index 57ab20ab11..f241aa5eba 100644 --- a/src/common/compat_time.h +++ b/src/common/compat_time.h @@ -196,6 +196,7 @@ monotime_coarse_diff_msec32(const monotime_coarse_t *start, // on a 64-bit platform, let's assume 64/64 division is cheap. return (int32_t) monotime_coarse_diff_msec(start, end); #else +#define USING_32BIT_MSEC_HACK return monotime_coarse_diff_msec32_(start, end); #endif } diff --git a/src/test/test_circuitmux.c b/src/test/test_circuitmux.c index 14c7598703..c81d53ae51 100644 --- a/src/test/test_circuitmux.c +++ b/src/test/test_circuitmux.c @@ -13,6 +13,8 @@ #include "scheduler.h" #include "test.h" +#include + /* XXXX duplicated function from test_circuitlist.c */ static channel_t * new_fake_channel(void) @@ -103,16 +105,19 @@ test_cmux_compute_ticks(void *arg) monotime_coarse_set_mock_time_nsec(now); tick = cell_ewma_get_current_tick_and_fraction(&rem); tt_uint_op(tick, OP_EQ, tick_zero); - tt_double_op(rem, OP_GT, .149999999); - tt_double_op(rem, OP_LT, .150000001); +#ifdef USING_32BIT_MSEC_HACK + const double tolerance = .0005; +#else + const double tolerance = .00000001; +#endif + tt_double_op(fabs(rem - .15), OP_LT, tolerance); /* 25 second later and we should be in another tick. */ now = START_NS + NS_PER_S * 25; monotime_coarse_set_mock_time_nsec(now); tick = cell_ewma_get_current_tick_and_fraction(&rem); tt_uint_op(tick, OP_EQ, tick_zero + 2); - tt_double_op(rem, OP_GT, .499999999); - tt_double_op(rem, OP_LT, .500000001); + tt_double_op(fabs(rem - .5), OP_LT, tolerance); done: ;