]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: reject some rare looping CFGs in shortcircuit
authorJosh Bleecher Snyder <josharian@gmail.com>
Mon, 22 Feb 2021 19:55:14 +0000 (11:55 -0800)
committerJosh Bleecher Snyder <josharian@gmail.com>
Mon, 22 Feb 2021 20:27:09 +0000 (20:27 +0000)
One CFGs that shortcircuit looks for is:

p   q
 \ /
  b
 / \
t   u

The test case creates a CFG like that in which p == t.
That caused the compiler to generate a (short-lived) invalid phi value.

Fix this with a relatively big hammer: Disallow single-length loops entirely.
This is probably overkill, but it such loops are very rare.
This doesn't change the generated code for anything in std.

It generates worse code for the test case:
It no longer compiles the entire function away.

Fixes #44465

Change-Id: Ib8cdcd6cc9d7f48b4dab253652038ace24eae152
Reviewed-on: https://go-review.googlesource.com/c/go/+/295130
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
src/cmd/compile/internal/ssa/shortcircuit.go
test/fixedbugs/issue44465.go [new file with mode: 0644]

index 7b4ee2e81c297a0306f6834f1f7b4d379ca05f70..c72c86a67ef37d071aacc153eac2353e6c317eb2 100644 (file)
@@ -266,6 +266,13 @@ func shortcircuitPhiPlan(b *Block, ctl *Value, cidx int, ti int64) func(*Value,
        // u is the "untaken" branch: the successor we never go to when coming in from p.
        u := b.Succs[1^ti].b
 
+       // In the following CFG matching, ensure that b's preds are entirely distinct from b's succs.
+       // This is probably a stronger condition than required, but this happens extremely rarely,
+       // and it makes it easier to avoid getting deceived by pretty ASCII charts. See #44465.
+       if p0, p1 := b.Preds[0].b, b.Preds[1].b; p0 == t || p1 == t || p0 == u || p1 == u {
+               return nil
+       }
+
        // Look for some common CFG structures
        // in which the outbound paths from b merge,
        // with no other preds joining them.
diff --git a/test/fixedbugs/issue44465.go b/test/fixedbugs/issue44465.go
new file mode 100644 (file)
index 0000000..8cb62ad
--- /dev/null
@@ -0,0 +1,21 @@
+// compile -d=ssa/check/seed
+
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This code caused an internal consistency error due to a bad shortcircuit optimization.
+
+package p
+
+func f() {
+       var b bool
+       if b {
+               b = true
+       }
+l:
+       for !b {
+               b = true
+               goto l
+       }
+}