Skip to content

Commit 02021b5

Browse files
jakebaileydeadprogram
authored andcommitted
runtime: preserve malloc allocations until free
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). Bump the Boehm library cache version because enabling atomic uncollectable allocations changes its compiled flags and exported API. 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.
1 parent 59fb104 commit 02021b5

23 files changed

Lines changed: 407 additions & 93 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
- gc: correct the old size calculation in block realloc
8585
- gc: correct the leaking allocator bounds and overflow checks
8686
- gc: move objHeader to the end of the block header
87+
- cgo: keep malloc allocations alive until free and stop the program for invalid or repeated free calls
8788
- fix the leaking GC build with the cores scheduler
8889
- rp2040: fix -gc=leaking and -gc=none
8990
- rp2: handle the RP2350 shared FIFO IRQ for GC (#5482)

builder/bdwgc.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ var BoehmGC = Library{
3030
// Use a minimal environment.
3131
"-DNO_MSGBOX_ON_ERROR", // don't call MessageBoxA on Windows
3232
"-DDONT_USE_ATEXIT",
33-
"-DNO_GETENV", // smaller binary, more predictable configuration
34-
"-DNO_CLOCK", // don't use system clock
35-
"-DNO_DEBUGGING", // reduce code size
36-
"-DGC_NO_FINALIZATION", // finalization is not used at the moment
33+
"-DNO_GETENV", // smaller binary, more predictable configuration
34+
"-DNO_CLOCK", // don't use system clock
35+
"-DNO_DEBUGGING", // reduce code size
36+
"-DGC_NO_FINALIZATION", // finalization is not used at the moment
37+
"-DGC_ATOMIC_UNCOLLECTABLE", // pointer-free storage retained until GC_free
3738

3839
// Special flag to work around the lack of __data_start in ld.lld.
3940
// TODO: try to fix this in LLVM/lld directly so we don't have to

builder/testdata/binary-size.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
target package code rodata data bss
2-
hifive1b examples/echo 4405 323 0 2268
3-
microbit examples/serial 2922 382 8 2264
4-
wioterminal examples/pininterrupt 8251 1717 148 7496
2+
hifive1b examples/echo 4533 323 0 2268
3+
microbit examples/serial 3002 382 8 2264
4+
wioterminal examples/pininterrupt 8331 1717 148 7496

compileopts/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
// library path in advance in several places).
2525
var libVersions = map[string]int{
2626
"musl": 3,
27-
"bdwgc": 2,
27+
"bdwgc": 3,
2828
"picolibc": 2,
2929
"wasmbuiltins": 1,
3030
}

main_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,13 @@ func TestBuild(t *testing.T) {
114114
// This makes it possible to run one specific test (instead of all),
115115
// which is especially useful to quickly check whether some changes
116116
// affect a particular target architecture.
117-
runPlatTests(optionsFromTarget(*testTarget, sema), tests, t)
117+
options := optionsFromTarget(*testTarget, sema)
118+
runPlatTests(options, tests, t)
119+
if *testTarget == "wasip1" {
120+
t.Run("cgo-realloc", func(t *testing.T) {
121+
runTest("cgo-realloc/", options, t, nil, nil)
122+
})
123+
}
118124
return
119125
}
120126

@@ -223,7 +229,11 @@ func TestBuild(t *testing.T) {
223229
})
224230
t.Run("WASIp1", func(t *testing.T) {
225231
t.Parallel()
226-
runPlatTests(optionsFromTarget("wasip1", sema), tests, t)
232+
options := optionsFromTarget("wasip1", sema)
233+
runPlatTests(options, tests, t)
234+
t.Run("cgo-realloc", func(t *testing.T) {
235+
runTest("cgo-realloc/", options, t, nil, nil)
236+
})
227237

228238
// Test with -gc=boehm.
229239
t.Run("gc.go-boehm", func(t *testing.T) {

src/runtime/arch_tinygowasm_malloc.go

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,29 @@
33
package runtime
44

55
import (
6-
"internal/gclayout"
6+
"internal/task"
77
"unsafe"
88
)
99

1010
// The below functions override the default allocator of wasi-libc. This ensures
1111
// code linked from other languages can allocate memory without colliding with
1212
// our GC allocations.
1313

14-
// Map of allocations, where the key is the allocated pointer and the value is
15-
// the size of the allocation.
16-
// TODO: make this a map[unsafe.Pointer]uintptr, since that results in slightly
17-
// smaller binaries. But for that to work, unsafe.Pointer needs to be seen as a
18-
// binary key (which it is not at the moment).
19-
// See https://github.com/tinygo-org/tinygo/pull/4898 for details.
20-
var allocs = make(map[*byte]uintptr)
14+
// Map of allocations, where the key is the allocation address and the value is
15+
// its size. Integer keys intentionally do not act as GC roots: manual
16+
// allocations are retained by the allocator until free.
17+
var allocs = make(map[uintptr]uintptr)
18+
var allocsLock task.PMutex
2119

2220
//export malloc
2321
func libc_malloc(size uintptr) unsafe.Pointer {
2422
if size == 0 {
2523
return nil
2624
}
27-
ptr := alloc(size, gclayout.NoPtrs.AsPtr())
28-
allocs[(*byte)(ptr)] = size
25+
ptr := allocManual(size)
26+
allocsLock.Lock()
27+
allocs[uintptr(ptr)] = size
28+
allocsLock.Unlock()
2929
return ptr
3030
}
3131

@@ -34,16 +34,22 @@ func libc_free(ptr unsafe.Pointer) {
3434
if ptr == nil {
3535
return
3636
}
37-
if _, ok := allocs[(*byte)(ptr)]; ok {
38-
delete(allocs, (*byte)(ptr))
37+
allocsLock.Lock()
38+
if _, ok := allocs[uintptr(ptr)]; ok {
39+
delete(allocs, uintptr(ptr))
40+
allocsLock.Unlock()
41+
freeManual(ptr)
3942
} else {
43+
allocsLock.Unlock()
4044
runtimeFatal("free: invalid pointer")
4145
}
4246
}
4347

4448
//export calloc
4549
func libc_calloc(nmemb, size uintptr) unsafe.Pointer {
46-
// No difference between calloc and malloc.
50+
if size != 0 && nmemb > ^uintptr(0)/size {
51+
return nil
52+
}
4753
return libc_malloc(nmemb * size)
4854
}
4955

@@ -54,22 +60,37 @@ func libc_realloc(oldPtr unsafe.Pointer, size uintptr) unsafe.Pointer {
5460
return nil
5561
}
5662

63+
var oldSize uintptr
64+
if oldPtr != nil {
65+
allocsLock.Lock()
66+
var ok bool
67+
oldSize, ok = allocs[uintptr(oldPtr)]
68+
allocsLock.Unlock()
69+
if !ok {
70+
runtimeFatal("realloc: invalid pointer")
71+
}
72+
}
73+
5774
// It's hard to optimize this to expand the current buffer with our GC, but
5875
// it is theoretically possible. For now, just always allocate fresh.
5976
// TODO: we could skip this if the new allocation is smaller than the old.
60-
ptr := alloc(size, gclayout.NoPtrs.AsPtr())
77+
ptr := allocManual(size)
6178

79+
allocsLock.Lock()
6280
if oldPtr != nil {
63-
if oldSize, ok := allocs[(*byte)(oldPtr)]; ok {
64-
oldBuf := unsafe.Slice((*byte)(oldPtr), oldSize)
65-
newBuf := unsafe.Slice((*byte)(ptr), size)
66-
copy(newBuf, oldBuf)
67-
delete(allocs, (*byte)(oldPtr))
68-
} else {
81+
if currentSize, ok := allocs[uintptr(oldPtr)]; !ok || currentSize != oldSize {
82+
allocsLock.Unlock()
6983
runtimeFatal("realloc: invalid pointer")
7084
}
85+
oldBuf := unsafe.Slice((*byte)(oldPtr), oldSize)
86+
newBuf := unsafe.Slice((*byte)(ptr), size)
87+
copy(newBuf, oldBuf)
88+
delete(allocs, uintptr(oldPtr))
89+
}
90+
allocs[uintptr(ptr)] = size
91+
allocsLock.Unlock()
92+
if oldPtr != nil {
93+
freeManual(oldPtr)
7194
}
72-
73-
allocs[(*byte)(ptr)] = size
7495
return ptr
7596
}

src/runtime/baremetal.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
package runtime
44

55
import (
6-
"internal/gclayout"
76
"sync/atomic"
87
"unsafe"
98
)
@@ -12,18 +11,20 @@ import (
1211
func libc_malloc(size uintptr) unsafe.Pointer {
1312
// Note: this zeroes the returned buffer which is not necessary.
1413
// The same goes for bytealg.MakeNoZero.
15-
return alloc(size, gclayout.NoPtrs.AsPtr())
14+
return allocManual(size)
1615
}
1716

1817
//export calloc
1918
func libc_calloc(nmemb, size uintptr) unsafe.Pointer {
20-
// No difference between calloc and malloc.
19+
if size != 0 && nmemb > ^uintptr(0)/size {
20+
return nil
21+
}
2122
return libc_malloc(nmemb * size)
2223
}
2324

2425
//export free
2526
func libc_free(ptr unsafe.Pointer) {
26-
free(ptr)
27+
freeManual(ptr)
2728
}
2829

2930
//export runtime_putchar

src/runtime/gc_blocks.go

Lines changed: 71 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ func (b gcBlock) free() {
205205

206206
// objHeader is a structure appended to every heap object to hold metadata.
207207
type objHeader struct {
208-
// next is the next object to scan after this.
209-
next *objHeader
208+
// next links the GC scan list. Manual allocations remain permanently marked
209+
// and use the otherwise invalid value 1 as an until-free marker.
210+
next uintptr
210211

211212
// layout holds the layout bitmap used to find pointers in the object.
212213
layout gcLayout
@@ -482,6 +483,7 @@ func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer {
482483
// Create the object header.
483484
size -= unsafe.Sizeof(objHeader{})
484485
header := (*objHeader)(unsafe.Add(pointer, size))
486+
header.next = 0
485487
header.layout = parseGCLayout(layout)
486488

487489
// We've claimed this allocation, now we can unlock the heap.
@@ -500,42 +502,61 @@ func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer {
500502
return pointer
501503
}
502504

503-
func realloc(ptr unsafe.Pointer, size uintptr) unsafe.Pointer {
504-
if ptr == nil {
505-
return alloc(size, gclayout.NoPtrs.AsPtr())
505+
// allocManual allocates pointer-free memory that remains live until freeManual.
506+
func allocManual(size uintptr) unsafe.Pointer {
507+
if size == 0 {
508+
return alloc_zero(size, gclayout.NoPtrs.AsPtr())
506509
}
510+
ptr := alloc(size, gclayout.NoPtrs.AsPtr())
507511

508-
// Find the first block of the original allocation.
509-
firstBlock := blockFromAddr(uintptr(ptr))
510-
511-
// Find the last block of the original allocation.
512-
lastBlock := firstBlock.findHead()
512+
gcLock.Lock()
513+
head := blockFromAddr(uintptr(ptr)).findHead()
514+
head.setState(blockStateMark)
515+
header := (*objHeader)(unsafe.Add(head.pointer(), bytesPerBlock-unsafe.Sizeof(objHeader{})))
516+
header.next = 1
517+
gcLock.Unlock()
518+
return ptr
519+
}
513520

514-
// Calculate the size of the original allocation body.
515-
oldSize := uintptr(lastBlock-firstBlock)*bytesPerBlock + (bytesPerBlock - unsafe.Sizeof(objHeader{}))
521+
func free(ptr unsafe.Pointer) {
522+
if ptr == nil {
523+
return
524+
}
516525

517-
if size <= oldSize {
518-
// The requested size is less than the old size.
519-
// There are likely scenarios for this:
520-
// - The caller intended to grow the allocation, but the original size
521-
// was rounded up by alloc to a multiple of the block size.
522-
// The rounded size is already sufficient.
523-
// - The caller intended to shrink the allocation.
524-
// We currently ignore this case.
525-
// Either way, the current allocation can be left alone.
526-
return ptr
526+
gcLock.Lock()
527+
addr := uintptr(ptr)
528+
if !isOnHeap(addr) || (addr-heapStart)%bytesPerBlock != 0 {
529+
gcLock.Unlock()
530+
runtimeFatal("free: invalid pointer")
527531
}
528532

529-
// Create a new allocation and copy the old data.
530-
newAlloc := alloc(size, gclayout.NoPtrs.AsPtr())
531-
memcpy(newAlloc, ptr, oldSize)
532-
free(ptr)
533+
firstBlock := blockFromAddr(addr)
534+
state := firstBlock.state()
535+
if state != blockStateTail && state != blockStateHead && state != blockStateMark {
536+
gcLock.Unlock()
537+
runtimeFatal("free: invalid pointer")
538+
}
533539

534-
return newAlloc
535-
}
540+
allocationStart := firstBlock
541+
for allocationStart != 0 && (allocationStart-1).state() == blockStateTail {
542+
allocationStart--
543+
}
544+
if allocationStart != firstBlock {
545+
gcLock.Unlock()
546+
runtimeFatal("free: invalid pointer")
547+
}
536548

537-
func free(ptr unsafe.Pointer) {
538-
// TODO: free blocks on request, when the compiler knows they're unused.
549+
lastBlock := firstBlock.findHead()
550+
header := (*objHeader)(unsafe.Add(lastBlock.pointer(), bytesPerBlock-unsafe.Sizeof(objHeader{})))
551+
if header.next != 1 {
552+
gcLock.Unlock()
553+
runtimeFatal("free: invalid pointer")
554+
}
555+
for block := firstBlock; block <= lastBlock; block++ {
556+
block.free()
557+
}
558+
insertFreeRange(firstBlock.pointer(), uintptr(lastBlock-firstBlock+1))
559+
gcLock.Unlock()
539560
}
540561

541562
// GC performs a garbage collection cycle.
@@ -666,7 +687,7 @@ func finishMark() {
666687
if obj == nil {
667688
return
668689
}
669-
scanList = obj.next
690+
scanList = (*objHeader)(unsafe.Pointer(obj.next))
670691

671692
// Check if the object may contain pointers.
672693
if obj.layout.pointerFree() {
@@ -724,7 +745,7 @@ func markRoot(addr, root uintptr) {
724745

725746
// Add the object to the scan list.
726747
header := (*objHeader)(unsafe.Add(head.pointer(), bytesPerBlock-unsafe.Sizeof(objHeader{})))
727-
header.next = scanList
748+
header.next = uintptr(unsafe.Pointer(scanList))
728749
scanList = header
729750
}
730751

@@ -758,7 +779,10 @@ func sweep() uintptr {
758779

759780
// Unmark the next head.
760781
block--
761-
block.unmark()
782+
header := (*objHeader)(unsafe.Add(block.pointer(), bytesPerBlock-unsafe.Sizeof(objHeader{})))
783+
if header.next != 1 {
784+
block.unmark()
785+
}
762786

763787
// Skip the tail.
764788
for block > 0 && (block-1).state() == blockStateTail {
@@ -903,5 +927,19 @@ func SetFinalizer(obj interface{}, finalizer interface{}) {
903927
// A nil pointer has nothing to finalize.
904928
return
905929
}
930+
931+
gcLock.Lock()
932+
addr := uintptr(objPtr)
933+
manual := false
934+
if isOnHeap(addr) {
935+
head := blockFromAddr(addr).findHead()
936+
header := (*objHeader)(unsafe.Add(head.pointer(), bytesPerBlock-unsafe.Sizeof(objHeader{})))
937+
manual = header.next == 1
938+
}
939+
gcLock.Unlock()
940+
if manual && finalizer != nil {
941+
runtimeFatal("runtime.SetFinalizer: manual allocation")
942+
}
943+
906944
registerFinalizer(uintptr(objPtr), finalizer)
907945
}

0 commit comments

Comments
 (0)