From 095b49f74a5e36ce401b899a0c16de4eeb46c054 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Thu, 8 Feb 2024 12:15:17 -0800 Subject: [PATCH] Sets up the unwinding frame limit on runtime error (#2029) Signed-off-by: Takeshi Yoneda --- internal/engine/compiler/engine.go | 4 +- internal/engine/interpreter/interpreter.go | 3 ++ .../engine/wazevo/backend/isa/amd64/stack.go | 5 +++ .../wazevo/backend/isa/arm64/unwind_stack.go | 5 +++ .../integration_test/engine/adhoc_test.go | 42 ++++++++++++++++++ .../testdata/huge_call_stack_unwind.wasm | Bin 0 -> 72 bytes .../testdata/huge_call_stack_unwind.wat | 16 +++++++ internal/wasmdebug/debug.go | 6 +++ 8 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 internal/integration_test/engine/testdata/huge_call_stack_unwind.wasm create mode 100644 internal/integration_test/engine/testdata/huge_call_stack_unwind.wat diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index 9362bc9b..6f46cb4f 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -873,6 +873,7 @@ func (ce *callEngine) deferredOnCall(ctx context.Context, m *wasm.ModuleInstance stackBasePointer := int(ce.stackBasePointerInBytes >> 3) functionListeners := make([]functionListenerInvocation, 0, 16) + left := wasmdebug.MaxFrames for { def := fn.definition() @@ -886,6 +887,7 @@ func (ce *callEngine) deferredOnCall(ctx context.Context, m *wasm.ModuleInstance } } builder.AddFrame(def.DebugName(), def.ParamTypes(), def.ResultTypes(), sources) + left-- if fn.parent.listener != nil { functionListeners = append(functionListeners, functionListenerInvocation{ @@ -895,7 +897,7 @@ func (ce *callEngine) deferredOnCall(ctx context.Context, m *wasm.ModuleInstance } callFrameOffset := callFrameOffset(fn.funcType) - if stackBasePointer != 0 { + if left > 0 && stackBasePointer != 0 { frame := *(*callFrame)(unsafe.Pointer(&ce.stack[stackBasePointer+callFrameOffset])) fn = frame.function pc = uint64(frame.returnAddress) diff --git a/internal/engine/interpreter/interpreter.go b/internal/engine/interpreter/interpreter.go index 2a24ff70..9b65e9a8 100644 --- a/internal/engine/interpreter/interpreter.go +++ b/internal/engine/interpreter/interpreter.go @@ -624,6 +624,9 @@ func (ce *callEngine) recoverOnCall(ctx context.Context, m *wasm.ModuleInstance, frameCount := len(ce.frames) functionListeners := make([]functionListenerInvocation, 0, 16) + if frameCount > wasmdebug.MaxFrames { + frameCount = wasmdebug.MaxFrames + } for i := 0; i < frameCount; i++ { frame := ce.popFrame() f := frame.f diff --git a/internal/engine/wazevo/backend/isa/amd64/stack.go b/internal/engine/wazevo/backend/isa/amd64/stack.go index f113ec75..9a70ac9d 100644 --- a/internal/engine/wazevo/backend/isa/amd64/stack.go +++ b/internal/engine/wazevo/backend/isa/amd64/stack.go @@ -4,6 +4,8 @@ import ( "encoding/binary" "reflect" "unsafe" + + "github.com/tetratelabs/wazero/internal/wasmdebug" ) func stackView(rbp, top uintptr) []byte { @@ -53,6 +55,9 @@ func UnwindStack(_, rbp, top uintptr, returnAddresses []uintptr) []uintptr { retAddr := binary.LittleEndian.Uint64(stackBuf[i+8:]) returnAddresses = append(returnAddresses, uintptr(retAddr)) i = callerRBP - uint64(rbp) + if len(returnAddresses) == wasmdebug.MaxFrames { + break + } } return returnAddresses } diff --git a/internal/engine/wazevo/backend/isa/arm64/unwind_stack.go b/internal/engine/wazevo/backend/isa/arm64/unwind_stack.go index f9550420..edb0e36e 100644 --- a/internal/engine/wazevo/backend/isa/arm64/unwind_stack.go +++ b/internal/engine/wazevo/backend/isa/arm64/unwind_stack.go @@ -4,6 +4,8 @@ import ( "encoding/binary" "reflect" "unsafe" + + "github.com/tetratelabs/wazero/internal/wasmdebug" ) // UnwindStack implements wazevo.unwindStack. @@ -55,6 +57,9 @@ func UnwindStack(sp, _, top uintptr, returnAddresses []uintptr) []uintptr { sizeOfArgRet := binary.LittleEndian.Uint64(stackBuf[i:]) i += 8 + sizeOfArgRet returnAddresses = append(returnAddresses, uintptr(retAddr)) + if len(returnAddresses) == wasmdebug.MaxFrames { + break + } } return returnAddresses } diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go index 162a5468..53ca3ead 100644 --- a/internal/integration_test/engine/adhoc_test.go +++ b/internal/integration_test/engine/adhoc_test.go @@ -36,6 +36,7 @@ type testCase struct { } var tests = map[string]testCase{ + "huge call stack unwind": {f: testHugeCallStackUnwind}, "imported mutable global": {f: testImportedMutableGlobalUpdate}, "huge stack": {f: testHugeStack}, "unreachable": {f: testUnreachable}, @@ -129,6 +130,8 @@ var ( globalExtendWasm []byte //go:embed testdata/infinite_loop.wasm infiniteLoopWasm []byte + //go:embed testdata/huge_call_stack_unwind.wasm + hugeCallStackUnwind []byte ) func testEnsureTerminationOnClose(t *testing.T, r wazero.Runtime) { @@ -2127,3 +2130,42 @@ func testImportedMutableGlobalUpdate(t *testing.T, r wazero.Runtime) { v = reExportedG.Get() require.Equal(t, uint64(2), v) } + +func testHugeCallStackUnwind(t *testing.T, r wazero.Runtime) { + ctx := context.Background() + _, err := r.Instantiate(ctx, hugeCallStackUnwind) + require.Error(t, err) + require.Equal(t, `start function[0] failed: wasm error: integer divide by zero +wasm stack trace: + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + .$0() + ... maybe followed by omitted frames`, err.Error()) +} diff --git a/internal/integration_test/engine/testdata/huge_call_stack_unwind.wasm b/internal/integration_test/engine/testdata/huge_call_stack_unwind.wasm new file mode 100644 index 0000000000000000000000000000000000000000..de5dde717f29772cfc2b08b5e164d34f72afc807 GIT binary patch literal 72 zcmZQbEY4+QU|?WmVN76PU}j=uU}I;jXLNkQ&dtHdz$MKn$)L>O$l%W60HheRRTu;q YxE+B4xl-H=+$?#CxvA_dj0}wF05V$$_5c6? literal 0 HcmV?d00001 diff --git a/internal/integration_test/engine/testdata/huge_call_stack_unwind.wat b/internal/integration_test/engine/testdata/huge_call_stack_unwind.wat new file mode 100644 index 00000000..6e244bd6 --- /dev/null +++ b/internal/integration_test/engine/testdata/huge_call_stack_unwind.wat @@ -0,0 +1,16 @@ +(module + (func + ;; check global and if it is not zero, call the function again + (if (i32.ne (global.get $g) (i32.const 0)) + (then + (global.set $g (i32.sub (global.get $g) (i32.const 1))) + (call 0) + ) + ) + ;; otherwise do i32.div by zero and crash + (i32.div_s (i32.const 0) (i32.const 0)) + drop + ) + (global $g (mut i32) (i32.const 1000)) + (start 0) +) diff --git a/internal/wasmdebug/debug.go b/internal/wasmdebug/debug.go index 0aa61369..deb67493 100644 --- a/internal/wasmdebug/debug.go +++ b/internal/wasmdebug/debug.go @@ -147,6 +147,9 @@ func (s *stackTrace) FromRecovered(recovered interface{}) error { } } +// MaxFrames is the maximum number of frames to include in the stack trace. +const MaxFrames = 30 + // AddFrame implements ErrorBuilder.AddFrame func (s *stackTrace) AddFrame(funcName string, paramTypes, resultTypes []api.ValueType, sources []string) { sig := signature(funcName, paramTypes, resultTypes) @@ -154,4 +157,7 @@ func (s *stackTrace) AddFrame(funcName string, paramTypes, resultTypes []api.Val for _, source := range sources { s.frames = append(s.frames, "\t"+source) } + if len(s.frames) == MaxFrames { + s.frames = append(s.frames, "... maybe followed by omitted frames") + } }