diff --git a/_fixtures/testfnpos.go b/_fixtures/testfnpos.go new file mode 100644 index 00000000..9572603f --- /dev/null +++ b/_fixtures/testfnpos.go @@ -0,0 +1,16 @@ +package main + +import "fmt" + +func f2() { + fmt.Printf("f2\n") +} + +func f1() { + fmt.Printf("f1\n") +} + +func main() { + f1() + f2() +} diff --git a/_fixtures/testunsafepointers.go b/_fixtures/testunsafepointers.go new file mode 100644 index 00000000..13bbecab --- /dev/null +++ b/_fixtures/testunsafepointers.go @@ -0,0 +1,17 @@ +package main + +import ( + "runtime" + "unsafe" +) + +func main() { + // We're going to produce a pointer with a bad address. + badAddr := uintptr(0x42) + unsafeDanglingPtrPtr := unsafe.Pointer(badAddr) + // We produce a **int, instead of more simply a *int, in order for the test + // program to test more complex Delve behavior. + danglingPtrPtr := (**int)(unsafeDanglingPtrPtr) + _ = danglingPtrPtr + runtime.Breakpoint() +} diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 94ef2784..1afa2294 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -1571,7 +1571,14 @@ func (scope *EvalScope) evalPointerDeref(node *ast.StarExpr) (*Variable, error) xev.Children[0].OnlyAddr = false return &(xev.Children[0]), nil } - rv := xev.maybeDereference() + xev.loadPtr() + if xev.Unreadable != nil { + val, ok := constant.Uint64Val(xev.Value) + if ok && val == 0 { + return nil, fmt.Errorf("couldn't read pointer: %w", xev.Unreadable) + } + } + rv := &xev.Children[0] if rv.Addr == 0 { return nil, fmt.Errorf("nil pointer dereference") } diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index bef2287d..58308942 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -124,6 +124,9 @@ type Variable struct { // number of elements to skip when loading a map mapSkip int + // Children lists the variables sub-variables. What constitutes a child + // depends on the variable's type. For pointers, there's one child + // representing the pointed-to variable. Children []Variable loaded bool @@ -1244,6 +1247,40 @@ func (v *Variable) maybeDereference() *Variable { } } +// loadPtr assumes that v is a pointer and loads its value. v also gets a child +// variable, representing the pointed-to value. If v is already loaded, +// loadPtr() is a no-op. +func (v *Variable) loadPtr() { + if len(v.Children) > 0 { + // We've already loaded this variable. + return + } + + t := v.RealType.(*godwarf.PtrType) + v.Len = 1 + + var child *Variable + if v.Unreadable == nil { + ptrval, err := readUintRaw(v.mem, v.Addr, t.ByteSize) + if err == nil { + child = v.newVariable("", ptrval, t.Type, DereferenceMemory(v.mem)) + } else { + // We failed to read the pointer value; mark v as unreadable. + v.Unreadable = err + } + } + + if v.Unreadable != nil { + // Pointers get a child even if their value can't be read, to + // maintain backwards compatibility. + child = v.newVariable("", 0 /* addr */, t.Type, DereferenceMemory(v.mem)) + child.Unreadable = fmt.Errorf("parent pointer unreadable: %w", v.Unreadable) + } + + v.Children = []Variable{*child} + v.Value = constant.MakeUint64(v.Children[0].Addr) +} + func loadValues(vars []*Variable, cfg LoadConfig) { for i := range vars { vars[i].loadValueInternal(0, cfg) @@ -1263,8 +1300,7 @@ func (v *Variable) loadValueInternal(recurseLevel int, cfg LoadConfig) { v.loaded = true switch v.Kind { case reflect.Ptr, reflect.UnsafePointer: - v.Len = 1 - v.Children = []Variable{*v.maybeDereference()} + v.loadPtr() if cfg.FollowPointers { // Don't increase the recursion level when dereferencing pointers // unless this is a pointer to interface (which could cause an infinite loop) diff --git a/pkg/proc/variables_test.go b/pkg/proc/variables_test.go index a46826ea..9af5956b 100644 --- a/pkg/proc/variables_test.go +++ b/pkg/proc/variables_test.go @@ -1610,3 +1610,66 @@ func TestEvalExpressionGenerics(t *testing.T) { } }) } + +// Test the behavior when reading dangling pointers produced by unsafe code. +func TestBadUnsafePtr(t *testing.T) { + withTestProcess("testunsafepointers", t, func(p *proc.Target, fixture protest.Fixture) { + assertNoError(p.Continue(), t, "Continue()") + + // danglingPtrPtr is a pointer with value 0x42, which is an unreadable + // address. + danglingPtrPtr, err := evalVariableWithCfg(p, "danglingPtrPtr", pnormalLoadConfig) + assertNoError(err, t, "eval returned an error") + t.Logf("danglingPtrPtr (%s): unreadable: %v. addr: 0x%x, value: %v", + danglingPtrPtr.TypeString(), danglingPtrPtr.Unreadable, danglingPtrPtr.Addr, danglingPtrPtr.Value) + assertNoError(danglingPtrPtr.Unreadable, t, "danglingPtrPtr is unreadable") + if val := danglingPtrPtr.Value; val == nil { + t.Fatal("Value not set danglingPtrPtr") + } + val, ok := constant.Uint64Val(danglingPtrPtr.Value) + if !ok { + t.Fatalf("Value not uint64: %v", danglingPtrPtr.Value) + } + if val != 0x42 { + t.Fatalf("expected value to be 0x42, got 0x%x", val) + } + if len(danglingPtrPtr.Children) != 1 { + t.Fatalf("expected 1 child, got: %d", len(danglingPtrPtr.Children)) + } + + badPtr, err := evalVariableWithCfg(p, "*danglingPtrPtr", pnormalLoadConfig) + assertNoError(err, t, "error evaluating *danglingPtrPtr") + t.Logf("badPtr: (%s): unreadable: %v. addr: 0x%x, value: %v", + badPtr.TypeString(), badPtr.Unreadable, badPtr.Addr, badPtr.Value) + if badPtr.Unreadable == nil { + t.Fatalf("badPtr should be unreadable") + } + if badPtr.Addr != 0x42 { + t.Fatalf("expected danglingPtr to point to 0x42, got 0x%x", badPtr.Addr) + } + if len(badPtr.Children) != 1 { + t.Fatalf("expected 1 child, got: %d", len(badPtr.Children)) + } + badPtrChild := badPtr.Children[0] + t.Logf("badPtr.Child (%s): unreadable: %v. addr: 0x%x, value: %v", + badPtrChild.TypeString(), badPtrChild.Unreadable, badPtrChild.Addr, badPtrChild.Value) + // We expect the dummy child variable to be marked as unreadable. + if badPtrChild.Unreadable == nil { + t.Fatalf("expected x to be unreadable, but got value: %v", badPtrChild.Value) + } + + // Evaluating **danglingPtrPtr fails. + _, err = evalVariableWithCfg(p, "**danglingPtrPtr", pnormalLoadConfig) + if err == nil { + t.Fatalf("expected error doing **danglingPtrPtr") + } + expErr := "couldn't read pointer" + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected \"%s\", got: \"%s\"", expErr, err) + } + nexpErr := "nil pointer dereference" + if strings.Contains(err.Error(), nexpErr) { + t.Fatalf("shouldn't have gotten \"%s\", but got: \"%s\"", nexpErr, err) + } + }) +}