From e26c1fd954d25b3de98e6804412e3c5080ef8e95 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 16 Jul 2019 13:59:59 -0400
Subject: [PATCH 092/108] FIXME: retain svalue sharing when merging
 region_models

---
 gcc/analyzer/region-model.cc                      | 42 +++++++++++++++++++++--
 gcc/testsuite/gcc.dg/analyzer/loop-2.c            |  7 ++--
 gcc/testsuite/gcc.dg/analyzer/loop.c              |  2 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c | 14 ++++----
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c |  2 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c   | 23 ++++++-------
 gcc/testsuite/gcc.dg/analyzer/paths-3.c           |  8 ++---
 gcc/testsuite/gcc.dg/analyzer/paths-4.c           |  4 +--
 gcc/testsuite/gcc.dg/analyzer/paths-7.c           |  4 +--
 9 files changed, 70 insertions(+), 36 deletions(-)

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 7525c2b..8deb91e 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4442,6 +4442,23 @@ model_merger::can_merge_values_p (svalue_id sid_a,
   if (!(sval_a && sval_b))
     return false;
 
+  /* Have they both already been mapped to the same new svalue_id?
+     If so, use it.  */
+  if (m_svalue_id_mapping)
+    {
+      svalue_id sid_a_in_m
+	= m_svalue_id_mapping->m_map_from_a_to_m.get_dst_for_src (sid_a);
+      svalue_id sid_b_in_m
+	= m_svalue_id_mapping->m_map_from_b_to_m.get_dst_for_src (sid_b);
+      if (!sid_a_in_m.null_p ()
+	  && !sid_b_in_m.null_p ()
+	  && sid_a_in_m == sid_b_in_m)
+	{
+	  *merged_sid = sid_a_in_m;
+	  return true;
+	}
+    }
+
 #if 0
   gcc_assert (sval_a->get_type () == sval_b->get_type ()
 	      || sval_a->get_type () == NULL_TREE
@@ -4456,8 +4473,6 @@ model_merger::can_merge_values_p (svalue_id sid_a,
   if (sval_a->get_kind () != sval_b->get_kind ()
       || sval_a->get_kind () == SK_UNKNOWN)
     {
-      /* FIXME: this loses information about unknown values
-	 that are shared between multiple regions.  */
       svalue *merged_sval = new unknown_svalue (type);
       *merged_sid = m_merged_model->add_svalue (merged_sval);
       record_svalues (sid_a, sid_b, *merged_sid);
@@ -5488,6 +5503,29 @@ test_state_merging ()
     ASSERT_EQ (model0, merged);
   }
 
+  /* Two regions sharing the same unknown svalue should continue sharing
+     an unknown svalue after self-merger.  */
+  {
+    test_region_model_context ctxt;
+    region_model model0;
+    svalue_id sid
+      = model0.set_to_new_unknown_value (model0.get_lvalue (x, &ctxt),
+					 integer_type_node);
+    model0.set_value (model0.get_lvalue (y, &ctxt), sid);
+    region_model model1 (model0);
+
+    /* They should be mergeable, and the result should be the same
+       (this requires an svalue_id_merger_mapping to work).  */
+    region_model merged;
+    svalue_id_merger_mapping sid_mapping (model0, model1);
+    ASSERT_TRUE (model0.can_merge_with_p (model1, &merged, &sid_mapping));
+    ASSERT_EQ (model0, merged);
+
+    /* In particular, we should have x == y.  */
+    ASSERT_EQ (merged.eval_condition (x, EQ_EXPR, y, &ctxt),
+	       tristate (tristate::TS_TRUE));
+  }
+
 #if 0
   {
     region_model model0;
diff --git a/gcc/testsuite/gcc.dg/analyzer/loop-2.c b/gcc/testsuite/gcc.dg/analyzer/loop-2.c
index e2ccc5a..2a7d0d9 100644
--- a/gcc/testsuite/gcc.dg/analyzer/loop-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/loop-2.c
@@ -23,8 +23,7 @@ void test(void)
 
   for (s.i=0; s.i<256; s.i++) {
     __analyzer_eval (s.i < 256); /* { dg-warning "TRUE" } */
-    /* { dg-warning "UNKNOWN" "" { target *-*-* } .-1 } */
-    /* TODO: this should report TRUE on the 2nd time, also.  */
+      /* (should report TRUE twice). */
 
     __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
 
@@ -36,9 +35,7 @@ void test(void)
       //__analyzer_eval (s.i >= 0); /* { d-todo-g-warning "TRUE" } */
   }
 
-  __analyzer_eval (s.i >= 256); /* { dg-warning "TRUE" "" { xfail *-*-* } } */
-  /* { dg-warning "UNKNOWN" "" { target *-*-* } .-1 } */
-  /* TODO(xfail^^^): it should figures out i >= 256.  */
+  __analyzer_eval (s.i >= 256); /* { dg-warning "TRUE" } */
 
   __analyzer_eval (s.i == 256); /* { dg-warning "TRUE" "" { xfail *-*-* } } */
   /* { dg-warning "UNKNOWN" "" { target *-*-* } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/loop.c b/gcc/testsuite/gcc.dg/analyzer/loop.c
index 6ef404f..bcc8c2a 100644
--- a/gcc/testsuite/gcc.dg/analyzer/loop.c
+++ b/gcc/testsuite/gcc.dg/analyzer/loop.c
@@ -9,7 +9,7 @@ void test(void)
 
   for (i=0; i<256; i++) {
       __analyzer_eval (i < 256); /* { dg-warning "TRUE" } */
-      /* { d-g-warning "TRUE" "" { target *-*-* } .-1 } */
+      /* (should report TRUE twice). */
 
       __analyzer_eval (i == 0); /* { dg-warning "TRUE" } */
       /* { dg-warning "FALSE" "" { xfail *-*-* } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
index 6fa2c130..f252df1 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
@@ -39,9 +39,9 @@ int test_repeated_predicate_1 (int n)
     result = sum;
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  // FIXME: why 5 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "8 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "8 exploded nodes" } */
+  // FIXME: why 8 here?
 
   if (n > 10)
     free (ptr); /* { dg-bogus "not on the heap" } */
@@ -104,8 +104,8 @@ int test_explicit_flag (int n)
     result = sum;
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
-  // FIXME: why 6 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "8 exploded nodes" } */
+  // FIXME: why 8 here?
 
   if (need_to_free)
     free (ptr); /* { dg-bogus "not on the heap" } */
@@ -139,8 +139,8 @@ int test_pointer_comparison (int n)
     result = sum;
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
-  // FIXME: why 6 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "8 exploded nodes" } */
+  // FIXME: why 8 here?
 
   if (ptr != buf)
     free (ptr); /* { dg-bogus "not on the heap" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
index 4eeec0c..d3bff3f 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
@@ -39,7 +39,7 @@ int test_1 (int n)
   }
 
   __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" "" { xfail *-*-* } } */
-  /* { dg-bogus "4 exploded nodes" "" { xfail *-*-* } .-1 } */
+  /* { dg-bogus "5 exploded nodes" "" { xfail *-*-* } .-1 } */
   /* TODO(xfail): should be 2 nodes here, for the malloc-vs-non-malloc case; erroneously getting more.  */
 
   return result; /* { dg-message "leak of 'ptr'" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
index 35ebe7f..44ecf53 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
@@ -48,17 +48,15 @@ int test_repeated_predicate_1 (int n)
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  // FIXME: why 5 here?
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  // FIXME: why 5 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
+  // FIXME: why 6 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
+  // FIXME: why 6 here?
 
   if (n > 10)
     free (ptr); /* { dg-bogus "not on the heap" } */
 
-  return result; /* { dg-bogus "leak" "" { xfail *-*-* } } */
-  /* TODO(xfail): this emits a bogus leak message due to state-merging
-     losing the (n > 10) condition, I think.  */
+  return result; /* { dg-bogus "leak" } */
 }
 
 /* A simpler version of the above.  */
@@ -107,8 +105,8 @@ int test_explicit_flag (int n)
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  // FIXME: why 5 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
+  // FIXME: why 6 here?
 
   if (need_to_free)
     free (ptr); /* { dg-bogus "not on the heap" } */
@@ -133,11 +131,12 @@ int test_pointer_comparison (int n)
 
   result = do_stuff (ptr, n);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
-  // FIXME: why 5 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
+  // FIXME: why 6 here?
 
   if (ptr != buf)
-    free (ptr); /* { dg-bogus "not on the heap" } */
+    free (ptr); /* { dg-bogus "not on the heap" "" { xfail *-*-* } } */
+  /* FIXME: how does this fail?  */
 
   return result; /* { dg-bogus "leak" } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-3.c b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
index efdeb51..738ce98 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-3.c
@@ -12,8 +12,8 @@ int test_1 (int a, int b)
     else
       p = malloc (32);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "4 exploded nodes" } */
-  __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 "3 exploded nodes" } */
 
   if (a > 5)
     {
@@ -34,8 +34,8 @@ int test_2 (int a, int b)
     else
       p = malloc (32);
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "4 exploded nodes" } */
-  __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 "3 exploded nodes" } */
 
   if (a > 6) /* different condition */
     {
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-4.c b/gcc/testsuite/gcc.dg/analyzer/paths-4.c
index 7a55f27..bdcc451 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-4.c
@@ -14,7 +14,7 @@ int test_1 (struct state *s)
   while (1)
     {
       __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
-      __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+      __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 exploded node" } */
       do_stuff (s, s->mode);
     }
 }
@@ -25,7 +25,7 @@ int test_2 (struct state *s)
   while (1)
     {
       __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 "1 exploded node" } */
       switch (s->mode)
 	{
 	case 0:
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-7.c b/gcc/testsuite/gcc.dg/analyzer/paths-7.c
index 6ed2f51..905bded 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-7.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-7.c
@@ -23,8 +23,8 @@ int test (int flag, void *ptr, int *p, int n)
     sum += foo (p[i]); /* { dg-bogus "uninitialized" } */
   result = sum;
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
-  // FIXME: why 6 here?
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "5 exploded nodes" } */
+  // FIXME: why 5 here?
 
   if (flag)
     free (ptr); /* { dg-warning "double-free of 'ptr'" } */
-- 
1.8.5.3

