interp.FilterStack: handle panics and small API improvements

When a panic happens, we want to get the stack trace from the oldest
panic, before runCfg unwinds everything.

However, at that point we don't know yet whether we will be recovered.
As a silly kludge, currently storing the oldest panic in a list on the
Interpreter struct which can then be queried once we're ready.

The approach taken is not strictly correct: if a panic is recovered, and
never queried, and later the same error occurs again and then is not
recovered, the wrong call stack will be returned.

interp/interp.go

@@ -20,6 +20,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"runtime"
+	"runtime/debug"
 	"strconv"
 	"strings"
 	"sync"
@@ -222,6 +223,7 @@ type Interpreter struct {
 
 	debugger *Debugger
 	calls    map[uintptr]*node // for translating runtime stacktrace, see FilterStack()
+	panics   []*Panic          // list of panics we have had, see GetOldestPanicForErr()
 }
 
 const (
@@ -250,6 +252,7 @@ var Symbols = Exports{
 		"Interpreter": reflect.ValueOf((*Interpreter)(nil)),
 		"Options":     reflect.ValueOf((*Options)(nil)),
 		"Panic":       reflect.ValueOf((*Panic)(nil)),
+		"IFunc":       reflect.ValueOf((*IFunc)(nil)),
 	},
 }
 
