internal/core/adt: fix index out of bound panic

The main issue here was that when a Vertex was cloned,
in overlay.go, its corresponding closeContext was cloned
first, alongside its group. Normally, the group of a
closeContext associated with a Vertex has its group
pointing to the Conjuncts of this Vertex, ensuring they
are always the same. The copy made in the overlay code,
however, only copied the slice pointers already allocated,
but the two slices would get out of sync when more
conjuncts were added. This caused the index to overrun.

This fix also exposed another issue where the group can
sometimes be empty. Although this is theoretically possible,
the conditions under which this appeared are suspicious.
This CL, therefore, added a TODO to investigate. A simplified
reproducer for this is:

A: v: "a" | "b"
h: [_]: A
h: a: v:  *"a" | string
h: b: v:  *h.a.v | string
h: ["b"]: v: *h.b.v | string

This example also exposes the counter issue, three of which
were added because of this change.

Tests were added in this CL, as they cause a panic otherwise.

Fixes #3597

A further related issue is fixed in a next CL.

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I61b6aafb2f439793388f9357001d8ec99a30da3f
Dispatch-Trailer: {"type":"trybot","CL":1206926,"patchset":1,"ref":"refs/changes/26/1206926/1","targetBranch":"master"}

cue/testdata/benchmarks/disjunctelim.txtar

@@ -47,7 +47,7 @@ Allocs: 572
 Retain: 0
 
 Unifications: 22
-Conjuncts:    825
+Conjuncts:    877
 Disjuncts:    618
 -- diff/-out/evalalpha/stats<==>+out/eval/stats --
 diff old new
@@ -68,7 +68,7 @@ diff old new
 -Conjuncts:    866
 -Disjuncts:    367
 +Unifications: 22
-+Conjuncts:    825
++Conjuncts:    877
 +Disjuncts:    618
 -- out/eval/stats --
 Leaks:  0

cue/testdata/benchmarks/issue1684.txtar

@@ -69,7 +69,7 @@ Allocs: 2135
 Retain: 0
 
 Unifications: 475
-Conjuncts:    4799
+Conjuncts:    4826
 Disjuncts:    918
 -- out/evalalpha --
 (struct){
@@ -153,7 +153,7 @@ diff old new
 -Conjuncts:    2480117
 -Disjuncts:    1064333
 +Unifications: 475
-+Conjuncts:    4799
++Conjuncts:    4826
 +Disjuncts:    918
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new

cue/testdata/eval/disjunctions.txtar

@@ -180,16 +180,40 @@ issue3434: full: {
 		}
 	}
 }
+-- issue3597.cue --
+issue3597: t1: {
+	#Schema: {
+		_ok: bool
+		{} | {
+			_ok: true
+			let Ok = _ok
+			{} | {x: Ok}
+		}
+	}
+
+	[string]: #Schema
+
+	foo: bar
+	bar: {}
+}
+issue3597: t2: {
+	ok: true
+	X: _ | {
+		let Ok = ok
+		_ | {x: Ok}
+	}
+	foo: X & {X}
+}
 -- out/eval/stats --
-Leaks:  0
-Freed:  449
-Reused: 433
-Allocs: 16
-Retain: 26
+Leaks:  8
+Freed:  561
+Reused: 547
+Allocs: 22
+Retain: 34
 
-Unifications: 210
-Conjuncts:    817
-Disjuncts:    451
+Unifications: 286
+Conjuncts:    1125
+Disjuncts:    571
 -- out/evalalpha --
 Errors:
 f: 2 errors in empty disjunction:
@@ -429,6 +453,60 @@ Result:
       }
     }
   }
+  issue3597: (struct){
+    t1: (struct){
+      #Schema: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+          x: (bool){ true }
+        }) }
+      foo: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1multi = 〈0;_ok〉
+          x: (bool){ true }
+        }) }
+      bar: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+          x: (bool){ true }
+        }) }
+    }
+    t2: (struct){
+      ok: (bool){ true }
+      X: (_){ |((_){ _ }, (_){
+          _
+          let Ok#2 = (bool){ true }
+        }, (struct){
+          let Ok#2 = (bool){ true }
+          x: (bool){ true }
+        }) }
+      foo: (_){ |((_){ _ }, (_){
+          _
+          let Ok#2 = (bool){ true }
+        }, (struct){
+          let Ok#2 = (bool){ true }
+          x: (bool){ true }
+        }) }
+    }
+  }
 }
 -- diff/-out/evalalpha<==>+out/eval --
 diff old new
@@ -477,6 +555,18 @@ diff old new
          }
        }
      }
+@@ -264,6 +255,9 @@
+           let Ok#1 = (bool){ true }
+         }, (#struct){
+           _ok: (bool){ true }
++          let Ok#1 = (bool){ true }
++        }, (#struct){
++          _ok: (bool){ true }
+           let Ok#1multi = 〈0;_ok〉
+           x: (bool){ true }
+         }) }
+-- diff/todo/p1 --
+issue3597.t1.foo: duplicate disjunct
 -- diff/todo/p3 --
 Missing error positions.
   It is probably okay to show name and val fields with original values, as long
@@ -730,6 +820,57 @@ Result:
       }
     }
   }
+  issue3597: (struct){
+    t1: (struct){
+      #Schema: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+          x: (bool){ true }
+        }) }
+      foo: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1multi = 〈0;_ok〉
+          x: (bool){ true }
+        }) }
+      bar: (#struct){ |((#struct){
+          _ok: (bool){ bool }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+        }, (#struct){
+          _ok: (bool){ true }
+          let Ok#1 = (bool){ true }
+          x: (bool){ true }
+        }) }
+    }
+    t2: (struct){
+      ok: (bool){ true }
+      X: (_){ |((_){ _ }, (_){
+          _
+          let Ok#2 = (bool){ true }
+        }, (struct){
+          let Ok#2 = (bool){ true }
+          x: (bool){ true }
+        }) }
+      foo: (_){ |((_){ _ }, (_){
+          _
+          let Ok#2 = (bool){ true }
+        }, (struct){
+          let Ok#2 = (bool){ true }
+          x: (bool){ true }
+        }) }
+    }
+  }
 }
 -- out/compile --
 --- in.cue
@@ -1041,3 +1182,37 @@ Result:
     }
   }
 }
+--- issue3597.cue
+{
+  issue3597: {
+    t1: {
+      #Schema: {
+        _ok: bool
+        ({}|{
+          _ok: true
+          let Ok#1 = 〈0;_ok〉
+          ({}|{
+            x: 〈1;let Ok#1〉
+          })
+        })
+      }
+      [string]: 〈0;#Schema〉
+      foo: 〈0;bar〉
+      bar: {}
+    }
+  }
+  issue3597: {
+    t2: {
+      ok: true
+      X: (_|{
+        let Ok#2 = 〈1;ok〉
+        (_|{
+          x: 〈1;let Ok#2〉
+        })
+      })
+      foo: (〈0;X〉 & {
+        〈1;X〉
+      })
+    }
+  }
+}

internal/core/adt/eval_test.go

@@ -90,6 +90,7 @@ var skipDebugDepErrors = map[string]int{
 	"disjunctions/elimination": 11,
 	"disjunctions/embed":       6,
 	"eval/conjuncts":           3,
+	"eval/disjunctions":        3,
 	"eval/notify":              17,
 	"fulleval/054_issue312":    1,
 	"scalars/embed":            1,

internal/core/adt/overlay.go

@@ -154,7 +154,12 @@ func (ctx *overlayContext) cloneVertex(x *Vertex) *Vertex {
 
 	v._cc.src = v
 	v._cc.parentConjuncts = v
+
+	// The group of the root closeContext should point to the Conjuncts field
+	// of the Vertex. As we already allocated the group, we use that allocation,
+	// but "move" it to v.Conjuncts.
 	v.Conjuncts = *v._cc.group
+	v._cc.group = &v.Conjuncts
 
 	if a := x.Arcs; len(a) > 0 {
 		// TODO(perf): reuse buffer.
@@ -280,13 +285,16 @@ func (ctx *overlayContext) allocCC(cc *closeContext) *closeContext {
 		}
 
 		if o.parent != nil {
-			// validate invariants
-			ca := *cc.parent.group
-			if ca[cc.parentIndex].x != cc.group {
-				panic("group misaligned")
+			// validate invariants.
+			// TODO: the group can sometimes be empty. Investigate why and
+			// whether this is valid.
+			if ca := *cc.parent.group; len(ca) > 0 {
+				if ca[cc.parentIndex].x != cc.group {
+					panic("group misaligned")
+				}
+
+				(*o.parent.group)[cc.parentIndex].x = o.group
 			}
-
-			(*o.parent.group)[cc.parentIndex].x = o.group
 		}
 	}