From df1d8270f119f5fbaa04cea87f3051571909859b Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 3 Jul 2019 14:53:18 -0400
Subject: [PATCH 62/63] FIXME: fixes and testing for program_point ordering

---
 gcc/analyzer/impl-4.cc                  | 273 +++++++++++++++++++++++---------
 gcc/analyzer/region-model.cc            |   2 +-
 gcc/testsuite/gcc.dg/analyzer/paths-3.c |   2 +-
 3 files changed, 201 insertions(+), 76 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index b2695e6..a2d624d 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -459,6 +459,11 @@ public:
   bool on_edge (exploded_graph &eg,
 		const superedge *succ);
 
+  static int cmp_within_supernode_1 (const program_point &point_a,
+				     const program_point &point_b);
+  static int cmp_within_supernode (const program_point &point_a,
+				   const program_point &point_b);
+
 //private:
   const supernode *m_supernode;
   const superedge *m_from_edge; /* for PK_BEFORE_SUPERNODE.  */
@@ -1248,6 +1253,7 @@ private:
     }
 
   private:
+    static int cmp_1 (const key_t &ka, const key_t &kb);
     static int cmp (const key_t &ka, const key_t &kb);
 
     int get_scc_id (const exploded_node *enode) const
@@ -2312,6 +2318,125 @@ program_point::get_stmt () const
     return NULL;
 }
 
+/* Comparator for program points within the same supernode,
+   for implementing worklist::key_t comparison operators.
+   Return negative if POINT_A is before POINT_B
+   Return positive if POINT_A is after POINT_B
+   Return 0 if they are equal.  */
+
+int
+program_point::cmp_within_supernode_1 (const program_point &point_a,
+				       const program_point &point_b)
+{
+  gcc_assert (point_a.m_supernode == point_b.m_supernode);
+
+  switch (point_a.m_kind)
+    {
+    default:
+      gcc_unreachable ();
+    case PK_BEFORE_SUPERNODE:
+      switch (point_b.m_kind)
+	{
+	default:
+	  gcc_unreachable ();
+	case PK_BEFORE_SUPERNODE:
+	  {
+	    int a_src_idx = -1;
+	    int b_src_idx = -1;
+	    if (point_a.m_from_edge)
+	      a_src_idx = point_a.m_from_edge->m_src->m_index;
+	    if (point_b.m_from_edge)
+	      b_src_idx = point_b.m_from_edge->m_src->m_index;
+	    return a_src_idx - b_src_idx;
+	  }
+	  break;
+
+	case PK_BEFORE_STMT:
+	case PK_AFTER_STMT:
+	case PK_AFTER_SUPERNODE:
+	  return -1;
+	}
+      break;
+    case PK_BEFORE_STMT:
+      switch (point_b.m_kind)
+	{
+	default:
+	  gcc_unreachable ();
+	case PK_BEFORE_SUPERNODE:
+	  return 1;
+
+	case PK_BEFORE_STMT:
+	  return point_a.m_stmt_idx - point_b.m_stmt_idx;
+
+	case PK_AFTER_STMT:
+	  if (point_a.m_stmt_idx != point_b.m_stmt_idx)
+	    return point_a.m_stmt_idx - point_b.m_stmt_idx;
+	  else
+	    return -1;
+
+	case PK_AFTER_SUPERNODE:
+	  return -1;
+	}
+      break;
+    case PK_AFTER_STMT:
+      switch (point_b.m_kind)
+	{
+	default:
+	  gcc_unreachable ();
+	case PK_BEFORE_SUPERNODE:
+	  return 1;
+
+	case PK_BEFORE_STMT:
+	  if (point_a.m_stmt_idx != point_b.m_stmt_idx)
+	    return point_a.m_stmt_idx - point_b.m_stmt_idx;
+	  else
+	    return 1;
+
+	case PK_AFTER_STMT:
+	  return point_a.m_stmt_idx - point_b.m_stmt_idx;
+
+	case PK_AFTER_SUPERNODE:
+	  return -1;
+	}
+      break;
+    case PK_AFTER_SUPERNODE:
+      switch (point_b.m_kind)
+	{
+	default:
+	  gcc_unreachable ();
+	case PK_BEFORE_SUPERNODE:
+	case PK_BEFORE_STMT:
+	case PK_AFTER_STMT:
+	  return 1;
+
+	case PK_AFTER_SUPERNODE:
+	  return 0;
+	}
+      break;
+    }
+}
+
+/* Comparator for program points within the same supernode,
+   for implementing worklist::key_t comparison operators.
+   Return negative if POINT_A is before POINT_B
+   Return positive if POINT_A is after POINT_B
+   Return 0 if they are equal.  */
+
+int
+program_point::cmp_within_supernode (const program_point &point_a,
+				     const program_point &point_b)
+{
+  int result = cmp_within_supernode_1 (point_a, point_b);
+
+  /* Check that the ordering is symmetric  */
+#if CHECKING_P
+  int reversed = cmp_within_supernode_1 (point_b, point_a);
+  gcc_assert (reversed == -result);
+#endif
+
+  return result;
+}
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* FIXME.  */
@@ -5097,7 +5222,7 @@ worklist::add_node (exploded_node *enode)
    Return 0 if they are equal.  */
 
 int
