macOS i386 fails time unit tests

circuitmux/compute_ticks: [forking] 
  FAIL src/test/test_circuitmux.c:109: assert(rem OP_LT .150000001): 0.1505 vs 0.15
  [compute_ticks FAILED]

mainloop/update_time_jumps: [forking] 
  FAIL src/test/test_mainloop.c:115: expected log to not contain entries  Captured logs:
    1. notice: "Your system clock just jumped 1800 seconds forward; assuming established circuits no longer work.\n"

  [update_time_jumps FAILED]

Should we set up macOS i386 CI to detect issues like this? Or should we remove macOS i386 from our supported platforms? (Apple won't support i386 in the next macOS, so in ~3 years time, we won't need to support it.)

changed milestone to %Tor: 0.3.5.x-final

added 034-backport 32-bit actualpoints::1 arm32 component::core tor/tor i386 macos milestone::Tor: 0.3.5.x-final owner::nickm priority::medium regression resolution::fixed reviewer::catalyst severity::normal status::closed type::defect labels

Are these intermittent or consistent?

I'd be okay with or without OSX i386 CI.

Trac:
Summary: macOS i386 fails time unit testa to macOS i386 fails time unit tests

Trac:
Cc: N/A to rl1987

We must check if this issue affects Linux and BSD on 0.3.4 before releasing 0.3.4.

Trac:
Keywords: i386 deleted, 035-roadmap-proposed, 035-must, 034-must added

Oops, wrong ticket for that comment.

Trac:
Keywords: 035-roadmap-proposed, 035-must, 034-must deleted, i386 added

Replying to nickm:

Are these intermittent or consistent?

Consistent, with exactly those numbers in the tests every time.

I don't see the same failures on macOS x86_64, but I do occasionally see odd time jumps after sleep on x86_64, see #26987 (moved). Maybe they are the same bug.

I'd be okay with or without OSX i386 CI.

Same here. Tor Browser doesn't distribute i386 builds on macOS, and Homebrew only supports 64-bit. (MacPorts still builds 32 bit tor, though.)

Trac:
Parent: N/A to #26987 (moved)

I'd like to fix this, but it's not a much-used platform. We should investigate a little more, and see if it's easy to fix.

Trac:
Milestone: Tor: 0.3.5.x-final to Tor: 0.3.4.x-final

Not related to #26987 (moved)

Trac:
Milestone: Tor: 0.3.4.x-final to Tor: 0.3.5.x-final
Parent: #26987 (moved) to N/A

Trac:
Parent: N/A to #27296 (moved)

I've started investigating these more closely.

One thing that would be helpful here would be to know which values mach_timebase_info() puts into its output on this platforms.

The first one is probably due to monotime_coarse_diff_msec32_() having some rounding error. I think that should be easy to resolve.

Trac:
Owner: N/A to nickm
Status: new to accepted

I think the second one may also be due to rounding error in that function.

Trac:
Keywords: N/A deleted, 034-backport, arm32, 32-bit added

Okay, I think I have it. The problem was in our 32-bit monotime_coarse_diff_msec32_ implementation: it wasn't precise enough, and it didn't try hard enough to avoid integer overflows. One of our unit tests was assuming an amount of precision it just couldn't provide.

Bugfix in branch bug27139_034; PR at https://github.com/torproject/tor/pull/334 .

I tested this by editing the compat_time.h header so that the monotime_coarse_diff_msec32() inline function would always use the 32-bit implementation.

Trac:
Status: accepted to needs_review

(This turns out not to have anything to do with its ostensible parent ticket)

Trac:
Parent: #27296 (moved) to N/A

Trac:
Reviewer: N/A to catalyst

I ran this branch with CC="clang -arch i386", and the necessary dependencies from MacPorts.

make check and make test-network-all passed.

Looks good to me! I only tested using nickm's strategy of forcing the use of the 32-bit computation on 64-bit, and trust teor's evidence.

I'm a little nervous about some of the fixed-point hackery here, but given that it's increasingly unlikely that 32-bit macOS will continue to be relevant, I guess we can fix it later if it turns out to be a problem.

Trac:
Status: needs_review to merge_ready

Replying to catalyst:

I'm a little nervous about some of the fixed-point hackery here, but given that it's increasingly unlikely that 32-bit macOS will continue to be relevant, I guess we can fix it later if it turns out to be a problem.

I downgraded macOS i386 to Maintained in our supported platforms policy: https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/SupportedPlatforms?action=diff&version=14

Okay, merged to 0.3.4 and forward!

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

Trac:
Actualpoints: N/A to 1

closed

added 8h of time spent

moved to tpo/core/tor#27139 (closed)