From ab4f3d8f41e97fd10069ae364a156b003040b60d Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Wed, 28 Feb 2024 21:12:48 +0100 Subject: [PATCH 1/9] spectest: reproduce segfault Signed-off-by: Edoardo Vacchi --- internal/integration_test/spectest/spectest.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/integration_test/spectest/spectest.go b/internal/integration_test/spectest/spectest.go index 87bc6fa138..fa57014d10 100644 --- a/internal/integration_test/spectest/spectest.go +++ b/internal/integration_test/spectest/spectest.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "math" + "runtime" "strconv" "strings" "testing" @@ -385,6 +386,7 @@ func RunCase(t *testing.T, testDataFS embed.FS, f string, ctx context.Context, c continue } t.Run(fmt.Sprintf("%s/line:%d", c.CommandType, c.Line), func(t *testing.T) { + defer runtime.GC() msg := fmt.Sprintf("%s:%d %s", wastName, c.Line, c.CommandType) switch c.CommandType { case "module": From 5ddc1508c3ba66a89ab04f68b7d2199a9e97baaf Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Wed, 28 Feb 2024 21:33:19 +0100 Subject: [PATCH 2/9] spectest: compile+instantiate module on `assert_uninstantiable` to avoid auto-close Signed-off-by: Edoardo Vacchi --- internal/integration_test/spectest/spectest.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/integration_test/spectest/spectest.go b/internal/integration_test/spectest/spectest.go index fa57014d10..da24a1434e 100644 --- a/internal/integration_test/spectest/spectest.go +++ b/internal/integration_test/spectest/spectest.go @@ -500,7 +500,12 @@ func RunCase(t *testing.T, testDataFS embed.FS, f string, ctx context.Context, c case "assert_uninstantiable": buf, err := testDataFS.ReadFile(testdataPath(c.Filename)) require.NoError(t, err, msg) - _, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig()) + // linking.wast references this module later, so while the instantiation + // should fail, we should not close it. InstantiateWithConfig() will auto-close + // on failure, so instead we compile then instantiate explicitly. + cm, err := r.CompileModule(ctx, buf) + require.NoError(t, err, msg) + _, err = r.InstantiateModule(ctx, cm, wazero.NewModuleConfig()) if c.Text == "out of bounds table access" { // This is not actually an instantiation error, but assert_trap in the original wast, but wast2json translates it to assert_uninstantiable. // Anyway, this spectest case expects the error due to active element offset ouf of bounds From a90416510cb3ec45650de9ca5679bbd848fba34f Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Wed, 28 Feb 2024 21:46:16 +0100 Subject: [PATCH 3/9] spectest: remove the GC call Signed-off-by: Edoardo Vacchi --- internal/integration_test/spectest/spectest.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/integration_test/spectest/spectest.go b/internal/integration_test/spectest/spectest.go index da24a1434e..f392586860 100644 --- a/internal/integration_test/spectest/spectest.go +++ b/internal/integration_test/spectest/spectest.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "math" - "runtime" "strconv" "strings" "testing" @@ -386,7 +385,6 @@ func RunCase(t *testing.T, testDataFS embed.FS, f string, ctx context.Context, c continue } t.Run(fmt.Sprintf("%s/line:%d", c.CommandType, c.Line), func(t *testing.T) { - defer runtime.GC() msg := fmt.Sprintf("%s:%d %s", wastName, c.Line, c.CommandType) switch c.CommandType { case "module": From be8ed97083a1f53cde1d3e790a48f7df9dbaf4fe Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 29 Feb 2024 12:36:30 +0100 Subject: [PATCH 4/9] reproducer Signed-off-by: Edoardo Vacchi --- .../integration_test/engine/adhoc_test.go | 28 ++++++++++++++++++ .../engine/testdata/linking1.wasm | Bin 0 -> 101 bytes .../engine/testdata/linking1.wat | 12 ++++++++ .../engine/testdata/linking2.wasm | Bin 0 -> 91 bytes .../engine/testdata/linking2.wat | 14 +++++++++ .../spectest/spectest_test.go | 1 + 6 files changed, 55 insertions(+) create mode 100644 internal/integration_test/engine/testdata/linking1.wasm create mode 100644 internal/integration_test/engine/testdata/linking1.wat create mode 100644 internal/integration_test/engine/testdata/linking2.wasm create mode 100644 internal/integration_test/engine/testdata/linking2.wat diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go index 3147e3eb54..386c39cef9 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,25 @@ 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() + mod, err := r.InstantiateWithConfig(ctx, linking1, wazero.NewModuleConfig().WithName("Ms")) + require.NoError(t, err) + m, err := r.CompileModule(ctx, linking2) + require.NoError(t, err) + _, err = r.InstantiateModule(ctx, m, wazero.NewModuleConfig()) + require.Error(t, err) + m.Close(ctx) + // This should not be necessary to fail. + runtime.GC() + _, err = mod.ExportedFunction("get table[0]").Call(ctx) // This should not SIGSEGV. + 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 0000000000000000000000000000000000000000..cd57daf2e15f05c053610803f06f15e67386effb GIT binary patch literal 101 zcmZQbEY4+QU|?WmWlUgTtY>CsVqjolVJrX&Ff%eRvKz9n<)-H57gaJbu$Cky<)kt) n@TRAhC_n_G4PqG>ctCux9Ei)vCCJ3i;K-oMz`zZn1%WgGcv2DV literal 0 HcmV?d00001 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 0000000000000000000000000000000000000000..22957bf342a2822cdb894c16acaa34831114e4a1 GIT binary patch literal 91 zcmWN_u?>JA6a~QdJ^@kOz-bu39l(Ud0)i16D<^UUca!oCu@wTKl>j~gkVRx|rleBO kTfh!|#Vscupi+G-ISzz}Bc@pl{QD`V0O$Jj)_cmi{-9hABme*a literal 0 HcmV?d00001 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" From 127dd847f2552bcb44965186fae352f4cf7a1970 Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 29 Feb 2024 17:51:08 +0100 Subject: [PATCH 5/9] Revert "spectest: compile+instantiate module on `assert_uninstantiable` to avoid auto-close" This reverts commit 5ddc1508c3ba66a89ab04f68b7d2199a9e97baaf. --- internal/integration_test/spectest/spectest.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/integration_test/spectest/spectest.go b/internal/integration_test/spectest/spectest.go index f392586860..87bc6fa138 100644 --- a/internal/integration_test/spectest/spectest.go +++ b/internal/integration_test/spectest/spectest.go @@ -498,12 +498,7 @@ func RunCase(t *testing.T, testDataFS embed.FS, f string, ctx context.Context, c case "assert_uninstantiable": buf, err := testDataFS.ReadFile(testdataPath(c.Filename)) require.NoError(t, err, msg) - // linking.wast references this module later, so while the instantiation - // should fail, we should not close it. InstantiateWithConfig() will auto-close - // on failure, so instead we compile then instantiate explicitly. - cm, err := r.CompileModule(ctx, buf) - require.NoError(t, err, msg) - _, err = r.InstantiateModule(ctx, cm, wazero.NewModuleConfig()) + _, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig()) if c.Text == "out of bounds table access" { // This is not actually an instantiation error, but assert_trap in the original wast, but wast2json translates it to assert_uninstantiable. // Anyway, this spectest case expects the error due to active element offset ouf of bounds From 4585e3e4e6aa2b2104560d73757f883457ea76a1 Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 29 Feb 2024 20:06:41 +0100 Subject: [PATCH 6/9] add comments, check result in test Signed-off-by: Edoardo Vacchi --- internal/integration_test/engine/adhoc_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go index 386c39cef9..049c812b3a 100644 --- a/internal/integration_test/engine/adhoc_test.go +++ b/internal/integration_test/engine/adhoc_test.go @@ -2185,15 +2185,21 @@ func testLinking(t *testing.T, r wazero.Runtime) { t.Skip() } ctx := context.Background() + // Instantiate the first module. mod, err := r.InstantiateWithConfig(ctx, linking1, wazero.NewModuleConfig().WithName("Ms")) 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() - _, err = mod.ExportedFunction("get table[0]").Call(ctx) // This should not SIGSEGV. + // 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]) } From 68409c761627e5dd018a5f3d0ddb8cd777a804c9 Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 29 Feb 2024 20:07:44 +0100 Subject: [PATCH 7/9] try fixing (but it won't work) by attaching the finalizer to compiledCode Signed-off-by: Edoardo Vacchi --- internal/engine/compiler/engine.go | 4 ++-- internal/engine/compiler/engine_cache.go | 2 +- internal/engine/compiler/engine_test.go | 15 +++++++-------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index 6f46cb4f79..9aa9edfb79 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -499,7 +499,7 @@ 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) { 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 +555,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) 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..eef962b06d 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") } From d2dce4fd259a07bdcad7fec4e3d94a2133ad2395 Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Mon, 4 Mar 2024 10:21:58 +0100 Subject: [PATCH 8/9] wip Signed-off-by: Edoardo Vacchi --- internal/engine/compiler/engine.go | 7 ++++++- internal/integration_test/engine/adhoc_test.go | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index 9aa9edfb79..609ac393f8 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log" "reflect" "runtime" "sort" @@ -500,6 +501,7 @@ func (s nativeCallStatusCode) String() (ret string) { // releaseCompiledModule is a runtime.SetFinalizer function that munmaps the compiledModule.executable. 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, @@ -675,7 +677,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 := uintptr(unsafe.Pointer(fptr)) + log.Printf("function instance reference %#x", u) + return u } // DoneInstantiation implements wasm.ModuleEngine. diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go index 049c812b3a..b8731a7e84 100644 --- a/internal/integration_test/engine/adhoc_test.go +++ b/internal/integration_test/engine/adhoc_test.go @@ -2196,10 +2196,12 @@ func testLinking(t *testing.T, r wazero.Runtime) { // However it traps upon instantiation. require.Error(t, err) m.Close(ctx) - // This should not be necessary to fail. + // 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) // This should not SIGSEGV. + res, err := mod.ExportedFunction("get table[0]").Call(ctx) require.NoError(t, err) require.Equal(t, uint64(0xdead), res[0]) } From 2036f4a79756d06241e2fd459141769bc2a67c0e Mon Sep 17 00:00:00 2001 From: Edoardo Vacchi Date: Thu, 7 Mar 2024 15:17:06 +0800 Subject: [PATCH 9/9] wip Signed-off-by: Edoardo Vacchi --- .../compiler/compiler_controlflow_test.go | 6 +- .../engine/compiler/compiler_post1_0_test.go | 68 ++++++++++--------- internal/engine/compiler/engine.go | 21 ++---- internal/engine/compiler/engine_test.go | 2 +- internal/engine/compiler/impl_amd64_test.go | 2 +- internal/engine/compiler/impl_arm64_test.go | 2 +- internal/engine/interpreter/interpreter.go | 30 ++++---- internal/engine/wazevo/call_engine.go | 4 +- internal/engine/wazevo/module_engine.go | 8 +-- .../integration_test/engine/adhoc_test.go | 1 + .../integration_test/engine/hammer_test.go | 42 ++++++++++-- internal/wasm/module_test.go | 2 +- internal/wasm/store.go | 6 +- internal/wasm/store_test.go | 20 +++--- internal/wasm/table.go | 5 +- internal/wasm/table_test.go | 8 +-- 16 files changed, 130 insertions(+), 97 deletions(-) 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 609ac393f8..a8ab28397c 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "log" "reflect" "runtime" "sort" @@ -501,7 +500,7 @@ func (s nativeCallStatusCode) String() (ret string) { // releaseCompiledModule is a runtime.SetFinalizer function that munmaps the compiledModule.executable. func releaseCompiledModule(cm *compiledCode) { - log.Printf("compiler: releasing compiled module %#x", cm.executable.Addr()) + // 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, @@ -678,8 +677,8 @@ 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 { fptr := &e.functions[funcIndex] - u := uintptr(unsafe.Pointer(fptr)) - log.Printf("function instance reference %#x", u) + u := unsafe.Pointer(fptr) + // log.Printf("function instance reference %#x", u) return u } @@ -705,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) } @@ -718,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. @@ -1198,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_test.go b/internal/engine/compiler/engine_test.go index eef962b06d..1ec414d017 100644 --- a/internal/engine/compiler/engine_test.go +++ b/internal/engine/compiler/engine_test.go @@ -240,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 b8731a7e84..374acd9210 100644 --- a/internal/integration_test/engine/adhoc_test.go +++ b/internal/integration_test/engine/adhoc_test.go @@ -2187,6 +2187,7 @@ func testLinking(t *testing.T, r wazero.Runtime) { 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) 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/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) }) }