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

Jump to bottom

cmd/compile: internal compiler error on import on mismatch between -p and package name #54542

Open
ALTree opened this issue Aug 19, 2022 · 4 comments
Open

cmd/compile: internal compiler error on import on mismatch between -p and package name #54542

ALTree opened this issue Aug 19, 2022 · 4 comments
Assignees
@mdempsky
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@ALTree
Copy link
Member

ALTree commented Aug 19, 2022 

$ gotip version
go version devel go1.20-6001c043dc Fri Aug 19 03:32:27 2022 +0000 linux/amd64

It's a small thing but I have encountered this by chance and I though I'd report it anyway, leaving for the compiler team to decide if it needs to be fixed.

Since unified was enabled by default, an internal compiler error can be triggered on manual invocations of go tool compile when there's mismatch between the package name as imported by main and the -p flag. Example:

$ tree
.
├── a.go
└── main.go

0 directories, 2 files

a.go

package a

func A() { println("a") }

main.go

package main

import "a"

func main() { a.A() }

Now:

$ gotip tool compile -p lie a.go         <--- note the -p flag value

$ gotip tool compile -I=. -p main main.go
<unknown line number>: internal compiler error: have package "lie" (0xc0003dd130), want package "a" (0xc0003dcfa0)

goroutine 1 [running]:
runtime/debug.Stack()
	/home/ZZZ/gotip/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x27005?, 0xc0?}, {0xd7c76d, 0x2a}, {0xc00011ca00, 0x4, 0x4})
	/home/ZZZ/gotip/src/cmd/compile/internal/base/print.go:227 +0x1d7
cmd/compile/internal/base.Fatalf(...)
	/home/ZZZ/gotip/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/base.Assertf(...)
	/home/ZZZ/gotip/src/cmd/compile/internal/base/print.go:246
cmd/compile/internal/noder.readPackage(0xc000107860?, 0xc0003dcfa0?, 0x0?)
	/home/ZZZ/gotip/src/cmd/compile/internal/noder/unified.go:260 +0x237
cmd/compile/internal/noder.readImportFile({0xc000027005?, 0xcec4a0?}, 0xc000000180, 0x7fa4029cc301?, 0xc0003cf140)
	/home/ZZZ/gotip/src/cmd/compile/internal/noder/import.go:244 +0x727
cmd/compile/internal/noder.(*gcimports).ImportFrom(0x140aee0?, {0xc000027005?, 0xc000060c00?}, {0xc000027005?, 0x2?}, 0x40f26d?)
	/home/ZZZ/gotip/src/cmd/compile/internal/noder/import.go:45 +0x3a
cmd/compile/internal/types2.(*Checker).importPackage(0xc000402000, {0xc0003ceff0, 0x3, 0x8}, {0xc000027005, 0x1}, {0xed11a0, 0x1})
	/home/ZZZ/gotip/src/cmd/compile/internal/types2/resolver.go:144 +0x19e
cmd/compile/internal/types2.(*Checker).collectObjects(0xc000402000)
	/home/ZZZ/gotip/src/cmd/compile/internal/types2/resolver.go:252 +0x1030
cmd/compile/internal/types2.(*Checker).checkFiles(0xc000402000, {0xc000012468, 0x1, 0x1})
	/home/ZZZ/gotip/src/cmd/compile/internal/types2/check.go:321 +0x12c
cmd/compile/internal/types2.(*Checker).Files(...)
...

This doesn't happen with GOEXPERIMENT=nounified, and in fact a subsequent go tool link invocation appears to produce a working binary. unified may want to either follow the old behaviour (no errors) or report an error message instead of an ICE.

cc @mdempsky
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 19, 2022
@ALTree ALTree added this to the Go1.20 milestone Aug 19, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 19, 2022
@mdempsky
Copy link
Member

Thanks, yeah, I need to change that to a proper error instead of an ICE. The Go compiler now requires you to always specify -p and that it needs to match the import path used in import statements.

@mdempsky mdempsky self-assigned this Aug 19, 2022
@mdempsky mdempsky added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 19, 2022
@mdempsky
Copy link
Member

mdempsky commented Aug 19, 2022 •
edited

This doesn't happen with GOEXPERIMENT=nounified, and in fact a subsequent go tool link invocation appears to produce a working binary.

FWIW, the binaries resulting from incorrect -p flag usage didn't necessary work correctly before, even when they linked successfully. For example:

$ cat a.go
package a

type T int

var X any = []T(nil)

$ cat main.go
package main

import "a"

func main() { println(a.X.([]a.T)) }

# using Go 1.19
$ go tool compile -p=b a.go
$ go tool compile -I . -p=main main.go
$ go tool link -L . main.o
$ ./a.out
panic: interface conversion: interface {} is []a.T, not []a.T (types from different scopes)

goroutine 1 [running]:
main.main()
	/tmp/main.go:5 +0x85

It's just that prior to unified IR, the export data format didn't actually record the -p flag, so we wouldn't catch mistakes like this.

For more history on the -p flag and how it slowly became essential to correct compilation, see the commit message in a987aaf.

@mdempsky mdempsky modified the milestones: Go1.20, Go1.21 Jan 10, 2023
@mdempsky
Copy link
Member

Bumping to 1.21. ICE instead of a proper diagnostic is ugly, but I expect typical users are unlikely to run into this in practice.

@tsaarni tsaarni mentioned this issue Feb 10, 2023
Closed
4 tasks
@cuonglm cuonglm mentioned this issue Feb 20, 2023
Closed
@mknyszek
Copy link
Contributor

mknyszek commented Jun 9, 2023

@mdempsky Based on your last message, it seems like this should happen during the dev cycle? Bumping to Backlog for now, but feel free to move it Go 1.22 if you plan to work on it. Thanks!

@mknyszek mknyszek modified the milestones: Go1.21, Backlog Jun 9, 2023
@ALTree ALTree mentioned this issue Oct 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdempsky mdempsky

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
4 participants
@mdempsky @mknyszek @ALTree @gopherbot