From 1d0cc9c727ac5e348b16cbe36b5bab56704445fe Mon Sep 17 00:00:00 2001
From: ysthakur <45539777+ysthakur@users.noreply.github.com>
Date: Sun, 1 Dec 2024 14:46:00 -0500
Subject: [PATCH] Delete entire bucket when removing pcrs

---
 broken.c                                      | 308 ++++++++++++++++++
 src/main/resources/runtime/runtime.h          |  32 +-
 src/test/scala/fred/FuzzTests.scala           | 115 +------
 .../{GenerateTypes.scala => GenUtil.scala}    | 118 ++++++-
 src/test/scala/fred/SCCTests.scala            |   8 +-
 5 files changed, 464 insertions(+), 117 deletions(-)
 create mode 100644 broken.c
 rename src/test/scala/fred/{GenerateTypes.scala => GenUtil.scala} (73%)

diff --git a/broken.c b/broken.c
new file mode 100644
index 0000000..8f0ed43
--- /dev/null
+++ b/broken.c
@@ -0,0 +1,308 @@
+#include "runtime.h"
+
+enum OptT0_kind { SomeT0_tag, NoneT0_tag };
+struct OptT0 {
+  int rc;
+  enum Color color;
+  int addedPCR;
+  enum OptT0_kind kind;
+  void (*print)();
+  union {
+    struct { struct T0* value_SomeT0; };
+    struct {  };
+  };
+};
+enum T0_kind { T0_tag };
+struct T0 {
+  int rc;
+  enum Color color;
+  int addedPCR;
+  enum T0_kind kind;
+  void (*print)();
+  struct OptT0* f0;
+  union {
+    struct {  };
+  };
+};
+void $free_OptT0(struct OptT0* this);
+void $free_T0(struct T0* this);
+void $decr_OptT0(struct OptT0* this);
+void $decr_T0(struct T0* this);
+void $markGray_OptT0(struct OptT0* this);
+void $markGray_T0(struct T0* this);
+void $scan_OptT0(struct OptT0* this);
+void $scan_T0(struct T0* this);
+void $scanBlack_OptT0(struct OptT0* this);
+void $scanBlack_T0(struct T0* this);
+void $collectWhite_OptT0(struct OptT0* this);
+void $collectWhite_T0(struct T0* this);
+void $print_OptT0(struct OptT0* this);
+void $print_T0(struct T0* this);
+struct OptT0* new$SomeT0(struct T0* value);
+struct OptT0* new$NoneT0();
+struct T0* new$T0(struct OptT0* f0);
+int main();
+void $free_OptT0(struct OptT0* this) {
+  fprintf(stderr, "Freeing OptT0\n");
+  switch (this->kind) {
+  case SomeT0_tag:
+    break;
+  case NoneT0_tag:
+    break;
+  }
+  free(this);
+}
+void $free_T0(struct T0* this) {
+  fprintf(stderr, "Freeing T0\n");
+  switch (this->kind) {
+  case T0_tag:
+    break;
+  }
+  free(this);
+}
+void $decr_OptT0(struct OptT0* this) {
+  fprintf(stderr, "Decrementing OptT0 (%p)\n", this);
+  if (--this->rc == 0) {
+    switch (this->kind) {
+    case SomeT0_tag:
+      $decr_T0(this->value_SomeT0);
+      break;
+    case NoneT0_tag:
+      break;
+    }
+    removePCR((void *) this, 0);
+    free(this);
+  } else {
+    addPCR(
+      (void *) this,
+      0,
+      (void *) $markGray_OptT0,
+      (void *) $scan_OptT0,
+      (void *) $collectWhite_OptT0);
+  }
+}
+void $decr_T0(struct T0* this) {
+  fprintf(stderr, "Decrementing T0 (%p)\n", this);
+  if (--this->rc == 0) {
+    switch (this->kind) {
+    case T0_tag:
+      $decr_OptT0(this->f0);
+      break;
+    }
+    removePCR((void *) this, 0);
+    free(this);
+  } else {
+    addPCR(
+      (void *) this,
+      0,
+      (void *) $markGray_T0,
+      (void *) $scan_T0,
+      (void *) $collectWhite_T0);
+  }
+}
+void $markGray_OptT0(struct OptT0* this) {
+  if (this->color == kGray) return;
+  this->color = kGray;
+  switch (this->kind) {
+  case SomeT0_tag:
+    this->value_SomeT0->rc --;
+    $markGray_T0(this->value_SomeT0);
+    break;
+  case NoneT0_tag:
+    break;
+  }
+}
+void $markGray_T0(struct T0* this) {
+  if (this->color == kGray) return;
+  this->color = kGray;
+  switch (this->kind) {
+  case T0_tag:
+    this->f0->rc --;
+    $markGray_OptT0(this->f0);
+    break;
+  }
+}
+void $scan_OptT0(struct OptT0* this) {
+  if (this->color != kGray) return;
+  if (this->rc > 0) {
+    $scanBlack_OptT0(this);
+    return;
+  }
+  this->color = kWhite;
+  switch (this->kind) {
+  case SomeT0_tag:
+    $scan_T0(this->value_SomeT0);
+    break;
+  case NoneT0_tag:
+    break;
+  }
+}
+void $scan_T0(struct T0* this) {
+  if (this->color != kGray) return;
+  if (this->rc > 0) {
+    $scanBlack_T0(this);
+    return;
+  }
+  this->color = kWhite;
+  switch (this->kind) {
+  case T0_tag:
+    $scan_OptT0(this->f0);
+    break;
+  }
+}
+void $scanBlack_OptT0(struct OptT0* this) {
+  if (this->color != kBlack) {
+    this->color = kBlack;
+    switch (this->kind) {
+    case SomeT0_tag:
+      this->value_SomeT0->rc ++;
+      $scanBlack_T0(this->value_SomeT0);
+      break;
+    case NoneT0_tag:
+      break;
+    }
+  }
+}
+void $scanBlack_T0(struct T0* this) {
+  if (this->color != kBlack) {
+    this->color = kBlack;
+    switch (this->kind) {
+    case T0_tag:
+      this->f0->rc ++;
+      $scanBlack_OptT0(this->f0);
+      break;
+    }
+  }
+}
+void $collectWhite_OptT0(struct OptT0* this) {
+  if (this->color == kWhite) {
+    this->color = kBlack;
+    switch (this->kind) {
+    case SomeT0_tag:
+      $collectWhite_T0(this->value_SomeT0);
+      break;
+    case NoneT0_tag:
+      break;
+    }
+    fprintf(stderr, "Removing OptT0\n");
+    struct FreeCell *curr = freeList;
+    freeList = malloc(sizeof(struct FreeCell));
+    freeList->obj = (void *) this;
+    freeList->next = curr;
+    freeList->free = (void *) $free_OptT0;
+  }
+}
+void $collectWhite_T0(struct T0* this) {
+  if (this->color == kWhite) {
+    this->color = kBlack;
+    switch (this->kind) {
+    case T0_tag:
+      $collectWhite_OptT0(this->f0);
+      break;
+    }
+    fprintf(stderr, "Removing T0\n");
+    struct FreeCell *curr = freeList;
+    freeList = malloc(sizeof(struct FreeCell));
+    freeList->obj = (void *) this;
+    freeList->next = curr;
+    freeList->free = (void *) $free_T0;
+  }
+}
+void $print_OptT0(struct OptT0* this) {
+  switch (this->kind) {
+  case SomeT0_tag:
+    printf("SomeT0 {");
+    printf("value=");
+    $print_T0(this->value_SomeT0);
+    printf(", ");
+    printf("}");
+    break;
+  case NoneT0_tag:
+    printf("NoneT0 {");
+    printf("}");
+    break;
+  }
+}
+void $print_T0(struct T0* this) {
+  switch (this->kind) {
+  case T0_tag:
+    printf("T0 {");
+    printf("f0=");
+    $print_OptT0(this->f0);
+    printf(", ");
+    printf("}");
+    break;
+  }
+}
+struct OptT0* new$SomeT0(struct T0* value) {
+  struct OptT0* $res = malloc(sizeof (struct OptT0));
+  $res->rc = 0;
+  $res->color = kBlack;
+  $res->addedPCR = 0;
+  $res->print = $print_OptT0;
+  $res->kind = SomeT0_tag;
+  $res->value_SomeT0 = value;
+  $res->value_SomeT0->rc ++;
+  return $res;
+}
+struct OptT0* new$NoneT0() {
+  struct OptT0* $res = malloc(sizeof (struct OptT0));
+  $res->rc = 0;
+  $res->color = kBlack;
+  $res->addedPCR = 0;
+  $res->print = $print_OptT0;
+  $res->kind = NoneT0_tag;
+  return $res;
+}
+struct T0* new$T0(struct OptT0* f0) {
+  struct T0* $res = malloc(sizeof (struct T0));
+  $res->rc = 0;
+  $res->color = kBlack;
+  $res->addedPCR = 0;
+  $res->print = $print_T0;
+  $res->kind = T0_tag;
+  $res->f0 = f0;
+  $res->f0->rc ++;
+  return $res;
+}
+int main() {
+  struct T0* vT0_0 = new$T0(new$NoneT0());
+  vT0_0->rc ++;
+  struct T0* vT0_2 = new$T0(new$NoneT0());
+  vT0_2->rc ++;
+  struct T0* vT0_1 = new$T0(new$NoneT0());
+  vT0_1->rc ++;
+  struct T0* vT0_6 = new$T0(new$NoneT0());
+  vT0_6->rc ++;
+  struct T0* vT0_5 = new$T0(new$NoneT0());
+  vT0_5->rc ++;
+  struct T0* vT0_3 = new$T0(new$NoneT0());
+  vT0_3->rc ++;
+  struct T0* vT0_4 = new$T0(new$NoneT0());
+  vT0_4->rc ++;
+  struct OptT0* oldValue$0 = vT0_5->f0;
+  vT0_5->f0 = new$SomeT0(vT0_6);
+  vT0_5->f0->rc ++;
+  $decr_OptT0(oldValue$0);
+  struct OptT0* oldValue$1 = vT0_0->f0;
+  vT0_0->f0 = new$SomeT0(vT0_0);
+  vT0_0->f0->rc ++;
+  $decr_OptT0(oldValue$1);
+  struct OptT0* oldValue$2 = vT0_3->f0;
+  vT0_3->f0 = new$SomeT0(vT0_5);
+  vT0_3->f0->rc ++;
+  $decr_OptT0(oldValue$2);
+  vT0_3->f0;
+  vT0_0->f0;
+  vT0_5->f0;
+  int ret$3 = 0;
+  $decr_T0(vT0_4);
+  $decr_T0(vT0_3);
+  $decr_T0(vT0_5);
+  $decr_T0(vT0_6);
+  $decr_T0(vT0_1);
+  $decr_T0(vT0_2);
+  $decr_T0(vT0_0);
+  processAllPCRs();
+  return ret$3;
+}
diff --git a/src/main/resources/runtime/runtime.h b/src/main/resources/runtime/runtime.h
index 5e292c2..e07a5b5 100644
--- a/src/main/resources/runtime/runtime.h
+++ b/src/main/resources/runtime/runtime.h
@@ -73,7 +73,7 @@ void addPCR(
   }
 
   struct PCR *pcr = malloc(sizeof(struct PCR));
