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: miscompilation of value switch involving generic interface types #53477

Closed
mdempsky opened this issue Jun 21, 2022 · 7 comments
Closed

cmd/compile: miscompilation of value switch involving generic interface types #53477

mdempsky opened this issue Jun 21, 2022 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.19

Comments

@mdempsky
Copy link
Member

https://go.dev/play/p/03EsH4Y3wv3

The above program should terminate quietly with success. However, in Go 1.18, it instead silently miscompiles into code that at runtime panics:

panic: interface conversion: main.X is not interface { M(go.shape.int_0) }: missing method M

Since this is a silent miscompilation, I think a fix and backport are appropriate. Ideally fix it to compile correctly, but even changing it to ICE would be preferable IMO.

/cc @randall77 @dr2chase @cuonglm
@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 21, 2022
@mdempsky mdempsky added this to the Go1.19 milestone Jun 21, 2022
@gopherbot
Copy link

Change https://go.dev/cl/413394 mentions this issue: test: add regress test for #53477

gopherbot pushed a commit that referenced this issue Jun 21, 2022
@mdempsky
test: add regress test for #53477
19ed442 
This test already passes for GOEXPERIMENT=unified; add regress test to
ensure it stays that way.

Updates #53477.

Change-Id: Ib7aa7428260595077052207899edcc044a6ab1c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/413394
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/413977 mentions this issue: [dev.unified] all: merge master (5a1c5b8) into dev.unified

gopherbot pushed a commit that referenced this issue Jun 24, 2022
@mdempsky
[dev.unified] all: merge master (5a1c5b8) into dev.unified
e7100ad 
Conflicts:

- test/run.go

  Textual conflict adding to the known failures list for the nounified
  frontend.

Merge List:

+ 2022-06-24 5a1c5b8 cmd/go: add per-package indexing for modules outside mod cache
+ 2022-06-24 b9c4d94 cmd/go/internal/list: update help info with Deprecated field
+ 2022-06-24 73475ef go/types, types2: print qualified object names in cycle errors
+ 2022-06-24 3e58ef6 go/types, types2: better errors for == when type sets are empty
+ 2022-06-24 d38f1d1 doc/go1.19: Linux race detector now requires glibc 2.17
+ 2022-06-23 de5329f debug/dwarf: handle malformed line table with bad program offset
+ 2022-06-23 15605ca embed: document additional file name restrictions
+ 2022-06-22 2e773a3 test: add test that causes gofrontend crash
+ 2022-06-22 ff17b7d cmd/compile: don't use dictionary convert to shaped empty interface
+ 2022-06-22 2a3b467 cmd/go: make module .zip files group/world readable
+ 2022-06-22 bdab4cf cmd/go, cmd/link: support failure to create _cgo_import.go
+ 2022-06-22 aca37d1 cmd/go: avoid indexing modules in GOROOT
+ 2022-06-22 111cdb5 all: update to current golang.org/x/sys revision
+ 2022-06-22 4045b1b cmd/compile: fix assert condition in generic method call
+ 2022-06-22 6bad7e8 compress/gzip: always close bodyReader in Example_compressingReader
+ 2022-06-22 606c6c3 encoding/xml: check nil pointer in DecodeElement
+ 2022-06-22 f571518 cmd/cgo: dont override declared struct type
+ 2022-06-22 92c9b81 net: don't set netGo = true on Windows with no cgo
+ 2022-06-22 be0b2a3 cmd/trace: add basic documentation to main page
+ 2022-06-22 b004c73 go/types, types2: fix parameter order dependence in type inference
+ 2022-06-21 f2c7e78 spec: document operations which accept []byte|string constrained types
+ 2022-06-21 ab422f2 runtime/trace: ignore fallback stacks in test
+ 2022-06-21 66685fb doc/go1.19: use correct link to sync/atomic docs
+ 2022-06-21 4b236b4 runtime: convert flaky semaphore linearity test into benchmark
+ 2022-06-21 530511b cmd/go/internal/modindex: avoid walking modules when not needed
+ 2022-06-21 c2d373d cmd/compile: allow 128-bit values to be spilled
+ 2022-06-21 19ed442 test: add regress test for #53477
+ 2022-06-20 3fcbfb0 doc/go1.19: fix HTML validation issues
+ 2022-06-18 527ace0 cmd/compile: skip substituting closures in unsafe builtins arguments
+ 2022-06-17 ec58e3f test: add regress test for #53419
+ 2022-06-17 103cc66 cmd/go/internal/modfetch: prevent duplicate hashes in go.sum
+ 2022-06-17 d42a488 sync: add more notes about Cond behavior
+ 2022-06-17 9e2f289 cmd/go/internal/work: log clearer detail for subprocess errors in (*Builder).toolID
+ 2022-06-17 dd2d00f net: fix flaky *TimeoutMustNotReturn tests
+ 2022-06-17 6c25ba6 go/token: delete unused File.set field
+ 2022-06-16 9068c68 cmd/dist: add package . to 'go test' commands
+ 2022-06-16 7bad615 runtime: write much more direct test for semaphore waiter scalability
+ 2022-06-16 f38a580 cmd/go: add more tracing

Change-Id: I912c5879165e03f4d7f8ac3ee9241d50fc92a419
@dr2chase
Copy link
Contributor

@randall77 did you see this yet?

@gopherbot
Copy link

Change https://go.dev/cl/414834 mentions this issue: cmd/compile: fix generic inter-inter comparisons from value switch statements

@dr2chase
Copy link
Contributor

Do we think this needs a backport to 1.18?

@mdempsky
Copy link
Member Author

@dr2chase I think so.

@gopherbot Please backport to Go 1.18. This is a silent miscompilation of valid Go code.

@gopherbot gopherbot mentioned this issue Jun 28, 2022
Closed
@gopherbot
Copy link

Backport issue(s) opened: #53587 (for 1.18).

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

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@mdempsky @jproberts
test: add regress test for golang#53477
6ac1b2b 
This test already passes for GOEXPERIMENT=unified; add regress test to
ensure it stays that way.

Updates golang#53477.

Change-Id: Ib7aa7428260595077052207899edcc044a6ab1c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/413394
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@cuonglm @jproberts
cmd/compile: fix generic inter-inter comparisons from value switch st…
bf64207 
…atements

If value is a non-empty interface and has shape, we still need to
convert it to an interface{} first.

Fixes golang#53477

Change-Id: I516063ba4429a6cc24c483758387ec13815fc63e
Reviewed-on: https://go-review.googlesource.com/c/go/+/414834
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@mdempsky @jproberts
test: add regress test for golang#53477
baf6570 
This test already passes for GOEXPERIMENT=unified; add regress test to
ensure it stays that way.

Updates golang#53477.

Change-Id: Ib7aa7428260595077052207899edcc044a6ab1c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/413394
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@cuonglm @jproberts
cmd/compile: fix generic inter-inter comparisons from value switch st…
a3e21a6 
…atements

If value is a non-empty interface and has shape, we still need to
convert it to an interface{} first.

Fixes golang#53477

Change-Id: I516063ba4429a6cc24c483758387ec13815fc63e
Reviewed-on: https://go-review.googlesource.com/c/go/+/414834
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@golang golang locked and limited conversation to collaborators Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@mdempsky @dr2chase @ianlancetaylor @gopherbot