cmd/compile: unexpected crash when using -G=3 with external symbols.

[Gh0u1L5]

What version of Go are you using (go version)?

$ go version
go version devel go1.17-48b7432e3f Thu Apr 15 01:54:41 2021 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

Firstly, create the following two files.

1. item/item.go

package item

type Int struct {
	value int
}

func NewInt() *Int {
	return &Int{0}
}

func (i *Int) Add(delta int) {
	i.value += delta
}

2. main.go

package main

import (
	"fmt"
	"test/item"
)

type wrapper[T any] struct {
	value T
}

func (w wrapper[T]) Get() T {
	return w.value
}

func main() {
	w := wrapper[item.Int]{}
	fmt.Println(w)
}

Then, execute the following commands to compile the codes.

$ go mod init test
$ go build -gcflags=-G=3 main.go

The compiler will first crash with the following logs:

# command-line-arguments
<autogenerated>:1: internal compiler error: method mismatch: wrapper[item.Int] for *wrapper[Int]

goroutine 1 [running]:
runtime/debug.Stack(0x1ba6480, 0xc00000e018, 0x0)
        /home/Gh0u1L5/Projects/go/src/runtime/debug/stack.go:24 +0x9f
cmd/compile/internal/base.FatalfAt(0x100000000001, 0x1a665bf, 0x1a, 0xc000389fd8, 0x2, 0x2)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/base/print.go:227 +0x1ea
cmd/compile/internal/base.Fatalf(...)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/typecheck.Lookdot(0xc000406300, 0xc00037f8f0, 0x0, 0x1bbaf00)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:1244 +0xccc
cmd/compile/internal/typecheck.tcDot(0xc000406300, 0xe, 0x1a15260, 0x1a49240)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/expr.go:499 +0x331
cmd/compile/internal/typecheck.typecheck1(0x1bbb628, 0xc000406300, 0xe, 0xc000406300, 0x120f92c)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:682 +0x1327
cmd/compile/internal/typecheck.typecheck(0x1bbb628, 0xc000406300, 0xe, 0x1, 0x1bb12e8)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:371 +0x63b
cmd/compile/internal/typecheck.tcCall(0xc0001799e0, 0x12, 0xc0004a39f0, 0xc0004a39f0)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/func.go:397 +0xac
cmd/compile/internal/typecheck.typecheck1(0x1bb9b98, 0xc0001799e0, 0x12, 0xc0001799e0, 0x20)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:715 +0xd9a
cmd/compile/internal/typecheck.typecheck(0x1bb9b98, 0xc0001799e0, 0x12, 0x3a98b01, 0x10000c00038abe0)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:371 +0x63b
cmd/compile/internal/typecheck.typecheckslice(0xc0000667c0, 0x1, 0x1, 0x12)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:177 +0x6c
cmd/compile/internal/typecheck.typecheckargs(0x1bbd288, 0xc0004a3b30)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:928 +0x1fe
cmd/compile/internal/typecheck.tcReturn(0xc0004a3b30, 0xc00049e97d, 0xc00040162f)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/stmt.go:343 +0x3b
cmd/compile/internal/typecheck.typecheck1(0x1bbb498, 0xc0004a3b30, 0x1, 0xc0004a3b30, 0xc00049e9a0)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:864 +0x2f1a
cmd/compile/internal/typecheck.typecheck(0x1bbb498, 0xc0004a3b30, 0x1, 0x1, 0x1bba368)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:371 +0x63b
cmd/compile/internal/typecheck.typecheckslice(0xc0000667d0, 0x1, 0x1, 0x1)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:177 +0x6c
cmd/compile/internal/typecheck.Stmts(...)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/typecheck/typecheck.go:36
cmd/compile/internal/reflectdata.methodWrapper(0xc0004a5960, 0xc000079e50, 0x1137201)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:1807 +0x14b5
cmd/compile/internal/reflectdata.methods(0xc0004a5960, 0x10, 0x34eea75a, 0x4)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:353 +0x485
cmd/compile/internal/reflectdata.uncommonSize(...)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:82
cmd/compile/internal/reflectdata.dcommontype(0xc0004df900, 0xc0004a5960, 0xc0004de300)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:719 +0x214
cmd/compile/internal/reflectdata.writeType(0xc0004a5960, 0xc0004a5960)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:1120 +0x450
cmd/compile/internal/reflectdata.dcommontype(0xc0004a1780, 0xc00037f8f0, 0x1014c00)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:693 +0xe6
cmd/compile/internal/reflectdata.writeType(0xc00037f8f0, 0xc0004e04f8)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:1145 +0x12b7
cmd/compile/internal/reflectdata.WriteRuntimeTypes()
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/reflectdata/reflect.go:1317 +0x34e
cmd/compile/internal/gc.dumpdata()
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/gc/obj.go:117 +0x91
cmd/compile/internal/gc.Main(0x1a83070)
        /home/Gh0u1L5/Projects/go/src/cmd/compile/internal/gc/main.go:304 +0x12ea
main.main()
        /home/Gh0u1L5/Projects/go/src/cmd/compile/main.go:54 +0x155

