runtime: mark all global variables explicitly for GC

[jakebailey]

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 custom anyway 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!

[Copilot]

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 new can 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.

[dgryski]

The net/mail failure on Mac is legit.

Edit: Oh, whatever you just pushed fixed it.

[jakebailey]

Yeah, the global analysis was happening after an optimization that then hid the allocs.

[jakebailey]

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

[dgryski]

LGTM plus it passes my test corpus. However, I wouldn't mind @niaow or @aykevl having a second look.

[dgryski]

There doesn't appear to be an equivalent comment in runtime/hashmap.go pointing back here for sync-i-ness.

[jakebailey]

I threw copilot at this for fun and it noticed some alloc bugs that this branch would have hit due to the fact that we could more eagerly GC stuff that could actually have been alloced manually. That covers a lot of the cases marked above, actually, and I don't think I agree that the rest should be conservative anymore.

[deadprogram]

@jakebailey can you please rebase this branch and resolve the merge conflicts? Thank you.

[deadprogram]

Sorry @jakebailey one last merge conflict to address here, please.

[deadprogram]

Thanks for this work @jakebailey. Here are the findings from an automated review of all 7 commits at 91e6995d.

The design is good. There is one blocking bug and one size concern.

1. Blocking: reflect.MakeChan gives no GC layout to the channel buffer

src/internal/reflectlite/value.go:2213 still declares two parameters.

//go:linkname chanMake runtime.chanMake
func chanMake(elementSize uintptr, bufSize uintptr) unsafe.Pointer

runtime.chanMake now has three parameters (src/runtime/chan.go:140). The same commit updated hashmapMake and hashmapMakeReflect, but not chanMake. Thus the channel buffer gets an undefined elementLayout.

A test program makes a chan T with reflect.MakeChan, sends one value, and then calls runtime.GC(). On -target=wasip1 the results are these.

Build	Result
dev	prints 42 hello in approximately 1 second
PR head	no output after 5 minutes
PR head with the change below	prints 42 hello

This change corrects it.

func chanMake(elementSize uintptr, bufSize uintptr, elementLayout unsafe.Pointer) unsafe.Pointer
...
ch := chanMake(elem.Size(), uintptr(size), elem.gcLayout())

CI does not show the failure because TestTinyMakeChan does not make the GC scan the buffer. A test that makes a channel and a map through reflect and then collects would find this.

2. Size of the root table for globals with many pointers

appendGCGlobalRoots writes one word for each pointer slot. It expands each array element by element. These are the sizes for a program with one var table [4096]*int global, for -target=wasip1 -size=short.

Build	code	data	flash
dev	12152	412	12564
PR head	13202	16928	30130

This is 17.6 kB more, or 2.4 times the flash, for one global array. The general overhead is small. Hello world goes from 60658 to 62872 bytes, which is 3.6% more. binary-size.txt shows 3.6%, 3.7% and 5.8% more code. But a continuous array of pointers must cost one entry, not 4096. Please think about a start address plus count encoding, or use the existing gcLayout bitmap for each global.

The root table also keeps each mutable global that holds a pointer through the final link.

3. free() has no isOnHeap check

src/runtime/gc_blocks.go:521 tests only for nil. C code can now get to it through libc_free on bare metal, where it was a no-op before. For an address that is not in the heap, blockFromAddr underflows, because the range check is only active with gcAsserts. Then findHead reads memory outside the heap. markRoot and SetFinalizer both call isOnHeap first. free must do the same. free also accepts a pointer into the middle of an object and then releases only the last part of it.

4. Questions

• Globals outside the module. makeGCGlobalRoots finds only module globals that have an initializer. Thus the GC no longer scans C static variables in picolibc, musl and CGo objects. The cgo rules do not permit C code to keep a Go pointer, so this is most probably correct. But it is a silent change for existing bare metal C code. Please record it in the commit message or the CHANGELOG.
• The pass ignores globals for which IsGlobalConstant() is true. This is correct only if a constant global can never hold a heap pointer. Is this true for all output of interp?
• makeGCGlobalRoots runs at the end of transform.Optimize. The ThinLTO backend and lld run their own pass pipeline after it, and that pipeline includes GlobalOpt. tinygo.gc.alloc.marker is no longer in the module at that time. Can GlobalOpt promote an allocation to a global there?
• builder/build.go sets HiddenVisibility on the string globals for -ldflags -X. Please tell why in the commit message.

