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: internal compiler error on import on mismatch between -p and package name #54542

Closed
ALTree opened this issue Aug 19, 2022 · 6 comments
Assignees
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

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
Contributor

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
Contributor

mdempsky commented Aug 19, 2022

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
Contributor

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.

@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
@cuonglm cuonglm self-assigned this Jul 3, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/596396 mentions this issue: cmd/compile: emit error message on mismatch import path

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/596515 mentions this issue: cmd/compile: add comment for the context on mismatch import path

Mchnan pushed a commit to Mchnan/go-sylixos that referenced this issue Jul 9, 2024
Fixes golang#54542

Change-Id: I16cfb84fc54892923106d0a6f0b3ba810886d077
Reviewed-on: https://go-review.googlesource.com/c/go/+/596396
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
gopherbot pushed a commit that referenced this issue Jul 31, 2024
Follow up suggestion in CL 596396.

Updates #54542

Change-Id: I47bf66684bb8397dc1cfbc4479e2279e59a40cfb
Reviewed-on: https://go-review.googlesource.com/c/go/+/596515
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

No branches or pull requests

5 participants