@@ -263,24 +266,6 @@ type _error struct {
 
 func (w _error) Error() string { return w.WError() }
 
-// Panic is an error recovered from a panic call in interpreted code.
-type Panic struct {
-	// Value is the recovered value of a call to panic.
-	Value interface{}
-
-	// Callers is the call stack obtained from the recover call.
-	// It may be used as the parameter to runtime.CallersFrames.
-	Callers []uintptr
-
-	// Stack is the call stack buffer for debug.
-	Stack []byte
-}
-
-// TODO: Capture interpreter stack frames also and remove
-// fmt.Fprintln(n.interp.stderr, oNode.cfgErrorf("panic")) in runCfg.
-
-func (e Panic) Error() string { return fmt.Sprint(e.Value) }
-
 // Walk traverses AST n in depth first order, call cbin function
 // at node entry and cbout function at node exit.
 func (n *node) Walk(in func(n *node) bool, out func(n *node)) {
@@ -338,6 +323,7 @@ func New(options Options) *Interpreter {
 		rdir:     map[string]bool{},
 		hooks:    &hooks{},
 		calls:    map[uintptr]*node{},
+		panics:   []*Panic{},
 	}
 
 	if i.opt.stdin = options.Stdin; i.opt.stdin == nil {
@@ -597,28 +583,46 @@ func (f *Func) Name() string {
 	return f.name
 }
 
-func (interp *Interpreter) FuncForCall(handle uintptr) (*Func, error) {
+type IFunc interface {
+	Entry() uintptr
+	FileLine(uintptr) (string, int)
+	Name() string
+}
+
+// return call if we know it, pass to runtime.FuncForPC otherwise
+func (interp *Interpreter) FuncForPC(handle uintptr) IFunc {
 	n, ok := interp.calls[handle]
 	if !ok {
-		return nil, fmt.Errorf("Call not found")
+		return runtime.FuncForPC(handle)
 	}
 	pos := n.interp.fset.Position(n.pos)
 	return &Func{
 		pos,
 		funcName(n),
 		handle,
-	}, nil
+	}
+}
+
+func (interp *Interpreter) FilteredStack() []byte {
+	return interp.FilterStack(debug.Stack())
+}
+
+func (interp *Interpreter) FilteredCallers() []uintptr {
+	pc := make([]uintptr, 64)
+	runtime.Callers(0, pc)
+	_, fPc := interp.FilterStackAndCallers(debug.Stack(), pc, 2)
+	return fPc
 }
 
 func (interp *Interpreter) FilterStack(stack []byte) []byte {
-	newStack, _ := interp.FilterStackAndCallers(stack, []uintptr{})
+	newStack, _ := interp.FilterStackAndCallers(stack, []uintptr{}, 2)
 	return newStack
 }
 
 // Given a runtime stacktrace and callers list, filter out the interpreter runtime
 // and replace it with the interpreted calls. Parses runtime stacktrace to figure
 // out which interp node by placing a magic value in parameters to runCfg and callBin
-func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr) ([]byte, []uintptr) {
+func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr, skip int) ([]byte, []uintptr) {
 	newFrames := [][]string{}
 	newCallers := []uintptr{}
 
@@ -638,6 +642,7 @@ func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr
 	}
 
 	// Parse stack in reverse order, because sometimes we want to skip frames
+	var lastInterpFrame int
 	for i := len(stackLines) - 1; i >= 0; i-- {
 		// Split stack trace into paragraphs (frames)
 		if len(stackLines[i]) == 0 || stackLines[i][0] == '\t' {
@@ -664,12 +669,15 @@ func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr
 		if callersIndex >= 0 {
 			callName := runtime.FuncForPC(callers[callersIndex]).Name()
 			if callName != strings.Split(p[0], "(")[0] {
-				// since we're walking stack and callers at the same time they
-				// should be in sync. If not, we stop messing with callers
-				for ; callersIndex >= 0; callersIndex-- {
-					newCallers = append(newCallers, callers[callersIndex])
+				// for some reason runtime.gopanic shows up as panic in stacktrace
+				if callName != "runtime.gopanic" || strings.Split(p[0], "(")[0] != "panic" {
+					// since we're walking stack and callers at the same time they
+					// should be in sync. If not, we stop messing with callers
+					for ; callersIndex >= 0; callersIndex-- {
+						newCallers = append(newCallers, callers[callersIndex])
+					}
+					callersIndex = dontSync
 				}
-				callersIndex = dontSync
 			}
 		}
 
@@ -703,13 +711,20 @@ func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr
 		}
 
 		var handle uintptr
+		originalExecNode := false
 
 		// A runCfg call refers to an interpreter level call
 		// grab callHandle from the first parameter to it
 		if strings.HasPrefix(funcPath[1], "runCfg(") {
 			fmt.Sscanf(funcPath[1], "runCfg(%v,", &handle)
 		}
 
+		// capture node that panicked
+		if strings.HasPrefix(funcPath[1], "runCfgPanic(") {
+			fmt.Sscanf(funcPath[1], "runCfgPanic(%v,", &handle)
+			originalExecNode = true
+		}
+
 		// callBin is a call to a binPkg
 		// the callHandle will be on the first or second function literal
 		if funcPath[1] == "callBin" &&
@@ -720,38 +735,53 @@ func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr
 			skipFrame = 2
 		}
 
-		// special case for panic builtin
-		if funcPath[1] == "_panic" && strings.HasPrefix(funcPath[2], "func1(") {
-			fmt.Sscanf(strings.Split(funcPath[2], "(")[1], "%v,", &handle)
-		}
-
 		if handle != 0 {
 			if callersIndex >= 0 {
 				newCallers = append(newCallers, handle)
 			}
 			n, ok := interp.calls[handle]
-			if !ok || n.kind != callExpr {
+
+			// Don't print scopes that weren't function calls
+			// (unless they're the node that caused the panic)
+			if !ok || (n.kind != callExpr && !originalExecNode) {
 				continue
 			}
+
 			pos := n.interp.fset.Position(n.pos)
-			newFrames = append(newFrames, []string{
+			newFrame := []string{
 				funcName(n) + "()",
 				fmt.Sprintf("\t%s", pos),
-			})
+			}
+
+			// we only find originalExecNode a few frames later
+			// so place it right after the last interpreted frame
+			if originalExecNode && len(newFrames) != lastInterpFrame {
+				newFrames = append(
+					newFrames[:lastInterpFrame+1],
+					newFrames[lastInterpFrame:]...)
+				newFrames[lastInterpFrame] = newFrame
+			} else {
+				newFrames = append(newFrames, newFrame)
+			}
+			lastInterpFrame = len(newFrames)
 		}
 	}
 
 	// reverse order because we parsed from bottom up, fix that now.
 	newStack := []string{}
-	for i := len(newFrames) - 1; i >= 0; i-- {
+	newStack = append(newStack, newFrames[len(newFrames)-1]...) // skip after goroutine id
+	for i := len(newFrames) - 2 - skip; i >= 0; i-- {
 		newStack = append(newStack, newFrames[i]...)
 	}
 	unreversedNewCallers := []uintptr{}
-	for i := len(newCallers) - 1; i >= 0; i-- {
-		unreversedNewCallers = append(unreversedNewCallers, newCallers[i])
-	}
-	if len(unreversedNewCallers) == 0 {
-		unreversedNewCallers = callers // just pass the original through
+	if len(newCallers) == 0 {
+		if len(callers) >= skip {
+			unreversedNewCallers = callers[skip:] // just pass the original through
+		}
+	} else {
+		for i := len(newCallers) - 1 - skip; i >= 0; i-- {
+			unreversedNewCallers = append(unreversedNewCallers, newCallers[i])
+		}
 	}
 
 	newStackJoined := strings.Join(newStack, "\n")
@@ -760,6 +790,64 @@ func (interp *Interpreter) FilterStackAndCallers(stack []byte, callers []uintptr
 	return newStackBytes, unreversedNewCallers
 }
 
+// Panic is an error recovered from a panic call in interpreted code.
+type Panic struct {
+	// Value is the recovered value of a call to panic.
+	Value interface{}
+
+	// Callers is the call stack obtained from the recover call.
+	// It may be used as the parameter to runtime.CallersFrames.
+	Callers []uintptr
+
+	// Stack is the call stack buffer for debug.
+	Stack []byte
+
+	// Interpreter runtime frames replaced by interpreted code
+	FilteredCallers []uintptr
+	FilteredStack   []byte
+}
+
+func (e Panic) Error() string {
+	return fmt.Sprintf("panic: %s\n%s\n", e.Value, e.FilteredStack)
+}
+
+// Store a panic record if this is an error we have not seen.
+// Not strictly correct: code might recover from err and never
+// call GetOldestPanicForErr(), and we later return the wrong one.
+func (interp *Interpreter) Panic(err interface{}) {
+	if len(interp.panics) > 0 && interp.panics[len(interp.panics)-1].Value == err {
+		return
+	}
+	pc := make([]uintptr, 64)
+	runtime.Callers(0, pc)
+	stack := debug.Stack()
+	fStack, fPc := interp.FilterStackAndCallers(stack, pc, 2)
+	interp.panics = append(interp.panics, &Panic{
+		Value:           err,
+		Callers:         pc,
+		Stack:           stack,
+		FilteredCallers: fPc,
+		FilteredStack:   fStack,
+	})
+}
+
+// We want to capture the full stacktrace from where the panic originated.
+// Return oldest panic that matches err. Then, clear out the list of panics.
+func (interp *Interpreter) GetOldestPanicForErr(err interface{}) *Panic {
+	if _, ok := err.(*Panic); ok {
+		return err.(*Panic)
+	}
+	r := (*Panic)(nil)
+	for i := len(interp.panics) - 1; i >= 0; i-- {
+		if interp.panics[i].Value == err {
+			r = interp.panics[i]
+			break
+		}
+	}
+	interp.panics = []*Panic{}
+	return r
+}
+
 // Eval evaluates Go code represented as a string. Eval returns the last result
 // computed by the interpreter, and a non nil error in case of failure.
 func (interp *Interpreter) Eval(src string) (res reflect.Value, err error) {

interp/program.go

@@ -5,8 +5,6 @@ import (
 	"go/ast"
 	"os"
 	"reflect"
-	"runtime"
-	"runtime/debug"
 )
 
 // A Program is Go code that has been parsed and compiled.
@@ -126,9 +124,8 @@ func (interp *Interpreter) Execute(p *Program) (res reflect.Value, err error) {
 	defer func() {
 		r := recover()
 		if r != nil {
-			var pc [64]uintptr // 64 frames should be enough.
-			n := runtime.Callers(1, pc[:])
-			err = Panic{Value: r, Callers: pc[:n], Stack: debug.Stack()}
+			interp.Panic(r)
+			err = interp.GetOldestPanicForErr(r)
 		}
 	}()
 

interp/run.go

@@ -172,6 +172,12 @@ func originalExecNode(n *node, exec bltn) *node {
 	return originalNode
 }
 
+// callHandle is just to show up in debug.Stack, see interp.FilterStack(), must be first arg
+//go:noinline
+func runCfgPanic(callHandle uintptr, o *node, err interface{}) {
+	o.interp.Panic(err)
+}
+
 // Functions set to run during execution of CFG.
 // runCfg executes a node AST by walking its CFG and running node builtin at each step.
 // callHandle is just to show up in debug.Stack, see interp.FilterStack(), must be first arg
@@ -188,7 +194,9 @@ func runCfg(callHandle uintptr, n *node, f *frame, funcNode, callNode *node) {
 			if oNode == nil {
 				oNode = n
 			}
-			fmt.Fprintln(n.interp.stderr, oNode.cfgErrorf("panic"))
+			// capture node that caused panic
+			handle := oNode.interp.addCall(oNode)
+			runCfgPanic(handle, oNode, f.recovered)
 			f.mutex.Unlock()
 			panic(f.recovered)
 		}
@@ -197,7 +205,7 @@ func runCfg(callHandle uintptr, n *node, f *frame, funcNode, callNode *node) {
 
 	dbg := n.interp.debugger
 	if dbg == nil {
-		for exec := n.exec; exec != nil && f.runid() == n.interp.runid(); {
+		for exec = n.exec; exec != nil && f.runid() == n.interp.runid(); {
 			exec = exec(f)
 		}
 		return
@@ -900,15 +908,9 @@ func _recover(n *node) {
 
 func _panic(n *node) {
 	value := genValue(n.child[1])
-	handle := n.interp.addCall(n)
-
-	// callHandle is to identify this call in debug stacktrace, see interp.FilterStack(). Must be first arg.
-	panicF := func(callHandle uintptr, f *frame) bltn {
-		panic(value(f))
-	}
 
 	n.exec = func(f *frame) bltn {
-		return panicF(handle, f)
+		return panic(value(f))
 	}
 }