analyzer: fix false leak reports when merging states [PR97074]
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Jan 2021 02:44:07 +0000 (21:44 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Jan 2021 02:44:07 +0000 (21:44 -0500)
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
gcc/analyzer/store.h
gcc/analyzer/svalue.cc
gcc/analyzer/svalue.h
gcc/testsuite/gcc.dg/analyzer/pr97074.c [new file with mode: 0644]

index b4a4d5f3bb2ae081c4e10da70162fcd478326cf9..23118d05685637b82cded4a1877664bfd8a650f3 100644 (file)
@@ -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;
index 2f97f4b68a51e464465a49022fe5041af6e95e2e..366439ce2dd1618ead2ed4cf627b12c5dbc8c4cd 100644 (file)
@@ -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 ();
index b2d98cfcac6913ac3c90e449ef19ebbd5070a74e..5bbd05e832725cabcb1e22ab5fe2abb66f727e6d 100644 (file)
@@ -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.  */
index 336c0b601c48ce4c65cc66eb96100f2070fb231d..0703cac8bb33b90af4d784a8a5fa4a19e5544521 100644 (file)
@@ -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 (file)
index 0000000..ccb3b61
--- /dev/null
@@ -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;
+}