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: please cherry pick internal compiler error fix for #23504 for 1.10.4 [1.10 backport] #26851

Closed
cwedgwood opened this issue Aug 7, 2018 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@cwedgwood
Copy link
Contributor

@randall77 It looks perhaps like the fix for #23504 didn't make it to 1.10.x releases (it's not in 1.9.x either, but I already patch those locally and am upgrading so less concerned).

Cherry picking the fix (4313d77) and rebuilding 1.10.3 works as expected for me.

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

go version go1.10.3 linux/amd64

What did you do?

https://play.golang.org/p/OWxLKfWXnnh

go build of

package main

func f(op string) {
        for len(op) > 1 && !(op[0] >= '0' || op[0] <= '9') {
        }
}

func main() {
        f("x")
}

What did you expect to see?

... no output ...

What did you see instead?

prog.go:4:16: internal compiler error: panic during layout while compiling f:

runtime error: index out of range

goroutine 7 [running]:
cmd/compile/internal/ssa.Compile.func1(0xc4203f3288, 0xc42000e500)
	/usr/local/go/src/cmd/compile/internal/ssa/compile.go:38 +0xc8
panic(0xbbc700, 0xf9d830)
	/usr/local/go/src/runtime/panic.go:502 +0x229
cmd/compile/internal/ssa.layout(0xc42000e500)
	/usr/local/go/src/cmd/compile/internal/ssa/layout.go:68 +0xdca
cmd/compile/internal/ssa.Compile(0xc42000e500)
	/usr/local/go/src/cmd/compile/internal/ssa/compile.go:70 +0x2bb
cmd/compile/internal/gc.buildssa(0xc420001200, 0x1, 0x0)
	/usr/local/go/src/cmd/compile/internal/gc/ssa.go:223 +0xb32
cmd/compile/internal/gc.compileSSA(0xc420001200, 0x1)
	/usr/local/go/src/cmd/compile/internal/gc/pgen.go:239 +0x39
cmd/compile/internal/gc.compileFunctions.func2(0xc4203e8180, 0xc4200125a0, 0x1)
	/usr/local/go/src/cmd/compile/internal/gc/pgen.go:289 +0x49
created by cmd/compile/internal/gc.compileFunctions
	/usr/local/go/src/cmd/compile/internal/gc/pgen.go:287 +0x11c



Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new

(apologies, this is the output from the playground as i now have it fixed locally so no error and am too lazy to rebuild it 'broken' right now)

@odeke-em odeke-em changed the title internal compiler error (bug #23504) in 1.10.3 cmd/compile: please cherry pick internal compiler error fix for #23504 for 1.10.4 Aug 7, 2018
@odeke-em
Copy link
Member

odeke-em commented Aug 7, 2018

Thank you @cwedgwood for reporting this!

I believe @randall77 has been looking at backporting to 1.10.* but also /cc @FiloSottile @andybons for auto-cherry picking/backporting.

@odeke-em odeke-em added this to the Go1.10.4 milestone Aug 7, 2018
@randall77
Copy link
Contributor

This is not a regression since it's been failing since Go 1.7. Normally we do not backport to fix such issues.
The fix is super simple, though. I'll put this issue in the queue for consideration.

@randall77 randall77 changed the title cmd/compile: please cherry pick internal compiler error fix for #23504 for 1.10.4 cmd/compile: please cherry pick internal compiler error fix for #23504 for 1.10.4 [1.10 backport] Aug 7, 2018
@randall77 randall77 added the CherryPickCandidate Used during the release process for point releases label Aug 7, 2018
@andybons
Copy link
Member

andybons commented Aug 9, 2018

@ianlancetaylor

@ianlancetaylor
Copy link
Contributor

I'm OK with backporting https://golang.org/cl/88955 to 1.10. I'll send a CL.

@gopherbot
Copy link

Change https://golang.org/cl/128855 mentions this issue: [release-branch.go1.10] cmd/compile: reset branch prediction when deleting a branch

@cwedgwood
Copy link
Contributor Author

@ianlancetaylor if this is deemed unsuitable for 1.10.x close this out as-is and it becomes a non-issue for 1.11.x in a little bit.

@gopherbot
Copy link

Closed by merging dea961e to release-branch.go1.10.

gopherbot pushed a commit that referenced this issue Aug 9, 2018
…eting a branch

When we go from a branch block to a plain block, reset the
branch prediction bit. Downstream passes asssume that if the
branch prediction is set, then the block has 2 successors.

Fixes #23504
Fixes #26851

Change-Id: I2898ec002228b2e34fe80ce420c6939201c0a5aa
Reviewed-on: https://go-review.googlesource.com/88955
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 4313d77)
Reviewed-on: https://go-review.googlesource.com/128855
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@andybons andybons added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Aug 9, 2018
@golang golang locked and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

6 participants