Skip to content

Commit ed80790

Browse files
committed
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 <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
1 parent e9c9683 commit ed80790

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,12 @@ opswitch:
550550
case OCLOSUREVAR, OCFUNC:
551551

552552
case OCALLINTER, OCALLFUNC, OCALLMETH:
553-
if n.Op == OCALLINTER {
553+
if n.Op == OCALLINTER || n.Op == OCALLMETH {
554+
// We expect both interface call reflect.Type.Method and concrete
555+
// call reflect.(*rtype).Method.
554556
usemethod(n)
557+
}
558+
if n.Op == OCALLINTER {
555559
markUsedIfaceMethod(n)
556560
}
557561

@@ -3710,6 +3714,16 @@ func usemethod(n *Node) {
37103714
}
37113715
}
37123716

3717+
// Don't mark reflect.(*rtype).Method, etc. themselves in the reflect package.
3718+
// Those functions may be alive via the itab, which should not cause all methods
3719+
// alive. We only want to mark their callers.
3720+
if myimportpath == "reflect" {
3721+
switch Curfn.Func.Nname.Sym.Name { // TODO: is there a better way than hardcoding the names?
3722+
case "(*rtype).Method", "(*rtype).MethodByName", "(*interfaceType).Method", "(*interfaceType).MethodByName":
3723+
return
3724+
}
3725+
}
3726+
37133727
// Note: Don't rely on res0.Type.String() since its formatting depends on multiple factors
37143728
// (including global variables such as numImports - was issue #19028).
37153729
// Also need to check for reflect package itself (see Issue #38515).

test/reflectmethod7.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run
2+
3+
// Copyright 2021 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// See issue 44207.
8+
9+
package main
10+
11+
import "reflect"
12+
13+
type S int
14+
15+
func (s S) M() {}
16+
17+
func main() {
18+
t := reflect.TypeOf(S(0))
19+
fn, ok := reflect.PtrTo(t).MethodByName("M")
20+
if !ok {
21+
panic("FAIL")
22+
}
23+
fn.Func.Call([]reflect.Value{reflect.New(t)})
24+
}

0 commit comments

Comments
 (0)