Skip to content

cmd/compile: constant overflows when assigned to package level var (Go 1.20 regression) #58293

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.

Sign up for GitHub

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

Jump to bottom
Closed
tklauser opened this issue Feb 3, 2023 · 7 comments
Closed

cmd/compile: constant overflows when assigned to package level var (Go 1.20 regression) #58293

tklauser opened this issue Feb 3, 2023 · 7 comments
Assignees
@cuonglm
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
Go1.21

Comments

@tklauser
Copy link
Member

tklauser commented Feb 3, 2023 •
edited
Loading

What version of Go are you using (go version)?

$ go version
go version go1.20 linux/amd64

Does this issue reproduce with the latest release?

Yes, however it does not reproduce with Go 1.19 or earlier. So this looks like a regression in Go 1.20.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tklauser/.cache/go-build"
GOENV="/home/tklauser/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/tklauser/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/tklauser/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/tklauser/src/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1006501225=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Some unit tests in the github.com/cilium/cilium/cilium/cmd package no longer built with a constant overflow error when being build with Go 1.20. They built fine and passed with Go 1.19 and earlier:

https://github.com/cilium/cilium/actions/runs/4083992015/jobs/7040172384

@ti-mo managed to come up with a minimal reproducer:

https://go.dev/play/p/ClBVgMEVVhG?v=gotip

What did you expect to see?

The reproducer (and the tests in package github.com/cilium/cilium/cmd) to build without errors using Go 1.20.

What did you see instead?

./prog.go:8:30: constant 3476277 overflows uint16
./prog.go:8:30: constant 3476224 overflows uint16

Go build failed.

The error only seems to occur if the result of bits.ReverseBytes16(13579) is assigned to a package level var. It builds fine if the result is assigned e.g. to a func level var.

Thanks to @ti-mo for helping me analyze this error and coming up with a minimal reproducer for playground.
@tklauser tklauser changed the title cmd/compile/internal/typecheck: constant overflows when assinged to package level var (Go 1.20 regression) cmd/compile/internal/typecheck: constant overflows when assigned to package level var (Go 1.20 regression) Feb 3, 2023
@tklauser tklauser added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 3, 2023
@tklauser tklauser mentioned this issue Feb 3, 2023
Closed
@tklauser tklauser changed the title cmd/compile/internal/typecheck: constant overflows when assigned to package level var (Go 1.20 regression) cmd/compile: constant overflows when assigned to package level var (Go 1.20 regression) Feb 3, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 3, 2023
@cuonglm
Copy link
Member

cuonglm commented Feb 3, 2023

The problem is with inline static init, you can pass -d=inlstaticinit=0 to workaround this issue.

@cuonglm cuonglm mentioned this issue Feb 4, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/465096 mentions this issue: cmd/compile: fix inline static init for constant expression

@cuonglm cuonglm self-assigned this Feb 4, 2023
@cuonglm cuonglm added this to the Go1.21 milestone Feb 4, 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 Feb 4, 2023
@tklauser
Copy link
Member Author

tklauser commented Feb 4, 2023

@gopherbot please consider this for backport to 1.20, it's a regression.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #58319 (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.

@gopherbot gopherbot mentioned this issue Feb 4, 2023
Closed
pchaigno added a commit to pchaigno/cilium that referenced this issue Feb 8, 2023
@pchaigno
Revert "chore(deps): update docker.io/library/golang docker tag to v1…
a6ea216 
….20.0"

This reverts commit 80de240.

The Golang v1.20.0 release includes a regression [1]. We should wait for
a new patch release before updating.

1 - golang/go#58293
Reported-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno mentioned this issue Feb 8, 2023
Merged
aanm pushed a commit to cilium/cilium that referenced this issue Feb 8, 2023
@pchaigno @aanm
Revert "chore(deps): update docker.io/library/golang docker tag to v1…
ee59a47 
….20.0"

This reverts commit 80de240.

The Golang v1.20.0 release includes a regression [1]. We should wait for
a new patch release before updating.

1 - golang/go#58293
Reported-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@mknyszek mknyszek moved this to In Progress in Go Compiler / Runtime Feb 8, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Feb 9, 2023
@cuonglm cuonglm mentioned this issue Feb 9, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/466277 mentions this issue: cmd/compile: rename EvalConst to EvalExpr

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/467015 mentions this issue: cmd/compile: disable inline static init optimization

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/467016 mentions this issue: cmd/compile: reenable inline static init

gopherbot pushed a commit that referenced this issue Feb 9, 2023
@cuonglm @gopherbot
cmd/compile: disable inline static init optimization
b7736cb 
There are a plenty of regression in 1.20 with this optimization. This CL
disable inline static init, so it's safer to backport to 1.20 branch.

The optimization will be enabled again during 1.21 cycle.

Updates #58293
Updates #58339
For #58293

Change-Id: If5916008597b46146b4dc7108c6b389d53f35e95
Reviewed-on: https://go-review.googlesource.com/c/go/+/467015
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
johanbrandhorst pushed a commit to Pryz/go that referenced this issue Feb 12, 2023
@cuonglm @johanbrandhorst
cmd/compile: remove constant arithmetic overflows during typecheck
7f33907 
Since go1.19, these errors are already reported by types2 for any user's
Go code. Compiler generated code, which looks like constant expression
should be evaluated as non-constant semantic, which allows overflows.

Fixes golang#58293

Change-Id: I6f0049a69bdb0a8d0d7a0db49c7badaa92598ea2
Reviewed-on: https://go-review.googlesource.com/c/go/+/465096
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
johanbrandhorst pushed a commit to Pryz/go that referenced this issue Feb 12, 2023
@cuonglm @johanbrandhorst
cmd/compile: disable inline static init optimization
bf480be 
There are a plenty of regression in 1.20 with this optimization. This CL
disable inline static init, so it's safer to backport to 1.20 branch.

The optimization will be enabled again during 1.21 cycle.

Updates golang#58293
Updates golang#58339
For golang#58293

Change-Id: If5916008597b46146b4dc7108c6b389d53f35e95
Reviewed-on: https://go-review.googlesource.com/c/go/+/467015
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopherbot pushed a commit that referenced this issue Apr 14, 2023
@cuonglm
cmd/compile: handle string concatenation in static init inliner
a141c58 
Static init inliner is using typecheck.EvalConst to handle string
concatenation expressions. But static init inliner may reveal constant
expressions after substitution, and the compiler needs to evaluate those
expressions in non-constant semantic. Using typecheck.EvalConst, which
always evaluates expressions in constant semantic, is not the right
choice.

For safety, this CL fold the logic to handle string concatenation to
static init inliner, so there won't be regression in handling constant
expressions in non-constant semantic. And also, future CL can simplify
typecheck.EvalConst logic.

Updates #58293
Updates #58339
Fixes #58439

Change-Id: I74068d99c245938e576afe9460cbd2b39677bbff
Reviewed-on: https://go-review.googlesource.com/c/go/+/466277
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
gopherbot pushed a commit that referenced this issue Apr 14, 2023
@cuonglm
cmd/compile: reenable inline static init
20c349e 
Updates #58293
Updates #58339
Fixes #58439

Change-Id: I06d2d92f86fa4a672d69515c4066d69d3e0fc75b
Reviewed-on: https://go-review.googlesource.com/c/go/+/467016
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@golang golang locked and limited conversation to collaborators Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@cuonglm cuonglm

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
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@tklauser @cuonglm @gopherbot and others