summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-05-23 22:54:34 +0300
committerPaul Buetow <paul@buetow.org>2026-05-23 22:54:34 +0300
commit4fbbb78125b58a7360d9441fbd175e658e03f534 (patch)
treef330e1fb83abcafec60acaee3e8c4462656a7614
parent976a47a21d9b04bd1b2f3f62137a1f18b34f947d (diff)
fix: prevent stack corruption in popTwo() and FastPower()
popTwo() used to pop b first, then a. If the second pop failed, b was already gone — the stack was silently corrupted. Fix: add ensureStackLength() check before any pop, preserving stack state on error. If the second pop fails (should not happen after the check, but added as belt-and-suspenders), restore b to the stack. FastPower() had the same issue (two sequential popStack calls). Replaced with popTwo() for consistency. Also fixed rpn_test.go: TestParseAndEvaluateInsufficientOperands shared a single RPN instance across test cases, which relied on the old stack-corruption behavior. Now each test case gets a fresh instance.
-rw-r--r--internal/rpn/operations_arithmetic.go7
-rw-r--r--internal/rpn/operations_helpers.go8
-rw-r--r--internal/rpn/rpn_test.go5
3 files changed, 11 insertions, 9 deletions
diff --git a/internal/rpn/operations_arithmetic.go b/internal/rpn/operations_arithmetic.go
index 8504d34..fd1b0fa 100644
--- a/internal/rpn/operations_arithmetic.go
+++ b/internal/rpn/operations_arithmetic.go
@@ -233,12 +233,7 @@ func (o *Operations) Modulo(stack *Stack) error {
// FastPower pops two values from stack, raises first to integer power of second (a ** b), and pushes result.
// Uses binary exponentiation for efficiency with large integer exponents.
func (o *Operations) FastPower(stack *Stack) error {
- b, err := popStack(stack, "**")
- if err != nil {
- return err
- }
-
- a, err := popStack(stack, "**")
+ a, b, err := popTwo(stack, "**")
if err != nil {
return err
}
diff --git a/internal/rpn/operations_helpers.go b/internal/rpn/operations_helpers.go
index 3d5f944..afdfa0e 100644
--- a/internal/rpn/operations_helpers.go
+++ b/internal/rpn/operations_helpers.go
@@ -19,7 +19,12 @@ func popStack(stack *Stack, op string) (StackValue, error) {
}
// popTwo pops two values from the stack for binary operations.
+// Uses ensureStackLength to prevent stack corruption on second Pop failure.
func popTwo(stack *Stack, op string) (StackValue, StackValue, error) {
+ if err := ensureStackLength(stack, 2, op); err != nil {
+ return nil, nil, err
+ }
+
b, err := stack.Pop()
if err != nil {
return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err)
@@ -27,6 +32,9 @@ func popTwo(stack *Stack, op string) (StackValue, StackValue, error) {
a, err := stack.Pop()
if err != nil {
+ // Should not happen given ensureStackLength check above.
+ // If it does (concurrent modification), restore b and return error.
+ stack.Push(b)
return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err)
}
diff --git a/internal/rpn/rpn_test.go b/internal/rpn/rpn_test.go
index 0460271..494e377 100644
--- a/internal/rpn/rpn_test.go
+++ b/internal/rpn/rpn_test.go
@@ -302,9 +302,6 @@ func TestParseAndEvaluateUnknownToken(t *testing.T) {
}
func TestParseAndEvaluateInsufficientOperands(t *testing.T) {
- v := NewVariables()
- r := NewRPN(v)
-
tests := []struct {
name string
input string
@@ -316,6 +313,8 @@ func TestParseAndEvaluateInsufficientOperands(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+ v := NewVariables()
+ r := NewRPN(v)
_, err := r.ParseAndEvaluate(tt.input)
if err == nil {
t.Errorf("%q should return error for insufficient operands", tt.input)