Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store function call state in a stack #294

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion interpreter/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 19 additions & 4 deletions interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@ 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
vars variable.Variable
lock sync.Mutex

options []context.Option

Expand All @@ -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],
}
}

Expand Down
8 changes: 4 additions & 4 deletions interpreter/statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions interpreter/statement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
70 changes: 46 additions & 24 deletions interpreter/subroutine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentatively we can define the max call stack frames and raise an error if exceeded, probably around 100 would be enough but up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems reasonable. I did a test and generated a silly call chain of 200 functions in a fiddle and it didn't cause an error. Seems like the limiting factor isn't the call depth but the overall workspace memory usage which would eventually be exhausted.

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 {
Expand All @@ -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
Expand Down
25 changes: 24 additions & 1 deletion interpreter/subroutine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
Expand Down Expand Up @@ -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)
})
}
}
Loading