5. Small points

• blockGlobalAllocPromotion uses each use of runtime.alloc as a call and reads call.Operand(1). A test for a call instruction is safer.
• libc_realloc allocates before it validates oldPtr. Thus the runtimeFatal("realloc: invalid pointer") path loses the new allocation.
• src/runtime/gc_custom.go:33 still shows realloc as a necessary function for a custom collector, but nothing calls it now.
• gclayout.Conservative is 2, and interp rejects it with the layout%2 != 1 test. Such an allocation must never get to interp. Please add a short comment.
• Only gc.custom uses findGlobals and os_windows_pe.go now. You can put build tags on them.

Verdict

Request changes. Item 1 is a correctness bug that GC can make into a hang. Item 2 can make a large increase in flash use for programs that have globals with many pointers. Items 3 to 5 are smaller.

[deadprogram]

@jakebailey please let me know when this is ready for next review.

[jakebailey]

It should be

[deadprogram]

Thanks for the quick changes @jakebailey. Here are the findings from an automated review of the new revision at 8cb0ef64.

Status of the earlier items

All of them are corrected.

• reflect.MakeChan now sends the element layout, and TestTinyMakeChan has a subtest that collects while the buffer holds a pointer. The test program from the earlier comment now prints 42 hello on -target=wasip1.
• The root table now holds (address, size) ranges and joins adjacent pointer slots. For the test program with one var table [4096]*int global, the flash size for -target=wasip1 goes down from 30130 bytes to 14091 bytes. The base is 12564 bytes.
• free now tests isOnHeap, block alignment, block state, the start of the allocation, and the manual marker.
• The other points are done. These are the call instruction test with a new unit test, the realloc validation before the allocation, the gc_custom.go text, the interp comment, and the visibility comment with the LangRef link. The removal of all findGlobals code is more than the earlier comment asked for.

New: the compiler stops on AVR for a global with a pointer at an odd offset

AVR has a pointer alignment of one byte. The data layout is p:16:8. Thus a pointer in a global structure can be at an odd offset. appendGCGlobalRootRange does not permit this.

package main

type entry struct {
	flag byte
	ptr  *int
}

var table [4]entry

//go:export getEntry
func getEntry(i int) *entry {
	return &table[i]
}

func main() {
	x := 1
	e := getEntry(1)
	e.flag = 2
	e.ptr = &x
	println(e.flag, *e.ptr)
}

tinygo build -target=arduino on this program gives a panic.

panic: global root range is not pointer aligned

goroutine 178 [running]:
github.com/tinygo-org/tinygo/transform.appendGCGlobalRootRange(...)
	transform/gc.go:413
github.com/tinygo-org/tinygo/transform.appendGCGlobalRootRanges(...)
	transform/gc.go:404
github.com/tinygo-org/tinygo/transform.makeGCGlobalRoots(...)
	transform/gc.go:319

The same program builds correctly for -target=microbit and -target=wasip1, because these targets align pointers to the pointer size.

The AVR smoke tests do not find this, because the optimizer removes such globals when the program does not use them. The //go:export function keeps the global.

Two possible corrections are these. Permit an unaligned range on targets where the pointer alignment is less than the pointer size, because AVR permits unaligned loads. Or scan the full global conservatively in this condition. In all conditions the compiler must give a diagnostic message and not a panic.

A test with an AVR data layout in transform finds this. This IR is sufficient.

target datalayout = "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8:16-a:8"
target triple = "avr-unknown-unknown"

%entry = type { i8, ptr }
@table = global [4 x %entry] zeroinitializer

Note on free

free now stops the program with runtimeFatal for a pointer that did not come from malloc. On bare metal this function did nothing before. Thus C code that frees static storage, or that frees the same address two times, now stops the program at runtime. This is correct behavior for the C standard, but it is a change. Please record it in the CHANGELOG.

Verdict

Approve after the AVR panic is corrected. All items from the first review are done.

[jakebailey]

Fixed both, PTAL

[deadprogram]

Looking good, thank you for all the work on this @jakebailey all feedback has been addressed so now merging!