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: ssa panics on a continue in a switch without a loop #19934

Closed
mvdan opened this issue Apr 11, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@mvdan
Copy link
Member

commented Apr 11, 2017

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

go version devel +69261ecad6 Tue Apr 11 12:25:55 2017 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

not relevant

What did you do?

Build f.go (reduced manually):

package foo

func Foo(a string) {
        switch a {
        case "b":
                continue
        }
}

What did you expect to see?

What Go 1.8.1 gives:

./f.go:6: continue is not in a loop

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xa248e4]

goroutine 1 [running]:
cmd/compile/internal/ssa.(*Block).AddEdgeTo(...)
        /home/mvdan/tip/src/cmd/compile/internal/ssa/block.go:150
cmd/compile/internal/gc.(*state).stmt(0xc4202a60e0, 0xc4202f21b0)
        /home/mvdan/tip/src/cmd/compile/internal/gc/ssa.go:783 +0x11e4
cmd/compile/internal/gc.(*state).stmtList(0xc4202a60e0, 0xc42034a040)
        /home/mvdan/tip/src/cmd/compile/internal/gc/ssa.go:503 +0x5a
cmd/compile/internal/gc.(*state).stmt(0xc4202a60e0, 0xc4202f2090)
        /home/mvdan/tip/src/cmd/compile/internal/gc/ssa.go:878 +0x48f4
cmd/compile/internal/gc.(*state).stmtList(0xc4202a60e0, 0xc4202c1fe0)
        /home/mvdan/tip/src/cmd/compile/internal/gc/ssa.go:503 +0x5a
cmd/compile/internal/gc.buildssa(0xc42009e280, 0x0)
        /home/mvdan/tip/src/cmd/compile/internal/gc/ssa.go:189 +0x84b
cmd/compile/internal/gc.compile(0xc42009e280)
        /home/mvdan/tip/src/cmd/compile/internal/gc/pgen.go:301 +0x112
cmd/compile/internal/gc.funccompile(0xc42009e280)
        /home/mvdan/tip/src/cmd/compile/internal/gc/dcl.go:1054 +0xb7
cmd/compile/internal/gc.Main(0xb9d528)
        /home/mvdan/tip/src/cmd/compile/internal/gc/main.go:545 +0x25e5
main.main()
        /home/mvdan/tip/src/cmd/compile/main.go:49 +0x95

Placing my bets on c03e75e by @josharian, will try that version its parent and report back.

@mvdan mvdan added this to the Go1.9 milestone Apr 11, 2017

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2017

Confirming the commit above is to blame; its tree panics, its parent's tree does not. Will look around the code in case the fix is somewhat easy.

Also not sure how to feel about being the only one who writes garbage code on tip.

@bradfitz bradfitz added the NeedsFix label Apr 11, 2017

@mvdan mvdan changed the title cmd/compile: ssa nil pointer dereference crash on tip cmd/compile: ssa panics on a continue in a switch without a loop Apr 11, 2017

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2017

Okay, found a fix. Mailing CL with test soon.

@mvdan mvdan assigned mvdan and unassigned josharian Apr 11, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Apr 11, 2017

CL https://golang.org/cl/40357 mentions this issue.

@gopherbot gopherbot closed this in 2923b14 Apr 12, 2017

@mvdan mvdan removed the NeedsFix label Apr 12, 2017

lparth added a commit to lparth/go that referenced this issue Apr 13, 2017

cmd/compile/internal/gc: don't panic on continue in switch
Continues outside of a loop are not allowed. Most of these possibilities
were tested in label1.go, but one was missing - a plain continue in a
switch/select but no enclosing loop.

This used to error with a "continue not in loop" in 1.8, but recently
was broken by c03e75e. In particular, innerloop does not only account
for loops, but also for switches and selects. Swap it by bools that
track whether breaks and continues should be allowed.

While at it, improve the wording of errors for breaks that are not where
they should be. Change "loop" by "loop, switch, or select" since they
can be used in any of those.

And add tests to make sure this isn't broken again. Use a separate func
since I couldn't get the compiler to crash on f() itself, possibly due
to the recursive call on itself.

Fixes golang#19934.

Change-Id: I8f09c6c2107fd95cac50efc2a8cb03cbc128c35e
Reviewed-on: https://go-review.googlesource.com/40357
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

@golang golang locked and limited conversation to collaborators Apr 12, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.