cmd/compile: special-case MethodByName(string literal) to keep the DCE enabled.

Normally, a call to MethodByName() disables the DCE because the linker
assumes that any method can be accessed this way. This pessimises
the code generation for k8s.io/apimachinery which needs MethodByName()
to verify whether or not a struct implements DeepCopyInto(). It cannot
cast a struct to `interface { DeepCopyInto() Foo }` because the return
type may vary. Instead, it does the following:

  if m := reflect.ValueOf(obj).MethodByName("DeepCopyInto"); ... {

In this case there is no need to disable the DCE altogether. It
suffices to add a relocation to keep methods named DeepCopyInto().

Fixes #62257.

Change-Id: I583c2f04d8309a8807de75cd962c04151baeeb1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/522436
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

1 files changed, 61 insertions, 22 deletions

diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go
index b4179dddb1..49b9576d36 100644
--- a/src/cmd/compile/internal/walk/expr.go
+++ b/src/cmd/compile/internal/walk/expr.go
@@ -12,11 +12,13 @@ import (
 
 	"cmd/compile/internal/base"
 	"cmd/compile/internal/ir"
+	"cmd/compile/internal/objw"
 	"cmd/compile/internal/reflectdata"
 	"cmd/compile/internal/staticdata"
 	"cmd/compile/internal/typecheck"
 	"cmd/compile/internal/types"
 	"cmd/internal/obj"
+	"cmd/internal/objabi"
 )
 
 // The result of walkExpr MUST be assigned back to n, e.g.
@@ -945,7 +947,8 @@ func bounded(n ir.Node, max int64) bool {
 	return false
 }
 
-// usemethod checks calls for uses of reflect.Type.{Method,MethodByName}.
+// usemethod checks calls for uses of Method and MethodByName of reflect.Value,
+// reflect.Type, reflect.(*rtype), and reflect.(*interfaceType).
 func usemethod(n *ir.CallExpr) {
 	// Don't mark reflect.(*rtype).Method, etc. themselves in the reflect package.
 	// Those functions may be alive via the itab, which should not cause all methods
@@ -957,10 +960,16 @@ func usemethod(n *ir.CallExpr) {
 			return
 		case fn == "(*interfaceType).Method", fn == "(*interfaceType).MethodByName":
 			return
+		case fn == "Value.Method", fn == "Value.MethodByName":
+			return
 		// StructOf defines closures that look up methods. They only look up methods
 		// reachable via interfaces. The DCE does not remove such methods. It is ok
 		// to not flag closures in StructOf as ReflectMethods and let the DCE run
 		// even if StructOf is reachable.
+		//
+		// (*rtype).MethodByName calls into StructOf so flagging StructOf as
+		// ReflectMethod would disable the DCE even when the name of a method
+		// to look up is a compile-time constant.
 		case strings.HasPrefix(fn, "StructOf.func"):
 			return
 		}
@@ -971,38 +980,68 @@ func usemethod(n *ir.CallExpr) {
 		return
 	}
 
-	// Looking for either direct method calls and interface method calls of:
-	//	reflect.Type.Method       - func(int) reflect.Method
-	//	reflect.Type.MethodByName - func(string) (reflect.Method, bool)
-	var pKind types.Kind
+	// looking for either direct method calls and interface method calls of:
+	//	reflect.Type.Method        - func(int) reflect.Method
+	//	reflect.Type.MethodByName  - func(string) (reflect.Method, bool)
+	//
+	//	reflect.Value.Method       - func(int) reflect.Value
+	//	reflect.Value.MethodByName - func(string) reflect.Value
+	methodName := dot.Sel.Name
+	t := dot.Selection.Type
+
+	// Check the number of arguments and return values.
+	if t.NumParams() != 1 || (t.NumResults() != 1 && t.NumResults() != 2) {
+		return
+	}
+
+	// Check the type of the argument.
+	switch pKind := t.Param(0).Type.Kind(); {
+	case methodName == "Method" && pKind == types.TINT,
+		methodName == "MethodByName" && pKind == types.TSTRING:
 
-	switch dot.Sel.Name {
-	case "Method":
-		pKind = types.TINT
-	case "MethodByName":
-		pKind = types.TSTRING
 	default:
+		// not a call to Method or MethodByName of reflect.{Type,Value}.
 		return
 	}
 
-	t := dot.Selection.Type
-	if t.NumParams() != 1 || t.Param(0).Type.Kind() != pKind {
+	// Check that first result type is "reflect.Method" or "reflect.Value".
+	// Note that we have to check sym name and sym package separately, as
+	// we can't check for exact string "reflect.Method" reliably
+	// (e.g., see #19028 and #38515).
+	switch s := t.Result(0).Type.Sym(); {
+	case s != nil && types.ReflectSymName(s) == "Method",
+		s != nil && types.ReflectSymName(s) == "Value":
+
+	default:
+		// not a call to Method or MethodByName of reflect.{Type,Value}.
 		return
 	}
-	switch t.NumResults() {
-	case 1:
-		// ok
-	case 2:
-		if t.Result(1).Type.Kind() != types.TBOOL {
-			return
+
+	var targetName ir.Node
+	switch dot.Op() {
+	case ir.ODOTINTER:
+		if methodName == "MethodByName" {
+			targetName = n.Args[0]
+		}
+	case ir.OMETHEXPR:
+		if methodName == "MethodByName" {
+			targetName = n.Args[1]
 		}
 	default:
-		return
+		base.FatalfAt(dot.Pos(), "usemethod: unexpected dot.Op() %s", dot.Op())
 	}
 
-	// Check that first result type is "reflect.Method". Note that we have to check sym name and sym package
-	// separately, as we can't check for exact string "reflect.Method" reliably (e.g., see #19028 and #38515).
-	if s := t.Result(0).Type.Sym(); s != nil && types.ReflectSymName(s) == "Method" {
+	if ir.IsConst(targetName, constant.String) {
+		name := constant.StringVal(targetName.Val())
+
+		var nameSym obj.LSym
+		nameSym.WriteString(base.Ctxt, 0, len(name), name)
+		objw.Global(&nameSym, int32(len(name)), obj.RODATA)
+
+		r := obj.Addrel(ir.CurFunc.LSym)
+		r.Type = objabi.R_USEGENERICIFACEMETHOD
+		r.Sym = &nameSym
+	} else {
 		ir.CurFunc.SetReflectMethod(true)
 		// The LSym is initialized at this point. We need to set the attribute on the LSym.
 		ir.CurFunc.LSym.Set(obj.AttrReflectMethod, true)