proc: Flag shadowed arguments as shadowed

Fixes #951

_fixtures/issue951.go

@@ -0,0 +1,21 @@
+package main
+
+import (
+	"fmt"
+	"runtime"
+)
+
+func main() {
+	shadow(42)
+}
+
+func shadow(i int) {
+	for i := 0; i < 5; i++ {
+		for i := 20; i < 25; i++ {
+			runtime.Breakpoint()
+			fmt.Println("another shadow", i)
+		}
+		fmt.Println("shadow", i)
+	}
+	fmt.Println("arg", i)
+}

pkg/proc/eval.go

@@ -2,7 +2,6 @@ package proc
 
 import (
 	"bytes"
-	"debug/dwarf"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -564,7 +563,7 @@ func (scope *EvalScope) evalIdent(node *ast.Ident) (*Variable, error) {
 		return nilVariable, nil
 	}
 
-	vars, err := scope.variablesByTag(dwarf.TagVariable, nil)
+	vars, err := scope.Locals()
 	if err != nil {
 		return nil, err
 	}
@@ -573,15 +572,6 @@ func (scope *EvalScope) evalIdent(node *ast.Ident) (*Variable, error) {
 			return vars[i], nil
 		}
 	}
-	args, err := scope.variablesByTag(dwarf.TagFormalParameter, nil)
-	if err != nil {
-		return nil, err
-	}
-	for i := range args {
-		if args[i].Name == node.Name {
-			return args[i], nil
-		}
-	}
 
 	// if it's not a local variable then it could be a package variable w/o explicit package name
 	if scope.Fn != nil {

pkg/proc/proc.go

@@ -524,7 +524,7 @@ func FrameToScope(bi *BinaryInfo, thread MemoryReadWriter, g *G, frames ...Stack
 
 	// Creates a cacheMem that will preload the entire stack frame the first
 	// time any local variable is read.
-	// Remember that the stack grows from downward in memory.
+	// Remember that the stack grows downward in memory.
 	minaddr := frames[0].Regs.SP()
 	var maxaddr uint64
 	if len(frames) > 1 && frames[0].SystemStack == frames[1].SystemStack {

pkg/proc/proc_test.go

@@ -3665,3 +3665,35 @@ func TestInlineStepOut(t *testing.T) {
 		{contStepout, 18},
 	})
 }
+
+func TestIssue951(t *testing.T) {
+	if ver, _ := goversion.Parse(runtime.Version()); ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 9, -1, 0, 0, ""}) {
+		t.Skip("scopes not implemented in <=go1.8")
+	}
+
+	withTestProcess("issue951", t, func(p proc.Process, fixture protest.Fixture) {
+		assertNoError(proc.Continue(p), t, "Continue()")
+		scope, err := proc.GoroutineScope(p.CurrentThread())
+		assertNoError(err, t, "GoroutineScope")
+		args, err := scope.FunctionArguments(normalLoadConfig)
+		assertNoError(err, t, "FunctionArguments")
+		t.Logf("%#v", args[0])
+		if args[0].Flags&proc.VariableShadowed == 0 {
+			t.Error("argument is not shadowed")
+		}
+		vars, err := scope.LocalVariables(normalLoadConfig)
+		assertNoError(err, t, "LocalVariables")
+		shadowed, notShadowed := 0, 0
+		for i := range vars {
+			t.Logf("var %d: %#v\n", i, vars[i])
+			if vars[i].Flags&proc.VariableShadowed != 0 {
+				shadowed++
+			} else {
+				notShadowed++
+			}
+		}
+		if shadowed != 1 || notShadowed != 1 {
+			t.Errorf("Wrong number of shadowed/non-shadowed local variables: %d %d", shadowed, notShadowed)
+		}
+	})
+}

pkg/proc/variables.go

@@ -58,6 +58,10 @@ const (
 	VariableShadowed
 	// VariableConstant means this variable is a constant value
 	VariableConstant
+	// VariableArgument means this variable is a function argument
+	VariableArgument
+	// VariableReturnArgument means this variable is a function return value
+	VariableReturnArgument
 )
 
 // Variable represents a variable. It contains the address, name,
