From b3d1d87fd762119ba9085475d678f75400b88d31 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 21 Nov 2022 11:26:31 +0900 Subject: [PATCH 1/9] Add -gc=custom option --- compileopts/config.go | 2 ++ compileopts/options.go | 2 +- compileopts/options_test.go | 6 ++++ src/internal/task/gc_stack_chain.go | 5 +-- src/internal/task/gc_stack_noop.go | 4 +-- src/runtime/arch_tinygowasm_malloc.go | 4 +-- src/runtime/gc_custom.go | 49 +++++++++++++++++++++++++++ src/runtime/gc_stack_portable.go | 21 +++++++----- 8 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 src/runtime/gc_custom.go diff --git a/compileopts/config.go b/compileopts/config.go index 6eaf5d6239..9f7586aec4 100644 --- a/compileopts/config.go +++ b/compileopts/config.go @@ -106,6 +106,8 @@ func (c *Config) GC() string { func (c *Config) NeedsStackObjects() bool { switch c.GC() { case "conservative": + fallthrough + case "custom": for _, tag := range c.BuildTags() { if tag == "tinygo.wasm" { return true diff --git a/compileopts/options.go b/compileopts/options.go index 11d8d40b23..ec20106265 100644 --- a/compileopts/options.go +++ b/compileopts/options.go @@ -8,7 +8,7 @@ import ( ) var ( - validGCOptions = []string{"none", "leaking", "conservative"} + validGCOptions = []string{"none", "leaking", "conservative", "custom"} validSchedulerOptions = []string{"none", "tasks", "asyncify"} validSerialOptions = []string{"none", "uart", "usb"} validPrintSizeOptions = []string{"none", "short", "full"} diff --git a/compileopts/options_test.go b/compileopts/options_test.go index 2845be3a42..25e75f9fe9 100644 --- a/compileopts/options_test.go +++ b/compileopts/options_test.go @@ -48,6 +48,12 @@ func TestVerifyOptions(t *testing.T) { GC: "conservative", }, }, + { + name: "GCOptionCustom", + opts: compileopts.Options{ + GC: "custom", + }, + }, { name: "InvalidSchedulerOption", opts: compileopts.Options{ diff --git a/src/internal/task/gc_stack_chain.go b/src/internal/task/gc_stack_chain.go index 07588d4093..db1c2c718b 100644 --- a/src/internal/task/gc_stack_chain.go +++ b/src/internal/task/gc_stack_chain.go @@ -1,5 +1,6 @@ -//go:build gc.conservative && tinygo.wasm -// +build gc.conservative,tinygo.wasm +//go:build (gc.conservative || gc.custom) && tinygo.wasm +// +build gc.conservative gc.custom +// +build tinygo.wasm package task diff --git a/src/internal/task/gc_stack_noop.go b/src/internal/task/gc_stack_noop.go index b1cfd44836..5609241d2b 100644 --- a/src/internal/task/gc_stack_noop.go +++ b/src/internal/task/gc_stack_noop.go @@ -1,5 +1,5 @@ -//go:build !gc.conservative || !tinygo.wasm -// +build !gc.conservative !tinygo.wasm +//go:build (!gc.conservative && !gc.custom) || !tinygo.wasm +// +build !gc.conservative,!gc.custom !tinygo.wasm package task diff --git a/src/runtime/arch_tinygowasm_malloc.go b/src/runtime/arch_tinygowasm_malloc.go index f7f0c5a2c5..85e64fba14 100644 --- a/src/runtime/arch_tinygowasm_malloc.go +++ b/src/runtime/arch_tinygowasm_malloc.go @@ -1,5 +1,5 @@ -//go:build tinygo.wasm && !custommalloc -// +build tinygo.wasm,!custommalloc +//go:build tinygo.wasm && !gc.custom +// +build tinygo.wasm,!gc.custom package runtime diff --git a/src/runtime/gc_custom.go b/src/runtime/gc_custom.go new file mode 100644 index 0000000000..801ca4d035 --- /dev/null +++ b/src/runtime/gc_custom.go @@ -0,0 +1,49 @@ +//go:build gc.custom +// +build gc.custom + +package runtime + +// This GC strategy allows an external GC to be plugged in instead of the builtin +// implementations. +// +// The custom implementation must provide the following functions in the runtime package +// using the go:linkname directive: +// +// - func initHeap() +// - func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer +// - func free(ptr unsafe.Pointer) +// - func GC() +// - func KeepAlive(x interface{}) +// - func SetFinalizer(obj interface{}, finalizer interface{}) +// +// In addition, if targeting wasi, the following functions should be exported for interoperability +// with wasi libraries that use them. Note, this requires the export directive, not go:linkname. +// +// - func malloc(size uintptr) unsafe.Pointer +// - func free(ptr unsafe.Pointer) +// - func calloc(nmemb, size uintptr) unsafe.Pointer +// - func realloc(oldPtr unsafe.Pointer, size uintptr) unsafe.Pointer + +import ( + "unsafe" +) + +func initHeap() + +func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer + +func free(ptr unsafe.Pointer) + +func GC() + +func KeepAlive(x interface{}) + +func SetFinalizer(obj interface{}, finalizer interface{}) + +func setHeapEnd(newHeapEnd uintptr) { + // Heap is in custom GC so ignore for when called from wasm initialization. +} + +func markRoots(start, end uintptr) { + // GC manages roots so ignore to allow gc_stack_portable to compile +} diff --git a/src/runtime/gc_stack_portable.go b/src/runtime/gc_stack_portable.go index b1073e4fa3..7d6bb7c589 100644 --- a/src/runtime/gc_stack_portable.go +++ b/src/runtime/gc_stack_portable.go @@ -1,5 +1,6 @@ -//go:build gc.conservative && tinygo.wasm -// +build gc.conservative,tinygo.wasm +//go:build (gc.conservative || gc.custom) && tinygo.wasm +// +build gc.conservative gc.custom +// +build tinygo.wasm package runtime @@ -39,13 +40,6 @@ type stackChainObject struct { // stackChainStart. Luckily we don't need to scan these, as these globals are // stored on the goroutine stack and are therefore already getting scanned. func markStack() { - // Hack to force LLVM to consider stackChainStart to be live. - // Without this hack, loads and stores may be considered dead and objects on - // the stack might not be correctly tracked. With this volatile load, LLVM - // is forced to consider stackChainStart (and everything it points to) as - // live. - volatile.LoadUint32((*uint32)(unsafe.Pointer(&stackChainStart))) - if task.OnSystemStack() { markRoots(getCurrentStackPointer(), stackTop) } @@ -61,3 +55,12 @@ func trackPointer(ptr unsafe.Pointer) func swapStackChain(dst **stackChainObject) { *dst, stackChainStart = stackChainStart, *dst } + +func init() { + // Hack to force LLVM to consider stackChainStart to be live. + // Without this hack, loads and stores may be considered dead and objects on + // the stack might not be correctly tracked. With this volatile load, LLVM + // is forced to consider stackChainStart (and everything it points to) as + // live. + volatile.LoadUint32((*uint32)(unsafe.Pointer(&stackChainStart))) +} From 74aed5e7916ea67f7f1dee17639c9e2931a2a70c Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 21 Nov 2022 11:57:26 +0900 Subject: [PATCH 2/9] Drift --- compileopts/options_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compileopts/options_test.go b/compileopts/options_test.go index 25e75f9fe9..6c2a7d72be 100644 --- a/compileopts/options_test.go +++ b/compileopts/options_test.go @@ -9,7 +9,7 @@ import ( func TestVerifyOptions(t *testing.T) { - expectedGCError := errors.New(`invalid gc option 'incorrect': valid values are none, leaking, conservative`) + expectedGCError := errors.New(`invalid gc option 'incorrect': valid values are none, leaking, conservative, custom`) expectedSchedulerError := errors.New(`invalid scheduler option 'incorrect': valid values are none, tasks, asyncify`) expectedPrintSizeError := errors.New(`invalid size option 'incorrect': valid values are none, short, full`) expectedPanicStrategyError := errors.New(`invalid panic option 'incorrect': valid values are print, trap`) From 79f62b28c536e16292dc816e1149fe0e3bc48ce6 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 22 Nov 2022 10:34:41 +0900 Subject: [PATCH 3/9] Load stack in init and markStack --- src/runtime/gc_stack_portable.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/runtime/gc_stack_portable.go b/src/runtime/gc_stack_portable.go index 7d6bb7c589..0a6f3c274a 100644 --- a/src/runtime/gc_stack_portable.go +++ b/src/runtime/gc_stack_portable.go @@ -40,6 +40,7 @@ type stackChainObject struct { // stackChainStart. Luckily we don't need to scan these, as these globals are // stored on the goroutine stack and are therefore already getting scanned. func markStack() { + loadStackChain() if task.OnSystemStack() { markRoots(getCurrentStackPointer(), stackTop) } @@ -57,6 +58,14 @@ func swapStackChain(dst **stackChainObject) { } func init() { + // At least currently, it is enough to load the stack chain once in the app to make + // sure stack tracking is emitted by LLVM, which will mostly be important when using + // gc=custom. In the future, this may change if LLVM optimizes away the stack chain + // because the load happens before the writes and we may need to revisit. + loadStackChain() +} + +func loadStackChain() { // Hack to force LLVM to consider stackChainStart to be live. // Without this hack, loads and stores may be considered dead and objects on // the stack might not be correctly tracked. With this volatile load, LLVM From d0124d7bc0417e6212186c3c724ddbc868af0416 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 13 Dec 2022 10:39:35 +0900 Subject: [PATCH 4/9] Update compileopts/config.go Co-authored-by: Ayke --- compileopts/config.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compileopts/config.go b/compileopts/config.go index 9f7586aec4..010c8e09ed 100644 --- a/compileopts/config.go +++ b/compileopts/config.go @@ -105,9 +105,7 @@ func (c *Config) GC() string { // that can be traced by the garbage collector. func (c *Config) NeedsStackObjects() bool { switch c.GC() { - case "conservative": - fallthrough - case "custom": + case "conservative", "custom": for _, tag := range c.BuildTags() { if tag == "tinygo.wasm" { return true From d9b9d9472b3bbc5f58e850ca2209d29a67dc9c81 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 16 Dec 2022 10:32:33 +0900 Subject: [PATCH 5/9] Revert loading stack change and require addRoots --- src/runtime/gc_custom.go | 20 ++++++++++++++++---- src/runtime/gc_stack_portable.go | 24 ++++++------------------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/runtime/gc_custom.go b/src/runtime/gc_custom.go index 801ca4d035..044a0ba29b 100644 --- a/src/runtime/gc_custom.go +++ b/src/runtime/gc_custom.go @@ -6,16 +6,21 @@ package runtime // This GC strategy allows an external GC to be plugged in instead of the builtin // implementations. // +// The interface defined in this file is not stable and can be broken at anytime, even +// across minor versions. +// // The custom implementation must provide the following functions in the runtime package // using the go:linkname directive: // // - func initHeap() // - func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer // - func free(ptr unsafe.Pointer) +// - func markRoots(start, end uintptr) // - func GC() // - func KeepAlive(x interface{}) // - func SetFinalizer(obj interface{}, finalizer interface{}) // +// // In addition, if targeting wasi, the following functions should be exported for interoperability // with wasi libraries that use them. Note, this requires the export directive, not go:linkname. // @@ -28,22 +33,29 @@ import ( "unsafe" ) +// initHeap is called when the heap is first initialized at program start. func initHeap() +// alloc is called to allocate memory. layout is currently not used. func alloc(size uintptr, layout unsafe.Pointer) unsafe.Pointer +// free is called to explicitly free a previously allocated pointer. func free(ptr unsafe.Pointer) +// markRoots is called with the start and end addresses to scan for references. +// It is currently only called with the top and bottom of the stack. +func markRoots(start, end uintptr) + +// GC is called to explicitly run garbage collection. func GC() +// KeepAlive is called to ensure the given object is kept alive until this function call. func KeepAlive(x interface{}) +// SetFinalizer is called to set a finalizer method to run when an object is about to be +// cleaned. func SetFinalizer(obj interface{}, finalizer interface{}) func setHeapEnd(newHeapEnd uintptr) { // Heap is in custom GC so ignore for when called from wasm initialization. } - -func markRoots(start, end uintptr) { - // GC manages roots so ignore to allow gc_stack_portable to compile -} diff --git a/src/runtime/gc_stack_portable.go b/src/runtime/gc_stack_portable.go index 0a6f3c274a..6b498f98ea 100644 --- a/src/runtime/gc_stack_portable.go +++ b/src/runtime/gc_stack_portable.go @@ -40,7 +40,12 @@ type stackChainObject struct { // stackChainStart. Luckily we don't need to scan these, as these globals are // stored on the goroutine stack and are therefore already getting scanned. func markStack() { - loadStackChain() + // Hack to force LLVM to consider stackChainStart to be live. + // Without this hack, loads and stores may be considered dead and objects on + // the stack might not be correctly tracked. With this volatile load, LLVM + // is forced to consider stackChainStart (and everything it points to) as + // live. + volatile.LoadUint32((*uint32)(unsafe.Pointer(&stackChainStart))) if task.OnSystemStack() { markRoots(getCurrentStackPointer(), stackTop) } @@ -56,20 +61,3 @@ func trackPointer(ptr unsafe.Pointer) func swapStackChain(dst **stackChainObject) { *dst, stackChainStart = stackChainStart, *dst } - -func init() { - // At least currently, it is enough to load the stack chain once in the app to make - // sure stack tracking is emitted by LLVM, which will mostly be important when using - // gc=custom. In the future, this may change if LLVM optimizes away the stack chain - // because the load happens before the writes and we may need to revisit. - loadStackChain() -} - -func loadStackChain() { - // Hack to force LLVM to consider stackChainStart to be live. - // Without this hack, loads and stores may be considered dead and objects on - // the stack might not be correctly tracked. With this volatile load, LLVM - // is forced to consider stackChainStart (and everything it points to) as - // live. - volatile.LoadUint32((*uint32)(unsafe.Pointer(&stackChainStart))) -} From 63b29440bab1f55f8eb0a0cb30bf8e15bd35e1d4 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 16 Dec 2022 10:35:04 +0900 Subject: [PATCH 6/9] Revert newline --- lib/mingw-w64 | 2 +- src/runtime/gc_stack_portable.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mingw-w64 b/lib/mingw-w64 index 8526cb6182..acc9b9d9eb 160000 --- a/lib/mingw-w64 +++ b/lib/mingw-w64 @@ -1 +1 @@ -Subproject commit 8526cb618269440a94810b94b77f8bd48c5c3396 +Subproject commit acc9b9d9eb63a13d8122cbac4882eb5f4ee2f679 diff --git a/src/runtime/gc_stack_portable.go b/src/runtime/gc_stack_portable.go index 6b498f98ea..3999e79c24 100644 --- a/src/runtime/gc_stack_portable.go +++ b/src/runtime/gc_stack_portable.go @@ -46,6 +46,7 @@ func markStack() { // is forced to consider stackChainStart (and everything it points to) as // live. volatile.LoadUint32((*uint32)(unsafe.Pointer(&stackChainStart))) + if task.OnSystemStack() { markRoots(getCurrentStackPointer(), stackTop) } From 36a73df8a1e4748c409370a78c795475cd207bab Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 16 Dec 2022 10:38:39 +0900 Subject: [PATCH 7/9] Revert unintended mingw update --- lib/mingw-w64 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mingw-w64 b/lib/mingw-w64 index acc9b9d9eb..8526cb6182 160000 --- a/lib/mingw-w64 +++ b/lib/mingw-w64 @@ -1 +1 @@ -Subproject commit acc9b9d9eb63a13d8122cbac4882eb5f4ee2f679 +Subproject commit 8526cb618269440a94810b94b77f8bd48c5c3396 From f58fe76081cf3af88b1743131788b6bc4cc72b94 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 19 Dec 2022 10:41:15 +0900 Subject: [PATCH 8/9] Remove methods that were moved --- src/runtime/gc_custom.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/runtime/gc_custom.go b/src/runtime/gc_custom.go index 044a0ba29b..2993724df1 100644 --- a/src/runtime/gc_custom.go +++ b/src/runtime/gc_custom.go @@ -17,8 +17,6 @@ package runtime // - func free(ptr unsafe.Pointer) // - func markRoots(start, end uintptr) // - func GC() -// - func KeepAlive(x interface{}) -// - func SetFinalizer(obj interface{}, finalizer interface{}) // // // In addition, if targeting wasi, the following functions should be exported for interoperability @@ -49,13 +47,6 @@ func markRoots(start, end uintptr) // GC is called to explicitly run garbage collection. func GC() -// KeepAlive is called to ensure the given object is kept alive until this function call. -func KeepAlive(x interface{}) - -// SetFinalizer is called to set a finalizer method to run when an object is about to be -// cleaned. -func SetFinalizer(obj interface{}, finalizer interface{}) - func setHeapEnd(newHeapEnd uintptr) { // Heap is in custom GC so ignore for when called from wasm initialization. } From 857e613d8483f3cb8b829658e4799843ba48361f Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 19 Dec 2022 11:47:21 +0900 Subject: [PATCH 9/9] Document runtime.markStack --- src/runtime/gc_custom.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/runtime/gc_custom.go b/src/runtime/gc_custom.go index 2993724df1..0d538619bf 100644 --- a/src/runtime/gc_custom.go +++ b/src/runtime/gc_custom.go @@ -9,6 +9,9 @@ package runtime // The interface defined in this file is not stable and can be broken at anytime, even // across minor versions. // +// runtime.markStack() must be called at the beginning of any GC cycle. //go:linkname +// on a function without a body can be used to access this internal function. +// // The custom implementation must provide the following functions in the runtime package // using the go:linkname directive: //