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: using non-exported generic type causes internal compiler error #48337

Open
IlyaFloppy opened this issue Sep 11, 2021 · 9 comments
Open
Assignees
Labels
Milestone

Comments

@IlyaFloppy
Copy link

@IlyaFloppy IlyaFloppy commented Sep 11, 2021

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

$ go version
go version devel go1.18-b32209d22d Thu Sep 9 23:18:18 2021 +0000 darwin/amd64

Does this issue reproduce with the latest release?

-

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ilya/Library/Caches/go-build"
GOENV="/Users/ilya/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/ilya/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/ilya/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/ilya/Development/goroot"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/ilya/Development/goroot/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel go1.18-b32209d22d Thu Sep 9 23:18:18 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/ilya/Development/observable/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7m/d4lybfs976gdy3bf8n25dxbc0000gn/T/go-build4228260199=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

func main() {
	obj := genericpkg.NewWrapperWithLock("this file does not import sync")
	obj.PrintWithLock()
}
package genericpkg

import (
	"fmt"
	"sync"
)

type WrapperWithLock[T any] interface {
	PrintWithLock()
}

func NewWrapperWithLock[T any](value T) WrapperWithLock[T] {
	return &wrapperWithLock[T]{
		Object: value,
	}
}

type wrapperWithLock[T any] struct {
	Lock   sync.Mutex
	Object T
}

func (w *wrapperWithLock[T]) PrintWithLock() {
	w.Lock.Lock()
	defer w.Lock.Unlock()

	fmt.Println(w.Object)
}

https://github.com/IlyaFloppy/strangeimports

What did you expect to see?

Successful compilation

$ go build -o /dev/null ./cmd/withoutsync
$

What did you see instead?

internal compiler error: missing import reader for sync.(*Mutex).Lock

Full error

# obstest/cmd/withoutsync
/Users/ilya/Development/strangeimports/genericpkg/wrapper_with_lock.go:24:13: internal compiler error: missing import reader for sync.(*Mutex).Lock

goroutine 1 [running]:
runtime/debug.Stack()
/Users/ilya/Development/goroot/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x11ae6a3, 0x0}, {0x1902d56, 0x1c}, {0xc0000cd3c0, 0x1, 0x1})
/Users/ilya/Development/goroot/src/cmd/compile/internal/base/print.go:227 +0x154
cmd/compile/internal/base.Fatalf(...)
/Users/ilya/Development/goroot/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/typecheck.ImportBody(0xc00040a580)
/Users/ilya/Development/goroot/src/cmd/compile/internal/typecheck/iimport.go:73 +0x1b9
cmd/compile/internal/typecheck.ImportedBody(0xc00040a580)
/Users/ilya/Development/goroot/src/cmd/compile/internal/typecheck/func.go:169 +0xad
cmd/compile/internal/inline.oldInline(0xc0004605a0, 0xc00040a580, 0x1)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:782 +0x53
cmd/compile/internal/inline.mkinlcall(0xc0004605a0, 0xc00040a580, 0x11b2862, 0xc00007a1b0, 0xc00007e160)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:734 +0xc85
cmd/compile/internal/inline.inlnode({0x1a68af8, 0xc0004605a0}, 0x20, 0x30, 0x2200108)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:591 +0x45d
cmd/compile/internal/inline.InlineCalls.func1({0x1a68af8, 0xc0004605a0})
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:513 +0x31
cmd/compile/internal/ir.editNodes({0xc000401620, 0x3, 0x203000}, 0xc00007e160)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/node_gen.go:1521 +0x74
cmd/compile/internal/ir.(*Func).editChildren(0xc0000cdad8, 0x11bb467)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/func.go:153 +0x2e
cmd/compile/internal/ir.EditChildren(...)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/visit.go:185
cmd/compile/internal/inline.InlineCalls(0xc00040af20)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:515 +0xf2
cmd/compile/internal/inline.InlinePackage.func1({0xc000408a20, 0x1, 0xc00040af20}, 0x0)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:71 +0x46
cmd/compile/internal/ir.(*bottomUpVisitor).visit(0xc000580000, 0xc00040af20)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/scc.go:127 +0x2f0
cmd/compile/internal/ir.VisitFuncsBottomUp({0xc000438a00, 0x8, 0xb}, 0x191f3e8)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/scc.go:60 +0x105
cmd/compile/internal/inline.InlinePackage()
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:58 +0x33
cmd/compile/internal/gc.Main(0x191f298)
/Users/ilya/Development/goroot/src/cmd/compile/internal/gc/main.go:246 +0xc7d
main.main()
/Users/ilya/Development/goroot/src/cmd/compile/main.go:55 +0xdd

