diff --git a/internal/engine/compiler/compiler_controlflow_test.go b/internal/engine/compiler/compiler_controlflow_test.go index 132a25ecb4..77c68abd4d 100644 --- a/internal/engine/compiler/compiler_controlflow_test.go +++ b/internal/engine/compiler/compiler_controlflow_test.go @@ -659,7 +659,7 @@ func TestCompiler_compileCallIndirect(t *testing.T) { env.addTable(&wasm.TableInstance{References: table}) cf := &function{typeID: 50} - table[0] = uintptr(unsafe.Pointer(cf)) + table[0] = unsafe.Pointer(cf) // Place the offset value. err = compiler.compileConstI32(targetOffset) @@ -737,7 +737,7 @@ func TestCompiler_compileCallIndirect(t *testing.T) { moduleInstance: env.moduleInstance, typeID: targetTypeID, } - table[i] = uintptr(unsafe.Pointer(&me.functions[i])) + table[i] = unsafe.Pointer(&me.functions[i]) } // Test to ensure that we can call all the functions stored in the table. @@ -827,7 +827,7 @@ func TestCompiler_callIndirect_largeTypeIndex(t *testing.T) { moduleInstance: env.moduleInstance, } me.functions = append(me.functions, f) - table[0] = uintptr(unsafe.Pointer(&f)) + table[0] = unsafe.Pointer(&f) } compiler := env.requireNewCompiler(t, &wasm.FunctionType{}, newCompiler, &wazeroir.CompilationResult{ diff --git a/internal/engine/compiler/compiler_post1_0_test.go b/internal/engine/compiler/compiler_post1_0_test.go index b76be97513..d0f3a13a79 100644 --- a/internal/engine/compiler/compiler_post1_0_test.go +++ b/internal/engine/compiler/compiler_post1_0_test.go @@ -476,7 +476,7 @@ func TestCompiler_compileMemoryInit(t *testing.T) { } func TestCompiler_compileElemDrop(t *testing.T) { - origins := []wasm.ElementInstance{{1}, {2}, {3}, {4}, {5}} + origins := []wasm.ElementInstance{{asReference(1)}, {asReference(2)}, {asReference(3)}, {asReference(4)}, {asReference(5)}} for i := 0; i < len(origins); i++ { t.Run(strconv.Itoa(i), func(t *testing.T) { @@ -595,7 +595,7 @@ func TestCompiler_compileTableCopy(t *testing.T) { table := make([]wasm.Reference, tableSize) env.addTable(&wasm.TableInstance{References: table}) for i := 0; i < tableSize; i++ { - table[i] = uintptr(i) + table[i] = asReference(uint(i)) } // Run code. @@ -604,7 +604,7 @@ func TestCompiler_compileTableCopy(t *testing.T) { if !tc.requireOutOfBoundsError { exp := make([]wasm.Reference, tableSize) for i := 0; i < tableSize; i++ { - exp[i] = uintptr(i) + exp[i] = asReference(uint(i)) } copy(exp[tc.destOffset:], exp[tc.sourceOffset:tc.sourceOffset+tc.size]) @@ -621,7 +621,7 @@ func TestCompiler_compileTableCopy(t *testing.T) { func TestCompiler_compileTableInit(t *testing.T) { elementInstances := []wasm.ElementInstance{ - {}, {1, 2, 3, 4, 5}, + {}, {asReference(1), asReference(2), asReference(3), asReference(4), asReference(5)}, } const tableSize = 100 @@ -680,7 +680,7 @@ func TestCompiler_compileTableInit(t *testing.T) { table := make([]wasm.Reference, tableSize) env.addTable(&wasm.TableInstance{References: table}) for i := 0; i < tableSize; i++ { - table[i] = uintptr(i) + table[i] = asReference(uint(i)) } code := asm.CodeSegment{} @@ -699,7 +699,7 @@ func TestCompiler_compileTableInit(t *testing.T) { require.Equal(t, nativeCallStatusCodeReturned, env.compilerStatus()) exp := make([]wasm.Reference, tableSize) for i := 0; i < tableSize; i++ { - exp[i] = uintptr(i) + exp[i] = asReference(uint(i)) } if inst := elementInstances[tc.elemIndex]; inst != nil { copy(exp[tc.destOffset:], inst[tc.sourceOffset:tc.sourceOffset+tc.copySize]) @@ -716,19 +716,19 @@ type dog struct{ name string } func TestCompiler_compileTableSet(t *testing.T) { externDog := &dog{name: "sushi"} - externrefOpaque := uintptr(unsafe.Pointer(externDog)) + externrefOpaque := unsafe.Pointer(externDog) funcref := &function{moduleInstance: &wasm.ModuleInstance{}} - funcrefOpaque := uintptr(unsafe.Pointer(funcref)) + funcrefOpaque := unsafe.Pointer(funcref) - externTable := &wasm.TableInstance{Type: wasm.RefTypeExternref, References: []wasm.Reference{0, 0, externrefOpaque, 0, 0}} - funcrefTable := &wasm.TableInstance{Type: wasm.RefTypeFuncref, References: []wasm.Reference{0, 0, 0, 0, funcrefOpaque}} + externTable := &wasm.TableInstance{Type: wasm.RefTypeExternref, References: []wasm.Reference{nil, nil, externrefOpaque, nil, nil}} + funcrefTable := &wasm.TableInstance{Type: wasm.RefTypeFuncref, References: []wasm.Reference{nil, nil, nil, nil, funcrefOpaque}} tables := []*wasm.TableInstance{externTable, funcrefTable} tests := []struct { name string tableIndex uint32 offset uint32 - in uintptr + in unsafe.Pointer expExtern bool expError bool }{ @@ -743,14 +743,14 @@ func TestCompiler_compileTableSet(t *testing.T) { name: "externref - nil", tableIndex: 0, offset: 1, - in: 0, + in: nil, expExtern: true, }, { name: "externref - out of bounds", tableIndex: 0, offset: 10, - in: 0, + in: nil, expError: true, }, { @@ -764,14 +764,14 @@ func TestCompiler_compileTableSet(t *testing.T) { name: "funcref - nil", tableIndex: 1, offset: 3, - in: 0, + in: nil, expExtern: false, }, { name: "funcref - out of bounds", tableIndex: 1, offset: 100000, - in: 0, + in: nil, expError: true, }, } @@ -795,7 +795,7 @@ func TestCompiler_compileTableSet(t *testing.T) { err = compiler.compileConstI32(operationPtr(wazeroir.NewOperationConstI32(tc.offset))) require.NoError(t, err) - err = compiler.compileConstI64(operationPtr(wazeroir.NewOperationConstI64(uint64(tc.in)))) + err = compiler.compileConstI64(operationPtr(wazeroir.NewOperationConstI64(uint64(uintptr(tc.in))))) require.NoError(t, err) err = compiler.compileTableSet(operationPtr(wazeroir.NewOperationTableSet(tc.tableIndex))) @@ -822,14 +822,14 @@ func TestCompiler_compileTableSet(t *testing.T) { if tc.expExtern { actual := dogFromPtr(externTable.References[tc.offset]) exp := externDog - if tc.in == 0 { + if tc.in == nil { exp = nil } require.Equal(t, exp, actual) } else { actual := functionFromPtr(funcrefTable.References[tc.offset]) exp := funcref - if tc.in == 0 { + if tc.in == nil { exp = nil } require.Equal(t, exp, actual) @@ -840,36 +840,36 @@ func TestCompiler_compileTableSet(t *testing.T) { } //go:nocheckptr ignore "pointer arithmetic result points to invalid allocation" -func dogFromPtr(ptr uintptr) *dog { - if ptr == 0 { +func dogFromPtr(ptr unsafe.Pointer) *dog { + if ptr == nil { return nil } - return (*dog)(unsafe.Pointer(ptr)) + return (*dog)(ptr) } //go:nocheckptr ignore "pointer arithmetic result points to invalid allocation" -func functionFromPtr(ptr uintptr) *function { - if ptr == 0 { +func functionFromPtr(ptr unsafe.Pointer) *function { + if ptr == nil { return nil } - return (*function)(unsafe.Pointer(ptr)) + return (*function)(ptr) } func TestCompiler_compileTableGet(t *testing.T) { externDog := &dog{name: "sushi"} - externrefOpaque := uintptr(unsafe.Pointer(externDog)) + externrefOpaque := unsafe.Pointer(externDog) funcref := &function{moduleInstance: &wasm.ModuleInstance{}} - funcrefOpaque := uintptr(unsafe.Pointer(funcref)) + funcrefOpaque := unsafe.Pointer(funcref) tables := []*wasm.TableInstance{ - {Type: wasm.RefTypeExternref, References: []wasm.Reference{0, 0, externrefOpaque, 0, 0}}, - {Type: wasm.RefTypeFuncref, References: []wasm.Reference{0, 0, 0, 0, funcrefOpaque}}, + {Type: wasm.RefTypeExternref, References: []wasm.Reference{nil, nil, externrefOpaque, nil, nil}}, + {Type: wasm.RefTypeFuncref, References: []wasm.Reference{nil, nil, nil, nil, funcrefOpaque}}, } tests := []struct { name string tableIndex uint32 offset uint32 - exp uintptr + exp unsafe.Pointer expError bool }{ { @@ -882,7 +882,7 @@ func TestCompiler_compileTableGet(t *testing.T) { name: "externref - nil", tableIndex: 0, offset: 4, - exp: 0, + exp: nil, }, { name: "externref - out of bounds", @@ -900,7 +900,7 @@ func TestCompiler_compileTableGet(t *testing.T) { name: "funcref - nil", tableIndex: 1, offset: 1, - exp: 0, + exp: nil, }, { name: "funcref - out of bounds", @@ -949,7 +949,7 @@ func TestCompiler_compileTableGet(t *testing.T) { } else { require.Equal(t, nativeCallStatusCodeReturned, env.compilerStatus()) require.Equal(t, uint64(1), env.stackPointer()) - require.Equal(t, uint64(tc.exp), env.stackTopAsUint64()) + require.Equal(t, uint64(uintptr(tc.exp)), env.stackTopAsUint64()) } }) } @@ -997,3 +997,7 @@ func TestCompiler_compileRefFunc(t *testing.T) { }) } } + +func asReference(u uint) wasm.Reference { + return unsafe.Pointer(uintptr(u)) +} diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index 6f46cb4f79..a8ab28397c 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -499,7 +499,8 @@ func (s nativeCallStatusCode) String() (ret string) { } // releaseCompiledModule is a runtime.SetFinalizer function that munmaps the compiledModule.executable. -func releaseCompiledModule(cm *compiledModule) { +func releaseCompiledModule(cm *compiledCode) { + // log.Printf("compiler: releasing compiled module %#x", cm.executable.Addr()) if err := cm.executable.Unmap(); err != nil { // munmap failure cannot recover, and happen asynchronously on the // finalizer thread. While finalizer functions can return errors, @@ -555,7 +556,7 @@ func (e *engine) CompileModule(_ context.Context, module *wasm.Module, listeners } // As this uses mmap, we need to munmap on the compiled machine code when it's GCed. - e.setFinalizer(cm, releaseCompiledModule) + e.setFinalizer(cm.compiledCode, releaseCompiledModule) ln := len(listeners) cmp := newCompiler() asmNodes := new(asmNodes) @@ -675,7 +676,10 @@ func (e *moduleEngine) ResolveImportedMemory(wasm.ModuleEngine) {} // FunctionInstanceReference implements the same method as documented on wasm.ModuleEngine. func (e *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Reference { - return uintptr(unsafe.Pointer(&e.functions[funcIndex])) + fptr := &e.functions[funcIndex] + u := unsafe.Pointer(fptr) + // log.Printf("function instance reference %#x", u) + return u } // DoneInstantiation implements wasm.ModuleEngine. @@ -700,7 +704,7 @@ func (e *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio panic(wasmruntime.ErrRuntimeInvalidTableAccess) } rawPtr := t.References[tableOffset] - if rawPtr == 0 { + if rawPtr == nil { panic(wasmruntime.ErrRuntimeInvalidTableAccess) } @@ -713,14 +717,8 @@ func (e *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio // functionFromUintptr resurrects the original *function from the given uintptr // which comes from either funcref table or OpcodeRefFunc instruction. -func functionFromUintptr(ptr uintptr) *function { - // Wraps ptrs as the double pointer in order to avoid the unsafe access as detected by race detector. - // - // For example, if we have (*function)(unsafe.Pointer(ptr)) instead, then the race detector's "checkptr" - // subroutine wanrs as "checkptr: pointer arithmetic result points to invalid allocation" - // https://github.com/golang/go/blob/1ce7fcf139417d618c2730010ede2afb41664211/src/runtime/checkptr.go#L69 - var wrapped *uintptr = &ptr - return *(**function)(unsafe.Pointer(wrapped)) +func functionFromUintptr(ptr wasm.Reference) *function { + return (*function)(ptr) } // Definition implements the same method as documented on wasm.ModuleEngine. @@ -1193,7 +1191,7 @@ func (ce *callEngine) builtinFunctionTableGrow(tables []*wasm.TableInstance) { table := tables[tableIndex] // verified not to be out of range by the func validation at compilation phase. num := ce.popValue() ref := ce.popValue() - res := table.Grow(uint32(num), uintptr(ref)) + res := table.Grow(uint32(num), unsafe.Pointer(uintptr(ref))) ce.pushValue(uint64(res)) } diff --git a/internal/engine/compiler/engine_cache.go b/internal/engine/compiler/engine_cache.go index de6a7e3dd0..a2734974ad 100644 --- a/internal/engine/compiler/engine_cache.go +++ b/internal/engine/compiler/engine_cache.go @@ -51,7 +51,7 @@ func (e *engine) getCompiledModule(module *wasm.Module, listeners []experimental } // As this uses mmap, we need to munmap on the compiled machine code when it's GCed. - e.setFinalizer(cm, releaseCompiledModule) + e.setFinalizer(cm.compiledCode, releaseCompiledModule) } return } diff --git a/internal/engine/compiler/engine_test.go b/internal/engine/compiler/engine_test.go index c68a05865f..1ec414d017 100644 --- a/internal/engine/compiler/engine_test.go +++ b/internal/engine/compiler/engine_test.go @@ -26,14 +26,14 @@ func requireSupportedOSArch(t *testing.T) { } } -type fakeFinalizer map[*compiledModule]func(module *compiledModule) +type fakeFinalizer map[*compiledCode]func(module *compiledCode) func (f fakeFinalizer) setFinalizer(obj interface{}, finalizer interface{}) { - cf := obj.(*compiledModule) + cf := obj.(*compiledCode) if _, ok := f[cf]; ok { // easier than adding a field for testing.T panic(fmt.Sprintf("BUG: %v already had its finalizer set", cf)) } - f[cf] = finalizer.(func(*compiledModule)) + f[cf] = finalizer.(func(*compiledCode)) } func TestCompiler_CompileModule(t *testing.T) { @@ -95,11 +95,10 @@ func TestCompiler_CompileModule(t *testing.T) { func TestCompiler_Releasecode_Panic(t *testing.T) { captured := require.CapturePanic(func() { - releaseCompiledModule(&compiledModule{ - compiledCode: &compiledCode{ - executable: makeCodeSegment(1, 2), - }, - }) + releaseCompiledModule(&compiledCode{ + executable: makeCodeSegment(1, 2), + }, + ) }) require.Contains(t, captured.Error(), "compiler: failed to munmap code segment") } @@ -241,7 +240,7 @@ func TestCallEngine_builtinFunctionTableGrow(t *testing.T) { ce.builtinFunctionTableGrow([]*wasm.TableInstance{table}) require.Equal(t, 1, len(table.References)) - require.Equal(t, uintptr(0xff), table.References[0]) + require.Equal(t, unsafe.Pointer(uintptr(0xff)), table.References[0]) } func ptrAsUint64(f *function) uint64 { diff --git a/internal/engine/compiler/impl_amd64_test.go b/internal/engine/compiler/impl_amd64_test.go index f6a8a6ae7c..d1ac902bd2 100644 --- a/internal/engine/compiler/impl_amd64_test.go +++ b/internal/engine/compiler/impl_amd64_test.go @@ -50,7 +50,7 @@ func TestAmd64Compiler_indirectCallWithTargetOnCallingConvReg(t *testing.T) { typeID: 0, } me.functions = append(me.functions, f) - table[0] = uintptr(unsafe.Pointer(&f)) + table[0] = unsafe.Pointer(&f) } compiler := env.requireNewCompiler(t, &wasm.FunctionType{}, newCompiler, &wazeroir.CompilationResult{ diff --git a/internal/engine/compiler/impl_arm64_test.go b/internal/engine/compiler/impl_arm64_test.go index da59b58f59..155dda15cf 100644 --- a/internal/engine/compiler/impl_arm64_test.go +++ b/internal/engine/compiler/impl_arm64_test.go @@ -47,7 +47,7 @@ func TestArm64Compiler_indirectCallWithTargetOnCallingConvReg(t *testing.T) { moduleInstance: env.moduleInstance, } me.functions = append(me.functions, f) - table[0] = uintptr(unsafe.Pointer(&f)) + table[0] = unsafe.Pointer(&f) } compiler := env.requireNewCompiler(t, &wasm.FunctionType{}, newCompiler, &wazeroir.CompilationResult{ diff --git a/internal/engine/interpreter/interpreter.go b/internal/engine/interpreter/interpreter.go index f9368be9aa..06c622f717 100644 --- a/internal/engine/interpreter/interpreter.go +++ b/internal/engine/interpreter/interpreter.go @@ -214,14 +214,8 @@ type function struct { // functionFromUintptr resurrects the original *function from the given uintptr // which comes from either funcref table or OpcodeRefFunc instruction. -func functionFromUintptr(ptr uintptr) *function { - // Wraps ptrs as the double pointer in order to avoid the unsafe access as detected by race detector. - // - // For example, if we have (*function)(unsafe.Pointer(ptr)) instead, then the race detector's "checkptr" - // subroutine wanrs as "checkptr: pointer arithmetic result points to invalid allocation" - // https://github.com/golang/go/blob/1ce7fcf139417d618c2730010ede2afb41664211/src/runtime/checkptr.go#L69 - var wrapped *uintptr = &ptr - return *(**function)(unsafe.Pointer(wrapped)) +func functionFromUintptr(ptr wasm.Reference) *function { + return (*function)(ptr) } type snapshot struct { @@ -497,7 +491,7 @@ func (e *moduleEngine) DoneInstantiation() {} // FunctionInstanceReference implements the same method as documented on wasm.ModuleEngine. func (e *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Reference { - return uintptr(unsafe.Pointer(&e.functions[funcIndex])) + return unsafe.Pointer(&e.functions[funcIndex]) } // NewFunction implements the same method as documented on wasm.ModuleEngine. @@ -514,7 +508,7 @@ func (e *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio panic(wasmruntime.ErrRuntimeInvalidTableAccess) } rawPtr := t.References[tableOffset] - if rawPtr == 0 { + if rawPtr == nil { panic(wasmruntime.ErrRuntimeInvalidTableAccess) } @@ -762,11 +756,11 @@ func (ce *callEngine) callNativeFunc(ctx context.Context, m *wasm.ModuleInstance panic(wasmruntime.ErrRuntimeInvalidTableAccess) } rawPtr := table.References[offset] - if rawPtr == 0 { + if rawPtr == nil { panic(wasmruntime.ErrRuntimeInvalidTableAccess) } - tf := functionFromUintptr(rawPtr) + tf := (*function)(rawPtr) if tf.typeID != typeIDs[op.U1] { panic(wasmruntime.ErrRuntimeIndirectCallTypeMismatch) } @@ -1769,7 +1763,7 @@ func (ce *callEngine) callNativeFunc(ctx context.Context, m *wasm.ModuleInstance panic(wasmruntime.ErrRuntimeInvalidTableAccess) } - ce.pushValue(uint64(table.References[offset])) + ce.pushValue(uint64(uintptr(table.References[offset]))) frame.pc++ case wazeroir.OperationKindTableSet: table := tables[op.U1] @@ -1780,7 +1774,7 @@ func (ce *callEngine) callNativeFunc(ctx context.Context, m *wasm.ModuleInstance panic(wasmruntime.ErrRuntimeInvalidTableAccess) } - table.References[offset] = uintptr(ref) // externrefs are opaque uint64. + table.References[offset] = asReference(ref) // externrefs are opaque uint64. frame.pc++ case wazeroir.OperationKindTableSize: table := tables[op.U1] @@ -1789,13 +1783,13 @@ func (ce *callEngine) callNativeFunc(ctx context.Context, m *wasm.ModuleInstance case wazeroir.OperationKindTableGrow: table := tables[op.U1] num, ref := ce.popValue(), ce.popValue() - ret := table.Grow(uint32(num), uintptr(ref)) + ret := table.Grow(uint32(num), asReference(ref)) ce.pushValue(uint64(ret)) frame.pc++ case wazeroir.OperationKindTableFill: table := tables[op.U1] num := ce.popValue() - ref := uintptr(ce.popValue()) + ref := asReference(ce.popValue()) offset := ce.popValue() if num+offset > uint64(len(table.References)) { panic(wasmruntime.ErrRuntimeInvalidTableAccess) @@ -4581,3 +4575,7 @@ func (ce *callEngine) callGoFuncWithStack(ctx context.Context, m *wasm.ModuleIns ce.stack = ce.stack[0 : len(ce.stack)-shrinkLen] } } + +func asReference(ptr uint64) wasm.Reference { + return unsafe.Pointer(uintptr(ptr)) +} diff --git a/internal/engine/wazevo/call_engine.go b/internal/engine/wazevo/call_engine.go index 22c24ea165..3e1a2d8a97 100644 --- a/internal/engine/wazevo/call_engine.go +++ b/internal/engine/wazevo/call_engine.go @@ -295,7 +295,7 @@ func (c *callEngine) callWithStack(ctx context.Context, paramResultStack []uint6 s := goCallStackView(c.execCtx.stackPointerBeforeGoCall) tableIndex, num, ref := uint32(s[0]), uint32(s[1]), uintptr(s[2]) table := mod.Tables[tableIndex] - s[0] = uint64(uint32(int32(table.Grow(num, ref)))) + s[0] = uint64(uint32(int32(table.Grow(num, unsafe.Pointer(ref))))) c.execCtx.exitCode = wazevoapi.ExitCodeOK afterGoFunctionCallEntrypoint(c.execCtx.goCallReturnAddress, c.execCtxPtr, uintptr(unsafe.Pointer(c.execCtx.stackPointerBeforeGoCall)), c.execCtx.framePointerBeforeGoCall) @@ -389,7 +389,7 @@ func (c *callEngine) callWithStack(ctx context.Context, paramResultStack []uint6 s := goCallStackView(c.execCtx.stackPointerBeforeGoCall) funcIndex := wasm.Index(s[0]) ref := mod.Engine.FunctionInstanceReference(funcIndex) - s[0] = uint64(ref) + s[0] = uint64(uintptr(ref)) c.execCtx.exitCode = wazevoapi.ExitCodeOK afterGoFunctionCallEntrypoint(c.execCtx.goCallReturnAddress, c.execCtxPtr, uintptr(unsafe.Pointer(c.execCtx.stackPointerBeforeGoCall)), c.execCtx.framePointerBeforeGoCall) diff --git a/internal/engine/wazevo/module_engine.go b/internal/engine/wazevo/module_engine.go index b72ac2b162..9efc9277e3 100644 --- a/internal/engine/wazevo/module_engine.go +++ b/internal/engine/wazevo/module_engine.go @@ -294,7 +294,7 @@ func (m *moduleEngine) DoneInstantiation() { func (m *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Reference { if funcIndex < m.module.Source.ImportFunctionCount { begin, _, _ := m.parent.offsets.ImportedFunctionOffset(funcIndex) - return uintptr(unsafe.Pointer(&m.opaque[begin])) + return unsafe.Pointer(&m.opaque[begin]) } localIndex := funcIndex - m.module.Source.ImportFunctionCount p := m.parent @@ -308,7 +308,7 @@ func (m *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Refe indexInModule: funcIndex, } m.localFunctionInstances = append(m.localFunctionInstances, lf) - return uintptr(unsafe.Pointer(lf)) + return unsafe.Pointer(lf) } // LookupFunction implements wasm.ModuleEngine. @@ -317,11 +317,11 @@ func (m *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio panic(wasmruntime.ErrRuntimeInvalidTableAccess) } rawPtr := t.References[tableOffset] - if rawPtr == 0 { + if rawPtr == nil { panic(wasmruntime.ErrRuntimeInvalidTableAccess) } - tf := wazevoapi.PtrFromUintptr[functionInstance](rawPtr) + tf := (*functionInstance)(rawPtr) if tf.typeID != typeId { panic(wasmruntime.ErrRuntimeIndirectCallTypeMismatch) } diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go index 3147e3eb54..374acd9210 100644 --- a/internal/integration_test/engine/adhoc_test.go +++ b/internal/integration_test/engine/adhoc_test.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "math" + "runtime" "strconv" "strings" "testing" @@ -72,6 +73,7 @@ var tests = map[string]testCase{ "many params many results / main / listener": {f: testManyParamsResultsMainListener}, "many params many results / call_many_consts_and_pick_last_vector": {f: testManyParamsResultsCallManyConstsAndPickLastVector}, "many params many results / call_many_consts_and_pick_last_vector / listener": {f: testManyParamsResultsCallManyConstsAndPickLastVectorListener}, + "linking a closed module should not segfault": {f: testLinking}, } func TestEngineCompiler(t *testing.T) { @@ -132,6 +134,10 @@ var ( infiniteLoopWasm []byte //go:embed testdata/huge_call_stack_unwind.wasm hugeCallStackUnwind []byte + //go:embed testdata/linking1.wasm + linking1 []byte + //go:embed testdata/linking2.wasm + linking2 []byte ) func testEnsureTerminationOnClose(t *testing.T, r wazero.Runtime) { @@ -2169,3 +2175,34 @@ wasm stack trace: .$0() ... maybe followed by omitted frames`, err.Error()) } + +// testLinking links two modules where the first exports a table and the second module imports it, +// overwriting one of the table entries with one of its functions. +// The first module exposes a function that invokes the functionref in the table. +// If the functionref belongs to failed or closed module, then the call should not fail. +func testLinking(t *testing.T, r wazero.Runtime) { + if !platform.CompilerSupported() { + t.Skip() + } + ctx := context.Background() + // Instantiate the first module. + mod, err := r.InstantiateWithConfig(ctx, linking1, wazero.NewModuleConfig().WithName("Ms")) + defer mod.Close(ctx) + require.NoError(t, err) + // The second module builds successfully. + m, err := r.CompileModule(ctx, linking2) + require.NoError(t, err) + // The second module instantiates and sets the table[0] field to point to its own $f function. + _, err = r.InstantiateModule(ctx, m, wazero.NewModuleConfig()) + // However it traps upon instantiation. + require.Error(t, err) + m.Close(ctx) + // Force a GC cycle. This should not cause the memory segment to become invalid. + // If the segment is invalid, the next function call will SIGSEGV. + runtime.GC() + time.Sleep(200 * time.Millisecond) + // The result is expected to be 0xdead, i.e., the result of linking2.$f. + res, err := mod.ExportedFunction("get table[0]").Call(ctx) + require.NoError(t, err) + require.Equal(t, uint64(0xdead), res[0]) +} diff --git a/internal/integration_test/engine/hammer_test.go b/internal/integration_test/engine/hammer_test.go index 3189550a1f..e5a6e53998 100644 --- a/internal/integration_test/engine/hammer_test.go +++ b/internal/integration_test/engine/hammer_test.go @@ -2,9 +2,6 @@ package adhoc import ( "context" - "sync" - "testing" - "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/experimental/opt" @@ -12,12 +9,16 @@ import ( "github.com/tetratelabs/wazero/internal/testing/hammer" "github.com/tetratelabs/wazero/internal/testing/require" "github.com/tetratelabs/wazero/sys" + "runtime" + "sync" + "testing" ) var hammers = map[string]testCase{ // Tests here are similar to what's described in /RATIONALE.md, but deviate as they involve blocking functions. - "close importing module while in use": {f: closeImportingModuleWhileInUse}, - "close imported module while in use": {f: closeImportedModuleWhileInUse}, + "close importing module while in use": {f: closeImportingModuleWhileInUse}, + "close imported module while in use": {f: closeImportedModuleWhileInUse}, + "linking a closed module should not segfault": {f: testLinkingHammer}, } func TestEngineCompiler_hammer(t *testing.T) { @@ -125,6 +126,37 @@ func closeModuleWhileInUse(t *testing.T, r wazero.Runtime, closeFn func(imported requireFunctionCall(t, importing.ExportedFunction("call_return_input")) } +// testLinkingHammer links two modules where the first exports a table and the second module imports it, +// overwriting one of the table entries with one of its functions. +// The first module exposes a function that invokes the functionref in the table. +// If the functionref belongs to failed or closed module, then the call should not fail. +func testLinkingHammer(t *testing.T, r wazero.Runtime) { + if !platform.CompilerSupported() { + t.Skip() + } + hammer.NewHammer(t, 1, 10).Run(func(name string) { + ctx := context.Background() + // Instantiate the first module. + mod, err := r.InstantiateWithConfig(ctx, linking1, wazero.NewModuleConfig().WithName("Ms")) + defer mod.Close(ctx) + require.NoError(t, err) + // The second module builds successfully. + m, err := r.CompileModule(ctx, linking2) + require.NoError(t, err) + // The second module instantiates and sets the table[0] field to point to its own $f function. + _, err = r.InstantiateModule(ctx, m, wazero.NewModuleConfig()) + // However it traps upon instantiation. + require.Error(t, err) + m.Close(ctx) + // This should not be necessary to fail. + runtime.GC() + // The result is expected to be 0xdead, i.e., the result of linking2.$f. + res, err := mod.ExportedFunction("get table[0]").Call(ctx) // This should not SIGSEGV. + require.NoError(t, err) + require.Equal(t, uint64(0xdead), res[0]) + }, func() {}) +} + func requireFunctionCall(t *testing.T, fn api.Function) { res, err := fn.Call(testCtx, 3) require.NoError(t, err) diff --git a/internal/integration_test/engine/testdata/linking1.wasm b/internal/integration_test/engine/testdata/linking1.wasm new file mode 100644 index 0000000000..cd57daf2e1 Binary files /dev/null and b/internal/integration_test/engine/testdata/linking1.wasm differ diff --git a/internal/integration_test/engine/testdata/linking1.wat b/internal/integration_test/engine/testdata/linking1.wat new file mode 100644 index 0000000000..bcf50d64ed --- /dev/null +++ b/internal/integration_test/engine/testdata/linking1.wat @@ -0,0 +1,12 @@ +;; Store is modified if the start function traps. +(module $Ms + (type $t (func (result i32))) + (memory (export "memory") 1) + (table (export "table") 1 funcref) + (func (export "get memory[0]") (type $t) + (i32.load8_u (i32.const 0)) + ) + (func (export "get table[0]") (type $t) + (call_indirect (type $t) (i32.const 0)) + ) +) diff --git a/internal/integration_test/engine/testdata/linking2.wasm b/internal/integration_test/engine/testdata/linking2.wasm new file mode 100644 index 0000000000..22957bf342 Binary files /dev/null and b/internal/integration_test/engine/testdata/linking2.wasm differ diff --git a/internal/integration_test/engine/testdata/linking2.wat b/internal/integration_test/engine/testdata/linking2.wat new file mode 100644 index 0000000000..832cf13a32 --- /dev/null +++ b/internal/integration_test/engine/testdata/linking2.wat @@ -0,0 +1,14 @@ + (module + (import "Ms" "memory" (memory 1)) + (import "Ms" "table" (table 1 funcref)) + (data (i32.const 0) "hello") + (elem (i32.const 0) $f) + (func $f (result i32) + (i32.const 0xdead) + ) + (func $main + (unreachable) + ) + (start $main) + ) + diff --git a/internal/integration_test/spectest/spectest_test.go b/internal/integration_test/spectest/spectest_test.go index ed8b3b8d75..609648c82c 100644 --- a/internal/integration_test/spectest/spectest_test.go +++ b/internal/integration_test/spectest/spectest_test.go @@ -1,6 +1,7 @@ package spectest import ( + _ "embed" "encoding/json" "math" "testing" diff --git a/internal/wasm/module_test.go b/internal/wasm/module_test.go index 6d8efe26ac..cded118e47 100644 --- a/internal/wasm/module_test.go +++ b/internal/wasm/module_test.go @@ -818,7 +818,7 @@ func TestModule_buildGlobals(t *testing.T) { mi.buildGlobals(m, func(funcIndex Index) Reference { require.Equal(t, localFuncRefInstructionIndex, funcIndex) - return 0x99999 + return Reference(uintptr(0x99999)) }) expectedGlobals := []*GlobalInstance{ imported[0], imported[1], diff --git a/internal/wasm/store.go b/internal/wasm/store.go index fe9d6d150d..873e796def 100644 --- a/internal/wasm/store.go +++ b/internal/wasm/store.go @@ -222,7 +222,7 @@ func (m *ModuleInstance) applyElements(elems []ElementSegment) { if table.Type == RefTypeExternref { for i := 0; i < len(elem.Init); i++ { - references[offset+uint32(i)] = Reference(0) + references[offset+uint32(i)] = nil } } else { for i, init := range elem.Init { @@ -233,7 +233,7 @@ func (m *ModuleInstance) applyElements(elems []ElementSegment) { var ref Reference if index, ok := unwrapElementInitGlobalReference(init); ok { global := m.Globals[index] - ref = Reference(global.Val) + ref = Reference(uintptr(global.Val)) } else { ref = m.Engine.FunctionInstanceReference(index) } @@ -558,7 +558,7 @@ func (g *GlobalInstance) initialize(importedGlobals []*GlobalInstance, expr *Con } case OpcodeRefFunc: v, _, _ := leb128.LoadUint32(expr.Data) - g.Val = uint64(funcRefResolver(v)) + g.Val = uint64(uintptr(funcRefResolver(v))) case OpcodeVecV128Const: g.Val, g.ValHi = binary.LittleEndian.Uint64(expr.Data[0:8]), binary.LittleEndian.Uint64(expr.Data[8:16]) } diff --git a/internal/wasm/store_test.go b/internal/wasm/store_test.go index b8d3d45dc9..f867da0f53 100644 --- a/internal/wasm/store_test.go +++ b/internal/wasm/store_test.go @@ -628,7 +628,7 @@ func TestGlobalInstance_initialize(t *testing.T) { &ConstantExpression{Opcode: OpcodeRefFunc, Data: []byte{1}}, func(funcIndex Index) Reference { require.Equal(t, Index(1), funcIndex) - return 0xdeadbeaf + return asReference(0xdeadbeaf) }, ) require.Equal(t, uint64(0xdeadbeaf), g.Val) @@ -967,7 +967,7 @@ func TestModuleInstance_applyElements(t *testing.T) { m := &ModuleInstance{} m.Tables = []*TableInstance{{Type: RefTypeExternref, References: make([]Reference, 10)}} for i := range m.Tables[0].References { - m.Tables[0].References[i] = 0xffff // non-null ref. + m.Tables[0].References[i] = asReference(0xffff) // non-null ref. } // This shouldn't panic. @@ -977,23 +977,23 @@ func TestModuleInstance_applyElements(t *testing.T) { {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: leb128_100}, Init: make([]Index, 5)}, // Iteration stops at this point, so the offset:5 below shouldn't be applied. {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: []byte{5}}, Init: make([]Index, 5)}, }) - require.Equal(t, []Reference{0, 0, 0, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff}, + require.Equal(t, []Reference{nil, nil, nil, asReference(0xffff), asReference(0xffff), asReference(0xffff), asReference(0xffff), asReference(0xffff), asReference(0xffff), asReference(0xffff)}, m.Tables[0].References) m.applyElements([]ElementSegment{ {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: []byte{5}}, Init: make([]Index, 5)}, }) - require.Equal(t, []Reference{0, 0, 0, 0xffff, 0xffff, 0, 0, 0, 0, 0}, m.Tables[0].References) + require.Equal(t, []Reference{nil, nil, nil, asReference(0xffff), asReference(0xffff), nil, nil, nil, nil, nil}, m.Tables[0].References) }) t.Run("funcref", func(t *testing.T) { e := &mockEngine{} me, err := e.NewModuleEngine(nil, nil) - me.(*mockModuleEngine).functionRefs = map[Index]Reference{0: 0xa, 1: 0xaa, 2: 0xaaa, 3: 0xaaaa} + me.(*mockModuleEngine).functionRefs = map[Index]Reference{0: asReference(0xa), 1: asReference(0xaa), 2: asReference(0xaaa), 3: asReference(0xaaaa)} require.NoError(t, err) m := &ModuleInstance{Engine: me, Globals: []*GlobalInstance{{}, {Val: 0xabcde}}} m.Tables = []*TableInstance{{Type: RefTypeFuncref, References: make([]Reference, 10)}} for i := range m.Tables[0].References { - m.Tables[0].References[i] = 0xffff // non-null ref. + m.Tables[0].References[i] = asReference(0xffff) // non-null ref. } // This shouldn't panic. @@ -1004,12 +1004,16 @@ func TestModuleInstance_applyElements(t *testing.T) { {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: leb128_100}, Init: make([]Index, 5)}, // Iteration stops at this point, so the offset:5 below shouldn't be applied. {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: []byte{5}}, Init: make([]Index, 5)}, }) - require.Equal(t, []Reference{0xa, 0xaa, 0xaaa, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xabcde}, + require.Equal(t, []Reference{asReference(0xa), asReference(0xaa), asReference(0xaaa), asReference(0xffff), asReference(0xffff), asReference(0xffff), asReference((0xffff)), asReference(0xffff), asReference(0xffff), asReference(0xabcde)}, m.Tables[0].References) m.applyElements([]ElementSegment{ {Mode: ElementModeActive, OffsetExpr: ConstantExpression{Opcode: OpcodeI32Const, Data: []byte{5}}, Init: []Index{0, ElementInitNullReference, 2}}, }) - require.Equal(t, []Reference{0xa, 0xaa, 0xaaa, 0xffff, 0xffff, 0xa, 0xffff, 0xaaa, 0xffff, 0xabcde}, + require.Equal(t, []Reference{asReference(0xa), asReference(0xaa), asReference(0xaaa), asReference(0xffff), asReference(0xffff), asReference(0xa), asReference((0xffff)), asReference(0xaaa), asReference(0xffff), asReference(0xabcde)}, m.Tables[0].References) }) } + +func asReference(u uint) Reference { + return Reference(uintptr(u)) +} diff --git a/internal/wasm/table.go b/internal/wasm/table.go index ac9fc49862..6c3840038f 100644 --- a/internal/wasm/table.go +++ b/internal/wasm/table.go @@ -3,6 +3,7 @@ package wasm import ( "fmt" "math" + "unsafe" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/internal/leb128" @@ -133,7 +134,7 @@ type TableInstance struct { type ElementInstance = []Reference // Reference is the runtime representation of RefType which is either RefTypeFuncref or RefTypeExternref. -type Reference = uintptr +type Reference = unsafe.Pointer // validateTable ensures any ElementSegment is valid. This caches results via Module.validatedActiveElementSegments. // Note: limitsType are validated by decoders, so not re-validated here. @@ -314,7 +315,7 @@ func (t *TableInstance) Grow(delta uint32, initialRef Reference) (currentLen uin newLen >= math.MaxUint32 || (t.Max != nil && newLen > int64(*t.Max)) { return 0xffffffff // = -1 in signed 32-bit integer. } - t.References = append(t.References, make([]uintptr, delta)...) + t.References = append(t.References, make([]Reference, delta)...) // Uses the copy trick for faster filling the new region with the initial value. // https://gist.github.com/taylorza/df2f89d5f9ab3ffd06865062a4cf015d diff --git a/internal/wasm/table_test.go b/internal/wasm/table_test.go index 1c5b255969..61531c6097 100644 --- a/internal/wasm/table_test.go +++ b/internal/wasm/table_test.go @@ -836,7 +836,7 @@ func TestModule_buildTables(t *testing.T) { }, importedGlobals: []*GlobalInstance{{Type: GlobalType{ValType: ValueTypeI32}, Val: 1}}, importedTables: []*TableInstance{{References: make([]Reference, 2), Min: 2}}, - expectedTables: []*TableInstance{{Min: 2, References: []Reference{0, 0}}}, + expectedTables: []*TableInstance{{Min: 2, References: []Reference{nil, nil}}}, }, { name: "imported global derived element offset - ignores min on imported table", @@ -854,7 +854,7 @@ func TestModule_buildTables(t *testing.T) { }, importedGlobals: []*GlobalInstance{{Type: GlobalType{ValType: ValueTypeI32}, Val: 1}}, importedTables: []*TableInstance{{References: make([]Reference, 2), Min: 2}}, - expectedTables: []*TableInstance{{Min: 2, References: []Reference{0, 0}}}, + expectedTables: []*TableInstance{{Min: 2, References: []Reference{nil, nil}}}, }, { name: "imported global derived element offset - two indices", @@ -1066,8 +1066,8 @@ func TestTableInstance_Grow(t *testing.T) { for _, tt := range tests { tc := tt t.Run(tc.name, func(t *testing.T) { - table := &TableInstance{References: make([]uintptr, tc.currentLen), Max: tc.max} - actual := table.Grow(tc.delta, 0) + table := &TableInstance{References: make([]Reference, tc.currentLen), Max: tc.max} + actual := table.Grow(tc.delta, nil) require.Equal(t, tc.exp, actual) }) }