*: remove uses of reflect.MethodByName from all of Delve

_scripts/gen-suitablemethods.go

@@ -0,0 +1,81 @@
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"go/format"
+	"go/token"
+	"go/types"
+	"io"
+	"log"
+	"os"
+	"strings"
+
+	"golang.org/x/tools/go/packages"
+)
+
+func must(err error, fmtstr string, args ...interface{}) {
+	if err != nil {
+		log.Fatalf(fmtstr, args...)
+	}
+}
+
+func main() {
+	buf := bytes.NewBuffer([]byte{})
+	fmt.Fprintf(buf, "// DO NOT EDIT: auto-generated using _scripts/gen-suitablemethods.go\n\n")
+	fmt.Fprintf(buf, "package rpccommon\n\n")
+	fmt.Fprintf(buf, "import ( \"reflect\"; \"github.com/go-delve/delve/service/rpc2\" )\n")
+
+	dorpc(buf, "rpc2.RPCServer", "rpc2", "2")
+	dorpc(buf, "RPCServer", "rpccommon", "Common")
+
+	src, err := format.Source(buf.Bytes())
+	must(err, "error formatting source: %v", err)
+
+	out := os.Stdout
+	if os.Args[1] != "-" {
+		if !strings.HasSuffix(os.Args[1], ".go") {
+			os.Args[1] += ".go"
+		}
+		out, err = os.Create(os.Args[1])
+		must(err, "error creating %s: %v", os.Args[1], err)
+	}
+	_, err = out.Write(src)
+	must(err, "error writing output: %v", err)
+	if out != os.Stdout {
+		must(out.Close(), "error closing %s: %v", os.Args[1], err)
+	}
+}
+
+func dorpc(buf io.Writer, typename, pkgname, outname string) {
+	fmt.Fprintf(buf, "func suitableMethods%s(s *%s, methods map[string]*methodType) {\n", outname, typename)
+
+	fset := &token.FileSet{}
+	cfg := &packages.Config{
+		Mode: packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedName | packages.NeedCompiledGoFiles | packages.NeedTypes,
+		Fset: fset,
+	}
+	pkgs, err := packages.Load(cfg, fmt.Sprintf("github.com/go-delve/delve/service/%s", pkgname))
+	must(err, "error loading packages: %v", err)
+	packages.Visit(pkgs, func(pkg *packages.Package) bool {
+		if pkg.PkgPath != fmt.Sprintf("github.com/go-delve/delve/service/%s", pkgname) {
+			return true
+		}
+		mset := types.NewMethodSet(types.NewPointer(pkg.Types.Scope().Lookup("RPCServer").Type()))
+		for i := 0; i < mset.Len(); i++ {
+			fn := mset.At(i).Obj().(*types.Func)
+			name := fn.Name()
+			if name[0] >= 'a' && name[0] <= 'z' {
+				continue
+			}
+			sig := fn.Type().(*types.Signature)
+			if sig.Params().Len() != 2 {
+				continue
+			}
+			fmt.Fprintf(buf, "methods[\"RPCServer.%s\"] = &methodType{ method: reflect.ValueOf(s.%s) }\n", name, name)
+
+		}
+		return true
+	}, nil)
+	fmt.Fprintf(buf, "}\n\n")
+}

cmd/dlv/dlv_test.go

@@ -374,6 +374,7 @@ func TestGeneratedDoc(t *testing.T) {
 	checkAutogenDoc(t, "pkg/terminal/starbind/starlark_mapping.go", "'go generate' inside pkg/terminal/starbind", runScript("_scripts/gen-starlark-bindings.go", "go", "-"))
 	checkAutogenDoc(t, "Documentation/cli/starlark.md", "'go generate' inside pkg/terminal/starbind", runScript("_scripts/gen-starlark-bindings.go", "doc/dummy", "Documentation/cli/starlark.md"))
 	checkAutogenDoc(t, "Documentation/faq.md", "'go run _scripts/gen-faq-toc.go Documentation/faq.md Documentation/faq.md'", runScript("_scripts/gen-faq-toc.go", "Documentation/faq.md", "-"))
+	checkAutogenDoc(t, "service/rpccommon/suitablemethods.go", "'go generate' inside service/rpccommon", runScript("_scripts/gen-suitablemethods.go", "-"))
 	if goversion.VersionAfterOrEqual(runtime.Version(), 1, 18) {
 		checkAutogenDoc(t, "_scripts/rtype-out.txt", "go run _scripts/rtype.go report _scripts/rtype-out.txt", runScript("_scripts/rtype.go", "report"))
 		runScript("_scripts/rtype.go", "check")
@@ -1346,7 +1347,7 @@ func TestVersion(t *testing.T) {
 	want1 := []byte("mod\tgithub.com/go-delve/delve")
 	want2 := []byte("dep\tgithub.com/google/go-dap")
 	if !bytes.Contains(got, want1) || !bytes.Contains(got, want2) {
-		t.Errorf("got %s\nwant %v and %v in the output", got, want1, want2)
+		t.Errorf("got %s\nwant %s and %s in the output", got, want1, want2)
 	}
 }
 
@@ -1462,3 +1463,14 @@ func TestUnixDomainSocket(t *testing.T) {
 	client.Detach(true)
 	cmd.Wait()
 }
+
+func TestDeadcodeEliminated(t *testing.T) {
+	dlvbin := protest.GetDlvBinary(t)
+	buf, err := exec.Command("go", "tool", "nm", dlvbin).CombinedOutput()
+	if err != nil {
+		t.Fatalf("error executing go tool nm %s: %v", dlvbin, err)
+	}
+	if strings.Contains(string(buf), "MethodByName") {
+		t.Fatal("output of go tool nm contains MethodByName")
+	}
+}

go.mod

@@ -13,8 +13,8 @@ require (
 	github.com/mattn/go-colorable v0.1.13
 	github.com/mattn/go-isatty v0.0.20
 	github.com/sirupsen/logrus v1.9.3
-	github.com/spf13/cobra v1.7.0
-	github.com/spf13/pflag v1.0.5
+	github.com/spf13/cobra v1.9.1
+	github.com/spf13/pflag v1.0.6
 	go.starlark.net v0.0.0-20231101134539-556fd59b42f6
 	golang.org/x/arch v0.11.0
 	golang.org/x/sys v0.26.0
@@ -24,7 +24,7 @@ require (
 )
 
 require (
-	github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
+	github.com/cpuguy83/go-md2man/v2 v2.0.6 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/mattn/go-runewidth v0.0.13 // indirect
 	github.com/rivo/uniseg v0.2.0 // indirect

go.sum

@@ -2,8 +2,8 @@ github.com/cilium/ebpf v0.11.0 h1:V8gS/bTCCjX9uUnkUFUpPsksM8n1lXBAvHcpiFk1X2Y=
 github.com/cilium/ebpf v0.11.0/go.mod h1:WE7CZAnqOL2RouJ4f1uyNhqr2P4CCvXFIqdRDUgWsVs=
 github.com/cosiner/argv v0.1.0 h1:BVDiEL32lwHukgJKP87btEPenzrrHUjajs/8yzaqcXg=
 github.com/cosiner/argv v0.1.0/go.mod h1:EusR6TucWKX+zFgtdUsKT2Cvg45K5rtpCcWz4hK06d8=
-github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
-github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
+github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0=
+github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/creack/pty v1.1.20 h1:VIPb/a2s17qNeQgDnkfZC35RScx+blkKF8GV68n80J4=
 github.com/creack/pty v1.1.20/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -45,10 +45,10 @@ github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
 github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
-github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I=
-github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0=
-github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
-github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
+github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo=
+github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0=
+github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o=
+github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=

pkg/version/buildinfo.go

@@ -2,29 +2,28 @@ package version
 
 import (
 	"bytes"
+	"fmt"
 	"runtime/debug"
-	"text/template"
 )
 
 func init() {
 	buildInfo = moduleBuildInfo
 }
 
-var buildInfoTmpl = ` mod	{{.Main.Path}}	{{.Main.Version}}	{{.Main.Sum}}
-{{range .Deps}} dep	{{.Path}}	{{.Version}}	{{.Sum}}{{if .Replace}}
-	=> {{.Replace.Path}}	{{.Replace.Version}}	{{.Replace.Sum}}{{end}}
-{{end}}`
-
 func moduleBuildInfo() string {
 	info, ok := debug.ReadBuildInfo()
 	if !ok {
 		return "not built in module mode"
 	}
 
 	buf := new(bytes.Buffer)
-	err := template.Must(template.New("buildinfo").Parse(buildInfoTmpl)).Execute(buf, info)
-	if err != nil {
-		panic(err)
+	fmt.Fprintf(buf, " mod\t%s\t%s\t%s\n", info.Main.Path, info.Main.Version, info.Main.Sum)
+	for _, dep := range info.Deps {
+		fmt.Fprintf(buf, " dep\t%s\t%s\t%s", dep.Path, dep.Version, dep.Sum)
+		if dep.Replace != nil {
+			fmt.Fprintf(buf, "\t=> %s\t%s\t%s", dep.Replace.Path, dep.Replace.Version, dep.Replace.Sum)
+		}
+		fmt.Fprintf(buf, "\n")
 	}
 	return buf.String()
 }

service/rpccommon/server.go

@@ -14,8 +14,6 @@ import (
 	"reflect"
 	"runtime"
 	"sync"
-	"unicode"
-	"unicode/utf8"
 
 	"github.com/go-delve/delve/pkg/logflags"
 	"github.com/go-delve/delve/pkg/version"
@@ -27,6 +25,8 @@ import (
 	"github.com/go-delve/delve/service/rpc2"
 )
 
+//go:generate go run ../../_scripts/gen-suitablemethods.go suitablemethods
+
 // ServerImpl implements a JSON-RPC server that can switch between two
 // versions of the API.
 type ServerImpl struct {
@@ -62,8 +62,7 @@ type RPCServer struct {
 }
 
 type methodType struct {
-	method      reflect.Method
-	Rcvr        reflect.Value
+	method      reflect.Value
 	ArgType     reflect.Type
 	ReplyType   reflect.Type
 	Synchronous bool
@@ -128,8 +127,10 @@ func (s *ServerImpl) Run() error {
 	s.methodMaps = make([]map[string]*methodType, 2)
 
 	s.methodMaps[1] = map[string]*methodType{}
-	suitableMethods(s.s2, s.methodMaps[1], s.log)
-	suitableMethods(rpcServer, s.methodMaps[1], s.log)
+
+	suitableMethods2(s.s2, s.methodMaps[1])
+	suitableMethodsCommon(rpcServer, s.methodMaps[1])
+	finishMethodsMapInit(s.methodMaps[1])
 
 	go func() {
 		defer s.listener.Close()
@@ -183,92 +184,15 @@ func (s *ServerImpl) serveConnectionDemux(c io.ReadWriteCloser) {
 	}
 }
 
-// Precompute the reflect type for error.  Can't use error directly
-// because Typeof takes an empty interface value.  This is annoying.
-var typeOfError = reflect.TypeOf((*error)(nil)).Elem()
-
-// Is this an exported - upper case - name?
-func isExported(name string) bool {
-	ch, _ := utf8.DecodeRuneInString(name)
-	return unicode.IsUpper(ch)
-}
-
-// Is this type exported or a builtin?
-func isExportedOrBuiltinType(t reflect.Type) bool {
-	for t.Kind() == reflect.Ptr {
-		t = t.Elem()
-	}
-	// PkgPath will be non-empty even for an exported type,
-	// so we need to check the type name as well.
-	return isExported(t.Name()) || t.PkgPath() == ""
-}
-
-// Fills methods map with the methods of receiver that should be made
-// available through the RPC interface.
-// These are all the public methods of rcvr that have one of those
-// two signatures:
-//
-//	func (rcvr ReceiverType) Method(in InputType, out *ReplyType) error
-//	func (rcvr ReceiverType) Method(in InputType, cb service.RPCCallback)
-func suitableMethods(rcvr interface{}, methods map[string]*methodType, log logflags.Logger) {
-	typ := reflect.TypeOf(rcvr)
-	rcvrv := reflect.ValueOf(rcvr)
-	sname := reflect.Indirect(rcvrv).Type().Name()
-	if sname == "" {
-		log.Debugf("rpc.Register: no service name for type %s", typ)
-		return
-	}
-	for m := 0; m < typ.NumMethod(); m++ {
-		method := typ.Method(m)
-		mname := method.Name
-		mtype := method.Type
-		// method must be exported
-		if method.PkgPath != "" {
-			continue
-		}
-		// Method needs three ins: (receive, *args, *reply) or (receiver, *args, *RPCCallback)
-		if mtype.NumIn() != 3 {
-			log.Warn("method", mname, "has wrong number of ins:", mtype.NumIn())
-			continue
-		}
-		// First arg need not be a pointer.
-		argType := mtype.In(1)
-		if !isExportedOrBuiltinType(argType) {
-			log.Warn(mname, "argument type not exported:", argType)
-			continue
-		}
-
-		replyType := mtype.In(2)
-		synchronous := replyType.String() != "service.RPCCallback"
-
-		if synchronous {
-			// Second arg must be a pointer.
-			if replyType.Kind() != reflect.Ptr {
-				log.Warn("method", mname, "reply type not a pointer:", replyType)
-				continue
-			}
-			// Reply type must be exported.
-			if !isExportedOrBuiltinType(replyType) {
-				log.Warn("method", mname, "reply type not exported:", replyType)
-				continue
-			}
-
-			// Method needs one out.
-			if mtype.NumOut() != 1 {
-				log.Warn("method", mname, "has wrong number of outs:", mtype.NumOut())
-				continue
-			}
-			// The return type of the method must be error.
-			if returnType := mtype.Out(0); returnType != typeOfError {
-				log.Warn("method", mname, "returns", returnType.String(), "not error")
-				continue
-			}
-		} else if mtype.NumOut() != 0 {
-			// Method needs zero outs.
-			log.Warn("method", mname, "has wrong number of outs:", mtype.NumOut())
-			continue
+func finishMethodsMapInit(methods map[string]*methodType) {
+	for name, method := range methods {
+		mtype := method.method.Type()
+		if mtype.NumIn() != 2 {
+			panic(fmt.Errorf("wrong number of inputs for method %s (%d)", name, mtype.NumIn()))
 		}
-		methods[sname+"."+mname] = &methodType{method: method, ArgType: argType, ReplyType: replyType, Synchronous: synchronous, Rcvr: rcvrv}
+		method.ArgType = mtype.In(0)
+		method.ReplyType = mtype.In(1)
+		method.Synchronous = method.ReplyType.String() != "service.RPCCallback"
 	}
 }
 
@@ -326,7 +250,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
 				s.log.Debugf("<- %s(%T%s)", req.ServiceMethod, argv.Interface(), argvbytes)
 			}
 			replyv = reflect.New(mtype.ReplyType.Elem())
-			function := mtype.method.Func
+			function := mtype.method
 			var returnValues []reflect.Value
 			var errInter interface{}
 			func() {
@@ -335,7 +259,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
 						errInter = newInternalError(ierr, 2)
 					}
 				}()
-				returnValues = function.Call([]reflect.Value{mtype.Rcvr, argv, replyv})
+				returnValues = function.Call([]reflect.Value{argv, replyv})
 				errInter = returnValues[0].Interface()
 			}()
 
@@ -358,15 +282,15 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
 				argvbytes, _ := json.Marshal(argv.Interface())
 				s.log.Debugf("(async %d) <- %s(%T%s)", req.Seq, req.ServiceMethod, argv.Interface(), argvbytes)
 			}
-			function := mtype.method.Func
+			function := mtype.method
 			ctl := &RPCCallback{s, sending, codec, req, make(chan struct{}), clientDisconnectChan}
 			go func() {
 				defer func() {
 					if ierr := recover(); ierr != nil {
 						ctl.Return(nil, newInternalError(ierr, 2))
 					}
 				}()
-				function.Call([]reflect.Value{mtype.Rcvr, argv, reflect.ValueOf(ctl)})
+				function.Call([]reflect.Value{argv, reflect.ValueOf(ctl)})
 			}()
 			<-ctl.setupDone
 		}