From d4cb92f27c48148bc269308ff6973338b6f1f7c9 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 31 Aug 2024 13:25:42 +0200 Subject: [PATCH] compiler: fix passing weirdly-padded structs to new goroutines The values were stored in the passed object as the values itself (not expanded like is common in the calling convention), and read back after assuming they were expanded. This often works for simple parameters (int, pointer, etc), but not for more complex parameters. Especially when there's padding. Found this while working on `//go:wasmexport`. --- compiler/goroutine.go | 2 +- .../testdata/goroutine-cortex-m-qemu-tasks.ll | 12 +++++------- compiler/testdata/goroutine-wasm-asyncify.ll | 12 +++++------- testdata/goroutines.go | 16 ++++++++++++++++ testdata/goroutines.txt | 1 + 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/compiler/goroutine.go b/compiler/goroutine.go index 60fdfaabf5..a235563450 100644 --- a/compiler/goroutine.go +++ b/compiler/goroutine.go @@ -49,7 +49,7 @@ func (b *builder) createGo(instr *ssa.Go) { // Get all function parameters to pass to the goroutine. var params []llvm.Value for _, param := range instr.Call.Args { - params = append(params, b.getValue(param, getPos(instr))) + params = append(params, b.expandFormalParam(b.getValue(param, getPos(instr)))...) } var prefix string diff --git a/compiler/testdata/goroutine-cortex-m-qemu-tasks.ll b/compiler/testdata/goroutine-cortex-m-qemu-tasks.ll index f0a692c0f2..f149f3a0cf 100644 --- a/compiler/testdata/goroutine-cortex-m-qemu-tasks.ll +++ b/compiler/testdata/goroutine-cortex-m-qemu-tasks.ll @@ -3,8 +3,6 @@ source_filename = "goroutine.go" target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" target triple = "thumbv7m-unknown-unknown-eabi" -%runtime._string = type { ptr, i32 } - @"main$string" = internal unnamed_addr constant [4 x i8] c"test", align 1 ; Function Attrs: allockind("alloc,zeroed") allocsize(0) @@ -150,12 +148,12 @@ define hidden void @main.startInterfaceMethod(ptr %itf.typecode, ptr %itf.value, entry: %0 = call align 4 dereferenceable(16) ptr @runtime.alloc(i32 16, ptr null, ptr undef) #9 store ptr %itf.value, ptr %0, align 4 - %1 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 1 + %1 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 1 store ptr @"main$string", ptr %1, align 4 - %.repack1 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 1, i32 1 - store i32 4, ptr %.repack1, align 4 - %2 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 2 - store ptr %itf.typecode, ptr %2, align 4 + %2 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 2 + store i32 4, ptr %2, align 4 + %3 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 3 + store ptr %itf.typecode, ptr %3, align 4 %stacksize = call i32 @"internal/task.getGoroutineStackSize"(i32 ptrtoint (ptr @"interface:{Print:func:{basic:string}{}}.Print$invoke$gowrapper" to i32), ptr undef) #9 call void @"internal/task.start"(i32 ptrtoint (ptr @"interface:{Print:func:{basic:string}{}}.Print$invoke$gowrapper" to i32), ptr nonnull %0, i32 %stacksize, ptr undef) #9 ret void diff --git a/compiler/testdata/goroutine-wasm-asyncify.ll b/compiler/testdata/goroutine-wasm-asyncify.ll index 1281857d31..699b9f2057 100644 --- a/compiler/testdata/goroutine-wasm-asyncify.ll +++ b/compiler/testdata/goroutine-wasm-asyncify.ll @@ -3,8 +3,6 @@ source_filename = "goroutine.go" target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20" target triple = "wasm32-unknown-wasi" -%runtime._string = type { ptr, i32 } - @"main$string" = internal unnamed_addr constant [4 x i8] c"test", align 1 ; Function Attrs: allockind("alloc,zeroed") allocsize(0) @@ -161,12 +159,12 @@ entry: %0 = call align 4 dereferenceable(16) ptr @runtime.alloc(i32 16, ptr null, ptr undef) #9 call void @runtime.trackPointer(ptr nonnull %0, ptr nonnull %stackalloc, ptr undef) #9 store ptr %itf.value, ptr %0, align 4 - %1 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 1 + %1 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 1 store ptr @"main$string", ptr %1, align 4 - %.repack1 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 1, i32 1 - store i32 4, ptr %.repack1, align 4 - %2 = getelementptr inbounds { ptr, %runtime._string, ptr }, ptr %0, i32 0, i32 2 - store ptr %itf.typecode, ptr %2, align 4 + %2 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 2 + store i32 4, ptr %2, align 4 + %3 = getelementptr inbounds { ptr, ptr, i32, ptr }, ptr %0, i32 0, i32 3 + store ptr %itf.typecode, ptr %3, align 4 call void @"internal/task.start"(i32 ptrtoint (ptr @"interface:{Print:func:{basic:string}{}}.Print$invoke$gowrapper" to i32), ptr nonnull %0, i32 65536, ptr undef) #9 ret void } diff --git a/testdata/goroutines.go b/testdata/goroutines.go index 66abc54fde..5ac3b73187 100644 --- a/testdata/goroutines.go +++ b/testdata/goroutines.go @@ -86,6 +86,10 @@ func main() { testCond() testIssue1790() + + done := make(chan int) + go testPaddedParameters(paddedStruct{x: 5, y: 7}, done) + <-done } func acquire(m *sync.Mutex) { @@ -243,3 +247,15 @@ func (f Foo) Wait() { time.Sleep(time.Microsecond) println(" ...waited") } + +type paddedStruct struct { + x uint8 + _ [0]int64 + y uint8 +} + +// Structs with interesting padding used to crash. +func testPaddedParameters(s paddedStruct, done chan int) { + println("paddedStruct:", s.x, s.y) + close(done) +} diff --git a/testdata/goroutines.txt b/testdata/goroutines.txt index 35c0cd44dd..1430ee0a20 100644 --- a/testdata/goroutines.txt +++ b/testdata/goroutines.txt @@ -26,3 +26,4 @@ called: Foo.Nowait called: Foo.Wait ...waited done with 'go on interface' +paddedStruct: 5 7