runtime: mark all global variables explicitly for GC - #5575
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces precise global GC root tracking and propagates GC layouts through runtime allocations to reduce false memory retention.
Changes:
- Generates explicit global-root tables from LLVM globals.
- Adds GC layouts for maps, channels, reflection, stacks, and raw allocations.
- Adds GC-root regression tests and updates expected compiler/interpreter output.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
transform/testdata/gc-stackslots.out.ll |
Updates expected root-table transformation. |
transform/testdata/gc-stackslots.ll |
Adds global-root transform inputs. |
transform/testdata/allocs.out.ll |
Updates expected allocation layouts. |
transform/testdata/allocs.ll |
Supplies pointer-free allocation layouts. |
transform/gc.go |
Generates explicit global-root functions and tables. |
testdata/map.go |
Reduces map-test memory use on small targets. |
testdata/gc.go |
Adds global map, channel, and reflection GC tests. |
src/runtime/hashmap.go |
Propagates map key, value, and bucket layouts. |
src/runtime/gc_stack_threads.go |
Uses explicit global marking. |
src/runtime/gc_stack_raw.go |
Uses explicit global marking. |
src/runtime/gc_stack_portable.go |
Uses explicit global marking. |
src/runtime/gc_stack_cores.go |
Uses explicit global marking. |
src/runtime/gc_precise.go |
Adds an explicit conservative layout marker. |
src/runtime/gc_leaking.go |
Marks reallocations pointer-free. |
src/runtime/gc_globals_none.go |
Adds no-op global marking. |
src/runtime/gc_globals_custom.go |
Retains custom-GC global scanning. |
src/runtime/gc_globals_boehm.go |
Marks roots eagerly through Boehm. |
src/runtime/gc_globals_blocks.go |
Marks generated root slots. |
src/runtime/gc_custom.go |
Updates custom-GC documentation. |
src/runtime/gc_boehm.go |
Declares eager Boehm marking. |
src/runtime/gc_blocks.go |
Marks reallocations pointer-free. |
src/runtime/chan.go |
Applies element layouts to channel buffers. |
src/runtime/baremetal.go |
Marks C allocations pointer-free. |
src/runtime/arch_tinygowasm_malloc.go |
Marks Wasm C allocations pointer-free. |
src/internal/task/task_stack.go |
Marks task stacks conservative. |
src/internal/task/task_asyncify.go |
Marks asyncify stacks conservative. |
src/internal/reflectlite/value.go |
Supplies layouts for reflected allocations and maps. |
src/internal/reflectlite/type.go |
Extends type descriptors with GC metadata. |
src/internal/gclayout/gclayout.go |
Adds pointer-pair and conservative layouts. |
interp/testdata/alloc.out.ll |
Updates interpreted allocation output. |
interp/testdata/alloc.ll |
Tests pointer-free interpreted allocation. |
interp/memory.go |
Rejects allocations without layouts. |
compiler/testdata/zeromap.ll |
Updates hashmap structure sizes. |
compiler/testdata/large.ll |
Verifies generated hashmap layout metadata. |
compiler/testdata/go1.21.ll |
Updates hashmap structure sizes. |
compiler/map.go |
Generates map allocation layout metadata. |
compiler/interface.go |
Embeds layouts in reflection descriptors. |
compiler/channel.go |
Generates channel element layouts. |
builder/sizes_test.go |
Updates expected binary sizes. |
Suppressed comments (2)
testdata/gc.go:202
- As above,
_ = new(...)need not produce a heap allocation, so this does not ensure that incorrectly collected reflected objects are overwritten before they are checked. Route the allocations through an escaping global to make this GC-liveness test effective.
testdata/gc.go:153 - The discarded
newcan be optimized away or stack-allocated, leaving the swept channel/object memory untouched and allowing this regression test to pass accidentally. Make these allocations escape so they genuinely pressure and overwrite the heap before the assertion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
21946f8 to
658d9a1
Compare
|
The Edit: Oh, whatever you just pushed fixed it. |
|
Yeah, the global analysis was happening after an optimization that then hid the allocs. |
91939ad to
73b7bb3
Compare
| {"wioterminal", "examples/pininterrupt", 8027, 1665, 132, 7488}, | ||
| {"hifive1b", "examples/echo", 4349, 323, 0, 2260}, | ||
| {"microbit", "examples/serial", 2882, 382, 8, 2256}, | ||
| {"wioterminal", "examples/pininterrupt", 8459, 1717, 148, 7488}, |
There was a problem hiding this comment.
I made a compromise; LLVM is prevented from promoting globals which contain pointers, such that they don't disappear, but other stuff can still be optimized. As such this one example increases from 132 to 148, but I did fix the others
| llvmValueSlotType = c.dataPtrType | ||
| } | ||
|
|
||
| // Keep this in sync with runtime.hashmapBucket and |
There was a problem hiding this comment.
There doesn't appear to be an equivalent comment in runtime/hashmap.go pointing back here for sync-i-ness.
| return nil | ||
| } | ||
| ptr := alloc(size, nil) | ||
| ptr := alloc(size, gclayout.NoPtrs.AsPtr()) |
There was a problem hiding this comment.
If we don't know what this memory is being used for, shouldn't it be Conservative instead of NoPtrs ?
| // it is theoretically possible. For now, just always allocate fresh. | ||
| // TODO: we could skip this if the new allocation is smaller than the old. | ||
| ptr := alloc(size, nil) | ||
| ptr := alloc(size, gclayout.NoPtrs.AsPtr()) |
| // Note: this zeroes the returned buffer which is not necessary. | ||
| // The same goes for bytealg.MakeNoZero. | ||
| return alloc(size, nil) | ||
| return alloc(size, gclayout.NoPtrs.AsPtr()) |
|
|
||
| // Create a new allocation and copy the old data. | ||
| newAlloc := alloc(size, nil) | ||
| newAlloc := alloc(size, gclayout.NoPtrs.AsPtr()) |
| func realloc(ptr unsafe.Pointer, size uintptr) unsafe.Pointer { | ||
| if ptr == nil { | ||
| return alloc(size, nil) | ||
| return alloc(size, gclayout.NoPtrs.AsPtr()) |
|
|
||
| func realloc(ptr unsafe.Pointer, size uintptr) unsafe.Pointer { | ||
| newAlloc := alloc(size, nil) | ||
| newAlloc := alloc(size, gclayout.NoPtrs.AsPtr()) |
There was a problem hiding this comment.
Conservative (although I guess doesn't matter because it's leaking...)
C malloc storage has explicit lifetime: it must remain allocated until free even when no GC-visible pointer references it. Treating it as an ordinary NoPtrs allocation breaks bare-metal C object graphs, while conservatively scanning arbitrary C bytes creates false Go roots. Add allocManual/freeManual so collectors can represent pointer-free, explicitly managed storage. Block GC keeps these objects permanently marked and releases their blocks on free; Boehm uses atomic uncollectable allocations; leaking and custom collectors provide equivalent behavior. Wasm keeps its allocation map only for validation and sizes, and WASIp2 realloc now copies min(oldSize, newSize). Also handle zero-size and overflowing allocations, serialize allocation registries, reject Go finalizers on manual storage, and add CGo regressions for C pointer graphs, hidden until-free allocations, repeated free/reuse, and allocation edge cases.
Fixes #5249
This came up in the testing of #5550, and like everything I touch, it ballooned into another big PR...
Basically, @dgryski noted that when testing something out of the test corpus, it was crashing with an OOM. The root cause turned out to be memory retention due to the conservative scanning of the global variables, which is done no matter which GC you're using. It was actually nothing to do with the Wasm code.
"Easy fix", I thought, as the compiler knows the location of all globals, so we can just build a big list at compile time, then mark the globals via that list.
That's great and all, but tinygo also has
interp, and it turned out to often return pointers to alloc'd memory as plain numbers, no type info for the allocs, which hid them the global analysis. For example, interp might peer right through the map types and optimize code away for a global map or something.And so, the first part of this PR is a change to ensure that allocs never happen with without GC layouts, both at interp time and runtime (which is effectively the same thing given what interp is doing, running the runtime code ahead of time)... This is a bit invasive and does increase binary sizes slightly to have the GC shape info there for use. BigGo of course does this too.
Then, once that's working, we can do precise scanning of globals. I started build tagging this for Wasm first, since that is where the test failed, but a later commit in the stack enables that everywhere such that no platform is conservatively scanning the global object space.
This in effect makes #5249's LLVM change requirement obsolete, since there's nothing special about
.rodata; we just have a list of the globals and use it.Boehm and custom are special; Boehm's API lacks a func to ask for marking just one address, it only wants ranges. If you attempt to give it a range per global, it seems to fall over due to the stress. A previous version of this PR just did a copy of all pointers into a big array and then gave that to Boehm, but Boehm turns out to have a special func that both marks and then actually does some work, which appears to fix the problem. Custom has no alternative and still passes a single pointer in as a range, but perhaps we should just delete
customanyway given the original use for it was to integrate Boehm externally...So anyway, I would suggest reviewing this one commit at a time. The effect of this is significantly reduced memory growth / leaking, given we no longer confuse random values stored in globals as heap pointers, which I think matters quite a bit given the limited address space of tinygo's target!