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

Comments

Projects
None yet
6 participants
@cwedgwood
Copy link
Contributor

commented Aug 7, 2018

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2018

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

@andybons

This comment has been minimized.

Copy link
Member

commented Aug 9, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2018

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

@gopherbot

This comment has been minimized.

Copy link

commented Aug 9, 2018

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

@cwedgwood

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2018

@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

This comment has been minimized.

Copy link

commented Aug 9, 2018

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

@gopherbot gopherbot closed this Aug 9, 2018

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

[release-branch.go1.10] cmd/compile: reset branch prediction when del…
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.