Skip to content

Commit

Permalink
compiler: fix passing weirdly-padded structs to new goroutines
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
aykevl authored and deadprogram committed Sep 5, 2024
1 parent ee5bc65 commit d4cb92f
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 15 deletions.
2 changes: 1 addition & 1 deletion compiler/goroutine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions compiler/testdata/goroutine-cortex-m-qemu-tasks.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions compiler/testdata/goroutine-wasm-asyncify.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 16 additions & 0 deletions testdata/goroutines.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions testdata/goroutines.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ called: Foo.Nowait
called: Foo.Wait
...waited
done with 'go on interface'
paddedStruct: 5 7

0 comments on commit d4cb92f

Please sign in to comment.