Hi,

Using attached untested patch, I managed to minimize a test-case failure for PR 66714.

The patch introduces two-phase marking in gt_cleare_cache:
- first phase, it loops over all the hash table entries and removes
  those which are dead
- second phase, it runs over all the live hash table entries and marks
  live items that are reachable from those live entries

By doing so, we make the behaviour of gt_cleare_cache independent of the order in which the entries are visited, turning:
- hard-to-trigger bugs which trigger for one visiting order but not for
  another, into
- more easily triggered bugs which trigger for any visiting order.

Any comments?

Thanks,
- Tom
Add checking in gt_cleare_cache

---
 gcc/hash-table.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index 12e0c96..c2ea112 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -1046,7 +1046,37 @@ gt_cleare_cache (hash_table<H> *h)
   if (!h)
     return;
 
-  for (typename table::iterator iter = h->begin (); iter != h->end (); ++iter)
+  typename table::iterator iter;
+
+#ifdef CHECKING
+  /* Say we have:
+     1. cache entry A, with keep_change_entry (A) == 1, and
+     2. cache entry B, with keep_change_entry (B) == 0, and
+     3. gt_ggc_mx (A) marks things live in such a way that keep_change_entry (B)
+        becomes 1.
+
+     In the loop at the end of the function, if A is visited first, then B is
+     kept.  If B is visited first, it is deleted.
+
+     We don't want the situation that the result of this function is dependent
+     on the order in which the entries are visited, so we consider this
+     situation a bug.
+
+     In order to stabilize the result of the function in presence of the bug, we
+     first clear all entries E with keep_change_entry (E) == 0.  By doing so, we
+     also maximize the impact of the liveness analysis done up until now, which
+     we hope makes it more likely that we run into bugs regarding that analysis.
+     We only do this when checking since it's more expensive.  */
+  for (iter = h->begin (); iter != h->end (); ++iter)
+    if (!table::is_empty (*iter) && !table::is_deleted (*iter))
+      {
+	int res = H::keep_cache_entry (*iter);
+	if (res == 0)
+	  h->clear_slot (&*iter);
+      }
+#endif
+
+  for (iter = h->begin (); iter != h->end (); ++iter)
     if (!table::is_empty (*iter) && !table::is_deleted (*iter))
       {
 	int res = H::keep_cache_entry (*iter);
-- 
1.9.1

Reply via email to