From ed8079096fe2e78d6dcb8002758774dca6d24eee Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Wed, 10 Feb 2021 12:43:18 -0500 Subject: [PATCH] cmd/compile: mark concrete call of reflect.(*rtype).Method as REFLECTMETHOD For functions that call reflect.Type.Method (or MethodByName), we mark it as REFLECTMETHOD, which tells the linker that methods can be retrieved via reflection and the linker keeps all exported methods live. Currently, this marking expects exactly the interface call reflect.Type.Method (or MethodByName). But now the compiler can devirtualize that call to a concrete call reflect.(*rtype).Method (or MethodByName), which is not handled and causing the linker to discard methods too aggressively. Handle the latter in this CL. Fixes #44207. Change-Id: Ia4060472dbff6ab6a83d2ca8e60a3e3f180ee832 Reviewed-on: https://go-review.googlesource.com/c/go/+/290950 Trust: Cherry Zhang Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/walk.go | 16 +++++++++++++++- test/reflectmethod7.go | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/reflectmethod7.go diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 2133a160b2b..98ebb239917 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -550,8 +550,12 @@ opswitch: case OCLOSUREVAR, OCFUNC: case OCALLINTER, OCALLFUNC, OCALLMETH: - if n.Op == OCALLINTER { + if n.Op == OCALLINTER || n.Op == OCALLMETH { + // We expect both interface call reflect.Type.Method and concrete + // call reflect.(*rtype).Method. usemethod(n) + } + if n.Op == OCALLINTER { markUsedIfaceMethod(n) } @@ -3710,6 +3714,16 @@ func usemethod(n *Node) { } } + // 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 + // alive. We only want to mark their callers. + if myimportpath == "reflect" { + switch Curfn.Func.Nname.Sym.Name { // TODO: is there a better way than hardcoding the names? + case "(*rtype).Method", "(*rtype).MethodByName", "(*interfaceType).Method", "(*interfaceType).MethodByName": + return + } + } + // Note: Don't rely on res0.Type.String() since its formatting depends on multiple factors // (including global variables such as numImports - was issue #19028). // Also need to check for reflect package itself (see Issue #38515). diff --git a/test/reflectmethod7.go b/test/reflectmethod7.go new file mode 100644 index 00000000000..42429978b45 --- /dev/null +++ b/test/reflectmethod7.go @@ -0,0 +1,24 @@ +// run + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// See issue 44207. + +package main + +import "reflect" + +type S int + +func (s S) M() {} + +func main() { + t := reflect.TypeOf(S(0)) + fn, ok := reflect.PtrTo(t).MethodByName("M") + if !ok { + panic("FAIL") + } + fn.Func.Call([]reflect.Value{reflect.New(t)}) +}