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: panic during dot-import function name collision #47201

Closed
zyedidia opened this issue Jul 14, 2021 · 8 comments
Closed

cmd/compile: panic during dot-import function name collision #47201

zyedidia opened this issue Jul 14, 2021 · 8 comments
Assignees
Labels
Milestone

Comments

@zyedidia
Copy link

@zyedidia zyedidia commented Jul 14, 2021

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

$ go version
go version go1.17rc1 linux/amd64

Does this issue reproduce with the latest release?

It reproduces with go1.17rc1 but not go1.16

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/zyedidia/.cache/go-build"
GOENV="/home/zyedidia/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/zyedidia/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/zyedidia/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/zyedidia/sdk/go1.17rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/zyedidia/sdk/go1.17rc1/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17rc1"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3213770622=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I created two files: test1.go and test2.go:

test1.go:

package main

import (
	. "fmt"
)

func test() {
	Println("foo")
}

test2.go:

package main

func Println() {}

func main() {}

Then I ran go1.17rc1 build. The names of the files are important for ordering.

What did you expect to see?

I expect to see an error telling me that a function from a dot-imported package collides with a function defined in the file, or maybe no error (this is the behavior of Go 1.16, but this also seems wrong). If alphabetical ordering of the files is swapped (by renaming test2.go to test0.go), I observe the correct behavior: Go 1.16 reports a number of errors, and Go 1.17rc1 reports the same ones:

./test1.go:4:2: Println redeclared during import "fmt"
	previous declaration at ./test0.go:3:6
./test1.go:4:2: imported and not used: "fmt"
./test1.go:8:9: too many arguments in call to Println
	have (string)
	want ()

What did you see instead?

However with the ordering from the original description above, Go 1.16 reports no issues (and successfully generates a binary) and Go 1.17rc1 fails with a panic:

panic: interface conversion: types.Object is nil, not *ir.Ident

goroutine 1 [running]:
cmd/compile/internal/typecheck.Redeclared({0x50, 0x0}, 0xc000419db0, {0xcec6ae, 0xd})
	/usr/local/go/src/cmd/compile/internal/typecheck/dcl.go:109 +0x2c5
cmd/compile/internal/typecheck.Declare(0xc00042aea0, 0x7)
	/usr/local/go/src/cmd/compile/internal/typecheck/dcl.go:77 +0x4d2
cmd/compile/internal/noder.(*noder).funcDecl(0xc0003ccc40, 0xc00040e060)
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:584 +0x585
cmd/compile/internal/noder.(*noder).decls(0xc0003ccc40, {0xc00041c0a0, 0x2, 0xc0004100c0})
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:331 +0x2c5
cmd/compile/internal/noder.(*noder).node(0xc0003ccc40)
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:285 +0xb8
cmd/compile/internal/noder.LoadPackage({0xc00001e250, 0x3, 0x0})
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:85 +0x358
cmd/compile/internal/gc.Main(0xd16418)
	/usr/local/go/src/cmd/compile/internal/gc/main.go:192 +0xb2e
main.main()
	/usr/local/go/src/cmd/compile/main.go:55 +0xdd

Go vet catches the error in both file orderings.

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Jul 14, 2021

go1.16 accepts this program, sounds like we need a backport cc @mdempsky @griesemer

This does not happen on dev.typeparams with types2.

Loading

@cherrymui cherrymui added this to the Go1.17 milestone Jul 14, 2021
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 14, 2021

sounds like we need a backport

This needs to be fixed in Go 1.17, but we don't need a backport as Go 1.17 has not been released. Thanks.

Loading

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Jul 14, 2021

sounds like we need a backport

This needs to be fixed in Go 1.17, but we don't need a backport as Go 1.17 has not been released. Thanks.

What I mean is that go1.16 accepts this program, it must not. Sounds like we need to backport a fix to go1.16 branch.

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 14, 2021

Okay, thanks. I missed that Go 1.16 behaves correctly only with certain ordering of the files.

Loading

@cuonglm cuonglm self-assigned this Jul 14, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 14, 2021

Change https://golang.org/cl/334589 mentions this issue: cmd/compile: set last declaration position for import dot

Loading

@gopherbot gopherbot closed this in c1cc9f9 Jul 15, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 16, 2021

Change https://golang.org/cl/334874 mentions this issue: [release-branch.go1.16] cmd/compile: fix wrong Block for dot import symbol

Loading

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Jul 16, 2021

@gopherbot please consider this for backport to 1.16

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 16, 2021

Backport issue(s) opened: #47244 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

Loading

steeve added a commit to znly/go that referenced this issue Aug 19, 2021
The compiler is relying on Sym.Def field to lookup symbol package in
DotImportRefs map. But the Sym.Def field is clear whenever the compiler
finish processing a file. If the dot import happen in file A, then the
redeclaration happen in file B, then the symbol lookup in file B will
see a nil Sym.Def, that cause the compiler crashes.

To fix this, we can interate over DotImportRefs and check for matching
symbol name and return the corresponding package. Though this operation
can be slow, but it only happens in invalid program, when printing error
message, so it's not worth to optimize it further.

Fixes golang#47201

Change-Id: I4ca1cb0a8e7432b19cf71434592a4cbb58d54adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/334589
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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
4 participants