Skip to content

Commit

Permalink
cmd/link: check for reflect.Value.MethodByName explicitly
Browse files Browse the repository at this point in the history
Currently we only check for reflect.Value.Method. And
reflect.Value.MethodByName is covered since it calls
reflect.Value.Method internally. But it is brittle to rely on
implementation detail of the reflect package. Check for
MethodByName explicitly.

Change-Id: Ifa8920e997524003dade03abc4fb3c4e64723643
Reviewed-on: https://go-review.googlesource.com/c/go/+/228881
Run-TryBot: Cherry Zhang <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
cherrymui committed Apr 19, 2020
1 parent a32262d commit af9ab6b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
5 changes: 3 additions & 2 deletions src/cmd/link/internal/ld/deadcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import (
//
// 1. direct call
// 2. through a reachable interface type
// 3. reflect.Value.Method, or reflect.Type.Method
// 3. reflect.Value.Method (or MethodByName), or reflect.Type.Method
// (or MethodByName)
//
// The first case is handled by the flood fill, a directly called method
// is marked as reachable.
Expand All @@ -33,7 +34,7 @@ import (
// as reachable. This is extremely conservative, but easy and correct.
//
// The third case is handled by looking to see if any of:
// - reflect.Value.Method is reachable
// - reflect.Value.Method or MethodByName is reachable
// - reflect.Type.Method or MethodByName is called (through the
// REFLECTMETHOD attribute marked by the compiler).
// If any of these happen, all bets are off and all exported methods
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/link/internal/ld/deadcode2.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ func deadcode2(ctxt *Link) {
d.flood()

methSym := ldr.Lookup("reflect.Value.Method", sym.SymVerABIInternal)
methByNameSym := ldr.Lookup("reflect.Value.MethodByName", sym.SymVerABIInternal)
if ctxt.DynlinkingGo() {
// Exported methods may satisfy interfaces we don't know
// about yet when dynamically linking.
Expand All @@ -230,7 +231,7 @@ func deadcode2(ctxt *Link) {
// Methods might be called via reflection. Give up on
// static analysis, mark all exported methods of
// all reachable types as reachable.
d.reflectSeen = d.reflectSeen || (methSym != 0 && ldr.AttrReachable(methSym))
d.reflectSeen = d.reflectSeen || (methSym != 0 && ldr.AttrReachable(methSym)) || (methByNameSym != 0 && ldr.AttrReachable(methByNameSym))

// Mark all methods that could satisfy a discovered
// interface as reachable. We recheck old marked interfaces
Expand Down

0 comments on commit af9ab6b

Please sign in to comment.