cmd/compile: report type loop for invalid recursive types

Similar to how we report initialization loops in initorder.go and type
alias loops in typecheck.go, this CL updates align.go to warn about
invalid recursive types. The code is based on the loop code from
initorder.go, with minimal changes to adapt from detecting
variable/function initialization loops to detecting type declaration
loops.

Thanks to Cuong Manh Le for investigating this, helping come up with
test cases, and exploring solutions.

Fixes #41575
Updates #41669.

Change-Id: Idb2cb8c5e1d645e62900e178fcb50af33e1700a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/258177
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/cmd/compile/internal/gc/align.go b/src/cmd/compile/internal/gc/align.go
index ab578ee8c7e82da9c8bff66792fe027d143973ac..5af403afa362210ffcd921358b980a24e6580569 100644 (file)
--- a/src/cmd/compile/internal/gc/align.go
+++ b/src/cmd/compile/internal/gc/align.go
@@ -5,7 +5,9 @@
 package gc
 
 import (
+       "bytes"
        "cmd/compile/internal/types"
+       "fmt"
        "sort"
 )
 
@@ -173,6 +175,91 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 {
        return o
 }
 
+// findTypeLoop searches for an invalid type declaration loop involving
+// type t and reports whether one is found. If so, path contains the
+// loop.
+//
+// path points to a slice used for tracking the sequence of types
+// visited. Using a pointer to a slice allows the slice capacity to
+// grow and limit reallocations.
+func findTypeLoop(t *types.Type, path *[]*types.Type) bool {
+       // We implement a simple DFS loop-finding algorithm. This
+       // could be faster, but type cycles are rare.
+
+       if t.Sym != nil {
+               // Declared type. Check for loops and otherwise
+               // recurse on the type expression used in the type
+               // declaration.
+
+               for i, x := range *path {
+                       if x == t {
+                               *path = (*path)[i:]
+                               return true
+                       }
+               }
+
+               *path = append(*path, t)
+               if findTypeLoop(asNode(t.Nod).Name.Param.Ntype.Type, path) {
+                       return true
+               }
+               *path = (*path)[:len(*path)-1]
+       } else {
+               // Anonymous type. Recurse on contained types.
+
+               switch t.Etype {
+               case TARRAY:
+                       if findTypeLoop(t.Elem(), path) {
+                               return true
+                       }
+               case TSTRUCT:
+                       for _, f := range t.Fields().Slice() {
+                               if findTypeLoop(f.Type, path) {
+                                       return true
+                               }
+                       }
+               case TINTER:
+                       for _, m := range t.Methods().Slice() {
+                               if m.Type.IsInterface() { // embedded interface
+                                       if findTypeLoop(m.Type, path) {
+                                               return true
+                                       }
+                               }
+                       }
+               }
+       }
+
+       return false
+}
+
+func reportTypeLoop(t *types.Type) {
+       if t.Broke() {
+               return
+       }
+
+       var l []*types.Type
+       if !findTypeLoop(t, &l) {
+               Fatalf("failed to find type loop for: %v", t)
+       }
+
+       // Rotate loop so that the earliest type declaration is first.
+       i := 0
+       for j, t := range l[1:] {
+               if typePos(t).Before(typePos(l[i])) {
+                       i = j + 1
+               }
+       }
+       l = append(l[i:], l[:i]...)
+
+       var msg bytes.Buffer
+       fmt.Fprintf(&msg, "invalid recursive type %v\n", l[0])
+       for _, t := range l {
+               fmt.Fprintf(&msg, "\t%v: %v refers to\n", linestr(typePos(t)), t)
+               t.SetBroke(true)
+       }
+       fmt.Fprintf(&msg, "\t%v: %v", linestr(typePos(l[0])), l[0])
+       yyerrorl(typePos(l[0]), msg.String())
+}
+
 // dowidth calculates and stores the size and alignment for t.
 // If sizeCalculationDisabled is set, and the size/alignment
 // have not already been calculated, it calls Fatal.