Adding imports in a file which uses provided code makes the error go away

import (
	"obstest/genericpkg"

	// these imports fix the error
	_ "fmt"
	_ "sync"
)

Returning value of exported type in NewWrapperWithLock[T any](value T) WrapperWithLock[T] also makes code compile without errors

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Sep 11, 2021

Seems inlining problem, it's build ok with:

go build -o /dev/null -gcflags=-l ./cmd/withoutsyn

and unified IR.

cc @danscales

@cuonglm cuonglm added this to the Go1.18 milestone Sep 11, 2021
@ahamlinman
Copy link

@ahamlinman ahamlinman commented Sep 12, 2021

I have an alternate reproducer that seems to demonstrate the same issue without using any standard library imports, and gives an error that might point more closely at the underlying problem.

$ gotip version
go version devel go1.18-ad97d204f0 Sun Sep 12 16:46:58 2021 +0000 linux/amd64
gotip env Output
$ gotip env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/alexhamlin/.cache/go-build"
GOENV="/home/alexhamlin/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/alexhamlin/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/alexhamlin/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/alexhamlin/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/alexhamlin/sdk/gotip/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.18-ad97d204f0 Sun Sep 12 16:46:58 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/home/alexhamlin/Desktop/typeparams-inlining-bug/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3969726503=/tmp/go-build -gno-record-gcc-switches"
-- go.mod --
module github.com/ahamlinman/typeparams-inlining-bug

go 1.18
-- main.go --
package main

import "github.com/ahamlinman/typeparams-inlining-bug/a"

func main() {
	a.NewMetaContainer()
}
-- a/a.go --
package a

type Container[T any] struct {
	X T
}

func NewContainer[T any](x T) *Container[T] {
	return &Container[T]{x}
}

type MetaContainer struct {
	C *Container[Value]
}

type Value struct{}

func NewMetaContainer() *MetaContainer {
	c := NewContainer(Value{})
	// c := &Container[Value]{Value{}} // <-- this works
	return &MetaContainer{c}
}
$ gotip build -gcflags=-m
# github.com/ahamlinman/typeparams-inlining-bug
./main.go:5:6: can inline main
./main.go:6:20: inlining call to a.NewMetaContainer
./main.go:6:20: inlining call to a.NewContainer[.shape.struct{}]
./main.go:6:20: &a.Container[.shape.struct{}]{...} does not escape
./main.go:6:20: &a.MetaContainer{...} does not escape
./main.go:6:20: cannot use ~R0 (type *a.Container[.shape.struct{}]) as type *a.Container[a.Value] in assignment
./main.go:6:20: cannot use a.x (type a.Value) as type .shape.struct {} in assignment

Same as above, compilation with -gcflags=-l does not exhibit the issue. It seems to be triggered specifically by the inlining of the type-parameterized constructor across packages, since removing that call also resolves the issue. I know almost nothing about Go compiler internals, but the mismatch between the defined type and the "shape" of that type might be a useful hint?

@danscales
Copy link

@danscales danscales commented Sep 13, 2021

The two cases listed look like distinct issues.

The second case is due to the fact that we are not correctly exporting/importing the IsShape/HasShape flag when importing shape types. We are importing shape types as part of importing functions like NewMetaContainer (including calls to stenciled functions with shape type params) as needed for inlining. If we isolate the knowledge to the types package, we can probably set the IsShape flag based on the type being in the shapes package. That may be preferable to changing the export format to know about an IsShape flag or a shape type. @randall77

I haven't tried to analyze the first issue yet.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 14, 2021

Change https://golang.org/cl/349869 mentions this issue: cmd/compile: set IsShape based on type being in the Shapes pkg

@danscales danscales self-assigned this Sep 14, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 14, 2021

Change https://golang.org/cl/349909 mentions this issue: cmd/compile: fix generic type handling in crawler

gopherbot pushed a commit that referenced this issue Sep 14, 2021
Move ShapePkg to types, and change types.NewNamed to automatically set
IsShape/HasShape if a type is in the shapes pkg. This means that
imported shape types will automatically have the correct
IsShape/HasShape flags, even though we are not explicitly
exporting/importing those flags.

Updates #48337

Change-Id: I8b6131a663205f73f395943c9d0c8bdb2a213401
Reviewed-on: https://go-review.googlesource.com/c/go/+/349869
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
gopherbot pushed a commit that referenced this issue Sep 17, 2021
There are a bunch of nodes beside ONAME and OTYPE, (such as OSTRUCTLIT
and OCOMPLIT) which can introduce a generic type that we need to mark.
So, just mark any generic type on any node in markInlBody. In this
particular issue, the type is introduced by an OSTRUCTLIT node.

