]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: special-case MethodByName(string literal) to keep the DCE enabled.
authorDominique Lefevre <domingolefevre@gmail.com>
Thu, 24 Aug 2023 07:15:29 +0000 (10:15 +0300)
committerCherry Mui <cherryyz@google.com>
Fri, 1 Sep 2023 15:06:51 +0000 (15:06 +0000)
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>

src/cmd/compile/internal/walk/expr.go

index b4179dddb18ffd586b3e1d8317f508e4e3fa5603..49b9576d369bf28341cceb8f5089c6f27a13490b 100644 (file)
@@ -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)