cmd/compile: ensure that operand of ORETURN is not double-walked

Inlining of switch statements into a RETURNed expression
can sometimes lead to the switch being walked twice, which
results in a miscompiled switch statement. The bug depends
on:

1) multiple results
2) named results
3) a return statement whose expression includes a call to a
function containing a switch statement that is inlined.

It may also be significant that the default case of that
switch is a panic(), though that's not proven.

Rearranged the walk case for ORETURN so that double walks are
not possible.  Added a test, because this is so fiddly.
Added a check against double walks, verified that it fires
w/o other fix.

Fixes #25776.

Change-Id: I2d594351fa082632512ef989af67eb887059729b
Reviewed-on: https://go-review.googlesource.com/118318
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go
index 433d38544ec3953e138287d52e82505c3e11f777..b668409a889ac8aa3109dbec0c1a32e2ab37b3b6 100644 (file)
--- a/src/cmd/compile/internal/gc/swt.go
+++ b/src/cmd/compile/internal/gc/swt.go
@@ -241,6 +241,11 @@ func walkswitch(sw *Node) {
 // search using if..goto, although binary search
 // is used with long runs of constants.
 func (s *exprSwitch) walk(sw *Node) {
+       // Guard against double walk, see #25776.
+       if sw.List.Len() == 0 && sw.Nbody.Len() > 0 {
+               Fatalf("second walk of switch")
+       }
+
        casebody(sw, nil)
 
        cond := sw.Left

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index 591c8f3bfecbce13fd422a8bea89581de546d2af..df7428a127fd50685a8c1586d2edc0f90ac91774 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -287,7 +287,6 @@ func walkstmt(n *Node) *Node {
                walkstmtlist(n.Rlist.Slice())
 
        case ORETURN:
-               walkexprlist(n.List.Slice(), &n.Ninit)
                if n.List.Len() == 0 {
                        break
                }
@@ -317,6 +316,9 @@ func walkstmt(n *Node) *Node {
 
                        if samelist(rl, n.List.Slice()) {
                                // special return in disguise
+                               // TODO(josharian, 1.12): is "special return" still relevant?
+                               // Tests still pass w/o this. See comments on https://go-review.googlesource.com/c/go/+/118318
+                               walkexprlist(n.List.Slice(), &n.Ninit)
                                n.List.Set(nil)
 
                                break
@@ -329,6 +331,7 @@ func walkstmt(n *Node) *Node {
                        n.List.Set(reorder3(ll))
                        break
                }
+               walkexprlist(n.List.Slice(), &n.Ninit)
 
                ll := ascompatte(nil, false, Curfn.Type.Results(), n.List.Slice(), 1, &n.Ninit)
                n.List.Set(ll)

diff --git a/test/fixedbugs/issue25776.go b/test/fixedbugs/issue25776.go
new file mode 100644 (file)
index 0000000..e05c0bc
--- /dev/null
+++ b/test/fixedbugs/issue25776.go
@@ -0,0 +1,99 @@
+// run
+
+// Copyright 2018 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.
+
+package main
+
+const (
+       Upper       = true
+       blas_Upper  = 121
+       badTriangle = "bad triangle"
+)
+
+// Triangular represents a triangular matrix. Triangular matrices are always square.
+type Triangular interface {
+       // Triangular returns the number of rows/columns in the matrix and its
+       // orientation.
+       Tryangle() (mmmm int, kynd bool)
+       Triangle() (mmmm int, kynd bool)
+}
+
+// blas64_Triangular represents a triangular matrix using the conventional storage scheme.
+type blas64_Triangular struct {
+       Stride int
+       Uplo   int
+}
+
+// TriDense represents an upper or lower triangular matrix in dense storage
+// format.
+type TriDense struct {
+       mat blas64_Triangular
+}
+
+func NewTriDense() *TriDense {
+       return &TriDense{
+               mat: blas64_Triangular{
+                       Stride: 3,
+                       Uplo:   blas_Upper,
+               },
+       }
+}
+
+func (t *TriDense) isUpper() bool {
+       return isUpperUplo(t.mat.Uplo)
+}
+
+func (t *TriDense) triKind() bool {
+       return isUpperUplo(t.mat.Uplo)
+}
+
+func isUpperUplo(u int) bool {
+       switch u {
+       case blas_Upper:
+               return true
+       default:
+               panic(badTriangle)
+       }
+}
+
+func (t *TriDense) IsZero() bool {
+       return t.mat.Stride == 0
+}
+
+//go:noinline
+func (t *TriDense) ScaleTri(f float64, a Triangular) {
+       n, kind := a.Triangle()
+       if kind == false {
+               println("ScaleTri n, kind=", n, ", ", kind, " (FAIL, expected true)")
+       }
+}
+
+//go:noinline
+func (t *TriDense) ScaleTry(f float64, a Triangular) {
+       n, kind := a.Tryangle()
+       if kind == false {
+               println("ScaleTry n, kind=", n, ", ", kind, " (FAIL, expected true)")
+       }
+}
+
+// Triangle failed (before fix)
+func (t *TriDense) Triangle() (nnnn int, kind bool) {
+       return 3, !t.IsZero() && t.triKind()
+}
+
+// Tryangle works -- difference is not-named output parameters.
+func (t *TriDense) Tryangle() (int, bool) {
+       return 3, !t.IsZero() && t.triKind()
+}
+
+func main() {
+       ta := NewTriDense()
+       n, kind := ta.Triangle()
+       if kind == false {
+               println("    main n, kind=", n, ", ", kind, " (FAIL, expected true)")
+       }
+       ta.ScaleTri(1, ta)
+       ta.ScaleTry(1, ta)
+}