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: bad walkinrange rewrites on constant above 2**63 [1.10 backport] #27247

Closed
gopherbot opened this issue Aug 26, 2018 · 3 comments
Closed
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@josharian requested issue #27143 to be considered for backport to the next 1.10 minor release.

Certainly seems to qualify for a backport.

Yeah. My hesitation was only that this appears to have existed for many releases, and the symptom is a spurious compiler error (not bad code generation). Nevertheless, I agree.

@gopherbot please open backport tracking issues.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 26, 2018
@gopherbot gopherbot added this to the Go1.10.5 milestone Aug 26, 2018
@josharian
Copy link
Contributor

@ALTree do you want to do the backport CL honors for this and #27246, since you wrote the fix? See https://golang.org/wiki/MinorReleases. Thanks!

@gopherbot
Copy link
Author

Change https://golang.org/cl/131595 mentions this issue: [release-branch.go1.10] cmd/compile: prevent overflow in walkinrange

@dmitshur dmitshur added the CherryPickApproved Used during the release process for point releases label Nov 1, 2018
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Nov 1, 2018
@gopherbot
Copy link
Author

Closed by merging 30ccc62 to release-branch.go1.10.

gopherbot pushed a commit that referenced this issue Nov 1, 2018
In the compiler frontend, walkinrange indiscriminately calls Int64()
on const CTINT nodes, even though Int64's return value is undefined
for anything over 2⁶³ (in practise, it'll return a negative number).

This causes the introduction of bad constants during rewrites of
unsigned expressions, which make the compiler reject valid Go
programs.

This change introduces a preliminary check that Int64() is safe to
call on the consts on hand. If it isn't, walkinrange exits without
doing any rewrite.

Fixes #27247

Change-Id: I2017073cae65468a521ff3262d4ea8ab0d7098d9
Reviewed-on: https://go-review.googlesource.com/130735
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 42cc4ca)
Reviewed-on: https://go-review.googlesource.com/c/131595
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Nov 1, 2019
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 FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

3 participants