Store function state in a stack

As functions are called store the current execution state for the function in a stack. This includes the local variables and re.group.{N} variables. This stack is used to detect recursion and prevent an interpreter panic. In the future this information could be used for including a VCL stack trace in error output or for accessing calling function state in the debugger.
ysugimoto · Apr 3, 2024 · b26ea74 · b26ea74
1 parent 65518ed
commit b26ea74
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 35 deletions.
diff --git a/interpreter/expression.go b/interpreter/expression.go
@@ -36,7 +36,7 @@ func (i *Interpreter) IdentValue(val string, withCondition bool) (value.Value, e
 	} else if _, ok := i.ctx.Ratecounters[val]; ok {
 		return &value.Ident{Value: val, Literal: true}, nil
 	} else if strings.HasPrefix(val, "var.") {
-		if v, err := i.localVars.Get(val); err != nil {
+		if v, err := i.StackPointer.Locals.Get(val); err != nil {
 			return value.Null, errors.WithStack(err)
 		} else {
 			return v, nil

diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go
@@ -22,9 +22,14 @@ import (
 	"github.com/ysugimoto/falco/parser"
 )
 
+type StackFrame struct {
+	Locals     variable.LocalVariables
+	Regex      map[string]*value.String
+	Subroutine *ast.SubroutineDeclaration
+}
+
 type Interpreter struct {
 	vars      variable.Variable
-	localVars variable.LocalVariables
 	lock      sync.Mutex
 
 	options []context.Option
@@ -36,15 +41,25 @@ type Interpreter struct {
 	IdentResolver func(v string) value.Value
 
 	TestingState State
+
+	Stack        []*StackFrame
+	StackPointer *StackFrame
 }
 
 func New(options ...context.Option) *Interpreter {
+	stack := []*StackFrame{
+		{
+			Locals: variable.LocalVariables{},
+			Regex:  map[string]*value.String{},
+		},
+	}
 	return &Interpreter{
 		options:      options,
 		cache:        cache.New(),
-		localVars:    variable.LocalVariables{},
 		Debugger:     DefaultDebugger{},
 		TestingState: NONE,
+		Stack:        stack,
+		StackPointer: stack[0],
 	}
 }
 

diff --git a/interpreter/statement.go b/interpreter/statement.go
@@ -196,7 +196,7 @@ func (i *Interpreter) ProcessBlockStatement(
 }
 
 func (i *Interpreter) ProcessDeclareStatement(stmt *ast.DeclareStatement) error {
-	return i.localVars.Declare(stmt.Name.Value, stmt.ValueType.Value)
+	return i.StackPointer.Locals.Declare(stmt.Name.Value, stmt.ValueType.Value)
 }
 
 func (i *Interpreter) ProcessReturnStatement(stmt *ast.ReturnStatement) State {
@@ -213,7 +213,7 @@ func (i *Interpreter) ProcessSetStatement(stmt *ast.SetStatement) error {
 	}
 
 	if strings.HasPrefix(stmt.Ident.Value, "var.") {
-		err = i.localVars.Set(stmt.Ident.Value, stmt.Operator.Operator, right)
+		err = i.StackPointer.Locals.Set(stmt.Ident.Value, stmt.Operator.Operator, right)
 	} else {
 		err = i.vars.Set(i.ctx.Scope, stmt.Ident.Value, stmt.Operator.Operator, right)
 	}
@@ -252,7 +252,7 @@ func (i *Interpreter) ProcessAddStatement(stmt *ast.AddStatement) error {
 func (i *Interpreter) ProcessUnsetStatement(stmt *ast.UnsetStatement) error {
 	var err error
 	if strings.HasPrefix(stmt.Ident.Value, "var.") {
-		err = i.localVars.Unset(stmt.Ident.Value)
+		err = i.StackPointer.Locals.Unset(stmt.Ident.Value)
 	} else {
 		err = i.vars.Unset(i.ctx.Scope, stmt.Ident.Value)
 	}
@@ -267,7 +267,7 @@ func (i *Interpreter) ProcessRemoveStatement(stmt *ast.RemoveStatement) error {
 	// Alias of unset
 	var err error
 	if strings.HasPrefix(stmt.Ident.Value, "var.") {
-		err = i.localVars.Unset(stmt.Ident.Value)
+		err = i.StackPointer.Locals.Unset(stmt.Ident.Value)
 	} else {
 		err = i.vars.Unset(i.ctx.Scope, stmt.Ident.Value)
 	}

diff --git a/interpreter/statement_test.go b/interpreter/statement_test.go
@@ -95,15 +95,15 @@ func TestDeclareStatement(t *testing.T) {
 
 	for _, tt := range tests {
 		ip := New(nil)
-		err := ip.localVars.Declare(tt.decl.Name.Value, tt.decl.ValueType.Value)
+		err := ip.StackPointer.Locals.Declare(tt.decl.Name.Value, tt.decl.ValueType.Value)
 		if err != nil {
 			if !tt.isError {
 				t.Errorf("%s: unexpected error returned: %s", tt.name, err)
 			}
 			continue
 		}
 
-		v, err := ip.localVars.Get(tt.decl.Name.Value)
+		v, err := ip.StackPointer.Locals.Get(tt.decl.Name.Value)
 		if err != nil {
 			t.Errorf("%s: %s varible must be declared: %s", tt.name, tt.decl.Name.Value, err)
 			continue
@@ -191,7 +191,7 @@ func TestSetStatement(t *testing.T) {
 
 	for _, tt := range tests {
 		ip := New(nil)
-		if err := ip.localVars.Declare("var.foo", "INTEGER"); err != nil {
+		if err := ip.StackPointer.Locals.Declare("var.foo", "INTEGER"); err != nil {
 			t.Errorf("%s: unexpected error returned: %s", tt.name, err)
 		}
 

diff --git a/interpreter/subroutine.go b/interpreter/subroutine.go
@@ -20,20 +20,47 @@ func (i *Interpreter) ProcessTestSubroutine(scope context.Scope, sub *ast.Subrou
 	return nil
 }
 
-func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (State, error) {
-	i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub))
+func (i *Interpreter) subroutineInStack(sub *ast.SubroutineDeclaration) bool {
+	for _, s := range i.Stack {
+		if s.Subroutine == sub {
+			return true
+		}
+	}
+	return false
+}
+
+func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) {
+	sf := &StackFrame{
+		Locals:     variable.LocalVariables{},
+		Regex:      make(map[string]*value.String),
+		Subroutine: sub,
+	}
+	i.Stack = append(i.Stack, sf)
+	i.StackPointer = sf
+	i.ctx.RegexMatchedValues = sf.Regex
+}
 
-	// Store the current values and restore after subroutine has ended
-	regex := i.ctx.RegexMatchedValues
-	local := i.localVars
-	i.ctx.RegexMatchedValues = make(map[string]*value.String)
-	i.localVars = variable.LocalVariables{}
+func (i *Interpreter) popStackFrame() {
+	var sf *StackFrame
+	sf, i.Stack = i.Stack[len(i.Stack)-1], i.Stack[:len(i.Stack)-1]
+	if len(i.Stack) > 0 {
+		i.StackPointer = i.Stack[len(i.Stack)-1]
+	} else {
+		i.StackPointer = nil
+	}
+	i.ctx.SubroutineCalls[sf.Subroutine.Name.Value]++
+	i.ctx.RegexMatchedValues = sf.Regex
+}
 
-	defer func() {
-		i.ctx.RegexMatchedValues = regex
-		i.localVars = local
-		i.ctx.SubroutineCalls[sub.Name.Value]++
-	}()
+func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (State, error) {
+	if i.subroutineInStack(sub) {
+		return NONE, errors.WithStack(
+			errors.Errorf("Recursion detected, subroutine %s already in stack", sub.Name.Value),
+		)
+	}
+	i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub))
+	i.pushStackFrame(sub)
+	defer i.popStackFrame()
 
 	// Try to extract fastly reserved subroutine macro
 	if err := i.extractBoilerplateMacro(sub); err != nil {
@@ -52,19 +79,14 @@ func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds Debug
 
 // nolint: gocognit
 func (i *Interpreter) ProcessFunctionSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (value.Value, State, error) {
+	if i.subroutineInStack(sub) {
+		return value.Null, NONE, errors.WithStack(
+			errors.Errorf("Recursion detected, subroutine %s already in stack", sub.Name.Value),
+		)
+	}
 	i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub))
-
-	// Store the current values and restore after subroutine has ended
-	regex := i.ctx.RegexMatchedValues
-	local := i.localVars
-	i.ctx.RegexMatchedValues = make(map[string]*value.String)
-	i.localVars = variable.LocalVariables{}
-
-	defer func() {
-		i.ctx.RegexMatchedValues = regex
-		i.localVars = local
-		i.ctx.SubroutineCalls[sub.Name.Value]++
-	}()
+	i.pushStackFrame(sub)
+	defer i.popStackFrame()
 
 	var err error
 	var debugState DebugState = ds

diff --git a/interpreter/subroutine_test.go b/interpreter/subroutine_test.go
@@ -44,6 +44,17 @@ func TestSubroutine(t *testing.T) {
 			}`,
 			isError: true,
 		},
+		{
+			name: "Recursion produces an error not a panic",
+			vcl: `sub func {
+				call func();
+			}
+
+			sub vcl_recv {
+				call func();
+			}`,
+			isError: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -58,6 +69,7 @@ func TestFunctionSubroutine(t *testing.T) {
 		name       string
 		vcl        string
 		assertions map[string]value.Value
+		isError    bool
 	}{
 		{
 			name: "Functional subroutine returns a value",
@@ -134,11 +146,22 @@ func TestFunctionSubroutine(t *testing.T) {
 				"req.http.X-Int-Value": &value.String{Value: "2"},
 			},
 		},
+		{
+			name: "Recursion produces an error not a panic",
+			vcl: `sub func STRING {
+				return func();
+			}
+
+			sub vcl_recv {
+				set req.http.foo = func();
+			}`,
+			isError: true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assertInterpreter(t, tt.vcl, context.RecvScope, tt.assertions, false)
+			assertInterpreter(t, tt.vcl, context.RecvScope, tt.assertions, tt.isError)
 		})
 	}
 }