* service: Prevent panics from crashing delve and killing the target

Catch all unrecovered proc and debugger panics in the service layer and
report them as errors, allow users to cleanly detach from the target
and quit.

Fixes #614

* proc: Next/Step should not panic if line info can not be found.

Fixes #683
This commit is contained in:
Alessandro Arzilli 2017-01-10 00:21:54 +01:00 committed by Derek Parker
parent 4dd627f669
commit 1afcc6c189
5 changed files with 109 additions and 9 deletions

7
_fixtures/issue683.go Normal file

@ -0,0 +1,7 @@
package main
import "fmt"
func main() {
fmt.Println("adssd") // put a breakpoint here
}

@ -3,6 +3,7 @@ package line
import ( import (
"bytes" "bytes"
"encoding/binary" "encoding/binary"
"errors"
"fmt" "fmt"
"github.com/derekparker/delve/dwarf/util" "github.com/derekparker/delve/dwarf/util"
@ -106,8 +107,13 @@ func (dbl *DebugLines) AllPCsForFileLine(f string, l int) (pcs []uint64) {
return return
} }
func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) []uint64 { var NoSourceError = errors.New("no source available")
func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) ([]uint64, error) {
lineInfo := dbl.GetLineInfo(filename) lineInfo := dbl.GetLineInfo(filename)
if lineInfo == nil {
return nil, NoSourceError
}
var ( var (
pcs []uint64 pcs []uint64
lastaddr uint64 lastaddr uint64
@ -125,7 +131,7 @@ func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) []uint6
pcs = append(pcs, sm.address) pcs = append(pcs, sm.address)
} }
} }
return pcs return pcs, nil
} }
func findAndExecOpcode(sm *StateMachine, buf *bytes.Buffer, b byte) { func findAndExecOpcode(sm *StateMachine, buf *bytes.Buffer, b byte) {

@ -2357,3 +2357,25 @@ func TestNegativeIntEvaluation(t *testing.T) {
} }
}) })
} }
func TestIssue683(t *testing.T) {
// Step panics when source file can not be found
withTestProcess("issue683", t, func(p *Process, fixture protest.Fixture) {
_, err := setFunctionBreakpoint(p, "main.main")
assertNoError(err, t, "setFunctionBreakpoint()")
assertNoError(p.Continue(), t, "First Continue()")
goterror := false
for i := 0; i < 20; i++ {
// eventually an error about the source file not being found will be
// returned, the important thing is that we shouldn't panic
err := p.Step()
if err != nil {
goterror = true
break
}
}
if !goterror {
t.Fatal("expeceted an error we didn't get")
}
})
}

