Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

compiler: fix passing weirdly-padded structs to new goroutines #4449

Merged
merged 2 commits into from
Sep 5, 2024
Merged

compiler: fix passing weirdly-padded structs to new goroutines #4449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85b64fd
compiler: move some code around to make the next bugfix easier
aykevl Aug 31, 2024
10b5f4c
compiler: fix passing weirdly-padded structs to new goroutines
aykevl Aug 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 33 additions & 31 deletions compiler/goroutine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,7 @@ import (

// createGo emits code to start a new goroutine.
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)))
}

var prefix string
var funcPtr llvm.Value
var funcType llvm.Type
hasContext := false
if callee := instr.Call.StaticCallee(); callee != nil {
// Static callee is known. This makes it easier to start a new
// goroutine.
var context llvm.Value
switch value := instr.Call.Value.(type) {
case *ssa.Function:
// Goroutine call is regular function call. No context is necessary.
case *ssa.MakeClosure:
// A goroutine call on a func value, but the callee is trivial to find. For
// example: immediately applied functions.
funcValue := b.getValue(value, getPos(instr))
context = b.extractFuncContext(funcValue)
default:
panic("StaticCallee returned an unexpected value")
}
if !context.IsNil() {
params = append(params, context) // context parameter
hasContext = true
}
funcType, funcPtr = b.getFunction(callee)
} else if builtin, ok := instr.Call.Value.(*ssa.Builtin); ok {
if builtin, ok := instr.Call.Value.(*ssa.Builtin); ok {
// We cheat. None of the builtins do any long or blocking operation, so
// we might as well run these builtins right away without the program
// noticing the difference.
Expand Down Expand Up @@ -74,6 +44,38 @@ func (b *builder) createGo(instr *ssa.Go) {
}
b.createBuiltin(argTypes, argValues, builtin.Name(), instr.Pos())
return
}

// Get all function parameters to pass to the goroutine.
var params []llvm.Value
for _, param := range instr.Call.Args {
params = append(params, b.expandFormalParam(b.getValue(param, getPos(instr)))...)
}

var prefix string
var funcPtr llvm.Value
var funcType llvm.Type
hasContext := false
if callee := instr.Call.StaticCallee(); callee != nil {
// Static callee is known. This makes it easier to start a new
// goroutine.
var context llvm.Value
switch value := instr.Call.Value.(type) {
case *ssa.Function:
// Goroutine call is regular function call. No context is necessary.
case *ssa.MakeClosure:
// A goroutine call on a func value, but the callee is trivial to find. For
// example: immediately applied functions.
funcValue := b.getValue(value, getPos(instr))
context = b.extractFuncContext(funcValue)
default:
panic("StaticCallee returned an unexpected value")
}
if !context.IsNil() {
params = append(params, context) // context parameter
hasContext = true
}
funcType, funcPtr = b.getFunction(callee)
} else if instr.Call.IsInvoke() {
// This is a method call on an interface value.
itf := b.getValue(instr.Call.Value, getPos(instr))
Expand Down
12 changes: 5 additions & 7 deletions compiler/testdata/goroutine-cortex-m-qemu-tasks.ll
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading