Support precise garbage collection with the Julia GC - #6558
Draft
fingolfin wants to merge 27 commits into
Draft
Conversation
The generated NewPlistFromArgs(MakeImmString(...), ...) evaluates every name before the list exists, so all but the last sit in a C array the collector cannot see while the later ones allocate. Store each name into the rooted temporary as soon as it is made. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NewPlistFromArgs(MakeImmString("data"), MakeImmString("val")) evaluates
both strings before the list exists, so the first sits in a C array the
collector cannot see while the second allocates. Store each name into
the list as soon as it is made.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add two hooks, GAP_GC_SAVE_STACK_STATE and GAP_GC_RESTORE_STACK_STATE, and call them wherever GAP sets up a setjmp for its error handling: the reader's TRY_IF_NO_ERROR, the GAP_TRY/GAP_CATCH helpers, and the libgap entry points. The state is saved before the setjmp and restored on the error path, next to the recursion depth that is already handled there. The point of this is the Julia GC integration. The Julia GC keeps a chain of root frames on the C stack. A GAP error longjmps out of the call chain that raised it, so the frames that chain registered are never unregistered, and after the jump they point into stack that no longer exists; the next collection then walks garbage. With the hooks, the head of that chain is saved and restored like the rest of the error state, which fixes this for every error GAP catches. GASMAN and Boehm find their roots by scanning the C stack and have nothing to record; for them the hooks compile to nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three static analyses guard this work: Julia's GCChecker, which checks that values are rooted across safepoints; its first-declaration check, which catches annotations clang would silently ignore; and Clang Thread Safety Analysis, which checks the safepoint annotations against each other. Add a runner for each, plus a sweep over the whole kernel. None of them is tied to a particular checkout: each reads the compile flags from a given build directory and locates the Julia tree, clang and the analyzer plugins from the -I flag recorded there. Every path can be overridden from the environment. These are developer aids with no build-system wiring; nothing in a normal build depends on them. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
A staged plan for moving the kernel off the conservative stack scanner, recording what is done, what is in progress, and what should only be attempted once the precise rooting path has more evidence behind it. This is a working document rather than reference material, and touches nothing else; drop it before merging if it is not wanted in the tree. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Introduce GAP-owned wrappers for the annotations Julia's static analysis tooling understands, plus the runtime rooting macros. Keeping our own spellings means GAP controls how they are exposed, and lets low-level headers use the annotations without pulling in julia.h. Everything here is inert unless an analyzer define is set: the annotations expand to nothing, and the rooting macros to `((void)0)` unless the Julia GC is in use. GASMAN and Boehm builds are unchanged. The header comment documents what the two analyses check, that they take opposite views of an unannotated function, where each annotation belongs, and why a rooted local must be initialised first. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The GC-stack bookkeeping they do must not itself be a place the collector can run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Julia's GC is precise: it finds roots by walking a stack of frames the program maintains, not by scanning the C stack. A GAP Obj held only in a C local is therefore invisible to it, and a collection triggered while that local is live can free or move the bag. Push a frame around each such local, and describe the kernel's existing rooting to the analyzer where a frame is not needed - which argument roots which, which accessor's result stays rooted by its container, and which functions must never reach a safepoint at all, marking functions above all. A frame stores the address of each local, so a rooted local must be initialised before the frame is pushed; the collector reads it as soon as the frame is pushed. That accounts for the many `Obj x = 0` changes. Use the fixed-arity GAP_GC_PUSH1..9 rather than GAP_GC_PUSHARGS for GAP values: PUSHARGS stores values instead of addresses and reads them with Julia's low-bit tag semantics, which collide with GAP's immediates. Verify with dev/run-julia-gc-analyzer-all.sh, which is clean across all 77 in-scope translation units at this commit. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Clang Thread Safety Analysis is the counterpart to Julia's `make -C src safesrc`, and checks the safepoint annotations against each other. Unlike the GC checker it has no implicit default: a function that reaches a safepoint must say so, and is flagged as soon as it calls one that does without saying so itself. The annotations were derived by starting at the allocation primitives and repeatedly annotating whatever the analysis flagged, until it went quiet. Each sits on the function's first declaration, since that is what callers in other translation units see; clang silently ignores one that appears only on a later declaration. Three functions are defined inside macro bodies and carry the annotation there: the operation wrappers in tracing.h, the math primitives in macfloat.c, and the sort helpers in sortbase.h. system.h is the exception worth reading. Panic never returns, so callers observe no safepoint and may remain non-safepoints themselves; its body does reach one, since SyExit calls jl_exit, which runs atexit handlers and Julia declares JL_CANSAFEPOINT. Promising callers one thing while the body opts out keeps every function that can panic - RetypeBag among them - from becoming a safepoint. Nothing here changes generated code. Stripping GAP_GC_CANSAFEPOINT from these files reproduces the parent tree exactly. Run the check with dev/run-safepoint-check.sh. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
That option is meant to turn off conservative stack scanning, but it had come to gate two unrelated things as well, so a build with it defined did not even start. Starting Julia was conditional on it. InitBags calls GAP_InitJuliaMemoryInterface, which is what invokes jl_init(), from a block guarded on stack scanning being enabled; with it disabled the first NewBag dereferenced a null current task. Whether GAP starts Julia depends only on whether GAP is the main program, not on how stacks are scanned. Marking GAP's global bags was conditional on it too. That loop lives in GapRootScanner, and the callback was only installed when stack scanning was enabled, so every global root - the symbol table among them - was collected on the first GC. Install the root scanner either way and guard only the stack walk inside it; scanning other tasks' stacks stays conditional, being purely conservative. With both separated, a DISABLE_STACK_SCAN build starts and runs into genuine rooting gaps rather than into this. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Julia's precise collector finds roots by walking GC frames, so a GAP object held in a C struct on the stack is invisible to it. Conservative stack scanning found these; the GC analyzer does not, because it checks rooting within a function rather than whether a stack struct holds objects. A clean analyzer run says nothing about them. Give each such struct a list of its GC slots, declared next to the struct so the two stay together, and compose the lists upward: a ReaderState roots its scanner and interpreter, which roots its coder. Push them with GAP_GC_PUSH_ROOTS, a thin wrapper that feeds a composed list to the existing fixed-arity macros. Its slot count must be a literal, since the wrapper pastes it onto GAP_GC_PUSH: that is also what makes a changed root list a compile error at every call site rather than a silent gap. The static asserts cover the other direction, a field appended to a struct without being added to its list. The reader state has seven such slots, and must be rooted immediately after it is zeroed: Match_ stores string and number literals into rs->s.ValueObj before anything else runs. The coder state built by SYNTAX_TREE_CODE needs the same, before CodeBegin, which allocates. Drop the local `stackNams` from both reader functions. It was a copy of rs->StackNams introduced only so there was a local to push, which is the wrong shape - root the field. Extracting the implicit-function preamble into a helper likewise removes `nams`, which needs no root of its own since PushPlist stores it into the rooted rs->StackNams and says so with GAP_GC_ROOTED_BY_ARG. Both functions then need exactly nine slots, so neither has to nest frames. GAP_IsRootedSlot asserts, in debug builds, that a state struct really was rooted by whoever created it. Nothing else catches that: not the compiler, not the analyzer, and not package code the analyzer never sees. It found the SYNTAX_TREE_CODE case above. With conservative stack scanning disabled, this takes the test suite from 62 completed files to passing all 318, though not yet reliably: a DISABLE_STACK_SCAN build still crashes intermittently, at varying points, so at least one rooted-by-luck struct remains. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
GASMAN's memory checking collects on every allocation, which the Julia GC build ignored. Make --enableMemCheck collect every nth bag allocation there too: period 1 cannot boot, a few hundred does, and GAP_MEMCHECK_PERIOD in the environment sets the period before startup so the checks cover boot. GASMAN_MEM_CHECK(n) changes it at run time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Four places keep a GAP object in a C local or parameter across a call that allocates, so a precise collector may reclaim it. Conservative stack scanning hides all four. CodeFuncExprEnd is the sharp one: <fexp> is reached through cs->CodeLVars, and stays alive only until that field is reassigned a few lines later, after which two allocating calls use it. The GC analyzer cannot see this, because it models the root relationship but not the assignment that breaks it. MakeFunction reads BODY_FUNC(fexp) after NewFunction has allocated, and SetupFuncInfo uses its <func> parameter after three allocating calls. InitLibrary in vars.c stores a function into BottomLVars through a raw header pointer without the write barrier, then allocates. Only the first was reached by the failure being chased; the other three are the same defect found by reading the surrounding code, and would each need a specific allocation to land in a specific window to bite. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Generated code keeps GAP objects in C locals: the arguments and locals of every handler, its temporaries, the values it passes to calls, and the function that generated init code creates. A precise collector sees none of them, so the compiler now emits GC frames for all of them - in groups of nine, nested, popped in reverse before every return - and the compiled-test corpus is regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GAP's compiler emits C that has to root its own locals, and getting that
wrong shows up as an intermittent crash a long way from the cause. The
checked-in expected outputs of tst/test-compile are a stable sample of
what the compiler emits, so analyzing them turns a codegen regression
into a diagnostic at the point of the mistake.
It works: on one of these files the analyzer reports 58 rooting problems
before the last commit's frames and 30 after, so it both detects the bug
class and measures progress against it.
The generated files live outside src/ and include "compiled.h", hence the
new JULIA_GC_ANALYZER_CFLAGS hook in the single-file runner.
Note this makes the sweep fail: 8 of the 9 files report, 100 diagnostics
in total, every one of them the same shape - a value passed as an
argument without ever being rooted, as in
DoOperation2Args( op, t_1, NewPlistFromArgs( t_2 ) );
Emitting a temporary for such arguments should clear all of them. The
kernel's own 77 translation units remain clean. The sweep is a developer
tool and is not run by CI, so nothing gates on this today.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The sweep invoked julia.GCChecker on its own, overriding the single-file runner's own default of core,julia.GCChecker. That is not a configuration Julia uses or tests: core is what terminates paths at noreturn calls, and without it the GC checker must model noreturn itself. Its model does not cover a callee that pops its own frame and then does not return, which GAP hits through CallErrorInner, so the caller's frame went missing and 27 files reported spurious "JL_GC_POP without corresponding push". Use Julia's CLANGSA_GC_CHECKERS list, defined once and shared. That drops those reports without any change to the checker, and removes the reason for the "Preserve caller GC frames on noreturn" patch we were carrying. Also clear the log directory at the start of a sweep. A log left from an earlier run is indistinguishable from one just written, which is an easy way to read a stale result as a current one. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The GC analyzer tracks rooting for Julia's own object types, which it recognises by name. GAP's Bag was made visible to it by a downstream patch adding "Bag" to that list, which cannot go upstream and would in any case claim any type whose name ends in "bag". Julia now offers JL_GC_TRACKED_TYPE for embedders to mark their own types. Wrap it as GAP_GC_TRACKED_TYPE and put it on struct OpaqueBag, which both Bag and Obj resolve to, so no patched Julia is needed. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
DoOperationNArgs collects the argument types into a C array on its own stack frame and hands it to GetMethodUncached, which keeps using it after calling a method's family predicate. That predicate is arbitrary GAP code and so a safepoint, and a precise collector cannot see a plain C array, so nothing keeps those types alive across the call. Put the array in a GC frame. ids[] beside it needs no rooting: ID_TYPE yields the type's number, which is an immediate. Conservative collectors keep the plain array, which their stack scan already finds. Whether the types are reachable by another route is not something either the collector or the analyzer can establish: they usually are, through the argument each was taken from, which is why this survives in practice. That is an argument about every TypeObjFuncs entry, not a local one, and it is not the contract the rest of the kernel is being held to. This does not come with a reproducer. Two crashes under GC stress -- a segfault in FuncLEN_LIST and an ELM_PLIST assertion inside GetMethodUncached itself -- led here, but neither reproduced once the array was rooted or before, so the fix is not known to address them. It closes a real hole either way. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
TypInputFile and TypOutputFile are C structs; the stream and the current-line string they hold were invisible to a precise collector. The root scanner now walks both chains. The marking callback and the functions it calls are not safepoints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Remove(list) takes the last element out of the list, then shrinks the list. Between those two steps the element is referenced by nothing but a C local, and SHRINK_PLIST is a safepoint by contract. Under a precise collector the element dies and Remove returns a dead object, which then turns into whatever next reuses its cell - a range, a plain list, an interpreter frame - in a list that is otherwise intact. The analyzer cannot see this: a value loaded from a rooted list counts as rooted by the list, and the checker has no way to notice the slot being cleared underneath it. RemList has the same shape with UNB_LIST. Root the element in both. Add(list, obj, pos) has the mirror image: its insert branch grows the list before obj is stored anywhere, and a growing ResizeBag allocates, so root obj there too. Only the growth case allocates today; a shrinking ResizeBag is in place. So Remove is latent in production and Add(list, obj, pos) is not. Under memory checking, where every resize collects, Remove fails outright: popping 64 fresh cyclotomics one by one returned dead objects, and Irr(SmallGroup(240, 109)) - whose Baum-Clausen step builds characters with Add and Remove - reproduced the corruption seen across the stress suite: a segfault in compiled code, an ELM_PLIST assertion in method dispatch, and the four ctblmono.tst diffs. All three pass now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Record how to build for memory checking, what GASMAN_MEM_CHECK(n) does and why period 1 cannot boot, the known reproducer, what the dead reference report looks like and how to read it, and the shape of bug the analyzer cannot see. Drop the advice to run the checker with julia.GCChecker alone, which is what produced the spurious missing-pop reports, and note the second GAP_GC_PUSHARGS user. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The insertion and shell sorts copy an element into a C temporary, shift the following elements up over its slot, and write it back once its place is found. Between the shift and the write-back nothing but the temporary refers to it, across every comparison made meanwhile. For the plain sorts that is survivable: the comparison receives the element as an argument, and a GAP comparison function roots its arguments. The parallel sorts do not get away with it. The element of the shadow list is moved in the same way but is never handed to the comparison, so when the comparison allocates it dies, and a dead pointer is written into the shadow list. That is what corrupted GAPInfo.PackagesInfo under memory checking: InitializePackagesInfoRecords sorts version strings with the package records as the shadow and CompareVersionNumbers as the comparison, and package.tst then found a string where a record had been. The memory checking report named the parent list, the dead record in it, and the sorting kernel on the C stack. Root the temporaries in the functions that take an element out of the list: Shell, Insertion, LimitedInsertion, and Swap, which after its first store holds the other element alone while a generic ASS_LIST may collect. Each instantiation supplies the frame for its own temporaries, two slots per temporary for the parallel sorts. The merge helpers need nothing: their temporary is always still in the list or the buffer. The analyzer cannot see this. An element loaded from a rooted list counts as rooted by the list, and the checker has no notion of the slot being overwritten by the shift. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The memory-checking build now aborts, with the parent bag, the slot, the GAP call stack and the GC frames, at the first reference the collector cannot vouch for, instead of leaving a corrupt heap to fail later: - a child that is dead or whose cell was reused, and a root that is neither 0 nor a live bag; the default marker used before a module registers its marking function is exempt, it is conservative; - every GAP_GC_PUSH* is recorded with its source location and every GAP_GC_POP checked against the record; push and pop also compare the frame with the stack pointer, since after an unwind that skipped GAP_GC_RESTORE_STACK_STATE the pops would quietly pop dead frames and keep record and chain consistent; - a root that holds the 0xAA fill pattern of an uninitialised local, for builds made with -ftrivial-auto-var-init=pattern. The sampled collections can be young ones: GAP_MEMCHECK_FULL_EVERY=n makes n-1 of every n samples young collections, which are cheap enough for period 1 and the only kind that exposes a missing write barrier, and the nth a full one that validates the old parents. GAP_MEMCHECK_START and GAP_MEMCHECK_STOP bound the checks to a window of allocations, and ResizeBag poisons the old body of a grown bag. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NewPlistFromArray copies from a C array the collector cannot see, the
compound literal or parameter pack behind NewPlistFromArgs, and the list
allocation is a safepoint. Any argument that is not also stored elsewhere
dies there; NewPlistFromArgs(MakeImmString("obj")) in compiled code lost
its string whenever a young collection landed in that window. So the
allocation now runs with the collector disabled: the copy is the only
step before the barrier.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NewBag allocates the masterpointer, then the body. The second allocation can collect, and the fresh masterpointer lived only in a C local until then - which the conservative stack scanner used to find. Without it a collection there freed the masterpointer, the body pointer was written into a free cell, and a dead bag was handed out; the crash came whenever the cell was reused, reading the reusing object's first word as a body pointer. That was the rare boot-time fault at a constant address in the optimized build. The memory-checking mode sampled only at NewBag's entry, so it never collected in that window. Sample inside AllocateBagMemory as well, which every body allocation goes through; with that, boot at any period reproduced the fault before the fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ConvVec8Bit looks the type up before it packs the list, and the element access inside the loop is a safepoint for the analyzer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The notes accumulated across the series; this is their state after the last of the memory-checking work. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6558 +/- ##
==========================================
- Coverage 78.98% 69.96% -9.03%
==========================================
Files 683 639 -44
Lines 294880 278598 -16282
Branches 8639 7672 -967
==========================================
- Hits 232912 194914 -37998
- Misses 60160 81960 +21800
+ Partials 1808 1724 -84 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lgoettgens
removed their request for review
September 4, 2026 21:13
Member
Member
I am not sure I follow the AI 😂 |
Member
Author
That wasn't the AI, that was me making a copy & paste mistake. I do not post un-edited AI generated PR descriptions, this one was already heavily edited, but humans make different mistakes compared to AI :-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Under the Julia GC, GAP objects held in C locals are currently kept alive by conservatively scanning the C stack. This PR makes the kernel describe those roots precisely instead, the way Julia's own runtime does, so that the stack scanner is no longer needed. It is opt-in: compiling with
-DDISABLE_STACK_SCANselects precise mode. Every other configuration is untouched: under GASMAN, Boehm, and Julia with stack scanning, all the new macros and annotations compile to nothing.What changes
Root frames. A C local that holds a GAP object across an allocation is registered with the collector:
GAP_GC_PUSH1(&x)..GAP_GC_PUSH9(...)push a frame of addresses,GAP_GC_POP()pops it (src/precise_gc.h). The macros wrap Julia'sJL_GC_PUSH*.Note that in order to support GAP immediate objects (such as produced by
INTOBJ_INT), aka tagged pointers, a patch for the Julia GC is needed; in fact a small 3 line patch is sufficient (and is required for this PR here), but I made a more elaborate version at JuliaLang/julia#62889 to also support the variadicJL_GC_PUSHARGS(which this PR here does not yet make use of). I hope to get this backported.Annotations for the analyzer.
GAP_GC_NOTSAFEPOINT,GAP_GC_CANSAFEPOINT,GAP_GC_ROOTED_BY_ARG(n),GAP_GC_GLOBALLY_ROOTEDand friends let Julia's GC static analyzer (a clang plugin that Julia uses on its own runtime in order to find GC unsafe code) check the kernel: every function that can collect is marked, every value held across such a call must be rooted. Two mechanical commits carry most of the annotations; the rest of the series is the rooting the analyzer then demanded.To fully make use of the analyzer part requires a second patch for Julia, see JuliaLang/julia#62928
Error unwinding. A GAP error
longjmps past the frames the aborted call chain pushed. #6556 adds hooks to the TRY/CATCH machinery to save and restore the collector's state; this PR includes that commit and uses the hooks.Memory-checking mode (
--enable-memory-checking,--enableMemCheck), which so far was a GASMAN feature, now works under the Julia GC: it collects every nth allocation (GAP_MEMCHECK_PERIOD), mixes young and full collections (GAP_MEMCHECK_FULL_EVERY, the young ones are what exposes a missing write barrier), and aborts at the first reference the collector cannot vouch for, printing the parent bag, the slot, the GAP call stack and the registered frames. It also records every push and pop and reports frames an unwind left behind. This is what found the issues below; the recipe is indev/julia-gc-handoff.md.Kernel issues. Precise rooting has no tolerance for objects held only in C memory, and the checks found a number of such places. The ones that also matter under GASMAN were split out and are merged (#6551, #6552, #6553, #6554). The remaining ones are Julia-only in effect but real: list elements held across a resize that drops them (
Remove), the temporaries of the sorting kernels, the type array in method dispatch, the objects behind open files,NewPlistFromArgs' argument array, andNewBagitself, which held the fresh masterpointer only in a C local while it allocated the body.What is missing resp. to be done?
🤖 Generated with Claude Code