@@ -616,12 +620,38 @@ func (scope *EvalScope) SetVariable(name, value string) error {
 
 // LocalVariables returns all local variables from the current function scope.
 func (scope *EvalScope) LocalVariables(cfg LoadConfig) ([]*Variable, error) {
-	return scope.variablesByTag(dwarf.TagVariable, &cfg)
+	vars, err := scope.Locals()
+	if err != nil {
+		return nil, err
+	}
+	vars = filterVariables(vars, func(v *Variable) bool {
+		return (v.Flags & (VariableArgument | VariableReturnArgument)) == 0
+	})
+	loadValues(vars, cfg)
+	return vars, nil
 }
 
 // FunctionArguments returns the name, value, and type of all current function arguments.
 func (scope *EvalScope) FunctionArguments(cfg LoadConfig) ([]*Variable, error) {
-	return scope.variablesByTag(dwarf.TagFormalParameter, &cfg)
+	vars, err := scope.Locals()
+	if err != nil {
+		return nil, err
+	}
+	vars = filterVariables(vars, func(v *Variable) bool {
+		return (v.Flags & (VariableArgument | VariableReturnArgument)) != 0
+	})
+	loadValues(vars, cfg)
+	return vars, nil
+}
+
+func filterVariables(vars []*Variable, pred func(v *Variable) bool) []*Variable {
+	r := make([]*Variable, 0, len(vars))
+	for i := range vars {
+		if pred(vars[i]) {
+			r = append(r, vars[i])
+		}
+	}
+	return r
 }
 
 // PackageVariables returns the name, value, and type of all package variables in the application.
@@ -825,6 +855,12 @@ func (v *Variable) maybeDereference() *Variable {
 	}
 }
 
+func loadValues(vars []*Variable, cfg LoadConfig) {
+	for i := range vars {
+		vars[i].loadValueInternal(0, cfg)
+	}
+}
+
 // Extracts the value of the variable at the given address.
 func (v *Variable) loadValue(cfg LoadConfig) {
 	v.loadValueInternal(0, cfg)
@@ -1876,20 +1912,17 @@ func (v *variablesByDepth) Swap(i int, j int) {
 }
 
 // Fetches all variables of a specific type in the current function scope
-func (scope *EvalScope) variablesByTag(tag dwarf.Tag, cfg *LoadConfig) ([]*Variable, error) {
+func (scope *EvalScope) Locals() ([]*Variable, error) {
 	if scope.Fn == nil {
 		return nil, errors.New("unable to find function context")
 	}
 
 	var vars []*Variable
 	var depths []int
-	varReader := reader.Variables(scope.BinInfo.dwarf, scope.Fn.offset, scope.PC, scope.Line, tag == dwarf.TagVariable)
+	varReader := reader.Variables(scope.BinInfo.dwarf, scope.Fn.offset, scope.PC, scope.Line, true)
 	hasScopes := false
 	for varReader.Next() {
 		entry := varReader.Entry()
-		if entry.Tag != tag {
-			continue
-		}
 		val, err := scope.extractVarInfoFromEntry(entry)
 		if err != nil {
 			// skip variables that we can't parse yet
@@ -1897,6 +1930,17 @@ func (scope *EvalScope) variablesByTag(tag dwarf.Tag, cfg *LoadConfig) ([]*Varia
 		}
 		vars = append(vars, val)
 		depth := varReader.Depth()
+		if entry.Tag == dwarf.TagFormalParameter {
+			if depth <= 1 {
+				depth = 0
+			}
+			isret, _ := entry.Val(dwarf.AttrVarParam).(bool)
+			if isret {
+				val.Flags |= VariableReturnArgument
+			} else {
+				val.Flags |= VariableArgument
+			}
+		}
 		depths = append(depths, depth)
 		if depth > 1 {
 			hasScopes = true
@@ -1933,9 +1977,6 @@ func (scope *EvalScope) variablesByTag(tag dwarf.Tag, cfg *LoadConfig) ([]*Varia
 			}
 			lvn[v.Name] = v
 		}
-		if cfg != nil {
-			v.loadValue(*cfg)
-		}
 	}
 
 	return vars, nil

service/api/types.go

@@ -170,6 +170,12 @@ const (
 
 	// VariableConstant means this variable is a constant value
 	VariableConstant
+
+	// VariableArgument means this variable is a function argument
+	VariableArgument
+
+	// VariableReturnArgument means this variable is a function return value
+	VariableReturnArgument
 )
 
 // Variable describes a variable.