addressing review comments: folding Functions and FunctionsDeep, redu…

…cing branches by using depth prefix, wrap using %w and other comments
go-delve · Apr 25, 2024 · 0751612 · 0751612
1 parent 413793e
commit 0751612
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 53 deletions.
diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go
@@ -1456,7 +1456,7 @@ func uniq(s []string) []string {
 }
 
 // Functions returns a list of functions in the target process.
-func (d *Debugger) Functions(filter string) ([]string, error) {
+func (d *Debugger) Functions(filter string, followCalls int) ([]string, error) {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
@@ -1470,7 +1470,15 @@ func (d *Debugger) Functions(filter string) ([]string, error) {
 	for t.Next() {
 		for _, f := range t.BinInfo().Functions {
 			if regex.MatchString(f.Name) {
-				funcs = append(funcs, f.Name)
+				if followCalls > 0 {
+					newfuncs, err := traverse(t, &f, 1, followCalls)
+					if err != nil {
+						return nil, fmt.Errorf("traverse failed with error %w", err)
+					}
+					funcs = append(funcs, newfuncs...)
+				} else {
+					funcs = append(funcs, f.Name)
+				}
 			}
 		}
 	}
@@ -1496,7 +1504,7 @@ func traverse(t proc.ValidTargets, f *proc.Function, depth int, FollowCalls int)
 	funcs = append(funcs, f.Name)
 	text, err := proc.Disassemble(t.Memory(), nil, t.Breakpoints(), t.BinInfo(), f.Entry, f.End)
 	if err != nil {
-		return nil, fmt.Errorf("disassemble failed with error %s", err.Error())
+		return nil, fmt.Errorf("disassemble failed with error %w", err)
 	}
 	depth++
 	for _, instr := range text {
@@ -1507,54 +1515,28 @@ func traverse(t proc.ValidTargets, f *proc.Function, depth int, FollowCalls int)
 			}
 			if depth <= FollowCalls {
 				children, err := traverse(t, cf, depth, FollowCalls)
-				funcs = append(funcs, children...)
 				if err != nil {
-					return nil, fmt.Errorf("disassemble failed with error %s", err.Error())
+					return nil, fmt.Errorf("traverse failed with error %w", err)
 				}
+				funcs = append(funcs, children...)
 			}
 		}
 	}
+	// The following code is needed to include defer function calls as they are invoked via funcname.func1 type
+	// naming, so check if funcname is a prefix in candidate function to include such functions
 	for _, fbinary := range t.BinInfo().Functions {
 		if strings.HasPrefix(fbinary.Name, f.Name) && fbinary.Name != f.Name {
 			children, err := traverse(t, &fbinary, depth, FollowCalls)
-			funcs = append(funcs, children...)
 			if err != nil {
-				return nil, fmt.Errorf("disassemble failed with error %s", err.Error())
+				return nil, fmt.Errorf("traverse failed with error %w", err)
 			}
+			funcs = append(funcs, children...)
 		}
 	}
 	return funcs, nil
 
 }
 
-// FunctionsDeep returns a list of functions and it's children upto required depth indicated by FollowCalls in the target process.
-func (d *Debugger) FunctionsDeep(filter string, FollowCalls int) ([]string, error) {
-	d.targetMutex.Lock()
-	defer d.targetMutex.Unlock()
-
-	regex, err := regexp.Compile(filter)
-	if err != nil {
-		return nil, fmt.Errorf("invalid filter argument: %s", err.Error())
-	}
-
-	funcs := []string{}
-	t := proc.ValidTargets{Group: d.target}
-	for t.Next() {
-		for _, f := range t.BinInfo().Functions {
-			if regex.MatchString(f.Name) {
-				newfuncs, err := traverse(t, &f, 1, FollowCalls)
-				funcs = append(funcs, newfuncs...)
-				if err != nil {
-					return nil, fmt.Errorf("traverse failed with error %s", err.Error())
-				}
-			}
-		}
-	}
-	sort.Strings(funcs)
-	funcs = uniq(funcs)
-	return funcs, nil
-}
-
 // Types returns all type information in the binary.
 func (d *Debugger) Types(filter string) ([]string, error) {
 	d.targetMutex.Lock()

diff --git a/service/rpc1/server.go b/service/rpc1/server.go
@@ -263,8 +263,8 @@ func (s *RPCServer) ListSources(filter string, sources *[]string) error {
 	return nil
 }
 
-func (s *RPCServer) ListFunctions(filter string, funcs *[]string) error {
-	fns, err := s.debugger.Functions(filter)
+func (s *RPCServer) ListFunctions(filter string, followCalls int, funcs *[]string) error {
+	fns, err := s.debugger.Functions(filter, followCalls)
 	if err != nil {
 		return err
 	}

diff --git a/service/rpc2/server.go b/service/rpc2/server.go
@@ -590,11 +590,7 @@ type ListFunctionsOut struct {
 func (s *RPCServer) ListFunctions(arg ListFunctionsIn, out *ListFunctionsOut) error {
 	var fns []string
 	var err error
-	if arg.FollowCalls > 0 {
-		fns, err = s.debugger.FunctionsDeep(arg.Filter, arg.FollowCalls)
-	} else {
-		fns, err = s.debugger.Functions(arg.Filter)
-	}
+	fns, err = s.debugger.Functions(arg.Filter, arg.FollowCalls)
 	if err != nil {
 		return err
 	}

diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go
@@ -799,19 +799,11 @@ func TestClientServer_infoLocals(t *testing.T) {
 }
 
 func matchFunctions(t *testing.T, funcs []string, expected []string, depth int) {
-	mismatch := false
-	var i int
-	for i = range funcs {
-		t.Logf("expected %s functions %s\n", expected[i], funcs[i])
+	for i := range funcs {
 		if funcs[i] != expected[i] {
-			mismatch = true
-			break
+			t.Fatalf("Function %s  not found in ListFunctions --follow-calls=%d output", expected[i], depth)
 		}
 	}
-	if mismatch {
-		t.Fatalf("Function %s  not found in ListFunctions --follow-calls=%d output", expected[i], depth)
-	}
-
 }
 
 func TestTraceFollowCallsCommand(t *testing.T) {
@@ -831,7 +823,7 @@ func TestTraceFollowCallsCommand(t *testing.T) {
 		depth = 4
 		functions, err = c.ListFunctions("main.callme", depth)
 		assertNoError(err, t, "ListFunctions()")
-		expected = []string{"main.callme", "main.callme2","main.callmed","main.callmee"}
+		expected = []string{"main.callme", "main.callme2", "main.callmed", "main.callmee"}
 		matchFunctions(t, functions, expected, depth)
 
 		depth = 5