Skip to content

Commit

Permalink
*: remove uses of reflect.MethodByName from all of Delve (#3916)
Browse files Browse the repository at this point in the history
When reflect.MethodByName is used the linker can not fully perform
deadcode elimination. This commit updates cobra and rewrites the
suitableMethods part of service/rpccommon so that reflect.MethodByName
is not used and the linker can fully execute deadcode elimination.

The executable size on go1.24.0 on linux is reduced from 25468606 bytes
to 22453382 bytes or a reduction of approximately 12%.

See also:

spf13/cobra#1956
https://github.com/aarzilli/whydeadcode
  • Loading branch information
aarzilli authored Feb 18, 2025
1 parent 10b7011 commit 7ea6d8f
Show file tree
Hide file tree
Showing 46 changed files with 1,401 additions and 2,518 deletions.
81 changes: 81 additions & 0 deletions _scripts/gen-suitablemethods.go
Original file line number Diff line number Diff line change
@@ -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")
}
14 changes: 13 additions & 1 deletion cmd/dlv/dlv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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")
}
}
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
17 changes: 8 additions & 9 deletions pkg/version/buildinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
114 changes: 19 additions & 95 deletions service/rpccommon/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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"
}
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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()
}()

Expand All @@ -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
}
Expand Down
Loading

0 comments on commit 7ea6d8f

Please sign in to comment.