diff mbox series

[committed] analyzer: fix false leak reports when merging states [PR97074]

Message ID 20210107024849.3181454-1-dmalcolm@redhat.com
State New
Headers show
Series [committed] analyzer: fix false leak reports when merging states [PR97074] | expand

Commit Message

David Malcolm Jan. 7, 2021, 2:48 a.m. UTC
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-6513-gbe6c485b24f2b47ac85380dd2bea025d353f1f91.

gcc/analyzer/ChangeLog:
	PR analyzer/97074
	* store.cc (binding_cluster::can_merge_p): Add "out_store" param
	and pass to calls to binding_cluster::make_unknown_relative_to.
	(binding_cluster::make_unknown_relative_to): Add "out_store"
	param.  Use it to mark base regions that are pointed to by
	pointers that become unknown as having escaped.
	(store::can_merge_p): Pass out_store to
	binding_cluster::can_merge_p.
	* store.h (binding_cluster::can_merge_p): Add "out_store" param.
	(binding_cluster::make_unknown_relative_to): Likewise.
	* svalue.cc (region_svalue::implicitly_live_p): New vfunc.
	* svalue.h (region_svalue::implicitly_live_p): New vfunc decl.

gcc/testsuite/ChangeLog:
	PR analyzer/97074
	* gcc.dg/analyzer/pr97074.c: New test.
---
 gcc/analyzer/store.cc                   | 23 +++++++++++++++---
 gcc/analyzer/store.h                    |  2 ++
 gcc/analyzer/svalue.cc                  | 16 +++++++++++++
 gcc/analyzer/svalue.h                   |  2 ++
 gcc/testsuite/gcc.dg/analyzer/pr97074.c | 32 +++++++++++++++++++++++++
 5 files changed, 72 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr97074.c
diff mbox series

Patch

diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index b4a4d5f3bb2..23118d05685 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -1177,6 +1177,7 @@  bool
 binding_cluster::can_merge_p (const binding_cluster *cluster_a,
 			      const binding_cluster *cluster_b,
 			      binding_cluster *out_cluster,
+			      store *out_store,
 			      store_manager *mgr,
 			      model_merger *merger)
 {
@@ -1197,14 +1198,14 @@  binding_cluster::can_merge_p (const binding_cluster *cluster_a,
     {
       gcc_assert (cluster_b != NULL);
       gcc_assert (cluster_b->m_base_region == out_cluster->m_base_region);
-      out_cluster->make_unknown_relative_to (cluster_b, mgr);
+      out_cluster->make_unknown_relative_to (cluster_b, out_store, mgr);
       return true;
     }
   if (cluster_b == NULL)
     {
       gcc_assert (cluster_a != NULL);
       gcc_assert (cluster_a->m_base_region == out_cluster->m_base_region);
-      out_cluster->make_unknown_relative_to (cluster_a, mgr);
+      out_cluster->make_unknown_relative_to (cluster_a, out_store, mgr);
       return true;
     }
 
@@ -1298,6 +1299,7 @@  binding_cluster::can_merge_p (const binding_cluster *cluster_a,
 
 void
 binding_cluster::make_unknown_relative_to (const binding_cluster *other,
+					   store *out_store,
 					   store_manager *mgr)
 {
   for (map_t::iterator iter = other->m_map.begin ();
@@ -1309,6 +1311,21 @@  binding_cluster::make_unknown_relative_to (const binding_cluster *other,
 	= mgr->get_svalue_manager ()->get_or_create_unknown_svalue
 	  (iter_sval->get_type ());
       m_map.put (iter_key, unknown_sval);
+
+      /* For any pointers in OTHER, the merger means that the
+	 concrete pointer becomes an unknown value, which could
+	 show up as a false report of a leak when considering what
+	 pointers are live before vs after.
+	 Avoid this by marking the base regions they point to as having
+	 escaped.  */
+      if (const region_svalue *region_sval
+	  = iter_sval->dyn_cast_region_svalue ())
+	{
+	  const region *base_reg
+	    = region_sval->get_pointee ()->get_base_region ();
+	  binding_cluster *c = out_store->get_or_create_cluster (base_reg);
+	  c->mark_as_escaped ();
+	}
     }
 }
 
@@ -2092,7 +2109,7 @@  store::can_merge_p (const store *store_a, const store *store_b,
       binding_cluster *out_cluster
 	= out_store->get_or_create_cluster (base_reg);
       if (!binding_cluster::can_merge_p (cluster_a, cluster_b,
-					 out_cluster, mgr, merger))
+					 out_cluster, out_store, mgr, merger))
 	return false;
     }
   return true;
diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h
index 2f97f4b68a5..366439ce2dd 100644
--- a/gcc/analyzer/store.h
+++ b/gcc/analyzer/store.h
@@ -434,9 +434,11 @@  public:
   static bool can_merge_p (const binding_cluster *cluster_a,
 			   const binding_cluster *cluster_b,
 			   binding_cluster *out_cluster,
+			   store *out_store,
 			   store_manager *mgr,
 			   model_merger *merger);
   void make_unknown_relative_to (const binding_cluster *other_cluster,
+				 store *out_store,
 				 store_manager *mgr);
 
   void mark_as_escaped ();
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index b2d98cfcac6..5bbd05e8327 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -509,6 +509,22 @@  region_svalue::accept (visitor *v) const
   m_reg->accept (v);
 }
 
+/* Implementation of svalue::implicitly_live_p vfunc for region_svalue.  */
+
+bool
+region_svalue::implicitly_live_p (const svalue_set &,
+				  const region_model *model) const
+{
+  /* Pointers into clusters that have escaped should be treated as live.  */
+  const region *base_reg = get_pointee ()->get_base_region ();
+  const store *store = model->get_store ();
+  if (const binding_cluster *c = store->get_cluster (base_reg))
+    if (c->escaped_p ())
+	return true;
+
+  return false;
+}
+
 /* Evaluate the condition LHS OP RHS.
    Subroutine of region_model::eval_condition for when we have a pair of
    pointers.  */
diff --git a/gcc/analyzer/svalue.h b/gcc/analyzer/svalue.h
index 336c0b601c4..0703cac8bb3 100644
--- a/gcc/analyzer/svalue.h
+++ b/gcc/analyzer/svalue.h
@@ -194,6 +194,8 @@  public:
 
   void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
   void accept (visitor *v) const FINAL OVERRIDE;
+  bool implicitly_live_p (const svalue_set &,
+			  const region_model *) const FINAL OVERRIDE;
 
   const region * get_pointee () const { return m_reg; }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr97074.c b/gcc/testsuite/gcc.dg/analyzer/pr97074.c
new file mode 100644
index 00000000000..ccb3b61bd41
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr97074.c
@@ -0,0 +1,32 @@ 
+#include "analyzer-decls.h"
+#define NULL ((void *)0)
+
+void *x, *y;
+
+void test_1 (int flag)
+{
+  void *p = __builtin_malloc (1024);
+  if (flag)
+    x = p;
+  else
+    y = p;
+} /* { dg-bogus "leak" } */
+
+struct s2
+{
+  void *f1;
+  void *f2;
+};
+
+struct s2 test_2 (int flag)
+{
+  struct s2 r;
+  r.f1 = NULL;
+  r.f2 = NULL;
+  void *p = __builtin_malloc (1024);
+  if (flag)
+    r.f1 = p;
+  else
+    r.f2 = p;
+  return r;
+}