From 4602c298860ae8d2c6631a00ea451b1cccedfe0c Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Thu, 10 Aug 2023 08:16:13 +0900 Subject: [PATCH 1/2] emscripten: explicitly checks if an api.Function is not nil Signed-off-by: Takeshi Yoneda --- internal/emscripten/emscripten.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/internal/emscripten/emscripten.go b/internal/emscripten/emscripten.go index f880a0c515..f8d3d95d3a 100644 --- a/internal/emscripten/emscripten.go +++ b/internal/emscripten/emscripten.go @@ -3,6 +3,7 @@ package emscripten import ( "context" "errors" + "fmt" "strconv" "github.com/tetratelabs/wazero/api" @@ -129,10 +130,7 @@ func (v *InvokeFunc) Call(ctx context.Context, mod api.Module, stack []uint64) { // We reuse savedStack to save allocations. We allocate with a size of 2 // here to accommodate for the input and output of setThrew. var savedStack [2]uint64 - err = mod.ExportedFunction("stackSave").CallWithStack(ctx, savedStack[:]) - if err != nil { - panic(err) - } + callOrPanic(ctx, mod, "stackSave", savedStack[:]) err = f.CallWithStack(ctx, stack) if err != nil { @@ -143,9 +141,7 @@ func (v *InvokeFunc) Call(ctx context.Context, mod api.Module, stack []uint64) { // This is the equivalent of "stackRestore(sp);". // Do not overwrite err here to preserve the original error. - if err := mod.ExportedFunction("stackRestore").CallWithStack(ctx, savedStack[:]); err != nil { - panic(err) - } + callOrPanic(ctx, mod, "stackRestore", savedStack[:]) // If we encounter ThrowLongjmpError, this means that the C code did a // longjmp, which in turn called _emscripten_throw_longjmp and that is @@ -163,9 +159,23 @@ func (v *InvokeFunc) Call(ctx context.Context, mod api.Module, stack []uint64) { // This is the equivalent of "_setThrew(1, 0);". savedStack[0] = 1 savedStack[1] = 0 - err = mod.ExportedFunction("setThrew").CallWithStack(ctx, savedStack[:]) + callOrPanic(ctx, mod, "setThrew", savedStack[:]) + } +} + +// maybeCallOrPanic calls a given function if it is exported, otherwise panics. +// +// This ensures if the given name is exported before calling it. In other words, this explicitly checks if an api.Function +// returned by api.Module.ExportedFunction is not nil. This is necessary because directly calling a method which is +// potentially nil interface can be fatal on some platforms due to a bug? in Go/QEMU. +// See https://github.com/tetratelabs/wazero/issues/1621 +func callOrPanic(ctx context.Context, m api.Module, name string, stack []uint64) { + if srf := m.ExportedFunction(name); srf != nil { + err := srf.CallWithStack(ctx, stack) if err != nil { - panic(err) // setThrew failed + panic(err) } + } else { + panic(fmt.Sprintf("%s not exported", name)) } } From c2db7e8f6e84927ea4f7f1df11f5c090cb0892ce Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Thu, 10 Aug 2023 08:23:03 +0900 Subject: [PATCH 2/2] test Signed-off-by: Takeshi Yoneda --- internal/emscripten/emscripten.go | 4 ++-- internal/emscripten/emscripten_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 internal/emscripten/emscripten_test.go diff --git a/internal/emscripten/emscripten.go b/internal/emscripten/emscripten.go index f8d3d95d3a..3262e67020 100644 --- a/internal/emscripten/emscripten.go +++ b/internal/emscripten/emscripten.go @@ -170,8 +170,8 @@ func (v *InvokeFunc) Call(ctx context.Context, mod api.Module, stack []uint64) { // potentially nil interface can be fatal on some platforms due to a bug? in Go/QEMU. // See https://github.com/tetratelabs/wazero/issues/1621 func callOrPanic(ctx context.Context, m api.Module, name string, stack []uint64) { - if srf := m.ExportedFunction(name); srf != nil { - err := srf.CallWithStack(ctx, stack) + if f := m.ExportedFunction(name); f != nil { + err := f.CallWithStack(ctx, stack) if err != nil { panic(err) } diff --git a/internal/emscripten/emscripten_test.go b/internal/emscripten/emscripten_test.go new file mode 100644 index 0000000000..daea7b3772 --- /dev/null +++ b/internal/emscripten/emscripten_test.go @@ -0,0 +1,26 @@ +package emscripten + +import ( + "context" + "testing" + + "github.com/tetratelabs/wazero/api" + "github.com/tetratelabs/wazero/experimental/wazerotest" + "github.com/tetratelabs/wazero/internal/testing/require" +) + +func Test_callOnPanic(t *testing.T) { + const exists = "f" + var called bool + f := wazerotest.NewFunction(func(context.Context, api.Module) { called = true }) + f.ExportNames = []string{exists} + m := wazerotest.NewModule(nil, f) + t.Run("exists", func(t *testing.T) { + callOrPanic(context.Background(), m, exists, nil) + require.True(t, called) + }) + t.Run("not exist", func(t *testing.T) { + err := require.CapturePanic(func() { callOrPanic(context.Background(), m, "not-exist", nil) }) + require.EqualError(t, err, "not-exist not exported") + }) +}