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: internal compiler error: out of range for go.shape.int64 #60601

Closed
rokkerruslan opened this issue Jun 5, 2023 · 14 comments
Closed
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rokkerruslan
Copy link

internal compiler error: 9223372036854775808 out of range for go.shape.int64

Please file a bug report including a short program that triggers the error.
https://go.dev/issue/new

Short program - https://go.dev/play/p/moV9U70P1pd

I am not attaching any information about the environment because it is reproduced on play.golang.org.

Yes, code is not ok, but compiler asks for a report.

@cuonglm
Copy link
Member

cuonglm commented Jun 5, 2023

Simpler reproducer:

package p

import (
	"unsafe"
)

func f[T int64](a T) T {
	return 1 << ((unsafe.Sizeof(a) * 8) - 1)
}

func g() {
	f(int64(1))
}

This seems to be a bug in Unified IR:

IIUC, Unified IR does constant folding for 1 << ((unsafe.Sizeof(a) * 8) - 1), which shoudn't be, because a is a type parameter, so the result of unsafe.Sizeof is not a Go constant:

The return value of Sizeof is a Go constant if the type of the argument x does not have variable size. (A type has variable size if it is a type parameter or if it is an array or struct type with elements of variable size).

In https://go-review.googlesource.com/c/go/+/469595, typecheck.EvalConst is removed, so the expression is not constant evaluated by the compiler anymore.

cc @mdempsky @griesemer

@cuonglm cuonglm added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 5, 2023
@cuonglm cuonglm added this to the Go1.21 milestone Jun 5, 2023
@griesemer
Copy link
Contributor

Since 1.21 seems ok, is this a backport issue at this point?

@cuonglm
Copy link
Member

cuonglm commented Jun 7, 2023

Since 1.21 seems ok, is this a backport issue at this point?

I'm afraid not.

Removing typecheck.EvalConst is the final CL in a chain, which the main purpose is simplifying the code base, but fixed this bug un-intentionally.

Backport the CL is too risky.

Users who face this issue can use GOEXPERIMENT=nounified to workaround this issue, so I'm leaning to not backport this.

@mdempsky
Copy link
Contributor

mdempsky commented Jun 7, 2023

I think we can probably just suppress the overflow errors in typecheck for 1.20. types2 will have already errored about any spec-required overflows.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/501735 mentions this issue: cmd/compile: supress overflow error in ir.IntVal

@cuonglm
Copy link
Member

cuonglm commented Jun 8, 2023

@gopherbot please backport this issue to go1.20, it's a compiler crash for legal user code.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #60675 (for 1.20).

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

@cuonglm
Copy link
Member

cuonglm commented Jun 8, 2023

Requested freeze exception for this.

cc @golang/release

@cuonglm cuonglm changed the title internal compiler error: out of range for go.shape.int64 internal compiler error: out of range for go.shape.int64 [freeze exception] Jun 8, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Jun 8, 2023

@cuonglm Thanks for letting us know, but I don't see what aspect of this needs a freeze exception and why. This seems to be a recently reported bug.

Please see https://golang.org/s/release#freeze-exceptions (we've made clarifying edits there recently) and if you still think a freeze exception is needed, please file it as a new issue with a detailed rationale. Thanks.

@dmitshur dmitshur changed the title internal compiler error: out of range for go.shape.int64 [freeze exception] cmd/compile: internal compiler error: out of range for go.shape.int64 Jun 8, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 8, 2023
@mdempsky
Copy link
Contributor

mdempsky commented Jun 8, 2023

I don't think this needs a freeze exception for Go 1.21.

It's only an issue with Go 1.20. We can commit a fix directly to release-branch.go1.20.

I think adding the regress test case to master for 1.21 is fine though.

@cuonglm
Copy link
Member

cuonglm commented Jun 9, 2023

@dmitshur @mdempsky Ah, thanks for the clarification.

I think the change to ir.IntVal needs to be land to master then backport, so that would require freeze exception. If we can commit change directly to 1.20 branch, then it does not.

@cuonglm
Copy link
Member

cuonglm commented Jun 9, 2023

I found a reproducer for 1.21, too:

package p

import (
	"unsafe"
)

func shift[T any]() int64 {
	return 1 << unsafe.Sizeof(*new(T))
}

func div[T any]() uintptr {
	return 1 / unsafe.Sizeof(*new(T))
}

func add[T any]() int64 {
	return 1<<63 - 1 + int64(unsafe.Sizeof(*new(T)))
}

func f() {
	shift[[62]byte]()
	shift[[63]byte]()
	shift[[64]byte]()
	shift[[100]byte]()
	shift[[1e6]byte]()

	div[[0]byte]()
	add[[1]byte]()
}

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 9, 2023
@cuonglm cuonglm self-assigned this Jun 9, 2023
@cuonglm cuonglm added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 9, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Jun 9, 2023

Hey all, I'm doing a sweep of the Go 1.21 milestone compiler/runtime issues. Sounds like this is both a Go 1.21 and Go 1.20 issue. Do y'all still plan to work on it for Go 1.21? Thanks.

@cuonglm
Copy link
Member

cuonglm commented Jun 10, 2023

Hey all, I'm doing a sweep of the Go 1.21 milestone compiler/runtime issues. Sounds like this is both a Go 1.21 and Go 1.20 issue. Do y'all still plan to work on it for Go 1.21? Thanks.

Hey @mknyszek , yes, https://go.dev/cl/501735 will fix this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants