From 47f1c613515a91ee3872b488fbceff82412de0eb Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 8 Jul 2019 16:46:28 -0400
Subject: [PATCH 065/108] FIXME: stabilize the sort within the worklist

---
 gcc/analyzer/impl-4.cc                          | 125 ++++++++++++++++++++++--
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c |  14 +--
 gcc/testsuite/gcc.dg/analyzer/paths-3.c         |   2 +-
 3 files changed, 128 insertions(+), 13 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index a2d624d..52aeece 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -309,7 +309,19 @@ public:
 
   int calc_recursion_depth () const;
 
+  static int cmp (const call_string &a,
+		  const call_string &b);
+
+  unsigned length () const { return m_return_edges.length (); }
+  const return_superedge *operator[] (unsigned idx) const
+  {
+    return m_return_edges[idx];
+  }
+
 private:
+  static int cmp_1 (const call_string &a,
+		    const call_string &b);
+
   auto_vec<const return_superedge *> m_return_edges;
 };
 
@@ -2183,6 +2195,69 @@ call_string::calc_recursion_depth () const
   return result;
 }
 
+/* Comparator for call strings.
+   Return negative if A is before B.
+   Return positive if B is after A.
+   Return 0 if they are equal.  */
+
+int
+call_string::cmp (const call_string &a,
+		  const call_string &b)
+{
+  int result = cmp_1 (a, b);
+
+  /* Check that the ordering is symmetric  */
+#if CHECKING_P
+  int reversed = cmp_1 (b, a);
+  gcc_assert (reversed == -result);
+#endif
+
+  /* We should only have 0 for equal pairs.  */
+  gcc_assert (result != 0
+	      || a == b);
+
+  return result;
+}
+
+/* Implementation of call_string::cmp.
+   This implements a version of lexicographical order.  */
+
+int
+call_string::cmp_1 (const call_string &a,
+		    const call_string &b)
+{
+  unsigned len_a = a.length ();
+  unsigned len_b = b.length ();
+
+  unsigned i = 0;
+  while (1)
+    {
+      /* Consider index i; the strings have been equal up to it.  */
+
+      /* Have both strings run out?  */
+      if (i >= len_a && i >= len_b)
+	return 0;
+
+      /* Otherwise, has just one of the strings run out?  */
+      if (i >= len_a)
+	return 1;
+      if (i >= len_b)
+	return -1;
+
+      /* Otherwise, compare the edges.  */
+      const return_superedge *edge_a = a[i];
+      const return_superedge *edge_b = b[i];
+      int src_cmp = edge_a->m_src->m_index - edge_b->m_src->m_index;
+      if (src_cmp)
+	return src_cmp;
+      int dest_cmp = edge_a->m_dest->m_index - edge_b->m_dest->m_index;
+      if (dest_cmp)
+	return dest_cmp;
+      i++;
+      // TODO: test coverage for this
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* FIXME.  */
@@ -5256,13 +5331,25 @@ worklist::key_t::cmp_1 (const worklist::key_t &ka, const worklist::key_t &kb)
   /* Order within supernode via program point.  */
   const program_point &point_a = ka.m_enode->get_point ();
   const program_point &point_b = kb.m_enode->get_point ();
-  int cmp = program_point::cmp_within_supernode (point_a, point_b);
-  if (cmp)
-    return cmp;
+  int within_snode_cmp = program_point::cmp_within_supernode (point_a, point_b);
+  if (within_snode_cmp)
+    return within_snode_cmp;
 
-  /* FIXME: otherwise, order by state, somehow, or
-     we'll have an unstable sort... */
-  return 0;
+  /* The points might vary by callstring; try sorting by callstring.  */
+  const call_string &call_string_a = point_a.m_call_string;
+  const call_string &call_string_b = point_b.m_call_string;
+  int cs_cmp = call_string::cmp (call_string_a, call_string_b);
+  if (cs_cmp)
+    return cs_cmp;
+
+  /* Otherwise, we ought to have the same program_point.  */
+  gcc_assert (point_a == point_b);
+
+  /* Otherwise, we have two enodes at the same program point but with
+     different states.  We don't have a good total ordering on states,
+     so order them by enode index, so that we have at least have a
+     stable sort.  */
+  return ka.m_enode->m_index - kb.m_enode->m_index;
 }
 
 /* Comparator for implementing worklist::key_t comparison operators.
@@ -5281,6 +5368,11 @@ worklist::key_t::cmp (const worklist::key_t &ka, const worklist::key_t &kb)
   gcc_assert (reversed == -result);
 #endif
 
+  /* We should only have 0 for equal (point, state) pairs.  */
+  gcc_assert (result != 0
+	      || (*ka.m_enode->get_ps_key ()
+		  == *kb.m_enode->get_ps_key ()));
+
   return result;
 }
 
@@ -7318,6 +7410,26 @@ test_sm_state_map ()
   // TODO: coverage for purging
 }
 
+/* FIXME.  */
+
+static void
+test_program_point_equality ()
+{
+  const supernode *snode = NULL;
+
+  const impl_4::call_string cs;
+
+  program_point a = program_point (snode, NULL, 0,
+				   impl_4::PK_BEFORE_SUPERNODE,
+				   cs);
+
+  program_point b = program_point (snode, NULL, 0,
+				   impl_4::PK_BEFORE_SUPERNODE,
+				   cs);
+  ASSERT_EQ (a, b);
+  // TODO: verify with non-empty callstrings, with different edges
+}
+
 /* Verify that program_point::cmp_within_supernode works as expected.  */
 
 static void
@@ -7382,6 +7494,7 @@ void
 analyzer_impl_4_cc_tests ()
 {
   test_sm_state_map ();
+  test_program_point_equality ();
   test_program_point_ordering ();
   test_assignments ();
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
index 176ed46..860e839 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
@@ -44,11 +44,13 @@ int test_repeated_predicate_1 (int n)
     ptr = buf;
 
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
-  // FIXME: why 3 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
+  // FIXME: why 5 here?
 
   if (n > 10)
     free (ptr); /* { dg-bogus "not on the heap" } */
@@ -102,8 +104,8 @@ int test_explicit_flag (int n)
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
-  // FIXME: why 3 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
+  // FIXME: why 5 here?
 
   if (need_to_free)
     free (ptr); /* { dg-bogus "not on the heap" } */
@@ -128,8 +130,8 @@ int test_pointer_comparison (int n)
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "7 exploded nodes" } */
-  // FIXME: why 7 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
+  // FIXME: why 5 here?
 
   if (ptr != buf)
     free (ptr); /* { dg-bogus "not on the heap" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-3.c b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
index bee0dd7..cf793fd 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
@@ -35,7 +35,7 @@ int test_2 (int a, int b)
       p = malloc (32);
 
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
 
   if (a > 6) /* different condition */
     {
-- 
1.8.5.3

