Skip to content

cmd/compile: Pow10 freeze the compiler on certain condition on Go 1.24 #71852

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

Closed
etrnal70 opened this issue Feb 20, 2025 · 12 comments
Closed

cmd/compile: Pow10 freeze the compiler on certain condition on Go 1.24 #71852

etrnal70 opened this issue Feb 20, 2025 · 12 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. Critical A critical problem that affects the availability or correctness of production systems built using Go FixPending Issues that have a fix which has not yet been reviewed or submitted. Implementation Issues describing a semantics-preserving change to the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@etrnal70
Copy link

etrnal70 commented Feb 20, 2025

Go version

go version go1.24.0 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/hanifrmn/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/hanifrmn/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/xd/37pvgts12bg_bk2wx9kp9gbm0000gp/T/go-build2186527099=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/hanifrmn/Repo/playground/go.mod'
GOMODCACHE='/Users/hanifrmn/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/hanifrmn/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/hanifrmn/sdk/go1.24.0'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/hanifrmn/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/hanifrmn/sdk/go1.24.0/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

package main

import (
	"math"
)

func main() {
	test(2)
}

func test(i int) {
	if i <= 0 {
		return
	}

	_ = math.Pow10(i + 2)
}

running go build will freeze the compiler

What did you see happen?

Running go build will freeze the compiler

Running with -gcflags=-l rans fine

What did you expect to see?

It compiles correctly on Go 1.23.6

@etrnal70 etrnal70 changed the title math: Pow10 freeze the compiler on certain condition math: Pow10 freeze the compiler on certain condition on Go 1.24 Feb 20, 2025
@Jorropo Jorropo added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. Implementation Issues describing a semantics-preserving change to the Go implementation. labels Feb 20, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 20, 2025
@Jorropo

This comment has been minimized.

@etrnal70

This comment has been minimized.

@etrnal70

This comment has been minimized.

@Jorropo
Copy link
Member

Jorropo commented Feb 20, 2025

Well I am very confused:

git bisect start
# status: waiting for both good and bad commits
# bad: [4700bc41750c1a80c21b00baa0a93a64eb79e7ac] test: trying to reproduce 71852
git bisect bad 4700bc41750c1a80c21b00baa0a93a64eb79e7ac
# status: waiting for good commit(s), bad commit known
# good: [a991f9c34d454d3d844f21dc08f2d05df35a8c60] [release-branch.go1.23] go1.23.6
git bisect good a991f9c34d454d3d844f21dc08f2d05df35a8c60
# good: [5e8a7316658c2f300a375041b6e0a606fec4c5f2] README: fix CC BY license name
git bisect good 5e8a7316658c2f300a375041b6e0a606fec4c5f2
# bad: [2fd2718f6b64000fa25e0c5d1ee48aa1426d5a6f] bufio: add example for ReadFrom and remove unused code
git bisect bad 2fd2718f6b64000fa25e0c5d1ee48aa1426d5a6f
# bad: [123594d3863b0a4b9094a569957d1bd94ebe7512] runtime: remove cloudwego/frugal unused linkname from comment
git bisect bad 123594d3863b0a4b9094a569957d1bd94ebe7512
# good: [206df8e7ad8a736b0dcbf5d98ab237b6784e5007] net/http: rename server receiver for consistency
git bisect good 206df8e7ad8a736b0dcbf5d98ab237b6784e5007
# good: [e126129d7612349874828685c2bcd49de498a1a0] cmd/compile/internal/ssa: combine shift and addition for riscv64 rva22u64
git bisect good e126129d7612349874828685c2bcd49de498a1a0
# bad: [a00195d304e6858406c6c9c961d253eeb8cb0aec] all: use t.Chdir in tests
git bisect bad a00195d304e6858406c6c9c961d253eeb8cb0aec
# good: [9b6efc25cddad4bebfa64fd8543afd07fbb0ab3e] go/ast: remove unused code
git bisect good 9b6efc25cddad4bebfa64fd8543afd07fbb0ab3e
# good: [c3f346a485f2fa97a7bdee82d587419b3823a1ba] math,os,os/*: use testenv.Executable
git bisect good c3f346a485f2fa97a7bdee82d587419b3823a1ba
# good: [d91a2e5b11cc1badd7c760a911218a64d91ac8b0] cmd: replace many sort.Interface with slices.Sort and SortFunc
git bisect good d91a2e5b11cc1badd7c760a911218a64d91ac8b0
# bad: [820f58a27f7f64f21353db6a071bf5dbf658924c] cmd/compile: compute Negation's limits from argument's limits
git bisect bad 820f58a27f7f64f21353db6a071bf5dbf658924c
# good: [4f2c0e5d0806a2f6fbe2d0704683d2b71d8191be] cmd/compile: compute Trunc's limits from argument's limits
git bisect good 4f2c0e5d0806a2f6fbe2d0704683d2b71d8191be
# good: [2f3165973f9bf1e1a95d108c57e5257f4655ee59] cmd/compile: compute Complement's limits from argument's limits
git bisect good 2f3165973f9bf1e1a95d108c57e5257f4655ee59
# first bad commit: [820f58a27f7f64f21353db6a071bf5dbf658924c] cmd/compile: compute Negation's limits from argument's limits

