From 5ec23663d0a4c54c5e8ec3d258613482eb6764a4 Mon Sep 17 00:00:00 2001
From: Dominik Honnef <dominik@honnef.co>
Date: Fri, 19 Jul 2019 00:48:11 +0200
Subject: [PATCH] go/cfg: stop once we've found the fallthrough target

The old code didn't stop after finding the fallthrough target, always
walking the entire list to the top. This would effectively break
fallthroughs, constructing an invalid graph, as the fallthrough target
would never be found.

The fix brings the loop condition in line with all the other
stack-walking loop conditions in the surrounding code, which abort
once the target block has been found.

Alternatively, the entire loop could been omitted, as 'fallthrough'
has to be the last statement in a case body and thus always refers to
the immediate element on the stack. However, since the builder already
handles malformed ASTs as gracefully as possible, it seemed better to
keep the loop and to construct a slightly less wrong graph in the
presence of malformed ASTs.

Before the fix, the following code

	func fn(x int) {
		for {
			switch x {
			case 1:
				println("case 1")
				fallthrough
			case 2:
				println("case 2")
			}
		}
	}

would result in the following graph. Note the presence of an
undefined.branch block.

	.0: # entry
		succs: 1

	.1: # for.body
		x
		1
		succs: 4 6

	.2: # for.done

	.3: # switch.done
		succs: 1

	.4: # switch.body
		println("case 1")
		succs: 7

	.5: # switch.body
		println("case 2")
		succs: 3

	.6: # switch.next
		2
		succs: 5 9

	.7: # undefined.branch

	.8: # unreachable.branch
		succs: 3

	.9: # switch.next
		succs: 3

After the fix, this graph is computed instead:

	.0: # entry
		succs: 1

	.1: # for.body
		x
		1
		succs: 4 6

	.2: # for.done

	.3: # switch.done
		succs: 1

	.4: # switch.body
		println("case 1")
		succs: 5

	.5: # switch.body
		println("case 2")
		succs: 3

	.6: # switch.next
		2
		succs: 5 8

	.7: # unreachable.branch
		succs: 3

	.8: # switch.next
		succs: 3

Change-Id: I3bb00eddec2a7da02cb929860f4c95cf477c848c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186857
Run-TryBot: Dominik Honnef <dominik@honnef.co>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
---
 go/cfg/builder.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/cfg/builder.go b/go/cfg/builder.go
index 24e1aba0..7f95a296 100644
--- a/go/cfg/builder.go
+++ b/go/cfg/builder.go
@@ -149,7 +149,7 @@ func (b *builder) branchStmt(s *ast.BranchStmt) {
 		}
 
 	case token.FALLTHROUGH:
-		for t := b.targets; t != nil; t = t.tail {
+		for t := b.targets; t != nil && block == nil; t = t.tail {
 			block = t._fallthrough
 		}