@@ -192,11 +279,7 @@ func dowidth(t *types.Type) {
        }
 
        if t.Width == -2 {
-               if !t.Broke() {
-                       t.SetBroke(true)
-                       yyerrorl(asNode(t.Nod).Pos, "invalid recursive type %v", t)
-               }
-
+               reportTypeLoop(t)
                t.Width = 0
                t.Align = 1
                return
@@ -308,10 +391,7 @@ func dowidth(t *types.Type) {
                checkwidth(t.Key())
 
        case TFORW: // should have been filled in
-               if !t.Broke() {
-                       t.SetBroke(true)
-                       yyerror("invalid recursive type %v", t)
-               }
+               reportTypeLoop(t)
                w = 1 // anything will do
 
        case TANY:

diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go
index b5527e2f8359bd3cc0f16324b38287eb4eee6942..07547df36e685367054d424ec57ab15f3908dc20 100644 (file)
--- a/src/cmd/compile/internal/gc/subr.go
+++ b/src/cmd/compile/internal/gc/subr.go
@@ -1921,3 +1921,13 @@ func ifaceData(pos src.XPos, n *Node, t *types.Type) *Node {
        ind.SetBounded(true)
        return ind
 }
+
+// typePos returns the position associated with t.
+// This is where t was declared or where it appeared as a type expression.
+func typePos(t *types.Type) src.XPos {
+       n := asNode(t.Nod)
+       if n == nil || !n.Pos.IsKnown() {
+               Fatalf("bad type: %v", t)
+       }
+       return n.Pos
+}

diff --git a/test/fixedbugs/bug195.go b/test/fixedbugs/bug195.go
index 496c0be6100e3d9db59f5bd80d92852e5025d89d..aef7bd2d8944a883c344b558dbdff1abb11aad54 100644 (file)
--- a/test/fixedbugs/bug195.go
+++ b/test/fixedbugs/bug195.go
@@ -6,22 +6,22 @@
 
 package main
 
-type I1 interface { I2 }       // ERROR "interface"
+type I1 interface{ I2 } // ERROR "interface"
 type I2 int
 
-type I3 interface { int }      // ERROR "interface"
+type I3 interface{ int } // ERROR "interface"
 
 type S struct {
-       x interface{ S }        // ERROR "interface"
+       x interface{ S } // ERROR "interface"
 }
-type I4 interface { // GC_ERROR "invalid recursive type"
-       I4      // GCCGO_ERROR "interface"
+type I4 interface { // GC_ERROR "invalid recursive type I4\n\tLINE: I4 refers to\n\tLINE: I4$"
+       I4 // GCCGO_ERROR "interface"
 }
 
-type I5 interface { // GC_ERROR "invalid recursive type"
-       I6      // GCCGO_ERROR "interface"
+type I5 interface { // GC_ERROR "invalid recursive type I5\n\tLINE: I5 refers to\n\tLINE+4: I6 refers to\n\tLINE: I5$"
+       I6 // GCCGO_ERROR "interface"
 }
 
 type I6 interface {
-       I5      // GCCGO_ERROR "interface"
+       I5 // GCCGO_ERROR "interface"
 }

diff --git a/test/fixedbugs/issue22904.go b/test/fixedbugs/issue22904.go
index 46cb7c048aedcfc81f95f68f29cced7eb70449a9..09f4a2118eb50b7568c5c39277e10919974d5625 100644 (file)
--- a/test/fixedbugs/issue22904.go
+++ b/test/fixedbugs/issue22904.go
@@ -9,8 +9,8 @@
 
 package p
 
-type a struct{ b }
-type b struct{ a } // ERROR "invalid recursive type"
+type a struct{ b } // ERROR "invalid recursive type"
+type b struct{ a }
 
 var x interface{}
 

diff --git a/test/fixedbugs/issue23823.go b/test/fixedbugs/issue23823.go
index 2f802d09886064a7a3c75e1e4546b5862b0a0e73..fe6cef1fb4d26a5f55644cf500121ec670d8e96f 100644 (file)
--- a/test/fixedbugs/issue23823.go
+++ b/test/fixedbugs/issue23823.go
@@ -10,6 +10,7 @@ type I1 = interface {
        I2
 }
 
-type I2 interface { // ERROR "invalid recursive type"
+// BAD: type loop should mention I1; see also #41669
+type I2 interface { // ERROR "invalid recursive type I2\n\tLINE: I2 refers to\n\tLINE: I2$"
        I1
 }

diff --git a/test/fixedbugs/issue41575.go b/test/fixedbugs/issue41575.go
new file mode 100644 (file)
index 0000000..d03d1c8
--- /dev/null
+++ b/test/fixedbugs/issue41575.go
@@ -0,0 +1,36 @@
+// errorcheck
+
+// Copyright 2020 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 p
+
+type T1 struct { // ERROR "invalid recursive type T1\n\tLINE: T1 refers to\n\tLINE+4: T2 refers to\n\tLINE: T1$"
+       f2 T2
+}
+
+type T2 struct {
+       f1 T1
+}
+
+type a b
+type b c // ERROR "invalid recursive type b\n\tLINE: b refers to\n\tLINE+1: c refers to\n\tLINE: b$"
+type c b
+
+type d e
+type e f
+type f f // ERROR "invalid recursive type f\n\tLINE: f refers to\n\tLINE: f$"
+
+type g struct { // ERROR "invalid recursive type g\n\tLINE: g refers to\n\tLINE: g$"
+       h struct {
+               g
+       }
+}
+
+type w x
+type x y // ERROR "invalid recursive type x\n\tLINE: x refers to\n\tLINE+1: y refers to\n\tLINE+2: z refers to\n\tLINE: x$"
+type y struct{ z }
+type z [10]x
+
+type w2 w // refer to the type loop again