@ -159,6 +159,13 @@ func (dbp *Process) next(stepInto bool) error {
return err return err
} }
success := false
defer func() {
if !success {
dbp.ClearInternalBreakpoints()
}
}()
csource := filepath.Ext(topframe.Current.File) != ".go" csource := filepath.Ext(topframe.Current.File) != ".go"
thread := dbp.CurrentThread thread := dbp.CurrentThread
currentGoroutine := false currentGoroutine := false
@ -182,14 +189,12 @@ func (dbp *Process) next(stepInto bool) error {
if instr.DestLoc != nil && instr.DestLoc.Fn != nil { if instr.DestLoc != nil && instr.DestLoc.Fn != nil {
if err := dbp.setStepIntoBreakpoint([]AsmInstruction{instr}, cond); err != nil { if err := dbp.setStepIntoBreakpoint([]AsmInstruction{instr}, cond); err != nil {
dbp.ClearInternalBreakpoints()
return err return err
} }
} else { } else {
// Non-absolute call instruction, set a StepBreakpoint here // Non-absolute call instruction, set a StepBreakpoint here
if _, err := dbp.SetBreakpoint(instr.Loc.PC, StepBreakpoint, cond); err != nil { if _, err := dbp.SetBreakpoint(instr.Loc.PC, StepBreakpoint, cond); err != nil {
if _, ok := err.(BreakpointExistsError); !ok { if _, ok := err.(BreakpointExistsError); !ok {
dbp.ClearInternalBreakpoints()
return err return err
} }
} }
@ -222,7 +227,6 @@ func (dbp *Process) next(stepInto bool) error {
bp, err := dbp.SetBreakpoint(deferpc, NextDeferBreakpoint, cond) bp, err := dbp.SetBreakpoint(deferpc, NextDeferBreakpoint, cond)
if err != nil { if err != nil {
if _, ok := err.(BreakpointExistsError); !ok { if _, ok := err.(BreakpointExistsError); !ok {
dbp.ClearInternalBreakpoints()
return err return err
} }
} }
@ -233,7 +237,10 @@ func (dbp *Process) next(stepInto bool) error {
} }
// Add breakpoints on all the lines in the current function // Add breakpoints on all the lines in the current function
pcs := dbp.lineInfo.AllPCsBetween(topframe.FDE.Begin(), topframe.FDE.End()-1, topframe.Current.File) pcs, err := dbp.lineInfo.AllPCsBetween(topframe.FDE.Begin(), topframe.FDE.End()-1, topframe.Current.File)
if err != nil {
return err
}
if !csource { if !csource {
var covered bool var covered bool
@ -254,6 +261,7 @@ func (dbp *Process) next(stepInto bool) error {
// Add a breakpoint on the return address for the current frame // Add a breakpoint on the return address for the current frame
pcs = append(pcs, topframe.Ret) pcs = append(pcs, topframe.Ret)
success = true
return dbp.setInternalBreakpoints(topframe.Current.PC, pcs, NextBreakpoint, cond) return dbp.setInternalBreakpoints(topframe.Current.PC, pcs, NextBreakpoint, cond)
} }

@ -1,6 +1,7 @@
package rpccommon package rpccommon
import ( import (
"bytes"
"errors" "errors"
"fmt" "fmt"
"io" "io"
@ -10,6 +11,7 @@ import (
"net/rpc" "net/rpc"
"net/rpc/jsonrpc" "net/rpc/jsonrpc"
"reflect" "reflect"
"runtime"
"sync" "sync"
"unicode" "unicode"
"unicode/utf8" "unicode/utf8"
@ -285,8 +287,18 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
if mtype.Synchronous { if mtype.Synchronous {
replyv = reflect.New(mtype.ReplyType.Elem()) replyv = reflect.New(mtype.ReplyType.Elem())
function := mtype.method.Func function := mtype.method.Func
returnValues := function.Call([]reflect.Value{mtype.Rcvr, argv, replyv}) var returnValues []reflect.Value
errInter := returnValues[0].Interface() var errInter interface{}
func() {
defer func() {
if ierr := recover(); ierr != nil {
errInter = newInternalError(ierr, 2)
}
}()
returnValues = function.Call([]reflect.Value{mtype.Rcvr, argv, replyv})
errInter = returnValues[0].Interface()
}()
errmsg := "" errmsg := ""
if errInter != nil { if errInter != nil {
errmsg = errInter.(error).Error() errmsg = errInter.(error).Error()
@ -296,7 +308,14 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
} else { } else {
function := mtype.method.Func function := mtype.method.Func
ctl := &RPCCallback{s, sending, codec, req} ctl := &RPCCallback{s, sending, codec, req}
go function.Call([]reflect.Value{mtype.Rcvr, argv, reflect.ValueOf(ctl)}) go func() {
defer func() {
if ierr := recover(); ierr != nil {
ctl.Return(nil, newInternalError(ierr, 2))
}
}()
function.Call([]reflect.Value{mtype.Rcvr, argv, reflect.ValueOf(ctl)})
}()
} }
} }
codec.Close() codec.Close()
@ -350,3 +369,41 @@ func (s *RPCServer) SetApiVersion(args api.SetAPIVersionIn, out *api.SetAPIVersi
s.s.config.APIVersion = args.APIVersion s.s.config.APIVersion = args.APIVersion
return nil return nil
} }
type internalError struct {
Err interface{}
Stack []internalErrorFrame
}
type internalErrorFrame struct {
Pc uintptr
Func string
File string
Line int
}
func newInternalError(ierr interface{}, skip int) *internalError {
r := &internalError{ierr, nil}
for i := skip; ; i++ {
pc, file, line, ok := runtime.Caller(i)
if !ok {
break
}
fname := "<unknown>"
fn := runtime.FuncForPC(pc)
if fn != nil {
fname = fn.Name()
}
r.Stack = append(r.Stack, internalErrorFrame{pc, fname, file, line})
}
return r
}
func (err *internalError) Error() string {
var out bytes.Buffer
fmt.Fprintf(&out, "Internal debugger error: %v\n", err.Err)
for _, frame := range err.Stack {
fmt.Fprintf(&out, "%s (%#x)\n\t%s%d\n", frame.Func, frame.Pc, frame.File, frame.Line)
}
return out.String()
}