-  fprintf(stderr, "[addPCR] Added PCR %p, prev = %p, scc: %d\n", pcr, *prev, scc);
+  fprintf(stderr, "[addPCR] Added PCR %p, prev = %p, scc: %d, obj: %p\n", pcr, *prev, scc, obj);
   pcr->obj = obj;
   pcr->markGray = markGray;
   pcr->scan = scan;
@@ -103,28 +103,46 @@ void removePCR(Common *obj, int scc)
   obj->addedPCR = 0;
   fprintf(stderr, "[removePCR] Trying to remove %p\n", obj);
 
+  struct PCRBucket **prevBucket = &pcrBuckets;
   struct PCRBucket *bucket = pcrBuckets;
   while (bucket->scc != scc)
   {
+    prevBucket = &bucket->next;
     bucket = bucket->next;
   }
 
-  struct PCR *head = bucket->first;
-  struct PCR **prev = &bucket->first;
+  if (bucket->first->obj == obj) {
+    if (bucket->last->obj == obj) {
+      // This was the only PCR in the bucket, so get rid of the bucket too
+      free(bucket->first);
+      *prevBucket = bucket->next;
+      free(bucket);
+      return;
+    } else {
+      bucket->first = bucket->first->next;
+      free(bucket->first);
+      return;
+    }
+  }
+
+  struct PCR *prev = bucket->first;
+  struct PCR *head = prev->next;
   while (head != NULL)
   {
     fprintf(stderr, "[removePCR] head = %p\n", head);
     if (head->obj == obj)
     {
-      fprintf(stderr, "[removePCR] Removed %p\n", head);
-      struct PCR *next = head->next;
+      fprintf(stderr, "[removePCR] Removed %p (obj=%p)\n", head, obj);
+      prev->next = head->next;
+      if (head == bucket->last) {
+        bucket->last = prev;
+      }
       free(head);
-      *prev = next;
       break;
     }
     else
     {
-      prev = &head->next;
+      prev = head;
       head = head->next;
     }
   }
