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: incorrect handling of iota in 1.18 #52438

Closed
willeccles opened this issue Apr 19, 2022 · 7 comments
Closed

cmd/compile: incorrect handling of iota in 1.18 #52438

willeccles opened this issue Apr 19, 2022 · 7 comments
Labels
NeedsInvestigation release-blocker
Milestone

Comments

@willeccles
Copy link

@willeccles willeccles commented Apr 19, 2022

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

$ go version
go version go1.18.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

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

What did you do?

Using golua, 1.18.1 causes panics which did not occur on previous versions of go. I have narrowed down the issue to this source file which functions differently now. Running this program on 1.18.1 yields different results than 1.17.9, but according to the language spec, 1.18.1 is exhibiting incorrect behavior.

What did you expect to see?

In the above program, OpAnd should have a value of 1 (1 + iota<<8 should be 1 + (0)<<8).

What did you see instead?

On go 1.17.9, the value is indeed 1. However, on go 1.18.1, the value is 257, meaning that iota here is incorrectly being set to 1 instead of 0. The language spec says that each constant declaration should reset iota to 0, which is not happening on 1.18.1.

@willeccles
Copy link
Author

@willeccles willeccles commented Apr 19, 2022

Original discussion of this issue can be found here: arnodel/golua#87

@ianlancetaylor ianlancetaylor changed the title iota: does not reset to 0 on constant declarations cmd/compile: incorrect handling of iota in 1.18 Apr 19, 2022
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 19, 2022

Here is a standalone program showing the problem. This program should not panic. It does not panic when compiled with Go 1.17 or earlier, or with gccgo. It does panic with Go 1.18.

package main

const C1 = 0 + iota<<8

const C2 = 1 + iota<<8

func main() {
	if C1 != 0 {
		panic(C1)
	}
	if C2 != 1 {
		panic(C2)
	}
}

CC @griesemer @mdempsky

@ianlancetaylor ianlancetaylor added NeedsInvestigation release-blocker labels Apr 19, 2022
@ianlancetaylor ianlancetaylor added this to the Go1.19 milestone Apr 19, 2022
@griesemer griesemer self-assigned this Apr 19, 2022
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 19, 2022

@gopherbot Please open a backport to 1.18.

This is an invalid compilation of a valid Go file. Earlier releases are unaffected.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 19, 2022

Backport issue(s) opened: #52441 (for 1.18).

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

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Apr 19, 2022

FWIW I tried to bisect this, and it blamed the commit where -G=3 was enabled by default; re-bisecting between 1.17 and 1.18 with -G=3 fails immediately because the issue appears to predate those releases. Going back further gets into the weeds since it's probably not something that was introduced via some specific commit thanks to the WIP-ness of the compiler at that point.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Apr 19, 2022

No need to bisect. This is due to a completely new type checker, so it won't be useful to pin-point to a single change. I've identified the bug and will have a fix in a little bit.

This is clearly a bad bug; but only manifests itself when using iota outside a grouped constant declaration, twice.

As a temporary work-around, you can change your code to:

// OpOr is a logical or (precedence 0)
const (OpOr Op = 0 + iota<<8)

// OpAnd is a logical and (precedence 1)
const (OpAnd Op = 1 + iota<<8)

(put parentheses around the const declarations).

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 19, 2022

Change https://go.dev/cl/401134 mentions this issue: cmd/compile/internal/types2: use correct value of iota

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants