analyzer: fix false leak involving params [PR98969]

This patch updates the svalue liveness code so that the initial value
of parameters at top-level functions to the analysis are treated as
live (since the values are presumably still live within the
outside-of-the-analysis calling code).

This fixes the false leak in PR analyzer/98969 seen on:

void
test (long int i)
{
  struct foo *f = (struct foo *)i;
  f->expr = __builtin_malloc (1024);
}

since the calling code can presumably still access the allocated
buffer via:
  ((struct foo *)i)->expr

The patch also removes the expected leak warnings from
g++.dg/analyzer/pr99064.C and gcc.dg/analyzer/pr96841.c, which now
appear to me to be false positives.

gcc/analyzer/ChangeLog:
	PR analyzer/98969
	* constraint-manager.cc (dead_svalue_purger::should_purge_p):
	Update for change to svalue::live_p.
	* program-state.cc (sm_state_map::on_liveness_change): Likewise.
	(program_state::detect_leaks): Likewise.
	* region-model-reachability.cc (reachable_regions::init_cluster):
	When dealing with a symbolic region, if the underlying pointer is
	implicitly live, add the region to the reachable regions.
	* region-model.cc (region_model::compare_initial_and_pointer):
	Move logic for detecting initial values of params to
	initial_svalue::initial_value_of_param_p.
	* svalue.cc (svalue::live_p): Convert "live_svalues" from a
	reference to a pointer; support it being NULL.
	(svalue::implicitly_live_p): Convert first param from a
	refererence to a pointer.
	(region_svalue::implicitly_live_p): Likewise.
	(constant_svalue::implicitly_live_p): Likewise.
	(initial_svalue::implicitly_live_p): Likewise.  Treat the initial
	values of params for the top level frame as still live.
	(initial_svalue::initial_value_of_param_p): New function, taken
	from a test in region_model::compare_initial_and_pointer.
	(unaryop_svalue::implicitly_live_p): Convert first param from a
	refererence to a pointer.
	(binop_svalue::implicitly_live_p): Likewise.
	(sub_svalue::implicitly_live_p): Likewise.
	(unmergeable_svalue::implicitly_live_p): Likewise.
	* svalue.h (svalue::live_p): Likewise.
	(svalue::implicitly_live_p): Likewise.
	(region_svalue::implicitly_live_p): Likewise.
	(constant_svalue::implicitly_live_p): Likewise.
	(initial_svalue::implicitly_live_p): Likewise.
	(initial_svalue::initial_value_of_param_p): New decl.
	(unaryop_svalue::implicitly_live_p): Convert first param from a
	refererence to a pointer.
	(binop_svalue::implicitly_live_p): Likewise.
	(sub_svalue::implicitly_live_p): Likewise.
	(unmergeable_svalue::implicitly_live_p): Likewise.

gcc/testsuite/ChangeLog:
	PR analyzer/98969
	* g++.dg/analyzer/pr99064.C: Convert dg-bogus to dg-warning.
	* gcc.dg/analyzer/pr96841.c: Add -Wno-analyzer-too-complex to
	options.  Remove false leak directive.
	* gcc.dg/analyzer/pr98969.c (test_1): Remove xfail from leak
	false positive.
	(test_3): New.

diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 23421080fbafcdf9a8d3a0a016c5de927bf493fe..4dadd200beeb63f13c28efad69c7cc2d8edfb333 100644 (file)
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -1643,7 +1643,7 @@ public:
 
   bool should_purge_p (const svalue *sval) const
   {
-    return !sval->live_p (m_live_svalues, m_model);
+    return !sval->live_p (&m_live_svalues, m_model);
   }
 
 private:

diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 60fed56cd655024a664e216ce8fb827f9ddf25a1..e427fff59d677de110e65878e94950fed265bbc3 100644 (file)
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -523,7 +523,7 @@ sm_state_map::on_liveness_change (const svalue_set &live_svalues,
        ++iter)
     {
       const svalue *iter_sval = (*iter).first;
-      if (!iter_sval->live_p (live_svalues, model))
+      if (!iter_sval->live_p (&live_svalues, model))
        {
          svals_to_unset.add (iter_sval);
          entry_t e = (*iter).second;
@@ -1201,7 +1201,7 @@ program_state::detect_leaks (const program_state &src_state,
         live in DEST_STATE: either explicitly reachable, or implicitly
         live based on the set of explicitly reachable svalues.
         Record those that have ceased to be live.  */
-      if (!sval->live_p (dest_svalues, dest_state.m_region_model))
+      if (!sval->live_p (&dest_svalues, dest_state.m_region_model))
        dead_svals.quick_push (sval);
     }
 

diff --git a/gcc/analyzer/region-model-reachability.cc b/gcc/analyzer/region-model-reachability.cc
index a988ffc14399a256443908574289aa255ac0e723..087185b4e45002648c21078a22a88c4b8c8682e5 100644 (file)
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -91,6 +91,8 @@ reachable_regions::init_cluster (const region *base_reg)
   if (const symbolic_region *sym_reg = base_reg->dyn_cast_symbolic_region ())
     {
       const svalue *ptr = sym_reg->get_pointer ();
+      if (ptr->implicitly_live_p (NULL, m_model))
+       add (base_reg, true);
       switch (ptr->get_kind ())
        {
        default:

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 2fc07c4964928bbc9a5e61b335a2942710289e97..159b0f18654c04cd43bdf6265e5fc4fb38ea9e9d 100644 (file)
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1980,18 +1980,8 @@ region_model::compare_initial_and_pointer (const initial_svalue *init,
   /* If we have a pointer to something within a stack frame, it can't be the
      initial value of a param.  */
   if (pointee->maybe_get_frame_region ())
-    {
-      const region *reg = init->get_region ();
-      if (tree reg_decl = reg->maybe_get_decl ())
-       if (TREE_CODE (reg_decl) == SSA_NAME)
-         {
-           tree ssa_name = reg_decl;
-           if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)
-               && SSA_NAME_VAR (ssa_name)
-               && TREE_CODE (SSA_NAME_VAR (ssa_name)) == PARM_DECL)
-             return tristate::TS_FALSE;
-         }
-    }
+    if (init->initial_value_of_param_p ())
+      return tristate::TS_FALSE;
 
   return tristate::TS_UNKNOWN;
 }

diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 5bbd05e832725cabcb1e22ab5fe2abb66f727e6d..d6305a37b9a6ee2602f830d2a8cbadbd5f07254b 100644 (file)
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -246,15 +246,18 @@ svalue::can_merge_p (const svalue *other,
 }
 
 /* Determine if this svalue is either within LIVE_SVALUES, or is implicitly
-   live with respect to LIVE_SVALUES and MODEL.  */
+   live with respect to LIVE_SVALUES and MODEL.
+   LIVE_SVALUES can be NULL, in which case determine if this svalue is
+   intrinsically live.  */
 
 bool
-svalue::live_p (const svalue_set &live_svalues,
+svalue::live_p (const svalue_set *live_svalues,
                const region_model *model) const
 {
   /* Determine if SVAL is explicitly live.  */
-  if (const_cast<svalue_set &> (live_svalues).contains (this))
-    return true;
+  if (live_svalues)
+    if (const_cast<svalue_set *> (live_svalues)->contains (this))
+      return true;
 
   /* Otherwise, determine if SVAL is implicitly live due to being made of
      other live svalues.  */
@@ -264,7 +267,7 @@ svalue::live_p (const svalue_set &live_svalues,
 /* Base implementation of svalue::implicitly_live_p.  */
 
 bool
-svalue::implicitly_live_p (const svalue_set &, const region_model *) const
+svalue::implicitly_live_p (const svalue_set *, const region_model *) const
 {
   return false;
 }
@@ -512,7 +515,7 @@ region_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for region_svalue.  */
 
 bool
-region_svalue::implicitly_live_p (const svalue_set &,
+region_svalue::implicitly_live_p (const svalue_set *,
                                  const region_model *model) const
 {
   /* Pointers into clusters that have escaped should be treated as live.  */
@@ -609,7 +612,7 @@ constant_svalue::accept (visitor *v) const
    Constants are implicitly live.  */
 
 bool
-constant_svalue::implicitly_live_p (const svalue_set &,
+constant_svalue::implicitly_live_p (const svalue_set *,
                                    const region_model *) const
 {
   return true;
@@ -749,7 +752,7 @@ initial_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for initial_svalue.  */
 
 bool
-initial_svalue::implicitly_live_p (const svalue_set &,
+initial_svalue::implicitly_live_p (const svalue_set *,
                                   const region_model *model) const
 {
   /* This svalue may be implicitly live if the region still implicitly
@@ -765,6 +768,31 @@ initial_svalue::implicitly_live_p (const svalue_set &,
        return true;
     }
 
+  /* Assume that the initial values of params for the top level frame
+     are still live, because (presumably) they're still
+     live in the external caller.  */
+  if (initial_value_of_param_p ())
+    if (const frame_region *frame_reg = m_reg->maybe_get_frame_region ())
+      if (frame_reg->get_calling_frame () == NULL)
+       return true;
+
+  return false;
+}
+
+/* Return true if this is the initial value of a function parameter.  */
+
+bool
+initial_svalue::initial_value_of_param_p () const
+{
+  if (tree reg_decl = m_reg->maybe_get_decl ())
+    if (TREE_CODE (reg_decl) == SSA_NAME)
+      {
+       tree ssa_name = reg_decl;
+       if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)
+           && SSA_NAME_VAR (ssa_name)
+           && TREE_CODE (SSA_NAME_VAR (ssa_name)) == PARM_DECL)
+         return true;
+      }
   return false;
 }
 
@@ -816,7 +844,7 @@ unaryop_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for unaryop_svalue.  */
 
 bool
-unaryop_svalue::implicitly_live_p (const svalue_set &live_svalues,
+unaryop_svalue::implicitly_live_p (const svalue_set *live_svalues,
                                   const region_model *model) const
 {
   return get_arg ()->live_p (live_svalues, model);
@@ -862,7 +890,7 @@ binop_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for binop_svalue.  */
 
 bool
-binop_svalue::implicitly_live_p (const svalue_set &live_svalues,
+binop_svalue::implicitly_live_p (const svalue_set *live_svalues,
                                 const region_model *model) const
 {
   return (get_arg0 ()->live_p (live_svalues, model)
@@ -919,7 +947,7 @@ sub_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for sub_svalue.  */
 
 bool
-sub_svalue::implicitly_live_p (const svalue_set &live_svalues,
+sub_svalue::implicitly_live_p (const svalue_set *live_svalues,
                               const region_model *model) const
 {
   return get_parent ()->live_p (live_svalues, model);
@@ -1136,7 +1164,7 @@ unmergeable_svalue::accept (visitor *v) const
 /* Implementation of svalue::implicitly_live_p vfunc for unmergeable_svalue.  */
 
 bool
-unmergeable_svalue::implicitly_live_p (const svalue_set &live_svalues,
+unmergeable_svalue::implicitly_live_p (const svalue_set *live_svalues,
                                       const region_model *model) const
 {
   return get_arg ()->live_p (live_svalues, model);

diff --git a/gcc/analyzer/svalue.h b/gcc/analyzer/svalue.h
index 0703cac8bb33b90af4d784a8a5fa4a19e5544521..672a89cccff75ccab7b0fa6afc0e547052d56d4f 100644 (file)
--- a/gcc/analyzer/svalue.h
+++ b/gcc/analyzer/svalue.h
@@ -128,9 +128,9 @@ public:
 
   virtual void accept (visitor *v) const  = 0;
 
-  bool live_p (const svalue_set &live_svalues,
+  bool live_p (const svalue_set *live_svalues,
               const region_model *model) const;
-  virtual bool implicitly_live_p (const svalue_set &live_svalues,
+  virtual bool implicitly_live_p (const svalue_set *live_svalues,
                                  const region_model *model) const;
 
   static int cmp_ptr (const svalue *, const svalue *);
@@ -194,7 +194,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   const region * get_pointee () const { return m_reg; }
@@ -243,7 +243,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   tree get_constant () const { return m_cst_expr; }
@@ -493,9 +493,11 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
+  bool initial_value_of_param_p () const;
+
   const region *get_region () const { return m_reg; }
 
  private:
@@ -564,7 +566,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   enum tree_code get_op () const { return m_op; }
@@ -653,7 +655,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   enum tree_code get_op () const { return m_op; }
@@ -734,7 +736,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   const svalue *get_parent () const { return m_parent_svalue; }
@@ -788,7 +790,7 @@ 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 &,
+  bool implicitly_live_p (const svalue_set *,
                          const region_model *) const FINAL OVERRIDE;
 
   const svalue *get_arg () const { return m_arg; }

diff --git a/gcc/testsuite/g++.dg/analyzer/pr99064.C b/gcc/testsuite/g++.dg/analyzer/pr99064.C
index a002219bf5809085de77ff9f716aee544a1e5fd9..9fa54f57814ca49f32c49a5bf3ed993c2a3e22c8 100644 (file)
--- a/gcc/testsuite/g++.dg/analyzer/pr99064.C
+++ b/gcc/testsuite/g++.dg/analyzer/pr99064.C
@@ -34,6 +34,6 @@ struct TPkcs11Token {
 void list_tokens() {
   for (__normal_iterator base = list_tokens_token_list.begin();;) {
     int *add_info = new int;
-    (*base).add_info = add_info; // { dg-bogus "leak" "PR analyzer/98969" { xfail *-*-* } }
+    (*base).add_info = add_info; // { dg-warning "leak" }
   }
 }

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96841.c b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
index d9d35f3dce8e32dccd6d3e7bfdbdbabd83d6b1ac..854666170541712cfcc36a10009243cdd2b3cb56 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/pr96841.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-O1 -Wno-builtin-declaration-mismatch" } */
+/* { dg-additional-options "-Wno-analyzer-too-complex -O1 -Wno-builtin-declaration-mismatch" } */
 
 int
 l8 (void);
@@ -18,6 +18,6 @@ bv (__SIZE_TYPE__ ny)
     {
       *mf = 0;
       (*mf)[ny] = (int *) malloc (sizeof (int));
-      th ((*mf)[ny]); /* { dg-warning "leak" } */
+      th ((*mf)[ny]);
     }
 }

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98969.c b/gcc/testsuite/gcc.dg/analyzer/pr98969.c
index 8298f26389823a38bc96666526253f735d04c19a..7e1587d7094d5bb3d30837ee003d9225ebeb25f7 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/pr98969.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr98969.c
@@ -8,7 +8,7 @@ test_1 (long int i)
 {
   struct foo *f = (struct foo *)i;
   f->expr = __builtin_malloc (1024);
-} /* { dg-bogus "leak" "PR analyzer/98969" { xfail *-*-* } } */
+} /* { dg-bogus "leak" } */
 
 void
 test_2 (long int i)
@@ -16,3 +16,10 @@ test_2 (long int i)
   __builtin_free (((struct foo *)i)->expr);
   __builtin_free (((struct foo *)i)->expr); /* { dg-warning "double-'free' of '\\*\\(\\(struct foo \\*\\)i\\)\\.expr'" } */
 }
+
+void
+test_3 (void *p)
+{
+  void **q = (void **)p;
+  *q = __builtin_malloc (1024);  
+}