Repository navigation
fix(libc): keep the _Cnd_timedwait deadline in the host clock's unit - #6580
oneandonlydean merged 2 commits into
Conversation
…ange A deadline of 10^10 seconds does not fit a nanosecond count but fits the host clock where system_clock counts microseconds, as on macOS. There the wait must block until it is woken; where the clock cannot hold the deadline, _Cnd_timedwait must throw the overflow error as for any other deadline past the clock's range.
The deadline was built as seconds plus nanoseconds, a nanosecond duration, and converted implicitly to a system_clock time point. Where system_clock counts microseconds, as on macOS, that conversion is lossy and does not compile, so libc did not build there. The seconds now stay in the clock's unit and only the nanoseconds are rounded up to it, so the wait never ends before the requested time. With a nanosecond system_clock nothing changes. Converting the nanosecond sum instead would compile but overflow above 2^63 nanoseconds, which the clock range check above allows on macOS: a 10^10 second deadline then wraps into the past and the wait times out at once.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Tested on Linux x86-64 (Release build, GCC/libstdc++), with 2854da2 merged into main 31cbb55:
AI-assisted: yes (Claude Code) |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Linux run, which the description says is still to come.
Ubuntu 26.04, Intel i7-8700, GCC 15.2 (libstdc++, nanosecond system_clock), Ninja, Release, BUILD_TESTING=ON, full build. main 31cbb55 with head 2854da2 merged on top.
- With the PR: libc builds with no new warnings;
guest_libc_cnd_timed_waitpasses 3 runs of 3. With a nanosecond clock, 1e10 s is above the clock's limit (about 9.2e9 s), so the new case takes thestd::overflow_errorbranch, as described. Threads.cppreverted tomain, test kept: it also passes. That is expected here:ceilto nanoseconds is the identity, so the change cannot show up on Linux; the red case is the macOS one in the description.- Non-GPU suite (
ctest -E "^agc|gpu|vulkan|video_out" -j8): 648/650, same asmainon this machine. The 2 failures (file_writer_binary,file_writer_text) also fail onmainhere.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Windows run, since Windows is untested. Windows 11, WinLibs GCC 15.2.0 r7 (MinGW), Release,
Not tested: a negative control on Windows. AI-assisted: yes (Claude Code) |
|
Linux and Windows results for 2854da2. The merge base is e16b2bc.
On both hosts the new case takes the overflow branch. The 1e10 s deadline is beyond the clock's 9223372036 s, so the test requires As expected, the test does not tell the fix from the old code on a nanosecond clock. There the deadline expression is the same before and after, so the with/without difference only shows on macOS. After restoring the branch's AI-assisted: yes |
What
libc does not build on macOS:
_Cnd_timedwaitturns its deadline into asystem_clock::time_pointfromseconds + nanoseconds, a nanosecond duration. On macOSsystem_clockcounts microseconds, so the implicit conversion is lossy and AppleClang rejects it:The seconds now stay in the clock's unit and only the nanoseconds are rounded up to it (
std::chrono::ceil), so the wait never ends before the requested time. With a nanosecondsystem_clock(Linux, MinGW) the expression is unchanged.Casting the nanosecond sum instead would compile but is wrong on macOS: the range check above allows deadlines up to the microsecond clock's limit (about 2.9e14 s), while the sum overflows above 2^63 ns (about 9.2e9 s). A deadline of 1e10 s then wraps into the past and the wait times out at once. The new test case waits on exactly that deadline: on a host whose clock holds it, the wait must block until a second thread wakes it; on a nanosecond clock it must throw the overflow error, as for any deadline past the clock's range.
Two commits: the test, then the fix. There are no further real steps for this change.
Tested
macOS 26.7.1, Apple M2, AppleClang 21.0.0, x86-64 build under Rosetta (as the full build selects), Release.
maindoes not build libc on macOS for other reasons as well, which #4627 addresses, so the test ran in a local tree ofmaince991f1 with #4627, #4630 and #6124 merged. The branch itself is based on e16b2bc and merges with it cleanly.guest_libc_cnd_timed_wait_testslinks only libc.main'sThreads.cpp: libc fails to compile with the error above.time_point_castof asys_time<nanoseconds>): builds; the new case aborts, 3 runs of 3.guest_libc_cnd_timed_wait_testspasses 5 runs of 5, the existing cases included.tools/check_conventions.py --base origin/mainandgit diff --checkpass.Linux and Windows, run by others on 2854da2 (thanks): jotitadev (Ubuntu 26.04, GCC 15.2) and coltken95 (Linux x86-64, GCC) and a WSL2 Ubuntu 24.04 run (GCC 13.3) pass
guest_libc_cnd_timed_wait; johnmaia (Windows 11, WinLibs GCC 15.2.0) passes it and the full suite. With a nanosecondsystem_clockthe new case takes the overflow branch, so it also passes withmain'sThreads.cppthere, as expected.Overlap: yabanana's AOT pull requests (#1835 and #3891, #4156, #4396, #4397, #4422) carry the same
ceilconversion inside their Apple ARM64 work. This lands it on its own with a test. @yabanana, if you'd rather keep it in your series, say so and I'll close this one. #6222 converts the nanosecond sum, which overflows as described above. #6044 movesGuestLibcCndTimedWait.cpp; whichever lands second rebases.Checklist
main; no other open PR implements the same functions (see Overlap above)NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)