From fc0d40144a9c65aaf9b851b5816d33f4107f0b75 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Fri, 10 Feb 2017 01:26:38 +0100 Subject: [PATCH] proc/variables: fix infinite recursion with pointer loop (#725) loadValue didn't react correctly to pointer loops going through slice -> interface{} -> slice or pointer -> interface{} -> pointer. --- _fixtures/testvariables2.go | 8 ++++++++ pkg/proc/proc_test.go | 15 +++++++++------ pkg/proc/variables.go | 9 +++++++-- service/api/prettyprint.go | 4 +++- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/_fixtures/testvariables2.go b/_fixtures/testvariables2.go index 9ef5108c..5f9b04c4 100644 --- a/_fixtures/testvariables2.go +++ b/_fixtures/testvariables2.go @@ -212,6 +212,14 @@ func main() { ninf := math.Inf(-1) nan := math.NaN() + var iface6 interface{} + var ptrinf *interface{} + iface6 = &ptrinf + ptrinf = &iface6 + + sliceinf := make([]interface{}, 1) + sliceinf[0] = sliceinf + var amb1 = 1 runtime.Breakpoint() for amb1 := 0; amb1 < 10; amb1++ { diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index ce279a1c..2ffadbf1 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -1440,14 +1440,17 @@ func TestIssue305(t *testing.T) { }) } -func TestIssue341(t *testing.T) { - // pointer loop through map entries +func TestPointerLoops(t *testing.T) { + // Pointer loops through map entries, pointers and slices + // Regression test for issue #341 withTestProcess("testvariables2", t, func(p *Process, fixture protest.Fixture) { assertNoError(p.Continue(), t, "Continue()") - t.Logf("requesting mapinf") - mapinf, err := evalVariable(p, "mapinf") - assertNoError(err, t, "EvalVariable()") - t.Logf("mapinf: %v\n", mapinf) + for _, expr := range []string{"mapinf", "ptrinf", "sliceinf"} { + t.Logf("requesting %s", expr) + v, err := evalVariable(p, expr) + assertNoError(err, t, fmt.Sprintf("EvalVariable(%s)", expr)) + t.Logf("%s: %v\n", expr, v) + } }) } diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index 1e284649..e9d8e899 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -812,7 +812,12 @@ func (v *Variable) loadValueInternal(recurseLevel int, cfg LoadConfig) { v.Children = []Variable{*v.maybeDereference()} if cfg.FollowPointers { // Don't increase the recursion level when dereferencing pointers - v.Children[0].loadValueInternal(recurseLevel, cfg) + // unless this is a pointer to interface (which could cause an infinite loop) + nextLvl := recurseLevel + if v.Children[0].Kind == reflect.Interface { + nextLvl++ + } + v.Children[0].loadValueInternal(nextLvl, cfg) } else { v.Children[0].OnlyAddr = true } @@ -1630,7 +1635,7 @@ func (v *Variable) loadInterface(recurseLevel int, loadData bool, cfg LoadConfig data = data.newVariable("data", data.Addr, typ) v.Children = []Variable{*data} - if loadData { + if loadData && recurseLevel <= cfg.MaxVariableRecurse { v.Children[0].loadValueInternal(recurseLevel, cfg) } else { v.Children[0].OnlyAddr = true diff --git a/service/api/prettyprint.go b/service/api/prettyprint.go index 3afb3593..7c53c1bc 100644 --- a/service/api/prettyprint.go +++ b/service/api/prettyprint.go @@ -87,7 +87,9 @@ func (v *Variable) writeTo(buf io.Writer, top, newlines, includeType bool, inden } data := v.Children[0] if data.Kind == reflect.Ptr { - if data.Children[0].Addr == 0 { + if len(data.Children) == 0 { + fmt.Fprintf(buf, "...") + } else if data.Children[0].Addr == 0 { fmt.Fprintf(buf, "nil") } else if data.Children[0].OnlyAddr { fmt.Fprintf(buf, "0x%x", v.Children[0].Addr)