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.

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

cmd/compile: internal compiler error: 'TestEnv': value generic.t1 (v164) incorrectly live at entry #66261

Closed
CarstenLeue opened this issue Mar 12, 2024 · 11 comments
Closed

cmd/compile: internal compiler error: 'TestEnv': value generic.t1 (v164) incorrectly live at entry #66261

CarstenLeue opened this issue Mar 12, 2024 · 11 comments
Assignees
@mdempsky
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.23

Comments

@CarstenLeue
Copy link

Go version

go version devel go1.23-a18aa0e3d1 Mon Mar 11 19:54:31 2024 +0000 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\CarstenLeue\AppData\Local\go-build
set GOENV=C:\Users\CarstenLeue\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\CarstenLeue\go\pkg\mod
set GONOPROXY=github.ibm.com
set GONOSUMDB=github.ibm.com
set GOOS=windows
set GOPATH=C:\Users\CarstenLeue\go
set GOPRIVATE=github.ibm.com
set GOPROXY=https://eu.artifactory.swg-devops.com/artifactory/api/go/sys-zaas-team-dev-go-virtual/
set GOROOT=C:\Users\CarstenLeue\sdk\gotip
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Users\CarstenLeue\sdk\gotip\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=devel go1.23-a18aa0e3d1 Mon Mar 11 19:54:31 2024 +0000
set GODEBUG=
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=C:\d\fp-go\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=c:\temp\Local\Temp\go-build3450821636=/tmp/go-build -gno-record-gcc-switches

What did you do?

I ran the testcases for https://github.com/IBM/fp-go using gotip

gotip test -run ^TestEnv$ .\...

What did you see happen?

The test bails out with an internal compiler error:

# github.com/IBM/fp-go/iooption [github.com/IBM/fp-go/iooption.test]
iooption/iooption_test.go:68:6: internal compiler error: 'TestEnv': value generic.t1 (v164) incorrectly live at entry

goroutine 66 [running]:
runtime/debug.Stack()
        C:/Users/CarstenLeue/sdk/gotip/src/runtime/debug/stack.go:26 +0x5e
cmd/compile/internal/base.FatalfAt({0x423730?, 0xc0?}, {0xc00051e180, 0x2d}, {0xc000507050, 0x3, 0x3})
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/base/print.go:225 +0x1d7
cmd/compile/internal/base.Fatalf(...)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/base/print.go:194
cmd/compile/internal/ssagen.(*ssafn).Fatalf(0xc000526918?, {0x26ea160?, 0xc0?}, {0xef98f0, 0x27}, {0xc00053a260, 0x2, 0xc000507001?})
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/ssa.go:8196 +0x119
cmd/compile/internal/ssagen.(*state).Fatalf(0xc000423828?, {0xef98f0?, 0xc001d94cc0?}, {0xc00053a260?, 0xc0026ea160?, 0xc00050a250?})
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/ssa.go:939 +0x64
cmd/compile/internal/ssagen.(*simplePhiState).insertPhis(...)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/phi.go:486
cmd/compile/internal/ssagen.(*state).insertPhis(0xc00011e600?)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/phi.go:45 +0xdc
cmd/compile/internal/ssagen.buildssa(0xc001d9a900, 0x0)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/ssa.go:571 +0x25ec
cmd/compile/internal/ssagen.Compile(0xc001d9a900, 0x0)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/ssagen/pgen.go:216 +0x3e
cmd/compile/internal/gc.compileFunctions.func5.1(0x0?)
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/gc/compile.go:182 +0x34
cmd/compile/internal/gc.compileFunctions.func3.1()
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/gc/compile.go:164 +0x30
created by cmd/compile/internal/gc.compileFunctions.func3 in goroutine 34
        C:/Users/CarstenLeue/sdk/gotip/src/cmd/compile/internal/gc/compile.go:163 +0x247

What did you expect to see?

The same test runs fine with go version go1.22.1 windows/amd64 and go1.20.11

I expect the test also to success for the latest gotip version.
@CarstenLeue CarstenLeue mentioned this issue Mar 12, 2024
Closed
@ALTree ALTree changed the title gotip: cmd/compile/internal/ssagen/ssa.go:8196 internal compiler error: 'TestEnv': value generic.t1 (v164) incorrectly live at entry cmd/compile: internal compiler error: 'TestEnv': value generic.t1 (v164) incorrectly live at entry Mar 12, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 12, 2024
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 12, 2024
@ALTree ALTree added this to the Go1.23 milestone Mar 12, 2024
@randall77
Copy link
Contributor

