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: ICE at composite literal assignment with alignment > PtrSize [1.19 backport] #54697
Labels
CherryPickApproved
Used during the release process for point releases
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
Milestone
Comments
gopherbot
added
the
CherryPickCandidate
Used during the release process for point releases
label
Aug 26, 2022
gopherbot
added
the
compiler/runtime
Issues related to the Go compiler and/or runtime.
label
Aug 26, 2022
Change https://go.dev/cl/425935 mentions this issue: |
heschi
added
the
CherryPickApproved
Used during the release process for point releases
label
Aug 31, 2022
gopherbot
removed
the
CherryPickCandidate
Used during the release process for point releases
label
Aug 31, 2022
Closed by merging 86e9e0e to release-branch.go1.19. |
gopherbot
pushed a commit
that referenced
this issue
Aug 31, 2022
…arger than PtrSize In typebits.Set we check that the offset is a multiple of the alignment, which makes perfect sense. But for values like atomic.Int64, which has 8-byte alignment even on 32-bit platforms (i.e. the alignment is larger than PtrSize), if it is on stack it may be under-aligned, as the stack frame is only PtrSize aligned. Normally we would prevent such values on stack, as the escape analysis force values with higher alignment to heap. But for a composite literal assignment like x = AlignedType{...}, the compiler creates an autotmp for the RHS then copies it to the LHS. The autotmp is on stack and may be under-aligned. Currently this may cause an ICE in the typebits.Set check. This CL makes it align the _offset_ of the autotmp to 8 bytes, which satisfies the check. Note that this is actually lying: the actual address at run time may not necessarily be 8-byte aligned as we only align SP to 4 bytes. The under-alignment is probably okay. The only purpose for the autotmp is to copy the value to the LHS, and the copying code we generate (at least currently) doesn't care the alignment beyond stack alignment. Updates #54638. Fixes #54697. Change-Id: I13c16afde2eea017479ff11dfc24092bcb8aba6a Reviewed-on: https://go-review.googlesource.com/c/go/+/425256 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 1211a62) Reviewed-on: https://go-review.googlesource.com/c/go/+/425935
bradfitz
pushed a commit
to tailscale/go
that referenced
this issue
Sep 8, 2022
…arger than PtrSize In typebits.Set we check that the offset is a multiple of the alignment, which makes perfect sense. But for values like atomic.Int64, which has 8-byte alignment even on 32-bit platforms (i.e. the alignment is larger than PtrSize), if it is on stack it may be under-aligned, as the stack frame is only PtrSize aligned. Normally we would prevent such values on stack, as the escape analysis force values with higher alignment to heap. But for a composite literal assignment like x = AlignedType{...}, the compiler creates an autotmp for the RHS then copies it to the LHS. The autotmp is on stack and may be under-aligned. Currently this may cause an ICE in the typebits.Set check. This CL makes it align the _offset_ of the autotmp to 8 bytes, which satisfies the check. Note that this is actually lying: the actual address at run time may not necessarily be 8-byte aligned as we only align SP to 4 bytes. The under-alignment is probably okay. The only purpose for the autotmp is to copy the value to the LHS, and the copying code we generate (at least currently) doesn't care the alignment beyond stack alignment. Updates golang#54638. Fixes golang#54697. Change-Id: I13c16afde2eea017479ff11dfc24092bcb8aba6a Reviewed-on: https://go-review.googlesource.com/c/go/+/425256 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 1211a62) Reviewed-on: https://go-review.googlesource.com/c/go/+/425935
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
CherryPickApproved
Used during the release process for point releases
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
@cherrymui requested issue #54638 to be considered for backport to the next 1.19 minor release.
The text was updated successfully, but these errors were encountered: