Repository navigation
Conversation
fingolfin
added
topic: kernel
release notes: not needed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6781 +/- ##
==========================================
+ Coverage 79.70% 79.72% +0.01%
==========================================
Files 684 684
Lines 295574 295595 +21
Branches 8684 8660 -24
==========================================
+ Hits 235594 235654 +60
+ Misses 58164 58125 -39
Partials 1816 1816 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| static jmp_buf RegsBags; | ||
| #ifndef HAVE___BUILTIN_UNWIND_INIT | ||
| // jmp_buf can be an int array (macOS); align it for reading Bags from it |
There was a problem hiding this comment.
Why is this a union with Bag and the other with void* ?
There was a problem hiding this comment.
No reason; both are void * now. The member only forces pointer alignment, because macOS declares jmp_buf as an int array.
Assisted-by: Generative AI
|
Looking around, I see several reports of __builtin_unwind_init not working correctly. Boehm has quite a block of compiler and platform exclusions here: At least, I’d be tempted to keep the setjmp capture and scan as well. That gives us another chance to find references if the builtin doesn’t do what we expect, and keeps that code exercised on our normal builds. With the current approach, the setjmp fallback will never run in CI. |
lgoettgens
left a comment
lgoettgens
left a comment
There was a problem hiding this comment.
I must admit that I don't really understand this, neither the problem nor the solution. Of course, I could just ask the AI if this change is sensible, but @fingolfin could just do the same with a fresh agent as well (and would probably understand the feedback)
Both GC backends could miss a bag reference that a caller held only in
a callee-saved register when the collection started, and then free a
bag still in use. No failure is known; whether one occurs depends on
the compiler's register allocation.
The register contents were lost in two ways:
- Both backends captured registers with setjmp, which stores some of
them mangled with a per-process key:
glibc 2.41 x86-64: rbp, rsp, return address
glibc 2.41 aarch64: sp, x30
macOS 26 x86-64: rbp, rsp, return address
macOS 26 arm64: x29, sp, x30
On macOS rbp and x29 always hold the frame pointer. With gcc's
default -fomit-frame-pointer at -O2 on x86-64 Linux, rbp is an
ordinary callee-saved register, so a bag held only in rbp was found
only if a function between its holder and the scan saved rbp on the
stack. With gcc 14 -O2 CollectBags does, but nothing guarantees it.
- The Julia GC root scanner scanned upward from its own jmp_buf. gcc on
aarch64 places callee-saved registers below the locals (gcc 14 on a
reduced copy of the function: x19-x21 at sp+16..32, the jmp_buf at
sp+88), so the caller values its prologue saved were not scanned.
Where configure finds __builtin_unwind_init, both backends now call it
in addition to setjmp. It makes the prologue of the calling function
save every callee-saved register, including the frame pointer and the
floating-point ones, in its frame; values live across a call are only
there or on the stack, so scanning that frame covers all of them. gcc
and clang support it and libgcc's unwinder relies on it, but it has
failed before: Boehm GC avoids it with gcc >= 11 after gcc 11.2 on
Cygwin x86-64 lost registers, and with clang < 8, where it was a no-op.
Keeping setjmp means such a failure costs nothing over the old capture.
Its jmp_buf is now aligned to pointer size for the word-wise scan
(macOS declares it as an int array).
The Julia GC scan now starts in a callee, as GASMAN's already does, so
that it covers the scanner's own frame.
Assisted-by: Generative AI
66f1788 to
aaf0fcb
Compare
|
Agreed, done. The exclusions on that list that matter for us are gcc >= 11, added after registers went missing with gcc 11.2 on Cygwin x86-64 (bdwgc/bdwgc@ffb9f1e82d; I found no gcc bug report for it), and clang < 8, where the builtin was a no-op. The setjmp capture and the jmp_buf scan now always run, and the builtin is an addition where configure finds it. So a broken builtin is no worse than master, and the setjmp path runs in CI. Assisted-by: Generative AI |
|
Thanks, I'm running some gap tests which the excessive memory checking flags on, just to give this another poke. Which should take a couple of days, but then I'll flag it reviewed for merging -- I don't expect anything to come up and this does solve that OSes like Mac have been increasingly messing around with jmpbuf, this is an improvement and I expected us to have to do something like this eventually. |
Both GC backends could miss a bag reference that a caller held only in a callee-saved register when the collection started, and then free a bag still in use. No failure is known; whether one occurs depends on the compiler's register allocation.
The register contents were lost in two ways:
Both backends captured registers with setjmp, which stores some of them mangled with a per-process key:
glibc 2.41 x86-64: rbp, rsp, return address
glibc 2.41 aarch64: sp, x30
macOS 26 x86-64: rbp, rsp, return address
macOS 26 arm64: x29, sp, x30
On macOS rbp and x29 always hold the frame pointer. With gcc's default -fomit-frame-pointer at -O2 on x86-64 Linux, rbp is an ordinary callee-saved register, so a bag held only in rbp was found only if a function between its holder and the scan saved rbp on the stack. With gcc 14 -O2 CollectBags does, but nothing guarantees it.
Where configure finds __builtin_unwind_init, both backends now call it in addition to setjmp. It makes the prologue of the calling function save every callee-saved register, including the frame pointer and the floating-point ones, in its frame; values live across a call are only there or on the stack, so scanning that frame covers all of them. gcc and clang support it and libgcc's unwinder relies on it, but it has failed before: Boehm GC avoids it with gcc >= 11 after gcc 11.2 on Cygwin x86-64 lost registers, and with clang < 8, where it was a no-op. Keeping setjmp means such a failure costs nothing over the old capture. Its jmp_buf is now aligned to pointer size for the word-wise scan (macOS declares it as an int array).
The Julia GC scan now starts in a callee, as GASMAN's already does, so that it covers the scanner's own frame.
Assisted-by: Generative AI
On the meta level, that's one more reason to want to use a precise GC...