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: incorrectly accepts package with name collision due to dot-imported function (was: panic during dot-import function name collision) [1.16 backport] #47244

Closed
gopherbot opened this issue Jul 16, 2021 · 7 comments

Comments

@gopherbot
Copy link

@cuonglm requested issue #47201 to be considered for backport to the next 1.16 minor release.

@gopherbot please consider this for backport to 1.16

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jul 16, 2021
@gopherbot gopherbot added this to the Go1.16.7 milestone Jul 16, 2021
@gopherbot
Copy link
Author

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

@mdempsky
Copy link
Member

mdempsky commented Jul 21, 2021

Edit: See #47244 (comment) instead; this assessment was based on a mistaken understanding of how the issue manifested in Go 1.16 (which differs from how it affects Go 1.17).

FWIW, I think the CL is trivially safe, but this may not meet the bar of "security issues, serious problems with no workaround, and documentation fixes" specified at https://github.com/golang/go/wiki/MinorReleases. In particular, this is an issue that the compiler crashes on particular invalid input rather than reporting an error. So I think it's not serious, and it also has a reasonable workaround (i.e., fix the problematic code). Dot imports also aren't very common.

@cuonglm
Copy link
Member

cuonglm commented Jul 21, 2021

@mdempsky But the problem is that the compiler compile successfully invalid program!

@mdempsky
Copy link
Member

@mdempsky But the problem is that the compiler compile successfully invalid program!

Oops, thanks. I forgot the issue affected Go 1.16 differently; I was just going by the issue title.

I think that the erroneous package would still be rejected by go/types, including due to go vet or go test (which runs vet by default). So it's perhaps still not a serious issue, but I think it's a bit more ambiguous.

@mdempsky mdempsky changed the title cmd/compile: panic during dot-import function name collision [1.16 backport] cmd/compile: incorrectly accepts package with name collision due to dot-imported function (was: panic during dot-import function name collision) [1.16 backport] Jul 21, 2021
@cuonglm
Copy link
Member

cuonglm commented Jul 21, 2021

I think that the erroneous package would still be rejected by go/types, including due to go vet or go test (which runs vet by default). So it's perhaps still not a serious issue, but I think it's a bit more ambiguous.

Fair point, but during development, I think developers often use go run *.go or go build *.go or go build. And it's actually make me surprise at first try when investigating #47201.

I think it's ok to not backport if this doesn't meet the bar.

@ianlancetaylor
Copy link
Contributor

I don't feel strongly about this but I'm inclined to think that this does not meet the bar for a backport. It's not good that the compiler accepts an invalid program, but the fact that it does so doesn't prevent anybody from using Go 1.16. The most one can say is that some people may accidentally run invalid Go programs, but that doesn't seem like a problem serious enough to require a fix in a minor release.

@cuonglm
Copy link
Member

cuonglm commented Jul 22, 2021

Closing this as it does not meet the bar for backporting.

@cuonglm cuonglm closed this as completed Jul 22, 2021
@dmitshur dmitshur removed the CherryPickCandidate Used during the release process for point releases label Aug 4, 2021
@golang golang locked and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants