proc,service: change FindLocation to work with multiple targets

Changes FindLocation to support multiple targets and adds an AddrPid
member to api.Breakpoint so that clients can set breakpoints by address
when multiple targets are connected (but at them moment this field is
ignored).

Updates go-delve#1653
Updates go-delve#2551

Author: aarzilli

pkg/locspec/locations.go

@@ -370,7 +370,7 @@ func (ale AmbiguousLocationError) Error() string {
 func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
 	limit := maxFindLocationCandidates
 	var candidateFiles []string
-	for _, sourceFile := range scope.BinInfo.Sources {
+	for _, sourceFile := range t.BinInfo().Sources {
 		substFile := sourceFile
 		if len(substitutePathRules) > 0 {
 			substFile = SubstitutePath(sourceFile, substitutePathRules)
@@ -387,10 +387,10 @@ func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope
 
 	var candidateFuncs []string
 	if loc.FuncBase != nil && limit > 0 {
-		candidateFuncs = loc.findFuncCandidates(scope, limit)
+		candidateFuncs = loc.findFuncCandidates(t.BinInfo(), limit)
 	}
 
-	if matching := len(candidateFiles) + len(candidateFuncs); matching == 0 {
+	if matching := len(candidateFiles) + len(candidateFuncs); matching == 0 && scope != nil {
 		// if no result was found this locations string could be an
 		// expression that the user forgot to prefix with '*', try treating it as
 		// such.
@@ -428,26 +428,26 @@ func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope
 	return []api.Location{addressesToLocation(addrs)}, nil
 }
 
-func (loc *NormalLocationSpec) findFuncCandidates(scope *proc.EvalScope, limit int) []string {
+func (loc *NormalLocationSpec) findFuncCandidates(bi *proc.BinaryInfo, limit int) []string {
 	candidateFuncs := map[string]struct{}{}
 	// See if it matches generic functions first
-	for fname := range scope.BinInfo.LookupGenericFunc() {
+	for fname := range bi.LookupGenericFunc() {
 		if len(candidateFuncs) >= limit {
 			break
 		}
-		if !loc.FuncBase.Match(&proc.Function{Name: fname}, scope.BinInfo.PackageMap) {
+		if !loc.FuncBase.Match(&proc.Function{Name: fname}, bi.PackageMap) {
 			continue
 		}
 		if loc.Base == fname {
 			return []string{fname}
 		}
 		candidateFuncs[fname] = struct{}{}
 	}
-	for _, f := range scope.BinInfo.LookupFunc {
+	for _, f := range bi.LookupFunc {
 		if len(candidateFuncs) >= limit {
 			break
 		}
-		if !loc.FuncBase.Match(f, scope.BinInfo.PackageMap) {
+		if !loc.FuncBase.Match(f, bi.PackageMap) {
 			continue
 		}
 		if loc.Base == f.Name {

pkg/proc/target_group.go

@@ -44,7 +44,8 @@ func NewGroup(t *Target) *TargetGroup {
 	}
 }
 
-// Targets returns a slice of targets in the group.
+// Targets returns a slice of all targets in the group, including the
+// ones that are no longer valid.
 func (grp *TargetGroup) Targets() []*Target {
 	return grp.targets
 }
@@ -57,7 +58,9 @@ func (grp *TargetGroup) Valid() (bool, error) {
 		if ok {
 			return true, nil
 		}
-		err0 = err
+		if err0 == nil {
+			err0 = err
+		}
 	}
 	return false, err0
 }
@@ -121,3 +124,25 @@ func (grp *TargetGroup) TargetForThread(thread Thread) *Target {
 	}
 	return nil
 }
+
+// ValidTargets iterates through all valid targets in Group.
+type ValidTargets struct {
+	*Target
+	Group *TargetGroup
+	start int
+}
+
+// Next moves to the next valid target, returns false if there aren't more
+// valid targets in the group.
+func (it *ValidTargets) Next() bool {
+	for i := it.start; i < len(it.Group.targets); i++ {
+		if ok, _ := it.Group.targets[i].Valid(); ok {
+			it.Target = it.Group.targets[i]
+			it.start = i + 1
+			return true
+		}
+	}
+	it.start = len(it.Group.targets)
+	it.Target = nil
+	return false
+}

pkg/terminal/command.go

@@ -1749,6 +1749,7 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 	for _, loc := range locs {
 		requestedBp.Addr = loc.PC
 		requestedBp.Addrs = loc.PCs
+		requestedBp.AddrPid = loc.PCPids
 		if tracepoint {
 			requestedBp.LoadArgs = &ShortLoadConfig
 		}

service/api/conversions.go

@@ -54,7 +54,7 @@ func ConvertLogicalBreakpoint(lbp *proc.LogicalBreakpoint) *Breakpoint {
 }
 
 // ConvertPhysicalBreakpoints adds informations from physical breakpoints to an API breakpoint.
-func ConvertPhysicalBreakpoints(b *Breakpoint, bps []*proc.Breakpoint) {
+func ConvertPhysicalBreakpoints(b *Breakpoint, pids []int, bps []*proc.Breakpoint) {
 	if len(bps) == 0 {
 		return
 	}
@@ -63,8 +63,9 @@ func ConvertPhysicalBreakpoints(b *Breakpoint, bps []*proc.Breakpoint) {
 	b.WatchType = WatchType(bps[0].WatchType)
 
 	lg := false
-	for _, bp := range bps {
+	for i, bp := range bps {
 		b.Addrs = append(b.Addrs, bp.Addr)
+		b.AddrPid = append(b.AddrPid, pids[i])
 		if b.FunctionName != bp.FunctionName && b.FunctionName != "" {
 			if !lg {
 				b.FunctionName = removeTypeParams(b.FunctionName)

service/api/types.go

@@ -80,6 +80,9 @@ type Breakpoint struct {
 	Addr uint64 `json:"addr"`
 	// Addrs is the list of addresses for this breakpoint.
 	Addrs []uint64 `json:"addrs"`
+	// AddrPid[i] is the PID associated with by Addrs[i], when debugging a
+	// single target process this is optional, otherwise it is mandatory.
+	AddrPid []int `json:"addrpid"`
 	// File is the source file for the breakpoint.
 	File string `json:"file"`
 	// Line is a line in File for the breakpoint.
@@ -192,6 +195,7 @@ type Location struct {
 	Line     int       `json:"line"`
 	Function *Function `json:"function,omitempty"`
 	PCs      []uint64  `json:"pcs,omitempty"`
+	PCPids   []int     `json:"pcpids,omitempty"`
 }
 
 // Stackframe describes one frame in a stack trace.

service/dap/server.go

@@ -1860,7 +1860,7 @@ func (s *Session) stoppedOnBreakpointGoroutineID(state *api.DebuggerState) (int,
 		return goid, nil
 	}
 	abp := api.ConvertLogicalBreakpoint(bp.Breakpoint.Logical)
-	api.ConvertPhysicalBreakpoints(abp, []*proc.Breakpoint{bp.Breakpoint})
+	api.ConvertPhysicalBreakpoints(abp, []int{0}, []*proc.Breakpoint{bp.Breakpoint})
 	return goid, abp
 }
 

service/debugger/debugger.go

@@ -45,6 +45,9 @@ var (
 
 	// ErrCoreDumpNotSupported is returned when core dumping is not supported
 	ErrCoreDumpNotSupported = errors.New("core dumping not supported")
+
+	// ErrNotImplementedWithMultitarget is returned for operations that are not implemented with multiple targets
+	ErrNotImplementedWithMultitarget = errors.New("not implemented for multiple targets")
 )
 
 // Debugger service.
@@ -386,6 +389,10 @@ func (d *Debugger) FunctionReturnLocations(fnName string) ([]uint64, error) {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
+	if len(d.target.Targets()) > 1 {
+		return nil, ErrNotImplementedWithMultitarget
+	}
+
 	var (
 		p = d.target.Selected
 		g = p.SelectedGoroutine()
@@ -604,10 +611,11 @@ func (d *Debugger) state(retLoadCfg *proc.LoadConfig, withBreakpointInfo bool) (
 		state.When, _ = d.target.When()
 	}
 
-	for _, t := range d.target.Targets() {
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
 		for _, bp := range t.Breakpoints().WatchOutOfScope {
 			abp := api.ConvertLogicalBreakpoint(bp.Logical)
-			api.ConvertPhysicalBreakpoints(abp, []*proc.Breakpoint{bp})
+			api.ConvertPhysicalBreakpoints(abp, []int{t.Pid()}, []*proc.Breakpoint{bp})
 			state.WatchOutOfScope = append(state.WatchOutOfScope, abp)
 		}
 	}
@@ -674,8 +682,9 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 		if runtime.GOOS == "windows" {
 			// Accept fileName which is case-insensitive and slash-insensitive match
 			fileNameNormalized := strings.ToLower(filepath.ToSlash(fileName))
+			t := proc.ValidTargets{Group: d.target}
 		caseInsensitiveSearch:
-			for _, t := range d.target.Targets() {
+			for t.Next() {
 				for _, symFile := range t.BinInfo().Sources {
 					if fileNameNormalized == strings.ToLower(filepath.ToSlash(symFile)) {
 						fileName = symFile
@@ -689,6 +698,7 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 		addrs, err = proc.FindFunctionLocation(d.target.Selected, requestedBp.FunctionName, requestedBp.Line)
 	case len(requestedBp.Addrs) > 0:
 		addrs = requestedBp.Addrs
+		//TODO(aarzilli): read requestedBp.AddrPid
 	default:
 		addrs = []uint64{requestedBp.Addr}
 	}
@@ -707,7 +717,18 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 
 func (d *Debugger) convertBreakpoint(lbp *proc.LogicalBreakpoint) *api.Breakpoint {
 	abp := api.ConvertLogicalBreakpoint(lbp)
-	api.ConvertPhysicalBreakpoints(abp, d.findBreakpoint(lbp.LogicalID))
+	bps := []*proc.Breakpoint{}
+	pids := []int{}
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
+		for _, bp := range t.Breakpoints().M {
+			if bp.LogicalID() == lbp.LogicalID {
+				bps = append(bps, bp)
+				pids = append(pids, t.Pid())
+			}
+		}
+	}
+	api.ConvertPhysicalBreakpoints(abp, pids, bps)
 	return abp
 }
 
@@ -846,7 +867,7 @@ func (d *Debugger) CreateEBPFTracepoint(fnName string) error {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
+		return ErrNotImplementedWithMultitarget
 	}
 	p := d.target.Selected
 	return p.SetEBPFTracepoint(fnName)
@@ -1045,23 +1066,22 @@ func isBpHitCondNotSatisfiable(bp *api.Breakpoint) bool {
 func (d *Debugger) Breakpoints(all bool) []*api.Breakpoint {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
-	p := d.target.Selected
 
 	abps := []*api.Breakpoint{}
 	if all {
-		for _, bp := range p.Breakpoints().M {
-			var abp *api.Breakpoint
-			if bp.Logical != nil {
-				abp = api.ConvertLogicalBreakpoint(bp.Logical)
-			} else {
-				abp = &api.Breakpoint{}
+		t := proc.ValidTargets{Group: d.target}
+		for t.Next() {
+			for _, bp := range t.Breakpoints().M {
+				var abp *api.Breakpoint
+				if bp.Logical != nil {
+					abp = api.ConvertLogicalBreakpoint(bp.Logical)
+				} else {
+					abp = &api.Breakpoint{}
+				}
+				api.ConvertPhysicalBreakpoints(abp, []int{t.Pid()}, []*proc.Breakpoint{bp})
+				abp.VerboseDescr = bp.VerboseDescr()
+				abps = append(abps, abp)
 			}
-			api.ConvertPhysicalBreakpoints(abp, []*proc.Breakpoint{bp})
-			abp.VerboseDescr = bp.VerboseDescr()
-			abps = append(abps, abp)
 		}
 	} else {
 		for _, lbp := range d.target.LogicalBreakpoints {
@@ -1422,7 +1442,8 @@ func (d *Debugger) Sources(filter string) ([]string, error) {
 	}
 
 	files := []string{}
-	for _, t := range d.target.Targets() {
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
 		for _, f := range t.BinInfo().Sources {
 			if regex.Match([]byte(f)) {
 				files = append(files, f)
@@ -1460,7 +1481,8 @@ func (d *Debugger) Functions(filter string) ([]string, error) {
 	}
 
 	funcs := []string{}
-	for _, t := range d.target.Targets() {
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
 		for _, f := range t.BinInfo().Functions {
 			if regex.MatchString(f.Name) {
 				funcs = append(funcs, f.Name)
@@ -1484,7 +1506,8 @@ func (d *Debugger) Types(filter string) ([]string, error) {
 
 	r := []string{}
 
-	for _, t := range d.target.Targets() {
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
 		types, err := t.BinInfo().Types()
 		if err != nil {
 			return nil, err
@@ -1932,13 +1955,6 @@ func (d *Debugger) FindLocation(goid, frame, deferredCall int, locStr string, in
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
-	if len(d.target.Targets()) != 1 {
-		//TODO(aarzilli): if there is more than one target process all must be
-		//searched and the addresses returned need to specify which target process
-		//they belong to.
-		panic("multiple targets not implemented")
-	}
-
 	if _, err := d.target.Valid(); err != nil {
 		return nil, err
 	}
@@ -1948,7 +1964,7 @@ func (d *Debugger) FindLocation(goid, frame, deferredCall int, locStr string, in
 		return nil, err
 	}
 
-	return d.findLocation(d.target.Selected, goid, frame, deferredCall, locStr, loc, includeNonExecutableLines, substitutePathRules)
+	return d.findLocation(goid, frame, deferredCall, locStr, loc, includeNonExecutableLines, substitutePathRules)
 }
 
 // FindLocationSpec will find the location specified by 'locStr' and 'locSpec'.
@@ -1959,34 +1975,39 @@ func (d *Debugger) FindLocationSpec(goid, frame, deferredCall int, locStr string
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
-	if len(d.target.Targets()) != 1 {
-		//TODO(aarzilli): if there is more than one target process all must be
-		//searched and the addresses returned need to specify which target process
-		//they belong to.
-		panic("multiple targets not implemented")
-	}
-
 	if _, err := d.target.Valid(); err != nil {
 		return nil, err
 	}
 
-	return d.findLocation(d.target.Selected, goid, frame, deferredCall, locStr, locSpec, includeNonExecutableLines, substitutePathRules)
+	return d.findLocation(goid, frame, deferredCall, locStr, locSpec, includeNonExecutableLines, substitutePathRules)
 }
 
-func (d *Debugger) findLocation(p *proc.Target, goid, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
-	s, _ := proc.ConvertEvalScope(p, goid, frame, deferredCall)
-
-	locs, err := locSpec.Find(p, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
-	for i := range locs {
-		if locs[i].PC == 0 {
-			continue
+func (d *Debugger) findLocation(goid, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
+	locations := []api.Location{}
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
+		pid := t.Pid()
+		s, _ := proc.ConvertEvalScope(t.Target, goid, frame, deferredCall)
+		locs, err := locSpec.Find(t.Target, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
+		if err != nil {
+			return nil, err
 		}
-		file, line, fn := p.BinInfo().PCToLine(locs[i].PC)
-		locs[i].File = file
-		locs[i].Line = line
-		locs[i].Function = api.ConvertFunction(fn)
+		for i := range locs {
+			if locs[i].PC == 0 {
+				continue
+			}
+			file, line, fn := t.BinInfo().PCToLine(locs[i].PC)
+			locs[i].File = file
+			locs[i].Line = line
+			locs[i].Function = api.ConvertFunction(fn)
+			locs[i].PCPids = make([]int, len(locs[i].PCs))
+			for j := range locs[i].PCs {
+				locs[i].PCPids[j] = pid
+			}
+		}
+		locations = append(locations, locs...)
 	}
-	return locs, err
+	return locations, nil
 }
 
 // Disassemble code between startPC and endPC.