-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: internal compiler error while building targets for kubernetes with master go #44370
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
Comments
@laboger ^ ^ |
internal compiler error
while building targets for kubernetes with master go
The relevant line which causes problem https://github.com/kubernetes/kubernetes/blob/960e5e78255dd148d4dae49f62e729ea940f4f07/test/e2e/framework/rc/rc_utils.go#L55 Seems another instance of inlining OCLOSURE. |
I found a failure similar. go version
go env
|
I think it is very likely this is due to the recent change to trampolines in https://go-review.googlesource.com/c/go/+/292490, since Kubernetes is large enough to require trampolines for long distance calls. |
@pmur FYI |
FYI: we have a CI job building Kubernetes using Golang tip version running every 4 hours: https://testgrid.k8s.io/sig-scalability-golang#build-and-push-k8s-at-golang-tip. These builds are later used to run load tests to detect performance regressions in Kubernetes coming from Golang. Last successful build job was using this CL as the tip: https://go-review.googlesource.com/c/go/+/275297, so something that was introduced later than the aforementioned "trampolines" change. |
Quick repro with k8s (no make required): git clone https://github.com/kubernetes/kubernetes
cd kubernetes/test/e2e/common
# Checkout current master, just so we're on the same page
git checkout 6235ad8c74f7
go build |
Change https://golang.org/cl/296049 mentions this issue: |
Added a flag '-d=inlfuncswithclosures=1' to allow inlining functions with closures, and change the default to off for now, until #44370 is fixed. Updates #44370. Change-Id: Ic17723aa5c091d91f5f5004d8b63ec7125257acf Reviewed-on: https://go-review.googlesource.com/c/go/+/296049 Run-TryBot: Dan Scales <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Dan Scales <[email protected]>
OK, if you sync to change 8027343 or beyond on master (tip), I disabled inlining functions with closures, so this bug should no longer happen. Meanwhile, I will work on figure out the problem and checking in the real fix. |
@danscales I feel like making it disabled by default because of one project is wrong, they could have easily disabled it for their builds. |
@OneOfOne I disagree. Just because they are the first to notice, doesn't mean this bug won't affect lots of others. Including introducing possible random trybot failures for everyone else. |
@OneOfOne Inlining of functions with closures is a new feature that hasn't been released yet, and we intentionally enabled it by default knowing it was still WIP to help smoke out issues like this. Now that we have a handful of concrete, reproducible issues to use for testing, it makes sense to turn it back off by default while we fix those issues. A few months worth of changes have been merged recently from dev.regabi and dev.typeparams, and they're suddenly getting much broader test exposure. Like @randall77 points out, it helps to rollback individual changes/features once we know they have issues so they don't hide further issues in other changes or ongoing development. |
Fair enough, for whatever reason I thought this was enabled since 1.15 and the regabi merge did something. Carry on, I appreciate all that work. |
Change https://golang.org/cl/296649 mentions this issue: |
Incidentally, it looks like CL 286434 (7eaaf28) is already helping prevent silent miscompilations by turning them into noisy ICEs. :) |
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?Operating sytem: Linux
Architecture: ppc64le and amd64
What did you do?
Building targets for kubernetes using the latest golang in master branch
What did you expect to see?
Kubernetes binaries build without error
What did you see instead?
Below compilation error:
The text was updated successfully, but these errors were encountered: