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: ICE due to bad ORL constant [1.15] #42753

Open
randall77 opened this issue Nov 20, 2020 · 7 comments
Open

cmd/compile: ICE due to bad ORL constant [1.15] #42753

randall77 opened this issue Nov 20, 2020 · 7 comments
Assignees
Milestone

Comments

@randall77
Copy link
Contributor

@randall77 randall77 commented Nov 20, 2020

package main

func f() uint32 {
	s := "\x01"
	x := -int32(s[0])
	return uint32(x) & 0x7fffffff
}

Compile with

go tool compile -d=ssa/check/on test.go

Generates

test.go:6:21: internal compiler error: 'f': bad int32 AuxInt value for v20

Ok at tip. Breaks for 1.15 and 1.14.

@randall77 randall77 added this to the Go1.15.6 milestone Nov 20, 2020
@randall77 randall77 self-assigned this Nov 20, 2020
@randall77
Copy link
Contributor Author

@randall77 randall77 commented Nov 20, 2020

@gopherbot please open backport issue for 1.14.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 20, 2020

Backport issue(s) opened: #42755 (for 1.14).

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

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 20, 2020

Change https://golang.org/cl/272028 mentions this issue: [release-branch.go1.15] cmd/compile: sign extend consant folding properly

@dmitshur dmitshur changed the title cmd/compile: ICE due to bad ORL constant cmd/compile: ICE due to bad ORL constant [1.15] Nov 20, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 20, 2020

Change https://golang.org/cl/272029 mentions this issue: cmd/compile: add test for 42753

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Nov 20, 2020

It seems this isn't a recent regression, it happens even with Go 1.5.4:

$ go1.5.4 build p.go
# command-line-arguments
fatal error: still in list
[...]
main.main()
	/usr/local/go/src/cmd/link/main.go:26 +0x189
$ echo $?
1

Our preference in https://golang.org/wiki/MinorReleases is not to backport, unless it's serious problem with no workaround. Given it's been around for this long, maybe it's not worth taking the risk of doing a backport, unless the fix is very safe. @randall77 Do you think there are reasons that justify doing a backport rather than letting this be a part of the upcoming Go 1.16 release (which will go through additional testing)?

@randall77
Copy link
Contributor Author

@randall77 randall77 commented Nov 20, 2020

The example isn't a full program, so it won't reproduce with go build. Not sure what that error you're getting is, but it isn't this one.

It looks like this bug started in 1.12 (probably the introduction of bit test instructions). 1.11 and earlier are ok.

This is similar to #41711 which had a backport approved.

As far as seriousness, I think it is an open question. It is an ICE, but a pretty rare one. We only came across it in generated code to test the compiler. It is conceivable that users will hit it, but we have no evidence for how likely that is.

It should never cause the compiler to generate incorrect code.

I don't think there's any easy workaround. You'd have to un-constant some expressions (by putting a constant in a global variable, say) to prevent the compiler optimizations involved. The error doesn't point to an obvious place to do that.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Nov 20, 2020

Ah, indeed, go1.5.4 was too old and fails even on simple Go programs for unrelated reasons (not to mention its compiler doesn't support ssa/check/on). Thanks for accurately determining that this regressed in Go 1.12, and providing additional information.

Approving as it is a serious issue without a good workaround. It applies to both 1.15 (this issue) and 1.14 (#42755).

gopherbot pushed a commit that referenced this issue Nov 21, 2020
This issue was already fixed at tip. Just adding the test that
failed on 1.14/1.15.

Update #42753

Change-Id: I00d13ade476b9c17190d762d7fdcb30cf6c83954
Reviewed-on: https://go-review.googlesource.com/c/go/+/272029
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.