820f58a is very simple, copying patterns and code from other patches.

I don't know when I'll be able to spend time investigating this,
I'll revert that for 1.24.1 but I think there is a latent bug hiding somewhere.

@Jorropo Jorropo added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Feb 20, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/650975 mentions this issue: Revert "cmd/compile: compute Negation's limits from argument's limits"

@Jorropo
Copy link
Member

Jorropo commented Feb 20, 2025

cc @golang/compiler

@prattmic prattmic added the Critical A critical problem that affects the availability or correctness of production systems built using Go label Feb 20, 2025
@Jorropo Jorropo modified the milestone: Go1.24.1 Feb 20, 2025
@Jorropo
Copy link
Member

Jorropo commented Feb 20, 2025

@gopherbot Please open backport issue for 1.24

This is a regression where non esoteric code that used to build now does not.
https://gophers.slack.com/archives/C6CM8NUEB/p1740060033459149

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #71855 (for 1.24).

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

@Jorropo
Copy link
Member

Jorropo commented Feb 20, 2025

Thanks to @etrnal70's reproducer I was able to preliminarily narrow down the issue to a bug in newLimit.
There is a loop permanently doing this operation in the flowLimit loop:

flowLimit: v51 = Rsh64Ux64 <uint> [false] v49 v46: sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 -> sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 false
flowLimit: v49 = Neg64 <int> v12: sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 -> sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 true
flowLimit: v51 = Rsh64Ux64 <uint> [false] v49 v46: sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 -> sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 false
flowLimit: v49 = Neg64 <int> v12: sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 -> sm,SM,um,UM=-9223372036854775808,9223372036854775807,0,18446744073709551615 true

Altho you might note that the limits do not change YET newLimit returns true which cause the loop to never break since it thinks the values are being forever narrowed down when they are not.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/650580 mentions this issue: cmd/compile: stop flowLimit search when we proved unsat

@Jorropo
Copy link
Member

Jorropo commented Feb 20, 2025

I've submitted two different proper fixes:

My original plan to revert 820f58a would not solve all regression cases, just this particular one.
I think the only correct revert would involve reverting most of @randall77's prove constants handling improvements, so I think we should consider backporting the fix altho this is more risky.

@Jorropo Jorropo changed the title math: Pow10 freeze the compiler on certain condition on Go 1.24 cmd/compile: Pow10 freeze the compiler on certain condition on Go 1.24 Feb 20, 2025
@Jorropo Jorropo removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 20, 2025
@dmitshur dmitshur added this to the Go1.25 milestone Feb 20, 2025
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. Critical A critical problem that affects the availability or correctness of production systems built using Go FixPending Issues that have a fix which has not yet been reviewed or submitted. Implementation Issues describing a semantics-preserving change to the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants