Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Gh0u1L5 opened this issue Apr 16, 2021 · 8 comments
Closed

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

Gh0u1L5 opened this issue Apr 16, 2021 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Gh0u1L5
Copy link

Gh0u1L5 commented Apr 16, 2021

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
}
  1. 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
Copy link
Author

Gh0u1L5 commented Apr 16, 2021

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
Copy link
Member

ALTree commented Apr 16, 2021

@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 ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 16, 2021
@ALTree
Copy link
Member

ALTree commented Apr 16, 2021

cc @griesemer

@mknyszek mknyszek added this to the Go1.17 milestone Apr 16, 2021
@mknyszek
Copy link
Contributor

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

@ALTree
Copy link
Member

ALTree commented Apr 16, 2021

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
Copy link
Contributor

CC @griesemer @findleyr

@griesemer
Copy link
Contributor

-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.

@griesemer griesemer self-assigned this Apr 19, 2021
@mdempsky
Copy link
Member

mdempsky commented Jul 2, 2021

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.

@mdempsky mdempsky closed this as completed Jul 2, 2021
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants