From: Dominique Lefevre Date: Thu, 24 Aug 2023 07:15:29 +0000 (+0300) Subject: cmd/compile: special-case MethodByName(string literal) to keep the DCE enabled. X-Git-Tag: go1.22rc1~1017 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=e802f211b0d3f9dac5c4ca56c9f83df0cb745070;p=gostls13.git 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 Reviewed-by: Dmitri Shuralyov Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot --- 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)