After a tough debugging, I managed to patch types.Identical to fix this crash.

diff --git a/src/cmd/compile/internal/types/identity.go b/src/cmd/compile/internal/types/identity.go
index dde9f51856..853e748f5d 100644
--- a/src/cmd/compile/internal/types/identity.go
+++ b/src/cmd/compile/internal/types/identity.go
@@ -28,7 +28,7 @@ func identical(t1, t2 *Type, cmpTags bool, assumedEqual map[typePair]struct{}) b
        if t1 == nil || t2 == nil || t1.kind != t2.kind || t1.Broke() || t2.Broke() {
                return false
        }
-       if t1.sym != nil || t2.sym != nil {
+       if (t1.sym != nil || t2.sym != nil) && t1.kind != TSTRUCT {
                // Special case: we keep byte/uint8 and rune/int32
                // separate for error messages. Treat them as equal.
                switch t1.kind {

Basically, a struct with type param now also has a non-nil sym. So if we find a struct with sym, instead of returning false, we should allow it to continue to the compare logic for structs.

However, the codes immediately triggered another crash with the following logs:

# command-line-arguments
type.main.wrapper[Int]: relocation target main.wrapper[Int].Get not defined

Apparently, the target here should be main.wrapper[item.Int]. I added some codes that explicitly replace every main.wrapper[Int] to main.wrapper[item.Int]. The compiler just worked like a charm.

Then I spend some time walking around, but failed to find where the compiler introduced the improper symbols. So I decide to file an issue here.

[Gh0u1L5]

Update: I've spent few more hours debugging the last crash, and eventually found a fix.

diff --git a/src/cmd/compile/internal/noder/types.go b/src/cmd/compile/internal/noder/types.go
index 8680559a41..fee6b5c09c 100644
--- a/src/cmd/compile/internal/noder/types.go
+++ b/src/cmd/compile/internal/noder/types.go
@@ -61,15 +61,19 @@ func (g *irgen) typ1(typ types2.Type) *types.Type {

 // instTypeName2 creates a name for an instantiated type, base on the type args
 // (given as types2 types).
-func instTypeName2(name string, targs []types2.Type) string {
+func instTypeName2(root *types2.Package, name string, targs []types2.Type) string {
        b := bytes.NewBufferString(name)
        b.WriteByte('[')
        for i, targ := range targs {
                if i > 0 {
                        b.WriteByte(',')
                }
-               tname := types2.TypeString(targ,
-                       func(*types2.Package) string { return "" })
+               tname := types2.TypeString(targ, func(pkg *types2.Package) string {
+                       if root == nil || pkg == nil || root.Path() == pkg.Path() {
+                               return ""
+                       }
+                       return pkg.Name()
+               })
                if strings.Index(tname, ", ") >= 0 {
                        // types2.TypeString puts spaces after a comma in a type
                        // list, but we don't want spaces in our actual type names
@@ -105,7 +109,7 @@ func (g *irgen) typ0(typ types2.Type) *types.Type {
                        // based on the names of the type arguments. We need a
                        // name to deal with recursive generic types (and it also
                        // looks better when printing types).
-                       instName := instTypeName2(typ.Obj().Name(), typ.TArgs())
+                       instName := instTypeName2(typ.Obj().Pkg(), typ.Obj().Name(), typ.TArgs())
                        s := g.pkg(typ.Obj().Pkg()).Lookup(instName)
                        if s.Def != nil {
                                // We have already encountered this instantiation,
@@ -262,7 +266,7 @@ func (g *irgen) fillinMethods(typ *types2.Named, ntyp *types.Type) {
                                // generic type, so we have to do a substitution to get
                                // the name/type of the method of the instantiated type,
                                // using m.Type().RParams() and typ.TArgs()
-                               inst2 := instTypeName2("", typ.TArgs())
+                               inst2 := instTypeName2(typ.Obj().Pkg(), "", typ.TArgs())
                                name := meth.Sym().Name
                                i1 := strings.Index(name, "[")
                                i2 := strings.Index(name[i1:], "]")

Please let me know if this fix looks good. I'll close this issue and open a PR.

[ALTree]

@Gh0u1L5

Please let me know if this fix looks good. I'll close this issue and open a PR.

Project members and code owners don't look at or review patches on the issue tracker (for CLA reasons I believe), if you want to propose a fix you'll need to send a PR or a gerrit change.

Also please don't close valid issues, they're closed automatically when a patch fixing them is merged.

[ALTree]

cc @griesemer

[mknyszek]

Actually, depending on what -G=3 is, this might not be a release blocker

[ALTree]

Reading #43931 (the Flag section in the OP) it looks like -G=3 definitely won't be enabled in 1.17 (and possibly not even 1.18), so I don't think a compiler crash on -G=3 will block the next release.

[ianlancetaylor]

CC @griesemer @findleyr

[griesemer]

-G=3 won't be enabled for 1.17 as @ALTree already pointed out. This is all work in progress. Leaving open so we can check this when we're ready.

[mdempsky]

This seems to be working with both -G=3 and GOEXPERIMENT=unified at tip on dev.typeparams. Please ping us to reopen if you're still able to reproduce.