Repository: tetratelabs/wazero
Pull request #47 (2021-11-16 04:50:08)

Removes option to disable stack overflow guard
This removes risk of compilation drift by removing the option to disable stack overflow protection.

codefromthecrypt: I owe vm_stack_test.go push test backfill prior to merge

Patch:
diff --git a/wasm/buildoptions/callstack_overflow_disabled.go b/wasm/buildoptions/callstack_overflow_disabled.go
deleted file mode 100644
index 0dcac1b0..00000000
--- a/wasm/buildoptions/callstack_overflow_disabled.go
+++ /dev/null
@@ -1,9 +0,0 @@
-//go:build disable_callstack_overflow_check
-// +build disable_callstack_overflow_check
-
-package buildoptions
-
-const (
-	CheckCallStackOverflow = false
-	CallStackHeightLimit   = 0
-)
diff --git a/wasm/buildoptions/callstack_overflow_enabled.go b/wasm/buildoptions/callstack_overflow_enabled.go
deleted file mode 100644
index 1c8fdd25..00000000
--- a/wasm/buildoptions/callstack_overflow_enabled.go
+++ /dev/null
@@ -1,9 +0,0 @@
-//go:build !disable_callstack_overflow_check
-// +build !disable_callstack_overflow_check
-
-package buildoptions
-
-const (
-	CheckCallStackOverflow = true
-	CallStackHeightLimit   = 2000
-)
diff --git a/wasm/buildoptions/stackoverflow.go b/wasm/buildoptions/stackoverflow.go
new file mode 100644
index 00000000..40d40e28
--- /dev/null
+++ b/wasm/buildoptions/stackoverflow.go
@@ -0,0 +1,7 @@
+package buildoptions
+
+// CallStackHeightLimit is the maximum WebAssembly call stack height. This allows wazero to raise
+// wasm.ErrCallStackOverflow instead of overflowing the Go runtime.
+//
+// The default value should suffice for most use cases. Those wishing to change this can via `go build -ldflags`.
+var CallStackHeightLimit = 2000
diff --git a/wasm/naivevm/vm_stack.go b/wasm/naivevm/vm_stack.go
index 69e3b583..04ad6b6c 100644
--- a/wasm/naivevm/vm_stack.go
+++ b/wasm/naivevm/vm_stack.go
@@ -10,6 +10,8 @@ const (
 	initialLabelStackHeight   = 10
 )
 
