diff --git a/Documentation/KnownBugs.md b/Documentation/KnownBugs.md index 68c04c72..f7e56331 100644 --- a/Documentation/KnownBugs.md +++ b/Documentation/KnownBugs.md @@ -1,6 +1,5 @@ # Known Bugs -- When a function defines two (or more) variables with the same name delve is unable to distinguish between them: `locals` will print both variables, `print` will randomly pick one. See [Issue #106](https://github.com/go-delve/delve/issues/106). - Delve does not currently support 32bit systems. This will usually manifest as a compiler error in `proc/disasm.go`. See [Issue #20](https://github.com/go-delve/delve/issues/20). - When Delve is compiled with versions of go prior to 1.7.0 it is not possible to set a breakpoint on a function in a remote package using the `Receiver.MethodName` syntax. See [Issue #528](https://github.com/go-delve/delve/issues/528). -- When running Delve on binaries compiled with a version of go prior to 1.9.0 `locals` will print all local variables, including ones that are out of scope. If there are multiple variables defined with the same name in the current function `print` will not be able to select the correct one for the current line. +- When running Delve on binaries compiled with a version of go prior to 1.9.0 `locals` will print all local variables, including ones that are out of scope, the shadowed flag will be applied arbitrarily. If there are multiple variables defined with the same name in the current function `print` will not be able to select the correct one for the current line. diff --git a/_fixtures/scopetest.go b/_fixtures/scopetest.go index 8d0e4ab0..ddb2f12d 100644 --- a/_fixtures/scopetest.go +++ b/_fixtures/scopetest.go @@ -142,6 +142,18 @@ func TestClosureScope() { f(3) f1(b) } +func TestClosureShadow() { + v := 1 + closure := func() { + f1(v) // v int = 1 + v := 2 + f1(v) // v int = 1, v int = 2 + for i := 0; i < 1; i++ { + f1(v) // v int = 1, v int = 2, i int = 0 + } + } + closure() +} func main() { ch <- 12 TestNestedFor() @@ -162,4 +174,5 @@ func main() { TestDiscontiguousRanges() TestDiscontiguousRanges() TestClosureScope() + TestClosureShadow() } diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index f321bc89..e0a52458 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -105,7 +105,6 @@ func (scope *EvalScope) Locals() ([]*Variable, error) { var vars []*Variable var depths []int varReader := reader.Variables(scope.image().dwarf, scope.Fn.offset, reader.ToRelAddr(scope.PC, scope.image().StaticBase), scope.Line, true, false) - hasScopes := false for varReader.Next() { entry := varReader.Entry() val, err := extractVarInfoFromEntry(scope.BinInfo, scope.image(), scope.Regs, scope.Mem, entry) @@ -127,9 +126,6 @@ func (scope *EvalScope) Locals() ([]*Variable, error) { } } depths = append(depths, depth) - if depth > 1 { - hasScopes = true - } } if err := varReader.Err(); err != nil { @@ -140,9 +136,7 @@ func (scope *EvalScope) Locals() ([]*Variable, error) { return vars, nil } - if hasScopes { - sort.Stable(&variablesByDepth{vars, depths}) - } + sort.Stable(&variablesByDepthAndDeclLine{vars, depths}) lvn := map[string]*Variable{} // lvn[n] is the last variable we saw named n @@ -160,12 +154,10 @@ func (scope *EvalScope) Locals() ([]*Variable, error) { v.DeclLine = declLine vars[i] = v } - if hasScopes { - if otherv := lvn[v.Name]; otherv != nil { - otherv.Flags |= VariableShadowed - } - lvn[v.Name] = v + if otherv := lvn[v.Name]; otherv != nil { + otherv.Flags |= VariableShadowed } + lvn[v.Name] = v } return vars, nil diff --git a/pkg/proc/scope_test.go b/pkg/proc/scope_test.go index dd646047..d616a71c 100644 --- a/pkg/proc/scope_test.go +++ b/pkg/proc/scope_test.go @@ -58,9 +58,10 @@ func TestScopeWithEscapedVariable(t *testing.T) { // the = and the initial value are optional and can only be specified if the // type is an integer type, float32, float64 or bool. // -// If multiple variables with the same name are specified -// LocalVariables+FunctionArguments should return them in the same order and -// EvalExpression should return the last one. +// If multiple variables with the same name are specified: +// 1. LocalVariables+FunctionArguments should return them in the same order and +// every variable except the last one should be marked as shadowed +// 2. EvalExpression should return the last one. func TestScope(t *testing.T) { if ver, _ := goversion.Parse(runtime.Version()); ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 9, -1, 0, 0, ""}) { return @@ -94,16 +95,12 @@ func TestScope(t *testing.T) { scope, _ := scopeCheck.checkLocalsAndArgs(p, t) - var prev *varCheck for i := range scopeCheck.varChecks { vc := &scopeCheck.varChecks[i] - if prev != nil && prev.name != vc.name { - prev.checkInScope(scopeCheck.line, scope, t) + if vc.shdw { + continue } - prev = vc - } - if prev != nil { - prev.checkInScope(scopeCheck.line, scope, t) + vc.checkInScope(scopeCheck.line, scope, t) } scopeCheck.ok = true @@ -130,6 +127,7 @@ type varCheck struct { name string typ string kind reflect.Kind + shdw bool // this variable should be shadowed hasVal bool intVal int64 uintVal uint64 @@ -229,7 +227,12 @@ func (check *scopeCheck) Parse(descr string, t *testing.T) { if err != nil { t.Fatalf("could not parse scope comment %q: %v", descr, err) } + } + } + for i := 1; i < len(check.varChecks); i++ { + if check.varChecks[i-1].name == check.varChecks[i].name { + check.varChecks[i-1].shdw = true } } } @@ -295,6 +298,10 @@ func (varCheck *varCheck) check(line int, v *proc.Variable, t *testing.T, ctxt s t.Errorf("%d: wrong type for %s (%s), got %s, expected %s", line, v.Name, ctxt, typ, varCheck.typ) } + if varCheck.shdw && v.Flags&proc.VariableShadowed == 0 { + t.Errorf("%d: expected shadowed %s variable", line, v.Name) + } + if !varCheck.hasVal { return } diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index eb7d1d29..7f01c71d 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -1946,16 +1946,21 @@ func (ctyp *constantType) describe(n int64) string { return "" } -type variablesByDepth struct { +type variablesByDepthAndDeclLine struct { vars []*Variable depths []int } -func (v *variablesByDepth) Len() int { return len(v.vars) } +func (v *variablesByDepthAndDeclLine) Len() int { return len(v.vars) } -func (v *variablesByDepth) Less(i int, j int) bool { return v.depths[i] < v.depths[j] } +func (v *variablesByDepthAndDeclLine) Less(i int, j int) bool { + if v.depths[i] == v.depths[j] { + return v.vars[i].DeclLine < v.vars[j].DeclLine + } + return v.depths[i] < v.depths[j] +} -func (v *variablesByDepth) Swap(i int, j int) { +func (v *variablesByDepthAndDeclLine) Swap(i int, j int) { v.depths[i], v.depths[j] = v.depths[j], v.depths[i] v.vars[i], v.vars[j] = v.vars[j], v.vars[i] }