From bec6a65b15ed9bdc1cdbeb1740a4c82a7ee8f573 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Fri, 19 Jan 2018 15:50:28 +0100 Subject: [PATCH] proc,prettyprint: guard against autodereferenced escaped pointers (#1077) Much like the bug in issue #1031 and commit f6f6f0bf13e4c708cb501202b83a6327a0f00e31 pointers can also escape to the heap and then have a zero address (and no children) when we autodereference. 1. Mark autodereferenced escaped variables with a 0 address as unreadable. 2. Add guards to the pretty printers for unsafe.Pointer and pointers. Fixes #1075 --- _fixtures/clientdo.go | 10 ++++++++++ pkg/proc/variables.go | 3 +++ service/api/prettyprint.go | 8 ++++++-- service/test/variables_test.go | 25 +++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 _fixtures/clientdo.go diff --git a/_fixtures/clientdo.go b/_fixtures/clientdo.go new file mode 100644 index 00000000..d2be3c53 --- /dev/null +++ b/_fixtures/clientdo.go @@ -0,0 +1,10 @@ +package main + +import ( + "net/http" + "net/url" +) + +func main() { + http.DefaultClient.Do(&http.Request{URL: &url.URL{}}) +} diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index eb89ec75..6d71e142 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -1938,6 +1938,9 @@ func (scope *EvalScope) variablesByTag(tag dwarf.Tag, cfg *LoadConfig) ([]*Varia for i, v := range vars { if name := v.Name; len(name) > 1 && name[0] == '&' { v = v.maybeDereference() + if v.Addr == 0 { + v.Unreadable = fmt.Errorf("no address for escaped variable") + } v.Name = name[1:] v.Flags |= VariableEscaped vars[i] = v diff --git a/service/api/prettyprint.go b/service/api/prettyprint.go index 40d7b842..4534b327 100644 --- a/service/api/prettyprint.go +++ b/service/api/prettyprint.go @@ -49,7 +49,7 @@ func (v *Variable) writeTo(buf io.Writer, top, newlines, includeType bool, inden case reflect.Array: v.writeArrayTo(buf, newlines, includeType, indent) case reflect.Ptr: - if v.Type == "" { + if v.Type == "" || len(v.Children) == 0 { fmt.Fprint(buf, "nil") } else if v.Children[0].OnlyAddr && v.Children[0].Addr != 0 { fmt.Fprintf(buf, "(%s)(0x%x)", v.Type, v.Children[0].Addr) @@ -58,7 +58,11 @@ func (v *Variable) writeTo(buf io.Writer, top, newlines, includeType bool, inden v.Children[0].writeTo(buf, false, newlines, includeType, indent) } case reflect.UnsafePointer: - fmt.Fprintf(buf, "unsafe.Pointer(0x%x)", v.Children[0].Addr) + if len(v.Children) == 0 { + fmt.Fprintf(buf, "unsafe.Pointer(nil)") + } else { + fmt.Fprintf(buf, "unsafe.Pointer(0x%x)", v.Children[0].Addr) + } case reflect.String: v.writeStringTo(buf) case reflect.Chan: diff --git a/service/test/variables_test.go b/service/test/variables_test.go index dff8d8b4..0ef00789 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -976,3 +976,28 @@ func TestConstants(t *testing.T) { } }) } + +func setFunctionBreakpoint(p proc.Process, fname string) (*proc.Breakpoint, error) { + addr, err := proc.FindFunctionLocation(p, fname, true, 0) + if err != nil { + return nil, err + } + return p.SetBreakpoint(addr, proc.UserBreakpoint, nil) +} + +func TestIssue1075(t *testing.T) { + withTestProcess("clientdo", t, func(p proc.Process, fixture protest.Fixture) { + _, err := setFunctionBreakpoint(p, "net/http.(*Client).Do") + assertNoError(err, t, "setFunctionBreakpoint") + assertNoError(proc.Continue(p), t, "Continue()") + for i := 0; i < 10; i++ { + scope, err := proc.GoroutineScope(p.CurrentThread()) + assertNoError(err, t, fmt.Sprintf("GoroutineScope (%d)", i)) + vars, err := scope.LocalVariables(pnormalLoadConfig) + assertNoError(err, t, fmt.Sprintf("LocalVariables (%d)", i)) + for _, v := range vars { + api.ConvertVar(v).SinglelineString() + } + } + }) +}