+var callStackHeightLimit = buildoptions.CallStackHeightLimit
+
 func drop(vm *naiveVirtualMachine) {
 	vm.operands.drop()
 	vm.activeFrame.pc++
@@ -137,16 +139,15 @@ func (s *frameStack) pop() *frame {
 }
 
 func (s *frameStack) push(val *frame) {
-	if buildoptions.CheckCallStackOverflow {
-		if buildoptions.CallStackHeightLimit <= s.sp {
+	newSp := s.sp + 1
+	if newSp == len(s.stack) {
+		if callStackHeightLimit <= newSp {
 			panic(wasm.ErrCallStackOverflow)
 		}
-	}
-	if s.sp+1 == len(s.stack) {
 		// grow stack
 		s.stack = append(s.stack, val)
 	} else {
-		s.stack[s.sp+1] = val
+		s.stack[newSp] = val
 	}
-	s.sp++
+	s.sp = newSp
 }
diff --git a/wasm/wazeroir/interpreter.go b/wasm/wazeroir/interpreter.go
index ff30d27c..6da8354c 100644
--- a/wasm/wazeroir/interpreter.go
+++ b/wasm/wazeroir/interpreter.go
@@ -14,6 +14,8 @@ import (
 	"github.com/tetratelabs/wazero/wasm/buildoptions"
 )
 
+var callStackHeightLimit = buildoptions.CallStackHeightLimit
+
 // interpreter implements wasm.Engine interface.
 // This is the direct interpreter of wazeroir operations.
 type interpreter struct {
@@ -70,23 +72,18 @@ func (it *interpreter) drop(r *InclusiveRange) {
 }
 
 func (it *interpreter) pushFrame(frame *interpreterFrame) {
-	if buildoptions.CheckCallStackOverflow {
-		if buildoptions.CallStackHeightLimit <= len(it.frames)-1 {
-			panic(wasm.ErrCallStackOverflow)
-		}
+	if callStackHeightLimit <= len(it.frames) {
+		panic(wasm.ErrCallStackOverflow)
 	}
 	it.frames = append(it.frames, frame)
 }
 
 func (it *interpreter) popFrame() (frame *interpreterFrame) {
-	// No need to check stack bound
-	// as we can assume that all the operations
-	// are valid thanks to analyzeFunction
-	// at module validation phase
-	// and wazeroir translation
-	// before compilation.
-	frame = it.frames[len(it.frames)-1]
-	it.frames = it.frames[:len(it.frames)-1]
+	// No need to check stack bound as we can assume that all the operations are valid thanks to analyzeFunction at
+	// module validation phase and wazeroir translation before compilation.
+	oneLess := len(it.frames) - 1
+	frame = it.frames[oneLess]
+	it.frames = it.frames[:oneLess]
 	return
 }
 
@@ -115,7 +112,7 @@ type interpreterOp struct {
 	f      *interpreterFunction
 }
 
-// Implements wasm.Engine for interpreter.
+// Compile Implements wasm.Engine for interpreter.
 func (it *interpreter) Compile(f *wasm.FunctionInstance) error {
 	if _, ok := it.functions[f]; ok {
 		return nil


Test patch:
diff --git a/wasm/naivevm/vm_stack_test.go b/wasm/naivevm/vm_stack_test.go
new file mode 100644
index 00000000..0157fcc6
--- /dev/null
+++ b/wasm/naivevm/vm_stack_test.go
@@ -0,0 +1,60 @@
+package naivevm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/tetratelabs/wazero/wasm/buildoptions"
+)
+
+func TestFrameStack_Push(t *testing.T) {
+	f1 := &frame{}
+	f2 := &frame{}
+
+	fs := newFrameStack()
+
+	fs.push(f1)
+	require.Equal(t, f1, fs.stack[0])
+	require.Equal(t, 0, fs.sp)
+
+	fs.push(f2)
+	require.Equal(t, f1, fs.stack[0])
+	require.Equal(t, f2, fs.stack[1])
+	require.Equal(t, 1, fs.sp)
+}
+
+func TestFrameStack_Push_Grows(t *testing.T) {
+	f := &frame{}
+
+	fs := newFrameStack()
+
+	for i := 0; i < initialLabelStackHeight; i++ {
+		fs.push(f)
+	}
+
+	f2 := &frame{}
+	fs.push(f2) // we expect to grow
+
+	require.Equal(t, f, fs.stack[initialLabelStackHeight-1])
+	require.Equal(t, f2, fs.stack[initialLabelStackHeight])
+	require.Equal(t, initialLabelStackHeight, fs.sp)
+}
+
+func TestFrameStack_Push_StackOverflow(t *testing.T) {
+	defer func() { callStackHeightLimit = buildoptions.CallStackHeightLimit }()
+
+	f := &frame{}
+
+	fs := newFrameStack()
+
+	// in naivevm, the stack is a slice with an initial capacity, allow growing 2 past this.
+	callStackHeightLimit = initialLabelStackHeight + 2
+
+	for i := 0; i < callStackHeightLimit; i++ {
+		fs.push(f)
+	}
+
+	// we're past our limit, so we should panic
+	require.Panics(t, func() { fs.push(f) })
+}
diff --git a/wasm/wazeroir/interpreter_test.go b/wasm/wazeroir/interpreter_test.go
new file mode 100644
index 00000000..818cc711
--- /dev/null
+++ b/wasm/wazeroir/interpreter_test.go
@@ -0,0 +1,40 @@
+package wazeroir
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/tetratelabs/wazero/wasm/buildoptions"
+)
+
+func TestInterpreter_PushFrame(t *testing.T) {
+	f1 := &interpreterFrame{}
+	f2 := &interpreterFrame{}
+
+	it := interpreter{}
+	require.Empty(t, it.frames)
+
+	it.pushFrame(f1)
+	require.Equal(t, []*interpreterFrame{f1}, it.frames)
+
+	it.pushFrame(f2)
+	require.Equal(t, []*interpreterFrame{f1, f2}, it.frames)
+}
+
+func TestInterpreter_PushFrame_StackOverflow(t *testing.T) {
+	defer func() { callStackHeightLimit = buildoptions.CallStackHeightLimit }()
+
+	callStackHeightLimit = 3
+
+	f1 := &interpreterFrame{}
+	f2 := &interpreterFrame{}
+	f3 := &interpreterFrame{}
+	f4 := &interpreterFrame{}
+
+	it := interpreter{}
+	it.pushFrame(f1)
+	it.pushFrame(f2)
+	it.pushFrame(f3)
+	require.Panics(t, func() { it.pushFrame(f4) })
+}