-worklist::key_t::cmp (const worklist::key_t &ka, const worklist::key_t &kb)
+worklist::key_t::cmp_1 (const worklist::key_t &ka, const worklist::key_t &kb)
 {
   /* First, order by SCC.  */
   int scc_id_a = ka.get_scc_id (ka.m_enode);
@@ -5126,87 +5251,37 @@ worklist::key_t::cmp (const worklist::key_t &ka, const worklist::key_t &kb)
   if (snode_a->m_index != snode_b->m_index)
     return snode_a->m_index - snode_b->m_index;
 
+  gcc_assert (snode_a == snode_b);
+
   /* 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 ();
-  switch (point_a.m_kind)
-    {
-    default:
-      gcc_unreachable ();
-    case PK_BEFORE_SUPERNODE:
-      switch (point_b.m_kind)
-	{
-	default:
-	  gcc_unreachable ();
-	case PK_BEFORE_SUPERNODE:
-	  return 0;
-
-	case PK_BEFORE_STMT:
-	case PK_AFTER_STMT:
-	case PK_AFTER_SUPERNODE:
-	  return -1;
-	}
-      break;
-    case PK_BEFORE_STMT:
-      switch (point_b.m_kind)
-	{
-	default:
-	  gcc_unreachable ();
-	case PK_BEFORE_SUPERNODE:
-	  return 1;
-
-	case PK_BEFORE_STMT:
-	  return point_a.m_stmt_idx - point_b.m_stmt_idx;
-
-	case PK_AFTER_STMT:
-	  if (point_a.m_stmt_idx != point_b.m_stmt_idx)
-	    return point_a.m_stmt_idx - point_b.m_stmt_idx;
-	  else
-	    return -1;
-
-	case PK_AFTER_SUPERNODE:
-	  return 1;
-	}
-      break;
-    case PK_AFTER_STMT:
-      switch (point_b.m_kind)
-	{
-	default:
-	  gcc_unreachable ();
-	case PK_BEFORE_SUPERNODE:
-	  return 1;
+  int cmp = program_point::cmp_within_supernode (point_a, point_b);
+  if (cmp)
+    return cmp;
 
-	case PK_BEFORE_STMT:
-	  if (point_a.m_stmt_idx != point_b.m_stmt_idx)
-	    return point_a.m_stmt_idx - point_b.m_stmt_idx;
-	  else
-	    return 1;
-
-	case PK_AFTER_STMT:
-	  return point_a.m_stmt_idx - point_b.m_stmt_idx;
+  /* FIXME: otherwise, order by state, somehow, or
+     we'll have an unstable sort... */
+  return 0;
+}
 
