Skip to content

Commit 7342911

Browse files
committed
*: remove uses of reflect.MethodByName from all of Delve
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
1 parent 9a39c3f commit 7342911

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1401
-2518
lines changed

_scripts/gen-suitablemethods.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"go/format"
7+
"go/token"
8+
"go/types"
9+
"io"
10+
"log"
11+
"os"
12+
"strings"
13+
14+
"golang.org/x/tools/go/packages"
15+
)
16+
17+
func must(err error, fmtstr string, args ...interface{}) {
18+
if err != nil {
19+
log.Fatalf(fmtstr, args...)
20+
}
21+
}
22+
23+
func main() {
24+
buf := bytes.NewBuffer([]byte{})
25+
fmt.Fprintf(buf, "// DO NOT EDIT: auto-generated using _scripts/gen-suitablemethods.go\n\n")
26+
fmt.Fprintf(buf, "package rpccommon\n\n")
27+
fmt.Fprintf(buf, "import ( \"reflect\"; \"github.com/go-delve/delve/service/rpc2\" )\n")
28+
29+
dorpc(buf, "rpc2.RPCServer", "rpc2", "2")
30+
dorpc(buf, "RPCServer", "rpccommon", "Common")
31+
32+
src, err := format.Source(buf.Bytes())
33+
must(err, "error formatting source: %v", err)
34+
35+
out := os.Stdout
36+
if os.Args[1] != "-" {
37+
if !strings.HasSuffix(os.Args[1], ".go") {
38+
os.Args[1] += ".go"
39+
}
40+
out, err = os.Create(os.Args[1])
41+
must(err, "error creating %s: %v", os.Args[1], err)
42+
}
43+
_, err = out.Write(src)
44+
must(err, "error writing output: %v", err)
45+
if out != os.Stdout {
46+
must(out.Close(), "error closing %s: %v", os.Args[1], err)
47+
}
48+
}
49+
50+
func dorpc(buf io.Writer, typename, pkgname, outname string) {
51+
fmt.Fprintf(buf, "func suitableMethods%s(s *%s, methods map[string]*methodType) {\n", outname, typename)
52+
53+
fset := &token.FileSet{}
54+
cfg := &packages.Config{
55+
Mode: packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedName | packages.NeedCompiledGoFiles | packages.NeedTypes,
56+
Fset: fset,
57+
}
58+
pkgs, err := packages.Load(cfg, fmt.Sprintf("github.com/go-delve/delve/service/%s", pkgname))
59+
must(err, "error loading packages: %v", err)
60+
packages.Visit(pkgs, func(pkg *packages.Package) bool {
61+
if pkg.PkgPath != fmt.Sprintf("github.com/go-delve/delve/service/%s", pkgname) {
62+
return true
63+
}
64+
mset := types.NewMethodSet(types.NewPointer(pkg.Types.Scope().Lookup("RPCServer").Type()))
65+
for i := 0; i < mset.Len(); i++ {
66+
fn := mset.At(i).Obj().(*types.Func)
67+
name := fn.Name()
68+
if name[0] >= 'a' && name[0] <= 'z' {
69+
continue
70+
}
71+
sig := fn.Type().(*types.Signature)
72+
if sig.Params().Len() != 2 {
73+
continue
74+
}
75+
fmt.Fprintf(buf, "methods[\"RPCServer.%s\"] = &methodType{ method: reflect.ValueOf(s.%s) }\n", name, name)
76+
77+
}
78+
return true
79+
}, nil)
80+
fmt.Fprintf(buf, "}\n\n")
81+
}

cmd/dlv/dlv_test.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ func TestGeneratedDoc(t *testing.T) {
374374
checkAutogenDoc(t, "pkg/terminal/starbind/starlark_mapping.go", "'go generate' inside pkg/terminal/starbind", runScript("_scripts/gen-starlark-bindings.go", "go", "-"))
375375
checkAutogenDoc(t, "Documentation/cli/starlark.md", "'go generate' inside pkg/terminal/starbind", runScript("_scripts/gen-starlark-bindings.go", "doc/dummy", "Documentation/cli/starlark.md"))
376376
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", "-"))
377+
checkAutogenDoc(t, "service/rpccommon/suitablemethods.go", "'go generate' inside service/rpccommon", runScript("_scripts/gen-suitablemethods.go", "-"))
377378
if goversion.VersionAfterOrEqual(runtime.Version(), 1, 18) {
378379
checkAutogenDoc(t, "_scripts/rtype-out.txt", "go run _scripts/rtype.go report _scripts/rtype-out.txt", runScript("_scripts/rtype.go", "report"))
379380
runScript("_scripts/rtype.go", "check")
@@ -1346,7 +1347,7 @@ func TestVersion(t *testing.T) {
13461347
want1 := []byte("mod\tgithub.1485827954.workers.dev/go-delve/delve")
13471348
want2 := []byte("dep\tgithub.1485827954.workers.dev/google/go-dap")
13481349
if !bytes.Contains(got, want1) || !bytes.Contains(got, want2) {
1349-
t.Errorf("got %s\nwant %v and %v in the output", got, want1, want2)
1350+
t.Errorf("got %s\nwant %s and %s in the output", got, want1, want2)
13501351
}
13511352
}
13521353

@@ -1462,3 +1463,14 @@ func TestUnixDomainSocket(t *testing.T) {
14621463
client.Detach(true)
14631464
cmd.Wait()
14641465
}
1466+
1467+
func TestDeadcodeEliminated(t *testing.T) {
1468+
dlvbin := protest.GetDlvBinary(t)
1469+
buf, err := exec.Command("go", "tool", "nm", dlvbin).CombinedOutput()
1470+
if err != nil {
1471+
t.Fatalf("error executing go tool nm %s: %v", dlvbin, err)
1472+
}
1473+
if strings.Contains(string(buf), "MethodByName") {
1474+
t.Fatal("output of go tool nm contains MethodByName")
1475+
}
1476+
}

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ require (
1313
github.com/mattn/go-colorable v0.1.13
1414
github.com/mattn/go-isatty v0.0.20
1515
github.com/sirupsen/logrus v1.9.3
16-
github.com/spf13/cobra v1.7.0
17-
github.com/spf13/pflag v1.0.5
16+
github.com/spf13/cobra v1.9.1
17+
github.com/spf13/pflag v1.0.6
1818
go.starlark.net v0.0.0-20231101134539-556fd59b42f6
1919
golang.org/x/arch v0.11.0
2020
golang.org/x/sys v0.26.0
@@ -24,7 +24,7 @@ require (
2424
)
2525

2626
require (
27-
github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
27+
github.com/cpuguy83/go-md2man/v2 v2.0.6 // indirect
2828
github.com/inconshreveable/mousetrap v1.1.0 // indirect
2929
github.com/mattn/go-runewidth v0.0.13 // indirect
3030
github.com/rivo/uniseg v0.2.0 // indirect

go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ github.com/cilium/ebpf v0.11.0 h1:V8gS/bTCCjX9uUnkUFUpPsksM8n1lXBAvHcpiFk1X2Y=
22
github.com/cilium/ebpf v0.11.0/go.mod h1:WE7CZAnqOL2RouJ4f1uyNhqr2P4CCvXFIqdRDUgWsVs=
33
github.com/cosiner/argv v0.1.0 h1:BVDiEL32lwHukgJKP87btEPenzrrHUjajs/8yzaqcXg=
44
github.com/cosiner/argv v0.1.0/go.mod h1:EusR6TucWKX+zFgtdUsKT2Cvg45K5rtpCcWz4hK06d8=
5-
github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
6-
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
5+
github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0=
6+
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
77
github.com/creack/pty v1.1.20 h1:VIPb/a2s17qNeQgDnkfZC35RScx+blkKF8GV68n80J4=
88
github.com/creack/pty v1.1.20/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
99
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
4545
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
4646
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
4747
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
48-
github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I=
49-
github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0=
50-
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
51-
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
48+
github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo=
49+
github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0=
50+
github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o=
51+
github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
5252
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
5353
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
5454
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=

pkg/version/buildinfo.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,28 @@ package version
22

33
import (
44
"bytes"
5+
"fmt"
56
"runtime/debug"
6-
"text/template"
77
)
88

99
func init() {
1010
buildInfo = moduleBuildInfo
1111
}
1212

13-
var buildInfoTmpl = ` mod {{.Main.Path}} {{.Main.Version}} {{.Main.Sum}}
14-
{{range .Deps}} dep {{.Path}} {{.Version}} {{.Sum}}{{if .Replace}}
15-
=> {{.Replace.Path}} {{.Replace.Version}} {{.Replace.Sum}}{{end}}
16-
{{end}}`
17-
1813
func moduleBuildInfo() string {
1914
info, ok := debug.ReadBuildInfo()
2015
if !ok {
2116
return "not built in module mode"
2217
}
2318

2419
buf := new(bytes.Buffer)
25-
err := template.Must(template.New("buildinfo").Parse(buildInfoTmpl)).Execute(buf, info)
26-
if err != nil {
27-
panic(err)
20+
fmt.Fprintf(buf, " mod\t%s\t%s\t%s\n", info.Main.Path, info.Main.Version, info.Main.Sum)
21+
for _, dep := range info.Deps {
22+
fmt.Fprintf(buf, " dep\t%s\t%s\t%s", dep.Path, dep.Version, dep.Sum)
23+
if dep.Replace != nil {
24+
fmt.Fprintf(buf, "\t=> %s\t%s\t%s", dep.Replace.Path, dep.Replace.Version, dep.Replace.Sum)
25+
}
26+
fmt.Fprintf(buf, "\n")
2827
}
2928
return buf.String()
3029
}

service/rpccommon/server.go

Lines changed: 19 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ import (
1414
"reflect"
1515
"runtime"
1616
"sync"
17-
"unicode"
18-
"unicode/utf8"
1917

2018
"github.com/go-delve/delve/pkg/logflags"
2119
"github.com/go-delve/delve/pkg/version"
@@ -27,6 +25,8 @@ import (
2725
"github.com/go-delve/delve/service/rpc2"
2826
)
2927

28+
//go:generate go run ../../_scripts/gen-suitablemethods.go suitablemethods
29+
3030
// ServerImpl implements a JSON-RPC server that can switch between two
3131
// versions of the API.
3232
type ServerImpl struct {
@@ -62,8 +62,7 @@ type RPCServer struct {
6262
}
6363

6464
type methodType struct {
65-
method reflect.Method
66-
Rcvr reflect.Value
65+
method reflect.Value
6766
ArgType reflect.Type
6867
ReplyType reflect.Type
6968
Synchronous bool
@@ -128,8 +127,10 @@ func (s *ServerImpl) Run() error {
128127
s.methodMaps = make([]map[string]*methodType, 2)
129128

130129
s.methodMaps[1] = map[string]*methodType{}
131-
suitableMethods(s.s2, s.methodMaps[1], s.log)
132-
suitableMethods(rpcServer, s.methodMaps[1], s.log)
130+
131+
suitableMethods2(s.s2, s.methodMaps[1])
132+
suitableMethodsCommon(rpcServer, s.methodMaps[1])
133+
finishMethodsMapInit(s.methodMaps[1])
133134

134135
go func() {
135136
defer s.listener.Close()
@@ -183,92 +184,15 @@ func (s *ServerImpl) serveConnectionDemux(c io.ReadWriteCloser) {
183184
}
184185
}
185186

186-
// Precompute the reflect type for error. Can't use error directly
187-
// because Typeof takes an empty interface value. This is annoying.
188-
var typeOfError = reflect.TypeOf((*error)(nil)).Elem()
189-
190-
// Is this an exported - upper case - name?
191-
func isExported(name string) bool {
192-
ch, _ := utf8.DecodeRuneInString(name)
193-
return unicode.IsUpper(ch)
194-
}
195-
196-
// Is this type exported or a builtin?
197-
func isExportedOrBuiltinType(t reflect.Type) bool {
198-
for t.Kind() == reflect.Ptr {
199-
t = t.Elem()
200-
}
201-
// PkgPath will be non-empty even for an exported type,
202-
// so we need to check the type name as well.
203-
return isExported(t.Name()) || t.PkgPath() == ""
204-
}
205-
206-
// Fills methods map with the methods of receiver that should be made
207-
// available through the RPC interface.
208-
// These are all the public methods of rcvr that have one of those
209-
// two signatures:
210-
//
211-
// func (rcvr ReceiverType) Method(in InputType, out *ReplyType) error
212-
// func (rcvr ReceiverType) Method(in InputType, cb service.RPCCallback)
213-
func suitableMethods(rcvr interface{}, methods map[string]*methodType, log logflags.Logger) {
214-
typ := reflect.TypeOf(rcvr)
215-
rcvrv := reflect.ValueOf(rcvr)
216-
sname := reflect.Indirect(rcvrv).Type().Name()
217-
if sname == "" {
218-
log.Debugf("rpc.Register: no service name for type %s", typ)
219-
return
220-
}
221-
for m := 0; m < typ.NumMethod(); m++ {
222-
method := typ.Method(m)
223-
mname := method.Name
224-
mtype := method.Type
225-
// method must be exported
226-
if method.PkgPath != "" {
227-
continue
228-
}
229-
// Method needs three ins: (receive, *args, *reply) or (receiver, *args, *RPCCallback)
230-
if mtype.NumIn() != 3 {
231-
log.Warn("method", mname, "has wrong number of ins:", mtype.NumIn())
232-
continue
233-
}
234-
// First arg need not be a pointer.
235-
argType := mtype.In(1)
236-
if !isExportedOrBuiltinType(argType) {
237-
log.Warn(mname, "argument type not exported:", argType)
238-
continue
239-
}
240-
241-
replyType := mtype.In(2)
242-
synchronous := replyType.String() != "service.RPCCallback"
243-
244-
if synchronous {
245-
// Second arg must be a pointer.
246-
if replyType.Kind() != reflect.Ptr {
247-
log.Warn("method", mname, "reply type not a pointer:", replyType)
248-
continue
249-
}
250-
// Reply type must be exported.
251-
if !isExportedOrBuiltinType(replyType) {
252-
log.Warn("method", mname, "reply type not exported:", replyType)
253-
continue
254-
}
255-
256-
// Method needs one out.
257-
if mtype.NumOut() != 1 {
258-
log.Warn("method", mname, "has wrong number of outs:", mtype.NumOut())
259-
continue
260-
}
261-
// The return type of the method must be error.
262-
if returnType := mtype.Out(0); returnType != typeOfError {
263-
log.Warn("method", mname, "returns", returnType.String(), "not error")
264-
continue
265-
}
266-
} else if mtype.NumOut() != 0 {
267-
// Method needs zero outs.
268-
log.Warn("method", mname, "has wrong number of outs:", mtype.NumOut())
269-
continue
187+
func finishMethodsMapInit(methods map[string]*methodType) {
188+
for name, method := range methods {
189+
mtype := method.method.Type()
190+
if mtype.NumIn() != 2 {
191+
panic(fmt.Errorf("wrong number of inputs for method %s (%d)", name, mtype.NumIn()))
270192
}
271-
methods[sname+"."+mname] = &methodType{method: method, ArgType: argType, ReplyType: replyType, Synchronous: synchronous, Rcvr: rcvrv}
193+
method.ArgType = mtype.In(0)
194+
method.ReplyType = mtype.In(1)
195+
method.Synchronous = method.ReplyType.String() != "service.RPCCallback"
272196
}
273197
}
274198

@@ -326,7 +250,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
326250
s.log.Debugf("<- %s(%T%s)", req.ServiceMethod, argv.Interface(), argvbytes)
327251
}
328252
replyv = reflect.New(mtype.ReplyType.Elem())
329-
function := mtype.method.Func
253+
function := mtype.method
330254
var returnValues []reflect.Value
331255
var errInter interface{}
332256
func() {
@@ -335,7 +259,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
335259
errInter = newInternalError(ierr, 2)
336260
}
337261
}()
338-
returnValues = function.Call([]reflect.Value{mtype.Rcvr, argv, replyv})
262+
returnValues = function.Call([]reflect.Value{argv, replyv})
339263
errInter = returnValues[0].Interface()
340264
}()
341265

@@ -358,15 +282,15 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
358282
argvbytes, _ := json.Marshal(argv.Interface())
359283
s.log.Debugf("(async %d) <- %s(%T%s)", req.Seq, req.ServiceMethod, argv.Interface(), argvbytes)
360284
}
361-
function := mtype.method.Func
285+
function := mtype.method
362286
ctl := &RPCCallback{s, sending, codec, req, make(chan struct{}), clientDisconnectChan}
363287
go func() {
364288
defer func() {
365289
if ierr := recover(); ierr != nil {
366290
ctl.Return(nil, newInternalError(ierr, 2))
367291
}
368292
}()
369-
function.Call([]reflect.Value{mtype.Rcvr, argv, reflect.ValueOf(ctl)})
293+
function.Call([]reflect.Value{argv, reflect.ValueOf(ctl)})
370294
}()
371295
<-ctl.setupDone
372296
}

0 commit comments

Comments
 (0)