git-bisect points to https://go-review.googlesource.com/c/go/+/567695
@mdempsky @thanm

@randall77
Copy link
Contributor

A simpler repro is just to clone the fp-go repro and do go test -c ./iooption.

@mdempsky mdempsky self-assigned this Mar 12, 2024
@mdempsky
Copy link
Member

mdempsky commented Mar 13, 2024 •
edited

Standalone repro: https://go.dev/play/p/sdcanBuV_ZT?v=gotip

Further minimized: https://go.dev/play/p/B2MzsIhXvMb?v=gotip

Without generics: https://go.dev/play/p/w6X6Sxtz9Gd?v=gotip

@mdempsky
Copy link
Member

Minimized reproducer:

package main

func main() {
	env := func() func(string) func() int {
		return func() func(string) func() int {
			return func(t3 string) func() int {
				return func() (_ int) {
					println(t3)
					return
				}
			}
		}()
	}()

	func(int) {}(env("XXX")())
}

The issue here is the compiler determines that env("XXX")() is a call to the innermost function literal, so it gets inlined as println(t3). But it's missing the t3 = "XXX" initialization. So the assertion is failing correctly.

We have logic that's meant to prevent us from trying to inline across frames:

go/src/cmd/compile/internal/inline/inl.go

Lines 828 to 829 in 508bb17

if len(c.ClosureVars) != 0 && c.ClosureVars[0].Outer.Curfn != caller {
return nil // inliner doesn't support inlining across closure frames

We also double-check that later during inlining:

go/src/cmd/compile/internal/noder/reader.go

Lines 3415 to 3416 in 508bb17

if cv.Outer.Curfn != callerfn {
base.FatalfAt(call.Pos(), "inlining closure call across frames")

I'm puzzled how CL 567695 causes the test case to now slip past these checks. I'm going to revert it while I investigate further.

@gopherbot
Copy link

Change https://go.dev/cl/571555 mentions this issue: Revert "cmd/compile/internal/inline: refactor fixpoint algorithm"

gopherbot pushed a commit that referenced this issue Mar 14, 2024
@mdempsky @gopherbot
Revert "cmd/compile/internal/inline: refactor fixpoint algorithm"
9159d71 
This reverts commit 28e0052.

Reason for revert: #66261

Change-Id: I9dfc8946c41e504c97ecad752971d760ae7a7416
Reviewed-on: https://go-review.googlesource.com/c/go/+/571555
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
@thanm
Copy link
Contributor

thanm commented Mar 14, 2024

Can we close this bug out, now that https://go.dev/cl/571555 has been submitted?

@mdempsky
Copy link
Member

Can we close this bug out, now that https://go.dev/cl/571555 has been submitted?

I'm still diagnosing the failure to roll forward again. I'd like to keep the issue open at least until we add a regress test to make sure it doesn't come up again in the future.

If I can't figure out the problem today, I'll send a CL to just add a regress test case so we can close it.

@thanm
Copy link
Contributor

thanm commented Mar 14, 2024

Right, makes sense. Thanks.

@gopherbot
Copy link

Change https://go.dev/cl/571815 mentions this issue: test/fixedbugs: add regress test for inlining failure

gopherbot pushed a commit that referenced this issue Mar 14, 2024
@mdempsky @gopherbot
test/fixedbugs: add regress test for inlining failure
412623c 
Still investigating, but adding the minimized reproducer as a regress
test case for now.

Updates #66261.

Change-Id: I20715b731f8c5b95616513d4a13e3ae083709031
Reviewed-on: https://go-review.googlesource.com/c/go/+/571815
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@CarstenLeue
Copy link
Author

I tested with go version devel go1.23-0a6f05e30f Sun Mar 17 15:57:54 2024 +0000 windows/amd64 and it's working as expected. Thank you!

@dmitshur dmitshur 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 Mar 18, 2024
@gopherbot
Copy link

Change https://go.dev/cl/573095 mentions this issue: cmd/compile/internal/typecheck: more selective OPAREN skipping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdempsky mdempsky

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.23
Development

No branches or pull requests
7 participants
@mdempsky @dmitshur @ALTree @randall77 @CarstenLeue @gopherbot @thanm