diff --git a/src/test/scala/fred/FuzzTests.scala b/src/test/scala/fred/FuzzTests.scala
index 3c5b550..043d983 100644
--- a/src/test/scala/fred/FuzzTests.scala
+++ b/src/test/scala/fred/FuzzTests.scala
@@ -15,8 +15,8 @@ class FuzzTests
   property("Simple generated programs") {
     given PropertyCheckConfiguration =
       PropertyCheckConfiguration(minSize = 1, sizeRange = 10)
-    forAll(GenerateTypes.genTypesAux().flatMap(GenerateTypes.genCode)) {
-      parsedFile => ExecTests.valgrindCheck(parsedFile, None, None)
+    forAll(GenUtil.genTypesAux().flatMap(GenUtil.genCode)) { parsedFile =>
+      ExecTests.valgrindCheck(parsedFile, None, None)
     }
   }
 
@@ -24,112 +24,17 @@ class FuzzTests
     given PropertyCheckConfiguration =
       PropertyCheckConfiguration(minSize = 1, sizeRange = 30)
 
-    def genVars(
-        typ: TypeDef,
-        prevTypes: Map[String, Map[String, Expr]]
-    ): Gen[Map[String, Expr]] = {
-      for {
-        size <- Gen.size
-        numVars <- Gen.choose(1, math.sqrt(size).ceil.toInt)
-        values <- GenerateTypes.sequence(1.to(numVars).map { i =>
-          GenerateTypes
-            .sequence(typ.cases.head.fields.map { field =>
-              prevTypes.get(field.typ.name) match {
-                case Some(vars) =>
-                  Gen
-                    .oneOf(vars.keySet)
-                    .map(field.name -> VarRef(_, Span.synth))
-                case None =>
-                  Gen.const(
-                    field.name -> CtorCall(
-                      Spanned(
-                        s"None${field.typ.name.stripPrefix("Opt")}",
-                        Span.synth
-                      ),
-                      Nil,
-                      Span.synth
-                    )
-                  )
-              }
-            })
-            .map { fieldArgs =>
-              CtorCall(typ.cases.head.name, fieldArgs, Span.synth)
-            }
-        })
-      } yield values.zipWithIndex.map { (value, i) =>
-        s"v${typ.name}_$i" -> value
-      }.toMap
-    }
-
-    /** @return A list of expressions that assign objects to create cycles */
-    def createCycles(
-        typ: TypeDef,
-        allVars: Map[String, Set[String]]
-    ): Gen[List[Expr]] = {
-      val currVars = allVars(typ.name)
-      GenerateTypes
-        .sequence(
-          typ.cases.head.fields.filter(_.typ.name.startsWith("Opt")).map {
-            field =>
-              val fieldType = field.typ.name.stripPrefix("Opt")
-              val candidates = allVars
-                .getOrElse(
-                  fieldType,
-                  throw new RuntimeException(
-                    s"[createCycles] No such type: $fieldType"
-                  )
-                )
-              Gen.someOf(currVars).flatMap { vars =>
-                GenerateTypes.sequence(vars.map { varName =>
-                  Gen.oneOf(candidates).map { ref =>
-                    SetFieldExpr(
-                      Spanned(varName, Span.synth),
-                      field.name,
-                      CtorCall(
-                        Spanned(s"Some$fieldType", Span.synth),
-                        List(
-                          (
-                            Spanned(GenerateTypes.SomeField, Span.synth),
-                            VarRef(ref, Span.synth)
-                          )
-                        ),
-                        Span.synth
-                      ),
-                      Span.synth
-                    )
-                  }
-                })
-              }
-          }
-        )
-        .map(_.flatten)
-    }
-
     val genFull = for {
       size <- Gen.size
-      rawTypes <- GenerateTypes.genTypesFullRandom(math.sqrt(size).ceil.toInt)
-      allTypes = GenerateTypes.addOptTypes(rawTypes)
+      numTypes <- Gen.choose(1, math.sqrt(size).toInt)
+      rawTypes <- GenUtil.genTypesFullRandom(numTypes)
+      allTypes = GenUtil.addOptTypes(rawTypes)
       sccs = Cycles.fromFile(ParsedFile(allTypes, Nil)).sccs
-      allVars <- sccs.foldRight(
-        Gen.const(Map.empty[String, Map[String, Expr]])
-      ) { (scc, prevVars) =>
-        prevVars.flatMap { prevVars =>
-          GenerateTypes
-            .sequence(
-              scc
-                .filterNot(_.name.startsWith("Opt"))
-                .map(typ => genVars(typ, prevVars).map(typ.name -> _))
-            )
-            .map(_.toMap ++ prevVars)
-        }
-      }
-      assignExprs <- GenerateTypes
-        .sequence(
-          allTypes.filterNot(_.name.startsWith("Opt")).map { typ =>
-            createCycles(typ, allVars.view.mapValues(_.keySet).toMap)
-          }
-        )
-        .map(_.flatten)
+      allVars <- GenUtil.genVars(sccs, size / numTypes)
+      assignExprs <- GenUtil.genAssignments(
+        allTypes.filterNot(_.name.startsWith("Opt")),
+        allVars.view.mapValues(_.keySet).toMap
+      )
     } yield {
       val finalRes = IntLiteral(0, Span.synth)
       val withAssignments = assignExprs.foldRight(finalRes: Expr)(
diff --git a/src/test/scala/fred/GenerateTypes.scala b/src/test/scala/fred/GenUtil.scala
similarity index 73%
rename from src/test/scala/fred/GenerateTypes.scala
rename to src/test/scala/fred/GenUtil.scala
index 8be28cf..d60273d 100644
--- a/src/test/scala/fred/GenerateTypes.scala
+++ b/src/test/scala/fred/GenUtil.scala
@@ -4,7 +4,7 @@ import scala.jdk.CollectionConverters.*
 
 import org.scalacheck.Gen
 
-object GenerateTypes {
+object GenUtil {
   val SomeField = "value"
 
   case class GenTypeAux(refs: List[Int])
@@ -80,6 +80,122 @@ object GenerateTypes {
     optTypes ::: safeTypes
   }
 
+  /** Create a bunch of variables of the given types
+    *
+    * @param sccs
+    * @param numVars
+    *   Number of variables per type
+    * @return
+    *   Maps type names to variables for that type. The inner map maps variable
+    *   names to expressions for their values
+    */
+  def genVars(
+      sccs: List[Set[TypeDef]],
+      numVars: Int
+  ): Gen[Map[String, Map[String, Expr]]] = {
+    def helper(
+        typ: TypeDef,
+        prevTypes: Map[String, Map[String, Expr]]
+    ): Gen[Map[String, Expr]] = {
+      GenUtil
+        .sequence(1.to(numVars).map { i =>
+          GenUtil
+            .sequence(typ.cases.head.fields.map { field =>
+              prevTypes.get(field.typ.name) match {
+                case Some(vars) =>
+                  Gen
+                    .oneOf(vars.keySet)
+                    .map(field.name -> VarRef(_, Span.synth))
+                case None =>
+                  Gen.const(
+                    field.name -> CtorCall(
+                      Spanned(
+                        s"None${field.typ.name.stripPrefix("Opt")}",
+                        Span.synth
+                      ),
+                      Nil,
+                      Span.synth
+                    )
+                  )
+              }
+            })
+            .map { fieldArgs =>
+              CtorCall(typ.cases.head.name, fieldArgs, Span.synth)
+            }
+        })
+        .map { values =>
+          values.zipWithIndex.map { (value, i) =>
+            s"v${typ.name}_$i" -> value
+          }.toMap
+        }
+    }
+
+    sccs.foldRight(
+      Gen.const(Map.empty[String, Map[String, Expr]])
+    ) { (scc, prevVars) =>
+      prevVars.flatMap { prevVars =>
+        GenUtil
+          .sequence(
+            scc
+              .filterNot(_.name.startsWith("Opt"))
+              .map(typ => helper(typ, prevVars).map(typ.name -> _))
+          )
+          .map(_.toMap ++ prevVars)
+      }
+    }
+  }
+
+  /** @return
+    *   A list of expressions that assign objects to fields inside other
+    *   objects. Returned list is not shuffled
+    */
+  def genAssignments(
+      types: List[TypeDef],
+      vars: Map[String, Set[String]]
+  ): Gen[List[Expr]] = {
+    def helper(typ: TypeDef): Gen[List[Expr]] = {
+      val currVars = vars(typ.name)
+      GenUtil
+        .sequence(
+          typ.cases.head.fields.filter(_.typ.name.startsWith("Opt")).map {
+            field =>
+              val fieldType = field.typ.name.stripPrefix("Opt")
+              val candidates = vars
+                .getOrElse(
+                  fieldType,
+                  throw new RuntimeException(
+                    s"[createCycles] No such type: $fieldType"
+                  )
+                )
+              Gen.someOf(currVars).flatMap { vars =>
+                GenUtil.sequence(vars.map { varName =>
+                  Gen.oneOf(candidates).map { ref =>
+                    SetFieldExpr(
+                      Spanned(varName, Span.synth),
+                      field.name,
+                      CtorCall(
+                        Spanned(s"Some$fieldType", Span.synth),
+                        List(
+                          (
+                            Spanned(GenUtil.SomeField, Span.synth),
+                            VarRef(ref, Span.synth)
+                          )
+                        ),
+                        Span.synth
+                      ),
+                      Span.synth
+                    )
+                  }
+                })
+              }
+          }
+        )
+        .map(_.flatten)
+    }
+
+    GenUtil.sequence(types.map(helper)).map(_.flatten)
+  }
+
   /** Generate a bunch of types. Each type is its own strongly-connected
     * component. Types that come later in the list can have references to types
     * that come earlier in the list, as well as themselves.
diff --git a/src/test/scala/fred/SCCTests.scala b/src/test/scala/fred/SCCTests.scala
index 27cf8c5..7a2c136 100644
--- a/src/test/scala/fred/SCCTests.scala
+++ b/src/test/scala/fred/SCCTests.scala
@@ -14,7 +14,7 @@ class SCCTests
   /** This is just a helper to avoid typing out calls to the TypeDef ctor */
   def createFile(graph: Map[String, Set[(Boolean, String)]]): ParsedFile = {
     ParsedFile(
-      GenerateTypes.toTypeDefs(
+      GenUtil.toTypeDefs(
         graph.view
           .mapValues(_.map((mut, typeName) => (mut, s"f$typeName", typeName)))
           .toMap
@@ -124,7 +124,7 @@ class SCCTests
     // This doesn't check that the algorithm doesn't lump every single type
     // into the same SCC.
     forAll(
-      Gen.sized { GenerateTypes.genTypesFullRandom(_) }.map(ParsedFile(_, Nil))
+      Gen.sized { GenUtil.genTypesFullRandom(_) }.map(ParsedFile(_, Nil))
     ) { file =>
       val cycles = Cycles.fromFile(file)
       validateSccs(file, cycles)
@@ -138,8 +138,8 @@ class SCCTests
     // references to types that come earlier in the list (or themselves).
     // Therefore, each type is its own strongly-connected component.
     val gen = Gen.sized { numSccs =>
-      GenerateTypes.sequence(0.until(numSccs).map { i =>
-        Gen.listOf(Gen.chooseNum(0, i)).map(GenerateTypes.GenTypeAux(_))
+      GenUtil.sequence(0.until(numSccs).map { i =>
+        Gen.listOf(Gen.chooseNum(0, i)).map(GenUtil.GenTypeAux(_))
       })
     }