Fix field lookup in cyclic structs.

Max Schaefer · Max Schaefer · commit 2df8c275e0d7 · 2020-06-19T08:16:09.000+01:00
diff --git a/change-notes/2020-06-19-cyclic-field-lookup.md b/change-notes/2020-06-19-cyclic-field-lookup.md
@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A bug has been fixed that could cause the analysis not to terminate in the presence of cycles through embedded struct fields.
diff --git a/ql/src/semmle/go/Types.qll b/ql/src/semmle/go/Types.qll
@@ -389,6 +389,7 @@ class StructType extends @structtype, CompositeType {
   ) {
     hasOwnField(_, name, tp, isEmbedded) and depth = 0 and isMethod = false
     or
+    not hasOwnField(_, name, _, _) and
     exists(Type embedded | hasEmbeddedField(embedded, depth - 1) |
       embedded.getUnderlyingType().(StructType).hasOwnField(_, name, tp, isEmbedded) and
       isMethod = false
diff --git a/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected b/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected
@@ -1,3 +1,4 @@
+| cyclic.go:4:3:4:3 | s | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
 | pkg1/embedding.go:19:23:19:26 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
 | pkg1/embedding.go:22:27:22:30 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
 | pkg1/embedding.go:25:24:25:31 | embedder | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
diff --git a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected
@@ -1,3 +1,4 @@
+| cyclic.go:4:3:4:3 | s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.s | s |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder | base |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder2 | base |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 | base |
diff --git a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected
@@ -1,3 +1,4 @@
+| cyclic.go:4:3:4:3 | s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types | s | s |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder | base |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder2 | base |
 | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder3 | base |
diff --git a/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected b/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected
@@ -19,3 +19,4 @@
 | embedder3 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 |
 | embedder4 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder4 |
 | ptrembedder | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.ptrembedder |
+| s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.s |
diff --git a/ql/test/library-tests/semmle/go/Types/StructFields.expected b/ql/test/library-tests/semmle/go/Types/StructFields.expected
@@ -31,3 +31,4 @@
 | embedder4 | pkg1/embedding.go:35:16:38:1 | struct type | base | base |
 | embedder4 | pkg1/embedding.go:35:16:38:1 | struct type | f | int |
 | ptrembedder | pkg1/embedding.go:22:18:22:32 | struct type | base | * base |
+| s | cyclic.go:3:8:5:1 | struct type | s | * s |
diff --git a/ql/test/library-tests/semmle/go/Types/Types.expected b/ql/test/library-tests/semmle/go/Types/Types.expected
@@ -19,3 +19,4 @@
 | embedder3 | embedder3 |
 | embedder4 | embedder4 |
 | ptrembedder | ptrembedder |
+| s | s |
diff --git a/ql/test/library-tests/semmle/go/Types/cyclic.go b/ql/test/library-tests/semmle/go/Types/cyclic.go
@@ -0,0 +1,5 @@
+package main
+
+type s struct {
+	*s
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+lgtm,codescanning`
	`2`	`+* A bug has been fixed that could cause the analysis not to terminate in the presence of cycles through embedded struct fields.`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+\| cyclic.go:4:3:4:3 \| s \| package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types \|`
`1`	`2`	`\| pkg1/embedding.go:19:23:19:26 \| base \| package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 \|`
`2`	`3`	`\| pkg1/embedding.go:22:27:22:30 \| base \| package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 \|`
`3`	`4`	`\| pkg1/embedding.go:25:24:25:31 \| embedder \| package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 \|`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+\| cyclic.go:4:3:4:3 \| s \| github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.s \| s \|`
`1`	`2`	`\| pkg1/embedding.go:19:23:19:26 \| base \| github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder \| base \|`
`2`	`3`	`\| pkg1/embedding.go:19:23:19:26 \| base \| github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder2 \| base \|`
`3`	`4`	`\| pkg1/embedding.go:19:23:19:26 \| base \| github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 \| base \|`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +package main
++
 +type s struct {
 +	*s
 +}