-	case PK_AFTER_SUPERNODE:
-	  return -1;
-	}
-      break;
-    case PK_AFTER_SUPERNODE:
-      switch (point_b.m_kind)
-	{
-	default:
-	  gcc_unreachable ();
-	case PK_BEFORE_SUPERNODE:
-	case PK_BEFORE_STMT:
-	case PK_AFTER_STMT:
-	  return 1;
+/* Comparator for implementing worklist::key_t comparison operators.
+   Return negative if KA is before KB
+   Return positive if KA is after KB
+   Return 0 if they are equal.  */
 
-	case PK_AFTER_SUPERNODE:
-	  return 0;
-	}
-      break;
-    }
+int
+worklist::key_t::cmp (const worklist::key_t &ka, const worklist::key_t &kb)
+{
+  int result = cmp_1 (ka, kb);
 
+  /* Check that the ordering is symmetric  */
+#if CHECKING_P
+  int reversed = cmp_1 (kb, ka);
+  gcc_assert (reversed == -result);
+#endif
 
-  return 0;
+  return result;
 }
 
 ////////////////////////////////////////////////////////////////////////////
@@ -7243,6 +7318,55 @@ test_sm_state_map ()
   // TODO: coverage for purging
 }
 
+/* Verify that program_point::cmp_within_supernode works as expected.  */
+
+static void
+test_program_point_ordering ()
+{
+  const supernode *snode = NULL;
+  const impl_4::call_string call_string;
+
+  /* Populate an array with various points within the same
+     snode, in order.  */
+  auto_delete_vec<program_point> points;
+  points.safe_push (new program_point (snode, NULL, 0,
+				       impl_4::PK_BEFORE_SUPERNODE,
+				       call_string));
+  points.safe_push (new program_point (snode, NULL, 0,
+				       impl_4::PK_BEFORE_STMT,
+				       call_string));
+  points.safe_push (new program_point (snode, NULL, 0,
+				       impl_4::PK_AFTER_STMT,
+				       call_string));
+  points.safe_push (new program_point (snode, NULL, 1,
+				       impl_4::PK_BEFORE_STMT,
+				       call_string));
+  points.safe_push (new program_point (snode, NULL, 1,
+				       impl_4::PK_AFTER_STMT,
+				       call_string));
+  points.safe_push (new program_point (snode, NULL, 0,
+				       impl_4::PK_AFTER_SUPERNODE,
+				       call_string));
+  /* Check all pairs.  */
+  unsigned i;
+  program_point *point_a;
+  FOR_EACH_VEC_ELT (points, i, point_a)
+    {
+      unsigned j;
+      program_point *point_b;
+      FOR_EACH_VEC_ELT (points, j, point_b)
+	{
+	  int cmp = program_point::cmp_within_supernode (*point_a, *point_b);
+	  if (i == j)
+	    ASSERT_EQ (cmp, 0);
+	  if (i < j)
+	    ASSERT_TRUE (cmp < 0);
+	  if (i > j)
+	    ASSERT_TRUE (cmp > 0);
+	}
+    }
+}
+
 /* FIXME.  */
 
 static void
@@ -7258,6 +7382,7 @@ void
 analyzer_impl_4_cc_tests ()
 {
   test_sm_state_map ();
+  test_program_point_ordering ();
   test_assignments ();
 }
 
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 0249b76..07bc03b 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -3967,7 +3967,7 @@ model_merger::can_merge_values_p (svalue_id sid_a,
       tree cst_a = cst_sval_a->get_constant ();
       tree cst_b = cst_sval_b->get_constant ();
       svalue *merged_sval;
-      if (cst_a == cst_b) // FIXME: other equality?
+      if (cst_a == cst_b) // FIXME: something other than pointer equality?
 	{
 	  /* If they are the same constant, merge as that constant value.  */
 	  merged_sval = new constant_svalue (cst_a);
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-3.c b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
index b6dea2d..bee0dd7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
@@ -34,7 +34,7 @@ int test_2 (int a, int b)
     else
       p = malloc (32);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
 
   if (a > 6) /* different condition */
-- 
1.8.5.3