Updates #48337

Change-Id: I271932518f0c1fb54d91a603e01a855c69df631d
Reviewed-on: https://go-review.googlesource.com/c/go/+/349909
Trust: Dan Scales <danscales@google.com>
Trust: Carlos Amedee <carlos@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@danscales
Copy link

@danscales danscales commented Sep 17, 2021

I believe both issues described in the issue are fixed now.

@danscales danscales closed this Sep 17, 2021
@IlyaFloppy
Copy link
Author

@IlyaFloppy IlyaFloppy commented Sep 18, 2021

I was getting the same error with other code (using latest master) so I made another reproducer.
https://github.com/IlyaFloppy/missingimportreader
Run

go test ./...

from project root.

Full error
# mir/common/pbadapt [mir/common/pbadapt.test]
/Users/ilya/Development/gobugs/missingimportreader/vendor/github.com/IlyaFloppy/observable/object.go:14:13: internal compiler error: missing import reader for sync.(*Mutex).Lock

goroutine 1 [running]:
runtime/debug.Stack()
/Users/ilya/Development/goroot/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x185c7e0, 0x0}, {0x18e8b6f, 0x1c}, {0xc0005513c0, 0x1, 0x1})
/Users/ilya/Development/goroot/src/cmd/compile/internal/base/print.go:227 +0x154
cmd/compile/internal/base.Fatalf(...)
/Users/ilya/Development/goroot/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/typecheck.ImportBody(0xc00043e6e0)
/Users/ilya/Development/goroot/src/cmd/compile/internal/typecheck/iimport.go:73 +0x1b9
cmd/compile/internal/typecheck.ImportedBody(0xc00043e6e0)
/Users/ilya/Development/goroot/src/cmd/compile/internal/typecheck/func.go:169 +0xad
cmd/compile/internal/inline.oldInline(0xc000599cb0, 0xc00043e6e0, 0x3)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:785 +0x53
cmd/compile/internal/inline.mkinlcall(0xc000599cb0, 0xc00043e6e0, 0x5a2900, 0xc0005a6c60, 0xc00086c220)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:737 +0xc85
cmd/compile/internal/inline.inlnode({0x1a46238, 0xc000599cb0}, 0x20, 0x30, 0x2200108)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:594 +0x4e5
cmd/compile/internal/inline.InlineCalls.func1({0x1a46238, 0xc000599cb0})
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:513 +0x31
cmd/compile/internal/ir.editNodes({0xc0000bfa70, 0x3, 0x1892840}, 0xc00086c220)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/node_gen.go:1521 +0x74
cmd/compile/internal/ir.(*Func).editChildren(0xc000551ad8, 0x11b74e7)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/func.go:153 +0x2e
cmd/compile/internal/ir.EditChildren(...)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/visit.go:185
cmd/compile/internal/inline.InlineCalls(0xc000892f20)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:515 +0xf2
cmd/compile/internal/inline.InlinePackage.func1({0xc0000c7250, 0x1, 0xc000892f20}, 0x0)
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:71 +0x46
cmd/compile/internal/ir.(*bottomUpVisitor).visit(0xc0005a6600, 0xc000892f20)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/scc.go:127 +0x2f0
cmd/compile/internal/ir.VisitFuncsBottomUp({0xc00001e700, 0xa, 0xb}, 0x1905200)
/Users/ilya/Development/goroot/src/cmd/compile/internal/ir/scc.go:60 +0x105
cmd/compile/internal/inline.InlinePackage()
/Users/ilya/Development/goroot/src/cmd/compile/internal/inline/inl.go:58 +0x33
cmd/compile/internal/gc.Main(0x19050b0)
/Users/ilya/Development/goroot/src/cmd/compile/internal/gc/main.go:246 +0xc7d
main.main()
/Users/ilya/Development/goroot/src/cmd/compile/main.go:55 +0xdd

FAIL mir/common/pbadapt [build failed]
FAIL


Should I open another issue for this one?

@danscales danscales reopened this Sep 20, 2021
@danscales
Copy link

@danscales danscales commented Sep 20, 2021

We can use this issue for the new case as well. I have a fix, but want to do more checking to make sure to get a general solution for these issues (which relate to exporting the types/functions bodies needed for inlining, as related to generic types that have unexported fields). Issue #48454 has similar problem/cause.

@ALTree ALTree changed the title Using non-exported generic type causes internal compiler error cmd/compile: using non-exported generic type causes internal compiler error Sep 20, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 21, 2021

Change https://golang.org/cl/351315 mentions this issue: cmd/compile: fix crawler for unexported fields with instantiated types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants