-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: out-of-bounds panic with uint32 conversion and modulus operation in Go 1.22.0 on arm64 [1.22 backport] #66076
Comments
Change https://go.dev/cl/573375 mentions this issue: |
Backport rationale: this bug causes a silent miscompilation of certain casts from smaller to larger integer types. |
Change https://go.dev/cl/573395 mentions this issue: |
Change https://go.dev/cl/573395 mentions this issue: |
Approved; serious problem with no workaround. |
When an opcode generates a known high bit state (typically, a sub-word operation that zeros the high bits), we can remove any subsequent extension operation that would be a no-op. x = (OP ...) y = (ZeroExt32to64 x) If OP zeros the high 32 bits, then we can replace y with x, as the zero extension doesn't do anything. However, x in this situation normally has a sub-word-sized type. The semantics of values in registers is typically that the high bits beyond the value's type size are junk. So although the opcode generating x *currently* zeros the high bits, after x is rewritten to another opcode it may not - rewrites of sub-word-sized values can trash the high bits. To fix, move the extension-removing rules to late lower. That ensures that their arguments won't be rewritten to change their high bits. I am also worried about spilling and restoring. Spilling and restoring doesn't preserve the high bits, but instead sets them to a known value (often 0, but in some cases it could be sign-extended). I am unable to come up with a case that would cause a problem here, so leaving for another time. Update #66076 Change-Id: I3b5c091b3b3278ccbb7f11beda8b56f4b6d3fde7 Reviewed-on: https://go-review.googlesource.com/c/go/+/568616 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit a46ecdc) Reviewed-on: https://go-review.googlesource.com/c/go/+/573375
…o-extended On amd64, we always zero-extend when loading arguments from the stack. On arm64, we extend based on the type. This causes problems with zeroUpper*Bits, which reports the top bits are zero when they aren't. Fix it to use the type to decide if the top bits are really zero. For tests, only f32 currently fails on arm64. Added other tests just for future-proofing. Fixes #66076 Change-Id: I2f13fb47198e139ef13c9a34eb1edc932eea3ee3 Reviewed-on: https://go-review.googlesource.com/c/go/+/571135 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit 6bf8b76) Reviewed-on: https://go-review.googlesource.com/c/go/+/573395 Reviewed-by: Cherry Mui <cherryyz@google.com>
Closed by merging 4658748 to release-branch.go1.22. |
Closed by merging 2c6d106 to release-branch.go1.22. |
@randall77 requested issue #66066 to be considered for backport to the next 1.22 minor release.
The text was updated successfully, but these errors were encountered: