analyzer: handle compound assignments [PR94378]
authorDavid Malcolm <dmalcolm@redhat.com>
Mon, 30 Mar 2020 21:02:59 +0000 (17:02 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Wed, 1 Apr 2020 19:34:40 +0000 (15:34 -0400)
PR analyzer/94378 reports a false -Wanalyzer-malloc-leak
when returning a struct containing a malloc-ed pointer.

The issue is that the assignment code was not handling
compound copies, only copying top-level values from region to region,
and not copying child values.

This patch introduces a region_model::copy_region function, using
it for assignments and when analyzing function return values.
It recursively copies nested values within structs, unions, and
arrays, fixing the bug.

gcc/analyzer/ChangeLog:
PR analyzer/94378
* checker-path.cc: Include "bitmap.h".
* constraint-manager.cc: Likewise.
* diagnostic-manager.cc: Likewise.
* engine.cc: Likewise.
(exploded_node::detect_leaks): Pass null region_id to pop_frame.
* program-point.cc: Include "bitmap.h".
* program-state.cc: Likewise.
* region-model.cc (id_set<region_id>::id_set): Convert to...
(region_id_set::region_id_set): ...this.
(svalue_id_set::svalue_id_set): New ctor.
(region_model::copy_region): New function.
(region_model::copy_struct_region): New function.
(region_model::copy_union_region): New function.
(region_model::copy_array_region): New function.
(stack_region::pop_frame): Drop return value.  Add
"result_dst_rid" param; if it is non-null, use copy_region to copy
the result to it.  Rather than capture and pass a single "known
used" return value to be used by purge_unused_values, instead
gather and pass a set of known used return values.
(root_region::pop_frame): Drop return value.  Add "result_dst_rid"
param.
(region_model::on_assignment): Use copy_region.
(region_model::on_return): Likewise for the result.
(region_model::on_longjmp): Pass null for pop_frame's
result_dst_rid.
(region_model::update_for_return_superedge): Pass the region for the
return value of the call, if any, to pop_frame, rather than setting
the lvalue for the lhs of the result.
(region_model::pop_frame): Drop return value.  Add
"result_dst_rid" param.
(region_model::purge_unused_svalues): Convert third param from an
svalue_id * to an svalue_id_set *, updating the initial populating
of the "used" bitmap accordingly.  Don't remap it when done.
(struct selftest::coord_test): New selftest fixture, extracted from...
(selftest::test_dump_2): ...here.
(selftest::test_compound_assignment): New selftest.
(selftest::test_stack_frames): Pass null to new param of pop_frame.
(selftest::analyzer_region_model_cc_tests): Call the new selftest.
* region-model.h (class id_set): Delete template.
(class region_id_set): Reimplement, using old id_set implementation.
(class svalue_id_set): Likewise.  Convert from auto_sbitmap to
auto_bitmap.
(region::get_active_view): New accessor.
(stack_region::pop_frame): Drop return value.  Add
"result_dst_rid" param.
(root_region::pop_frame): Likewise.
(region_model::pop_frame): Likewise.
(region_model::copy_region): New decl.
(region_model::purge_unused_svalues): Convert third param from an
svalue_id * to an svalue_id_set *.
(region_model::copy_struct_region): New decl.
(region_model::copy_union_region): New decl.
(region_model::copy_array_region): New decl.

gcc/testsuite/ChangeLog:
PR analyzer/94378
* gcc.dg/analyzer/compound-assignment-1.c: New test.
* gcc.dg/analyzer/compound-assignment-2.c: New test.
* gcc.dg/analyzer/compound-assignment-3.c: New test.

13 files changed:
gcc/analyzer/ChangeLog
gcc/analyzer/checker-path.cc
gcc/analyzer/constraint-manager.cc
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/engine.cc
gcc/analyzer/program-point.cc
gcc/analyzer/program-state.cc
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c [new file with mode: 0644]

index 98093cd4f35913122aa95762b2cdacda16efc76c..3f136f45c20d50182b6b2217f5ff13267fa2b7b2 100644 (file)
@@ -1,3 +1,60 @@
+2020-04-01  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/94378
+       * checker-path.cc: Include "bitmap.h".
+       * constraint-manager.cc: Likewise.
+       * diagnostic-manager.cc: Likewise.
+       * engine.cc: Likewise.
+       (exploded_node::detect_leaks): Pass null region_id to pop_frame.
+       * program-point.cc: Include "bitmap.h".
+       * program-state.cc: Likewise.
+       * region-model.cc (id_set<region_id>::id_set): Convert to...
+       (region_id_set::region_id_set): ...this.
+       (svalue_id_set::svalue_id_set): New ctor.
+       (region_model::copy_region): New function.
+       (region_model::copy_struct_region): New function.
+       (region_model::copy_union_region): New function.
+       (region_model::copy_array_region): New function.
+       (stack_region::pop_frame): Drop return value.  Add
+       "result_dst_rid" param; if it is non-null, use copy_region to copy
+       the result to it.  Rather than capture and pass a single "known
+       used" return value to be used by purge_unused_values, instead
+       gather and pass a set of known used return values.
+       (root_region::pop_frame): Drop return value.  Add "result_dst_rid"
+       param.
+       (region_model::on_assignment): Use copy_region.
+       (region_model::on_return): Likewise for the result.
+       (region_model::on_longjmp): Pass null for pop_frame's
+       result_dst_rid.
+       (region_model::update_for_return_superedge): Pass the region for the
+       return value of the call, if any, to pop_frame, rather than setting
+       the lvalue for the lhs of the result.
+       (region_model::pop_frame): Drop return value.  Add
+       "result_dst_rid" param.
+       (region_model::purge_unused_svalues): Convert third param from an
+       svalue_id * to an svalue_id_set *, updating the initial populating
+       of the "used" bitmap accordingly.  Don't remap it when done.
+       (struct selftest::coord_test): New selftest fixture, extracted from...
+       (selftest::test_dump_2): ...here.
+       (selftest::test_compound_assignment): New selftest.
+       (selftest::test_stack_frames): Pass null to new param of pop_frame.
+       (selftest::analyzer_region_model_cc_tests): Call the new selftest.
+       * region-model.h (class id_set): Delete template.
+       (class region_id_set): Reimplement, using old id_set implementation.
+       (class svalue_id_set): Likewise.  Convert from auto_sbitmap to
+       auto_bitmap.
+       (region::get_active_view): New accessor.
+       (stack_region::pop_frame): Drop return value.  Add
+       "result_dst_rid" param.
+       (root_region::pop_frame): Likewise.
+       (region_model::pop_frame): Likewise.
+       (region_model::copy_region): New decl.
+       (region_model::purge_unused_svalues): Convert third param from an
+       svalue_id * to an svalue_id_set *.
+       (region_model::copy_struct_region): New decl.
+       (region_model::copy_union_region): New decl.
+       (region_model::copy_array_region): New decl.
+
 2020-03-27  David Malcolm  <dmalcolm@redhat.com>
 
        * program-state.cc (selftest::test_program_state_dumping): Update
index c781cd8dbeba69e969f75e8971986290fd6fdc99..c07273460d55a3b89482a23b4749a7d6c2a23c8c 100644 (file)
@@ -42,6 +42,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/sm.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "ordered-hash-map.h"
 #include "selftest.h"
index a62354356816da896382f92fc0cb3d20d8764422..786bd6ccf85974b57ff3a31b538954d491c97e98 100644 (file)
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "digraph.h"
 #include "analyzer/supergraph.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "analyzer/region-model.h"
 #include "analyzer/constraint-manager.h"
index 4b884c7a45256a5cba7d25826a09950daff80a53..e132a5625de4b3e9938a1b72f37d08d53136cec2 100644 (file)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "fibonacci_heap.h"
 #include "shortest-paths.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "selftest.h"
 #include "ordered-hash-map.h"
index befd4836cb4eda04c4d575af9eca0229abda4358..880e70fb2ba8a8602f2e97eb2e16b3be574a16d2 100644 (file)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "function.h"
 #include "pretty-print.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "ordered-hash-map.h"
 #include "selftest.h"
@@ -1361,7 +1362,8 @@ exploded_node::detect_leaks (exploded_graph &eg) const
                                  &old_state, &new_state,
                                  NULL,
                                  get_stmt ());
-  new_state.m_region_model->pop_frame (true, &stats, &ctxt);
+  new_state.m_region_model->pop_frame (region_id::null (),
+                                      true, &stats, &ctxt);
 }
 
 /* Dump the successors and predecessors of this enode to OUTF.  */
index 2e7f76941113b314f2f832eb7cb92612c2097f67..7bf2de4e3bfe08547e72f590f92630b7acfa73c5 100644 (file)
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/supergraph.h"
 #include "analyzer/program-point.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "selftest.h"
 #include "analyzer/region-model.h"
index a6049604ca847810413ded65fe4bc993cdd903d9..43396c658274d7744f0c2915191fee7728078105 100644 (file)
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/sm.h"
 #include "sbitmap.h"
+#include "bitmap.h"
 #include "tristate.h"
 #include "ordered-hash-map.h"
 #include "selftest.h"
index 68152526486ab03a4cf72de13fabba2247ec046b..30fe72b90e995d412183006756925d9a5456826d 100644 (file)
@@ -243,17 +243,22 @@ region_id::validate (const region_model &model) const
   gcc_assert (null_p () || m_idx < (int)model.get_num_regions ());
 }
 
-/* class id_set.  */
+/* class region_id_set.  */
 
-/* id_set<region_id>'s ctor.  */
-
-template<>
-id_set<region_id>::id_set (const region_model *model)
+region_id_set::region_id_set (const region_model *model)
 : m_bitmap (model->get_num_regions ())
 {
   bitmap_clear (m_bitmap);
 }
 
+/* class svalue_id_set.  */
+
+svalue_id_set::svalue_id_set ()
+: m_bitmap (NULL)
+{
+  bitmap_clear (m_bitmap);
+}
+
 /* class svalue and its various subclasses.  */
 
 /* class svalue.  */
@@ -1219,6 +1224,127 @@ region::get_inherited_child_sid (region *child,
   return svalue_id::null ();
 }
 
+/* Copy from SRC_RID to DST_RID, using CTXT for any issues that occur.
+   Copy across any value for the region, and handle structs, unions
+   and arrays recursively.  */
+
+void
+region_model::copy_region (region_id dst_rid, region_id src_rid,
+                          region_model_context *ctxt)
+{
+  gcc_assert (!dst_rid.null_p ());
+  gcc_assert (!src_rid.null_p ());
+  if (dst_rid == src_rid)
+    return;
+  region *dst_reg = get_region (dst_rid);
+  region *src_reg = get_region (src_rid);
+
+  /* Copy across any value for the src region itself.  */
+  svalue_id sid = src_reg->get_value (*this, true, ctxt);
+  set_value (dst_rid, sid, ctxt);
+
+  if (dst_reg->get_kind () != src_reg->get_kind ())
+    return;
+
+  /* Copy across child regions for structs, unions, and arrays.  */
+  switch (dst_reg->get_kind ())
+    {
+    case RK_PRIMITIVE:
+      return;
+    case RK_STRUCT:
+      {
+       struct_region *dst_sub = as_a <struct_region *> (dst_reg);
+       struct_region *src_sub = as_a <struct_region *> (src_reg);
+       copy_struct_region (dst_rid, dst_sub, src_sub, ctxt);
+      }
+      return;
+    case RK_UNION:
+      {
+       union_region *src_sub = as_a <union_region *> (src_reg);
+       copy_union_region (dst_rid, src_sub, ctxt);
+      }
+      return;
+    case RK_FRAME:
+    case RK_GLOBALS:
+    case RK_CODE:
+    case RK_FUNCTION:
+      return;
+    case RK_ARRAY:
+      {
+       array_region *dst_sub = as_a <array_region *> (dst_reg);
+       array_region *src_sub = as_a <array_region *> (src_reg);
+       copy_array_region (dst_rid, dst_sub, src_sub, ctxt);
+      }
+      return;
+    case RK_STACK:
+    case RK_HEAP:
+    case RK_ROOT:
+    case RK_SYMBOLIC:
+      return;
+    }
+}
+
+/* Subroutine of region_model::copy_region for copying the child
+   regions for a struct.  */
+
+void
+region_model::copy_struct_region (region_id dst_rid,
+                                 struct_region *dst_reg,
+                                 struct_region *src_reg,
+                                 region_model_context *ctxt)
+{
+  for (map_region::iterator_t iter = src_reg->begin ();
+       iter != src_reg->end (); ++iter)
+    {
+      tree src_key = (*iter).first;
+      region_id src_field_rid = (*iter).second;
+      region *src_field_reg = get_region (src_field_rid);
+      region_id dst_field_rid
+       = dst_reg->get_or_create (this, dst_rid, src_key,
+                                 src_field_reg->get_type (), ctxt);
+      copy_region (dst_field_rid, src_field_rid, ctxt);
+    }
+}
+
+/* Subroutine of region_model::copy_region for copying the active
+   child region for a union.  */
+
+void
+region_model::copy_union_region (region_id dst_rid,
+                                union_region *src_reg,
+                                region_model_context *ctxt)
+{
+  region_id src_active_view_rid = src_reg->get_active_view ();
+  if (src_active_view_rid.null_p ())
+    return;
+  region *src_active_view = get_region (src_active_view_rid);
+  tree type = src_active_view->get_type ();
+  region_id dst_active_view_rid = get_or_create_view (dst_rid, type, ctxt);
+  copy_region (dst_active_view_rid, src_active_view_rid, ctxt);
+}
+
+/* Subroutine of region_model::copy_region for copying the child
+   regions for an array.  */
+
+void
+region_model::copy_array_region (region_id dst_rid,
+                                array_region *dst_reg,
+                                array_region *src_reg,
+                                region_model_context *ctxt)
+{
+  for (array_region::iterator_t iter = src_reg->begin ();
+       iter != src_reg->end (); ++iter)
+    {
+      array_region::key_t src_key = (*iter).first;
+      region_id src_field_rid = (*iter).second;
+      region *src_field_reg = get_region (src_field_rid);
+      region_id dst_field_rid
+       = dst_reg->get_or_create (this, dst_rid, src_key,
+                                 src_field_reg->get_type (), ctxt);
+      copy_region (dst_field_rid, src_field_rid, ctxt);
+    }
+}
+
 /* Generate a hash value for this region.  The work is done by the
    add_to_hash vfunc.  */
 
@@ -2630,21 +2756,21 @@ stack_region::get_current_frame_id () const
 
 /* Pop the topmost frame_region from this stack.
 
+   If RESULT_DST_RID is non-null, copy any return value from the frame
+   into RESULT_DST_RID's region.
+
    Purge the frame region and all its descendent regions.
    Convert any pointers that point into such regions into
    POISON_KIND_POPPED_STACK svalues.
 
-   Return the ID of any return value from the frame.
-
    If PURGE, then purge all unused svalues, with the exception of any
-   return value for the frame, which is temporarily
-   preserved in case no regions reference it, so it can
-   be written into a region in the caller.
+   returned values.
 
    Accumulate stats on purged entities into STATS.  */
 
-svalue_id
-stack_region::pop_frame (region_model *model, bool purge, purge_stats *stats,
+void
+stack_region::pop_frame (region_model *model, region_id result_dst_rid,
+                        bool purge, purge_stats *stats,
                         region_model_context *ctxt)
 {
   gcc_assert (m_frame_rids.length () > 0);
@@ -2653,11 +2779,35 @@ stack_region::pop_frame (region_model *model, bool purge, purge_stats *stats,
   frame_region *frame = model->get_region<frame_region> (frame_rid);
 
   /* Evaluate the result, within the callee frame.  */
-  svalue_id result_sid;
+  svalue_id_set returned_sids;
   tree fndecl = frame->get_function ()->decl;
   tree result = DECL_RESULT (fndecl);
   if (result && TREE_TYPE (result) != void_type_node)
-    result_sid = model->get_rvalue (result, ctxt);
+    {
+      if (!result_dst_rid.null_p ())
+       {
+         /* Copy the result to RESULT_DST_RID.  */
+         model->copy_region (result_dst_rid, model->get_lvalue (result, ctxt),
+                             ctxt);
+       }
+      if (purge)
+       {
+         /* Populate returned_sids, to avoid purging them.  */
+         region_id return_rid = model->get_lvalue (result, NULL);
+         region_id_set returned_rids (model);
+         model->get_descendents (return_rid, &returned_rids,
+                                 region_id::null ());
+         for (unsigned i = 0; i < model->get_num_regions (); i++)
+           {
+             region_id rid = region_id::from_int (i);
+             if (returned_rids.region_p (rid))
+               {
+                 svalue_id sid = model->get_region (rid)->get_value_direct ();
+                 returned_sids.add_svalue (sid);
+               }
+           }
+       }
+    }
 
   /* Pop the frame RID.  */
   m_frame_rids.pop ();
@@ -2667,13 +2817,11 @@ stack_region::pop_frame (region_model *model, bool purge, purge_stats *stats,
                                        stats,
                                        ctxt ? ctxt->get_logger () : NULL);
 
-  /* Delete unused svalues, but don't delete the return value.  */
+  /* Delete unused svalues, but don't delete the return value(s).  */
   if (purge)
-    model->purge_unused_svalues (stats, ctxt, &result_sid);
+    model->purge_unused_svalues (stats, ctxt, &returned_sids);
 
   model->validate ();
-
-  return result_sid;
 }
 
 /* Implementation of region::add_to_hash vfunc for stack_region.  */
@@ -3039,12 +3187,13 @@ root_region::get_current_frame_id (const region_model &model) const
 /* Pop the topmost frame_region from this root_region's stack;
    see the comment for stack_region::pop_frame.  */
 
-svalue_id
-root_region::pop_frame (region_model *model, bool purge, purge_stats *out,
+void
+root_region::pop_frame (region_model *model, region_id result_dst_rid,
+                       bool purge, purge_stats *out,
                        region_model_context *ctxt)
 {
   stack_region *stack = model->get_region <stack_region> (m_stack_rid);
-  return stack->pop_frame (model, purge, out, ctxt);
+  stack->pop_frame (model, result_dst_rid, purge, out, ctxt);
 }
 
 /* Return the region_id of the stack region, creating it if doesn't
@@ -4139,8 +4288,8 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
     case PARM_DECL:
       {
        /* LHS = VAR;  */
-       svalue_id var_sid = get_rvalue (rhs1, ctxt);
-       set_value (lhs_rid, var_sid, ctxt);
+       region_id rhs_rid = get_lvalue (rhs1, ctxt);
+       copy_region (lhs_rid, rhs_rid, ctxt);
       }
       break;
 
@@ -4601,7 +4750,7 @@ region_model::on_return (const greturn *return_stmt, region_model_context *ctxt)
   tree rhs = gimple_return_retval (return_stmt);
 
   if (lhs && rhs)
-    set_value (get_lvalue (lhs, ctxt), get_rvalue (rhs, ctxt), ctxt);
+    copy_region (get_lvalue (lhs, ctxt), get_lvalue (rhs, ctxt), ctxt);
 }
 
 /* Update this model for a call and return of setjmp/sigsetjmp at CALL within
@@ -4656,7 +4805,7 @@ region_model::on_longjmp (const gcall *longjmp_call, const gcall *setjmp_call,
   while (get_stack_depth () > setjmp_stack_depth)
     {
       /* Don't purge unused svalues yet, as we're using fake_retval_sid.  */
-      pop_frame (false, NULL, ctxt);
+      pop_frame (region_id::null (), false, NULL, ctxt);
     }
 
   gcc_assert (get_stack_depth () == setjmp_stack_depth);
@@ -5997,31 +6146,39 @@ region_model::update_for_call_superedge (const call_superedge &call_edge,
   push_frame (call_edge.get_callee_function (), &arg_sids, ctxt);
 }
 
-/* Pop the top-most frame_region from the stack, and store the svalue
-   for any returned value into the region for the lvalue of the LHS of
+/* Pop the top-most frame_region from the stack, and copy the return
+   region's values (if any) into the region for the lvalue of the LHS of
    the call (if any).  */
 
 void
 region_model::update_for_return_superedge (const return_superedge &return_edge,
                                           region_model_context *ctxt)
 {
-  purge_stats stats;
-  svalue_id result_sid = pop_frame (true, &stats, ctxt);
-  // TODO: do something with the stats?
-
-  if (result_sid.null_p ())
-    return;
+  region_id stack_rid = get_stack_region_id ();
+  stack_region *stack = get_region <stack_region> (stack_rid);
 
-  /* Set the result of the call, within the caller frame.  */
+  /* Get the region for the result of the call, within the caller frame.  */
+  region_id result_dst_rid;
   const gcall *call_stmt = return_edge.get_call_stmt ();
   tree lhs = gimple_call_lhs (call_stmt);
   if (lhs)
-    set_value (get_lvalue (lhs, ctxt), result_sid, ctxt);
-  else
+    {
+      /* Normally we access the top-level frame, which is:
+          path_var (expr, stack->get_num_frames () - 1)
+        whereas here we need the caller frame, hence "- 2" here.  */
+      gcc_assert (stack->get_num_frames () >= 2);
+      result_dst_rid = get_lvalue (path_var (lhs, stack->get_num_frames () - 2),
+                                  ctxt);
+    }
+
+  purge_stats stats;
+  stack->pop_frame (this, result_dst_rid, true, &stats, ctxt);
+  // TODO: do something with the stats?
+
+  if (!lhs)
     {
       /* This could be a leak; try purging again, but this time,
-        don't special-case the result_sid.  */
-      purge_stats stats;
+        don't special-case the result sids (as was done in pop_frame).  */
       purge_unused_svalues (&stats, ctxt);
     }
 }
@@ -6189,11 +6346,12 @@ region_model::get_current_function () const
 /* Pop the topmost frame_region from this region_model's stack;
    see the comment for stack_region::pop_frame.  */
 
-svalue_id
-region_model::pop_frame (bool purge, purge_stats *out,
+void
+region_model::pop_frame (region_id result_dst_rid,
+                        bool purge, purge_stats *out,
                         region_model_context *ctxt)
 {
-  return get_root_region ()->pop_frame (this, purge, out, ctxt);
+  get_root_region ()->pop_frame (this, result_dst_rid, purge, out, ctxt);
 }
 
 /* Get the number of frames in this region_model's stack.  */
@@ -6374,17 +6532,19 @@ private:
    number of redundant unknown values could grow without bounds, and each
    such model would be treated as distinct.
 
-   If KNOWN_USED is non-NULL, treat *KNOWN_USED as used (this is for
+   If KNOWN_USED_SIDS is non-NULL, treat *KNOWN_USED_SIDS as used (this is for
    handling values being returned from functions as their frame is popped,
    since otherwise we'd have to simultaneously determine both the rvalue
    of the return expr in the callee frame and the lvalue for the gcall's
    assignment in the caller frame, and it seems cleaner to express all
-   lvalue and rvalue lookups implicitly relative to a "current" frame).  */
+   lvalue and rvalue lookups implicitly relative to a "current" frame).
+   The svalue_ids in *KNOWN_USED_SIDS are not remapped and hence this
+   call makes it invalid.  */
 
 void
 region_model::purge_unused_svalues (purge_stats *stats,
                                    region_model_context *ctxt,
-                                   svalue_id *known_used_sid)
+                                   svalue_id_set *known_used_sids)
 {
   // TODO: might want to avoid a vfunc call just to do logging here:
   logger *logger = ctxt ? ctxt->get_logger () : NULL;
@@ -6394,9 +6554,14 @@ region_model::purge_unused_svalues (purge_stats *stats,
   auto_sbitmap used (m_svalues.length ());
   bitmap_clear (used);
 
-  if (known_used_sid)
-    if (!known_used_sid->null_p ())
-      bitmap_set_bit (used, known_used_sid->as_int ());
+  if (known_used_sids)
+    {
+      /* We can't use an sbitmap for known_used_sids as the number of
+        svalues could have grown since it was created.  */
+      for (unsigned i = 0; i < get_num_svalues (); i++)
+       if (known_used_sids->svalue_p (svalue_id::from_int (i)))
+         bitmap_set_bit (used, i);
+    }
 
   /* Walk the regions, marking sids that are used.  */
   unsigned i;
@@ -6491,9 +6656,6 @@ region_model::purge_unused_svalues (purge_stats *stats,
        stats->m_num_svalues++;
     }
 
-  if (known_used_sid)
-    map.update (known_used_sid);
-
   validate ();
 }
 
@@ -7459,25 +7621,39 @@ make_test_compound_type (const char *name, bool is_struct,
   return t;
 }
 
+/* Selftest fixture for creating the type "struct coord {int x; int y; };".  */
+
+struct coord_test
+{
+  coord_test ()
+  {
+    auto_vec<tree> fields;
+    m_x_field = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
+                              get_identifier ("x"), integer_type_node);
+    fields.safe_push (m_x_field);
+    m_y_field = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
+                              get_identifier ("y"), integer_type_node);
+    fields.safe_push (m_y_field);
+    m_coord_type = make_test_compound_type ("coord", true, &fields);
+  }
+
+  tree m_x_field;
+  tree m_y_field;
+  tree m_coord_type;
+};
+
 /* Verify that dumps can show struct fields.  */
 
 static void
 test_dump_2 ()
 {
-  auto_vec<tree> fields;
-  tree x_field = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
-                            get_identifier ("x"), integer_type_node);
-  fields.safe_push (x_field);
-  tree y_field = build_decl (UNKNOWN_LOCATION, FIELD_DECL,
-                            get_identifier ("y"), integer_type_node);
-  fields.safe_push (y_field);
-  tree coord_type = make_test_compound_type ("coord", true, &fields);
-
-  tree c = build_global_decl ("c", coord_type);
-  tree c_x = build3 (COMPONENT_REF, TREE_TYPE (x_field),
-                    c, x_field, NULL_TREE);
-  tree c_y = build3 (COMPONENT_REF, TREE_TYPE (y_field),
-                    c, y_field, NULL_TREE);
+  coord_test ct;
+
+  tree c = build_global_decl ("c", ct.m_coord_type);
+  tree c_x = build3 (COMPONENT_REF, TREE_TYPE (ct.m_x_field),
+                    c, ct.m_x_field, NULL_TREE);
+  tree c_y = build3 (COMPONENT_REF, TREE_TYPE (ct.m_y_field),
+                    c, ct.m_y_field, NULL_TREE);
 
   tree int_17 = build_int_cst (integer_type_node, 17);
   tree int_m3 = build_int_cst (integer_type_node, -3);
@@ -7881,6 +8057,41 @@ test_assignment ()
   ASSERT_DUMP_EQ (model, true, "y: 0, {x}: unknown, x == y");
 }
 
+/* Verify that compound assignments work as expected.  */
+
+static void
+test_compound_assignment ()
+{
+  coord_test ct;
+
+  tree c = build_global_decl ("c", ct.m_coord_type);
+  tree c_x = build3 (COMPONENT_REF, TREE_TYPE (ct.m_x_field),
+                    c, ct.m_x_field, NULL_TREE);
+  tree c_y = build3 (COMPONENT_REF, TREE_TYPE (ct.m_y_field),
+                    c, ct.m_y_field, NULL_TREE);
+  tree d = build_global_decl ("d", ct.m_coord_type);
+  tree d_x = build3 (COMPONENT_REF, TREE_TYPE (ct.m_x_field),
+                    d, ct.m_x_field, NULL_TREE);
+  tree d_y = build3 (COMPONENT_REF, TREE_TYPE (ct.m_y_field),
+                    d, ct.m_y_field, NULL_TREE);
+
+  tree int_17 = build_int_cst (integer_type_node, 17);
+  tree int_m3 = build_int_cst (integer_type_node, -3);
+
+  region_model model;
+  model.set_value (c_x, int_17, NULL);
+  model.set_value (c_y, int_m3, NULL);
+
+  ASSERT_DUMP_EQ (model, true, "c.x: 17, c.y: -3");
+
+  /* Copy c to d.  */
+  model.copy_region (model.get_lvalue (d, NULL), model.get_lvalue (c, NULL),
+                    NULL);
+  /* Check that the fields have the same svalues.  */
+  ASSERT_EQ (model.get_rvalue (c_x, NULL), model.get_rvalue (d_x, NULL));
+  ASSERT_EQ (model.get_rvalue (c_y, NULL), model.get_rvalue (d_y, NULL));
+}
+
 /* Verify the details of pushing and popping stack frames.  */
 
 static void
@@ -7986,7 +8197,7 @@ test_stack_frames ()
 
   /* Pop the "child_fn" frame from the stack.  */
   purge_stats purged;
-  model.pop_frame (true, &purged, &ctxt);
+  model.pop_frame (region_id::null (), true, &purged, &ctxt);
 
   /* We should have purged the unknown values for x and y. */
   ASSERT_EQ (purged.m_num_svalues, 2);
@@ -8671,6 +8882,7 @@ analyzer_region_model_cc_tests ()
   test_purging_by_criteria ();
   test_purge_unused_svalues ();
   test_assignment ();
+  test_compound_assignment ();
   test_stack_frames ();
   test_get_representative_path_var ();
   test_canonicalization_1 ();
index 235db72141eb81d7e47639146877b229c9be3017..2c9ee39a38dc105769aefeec86950979565ad9c7 100644 (file)
@@ -370,25 +370,24 @@ one_way_id_map<T>::update (T *id) const
   *id = get_dst_for_src (*id);
 }
 
-/* A set of IDs within a region_model (either svalue_id or region_id).  */
+/* A set of region_ids within a region_model.  */
 
-template <typename T>
-class id_set
+class region_id_set
 {
 public:
-  id_set (const region_model *model);
+  region_id_set (const region_model *model);
 
-  void add_region (id)
+  void add_region (region_id rid)
   {
-    if (!id.null_p ())
-      bitmap_set_bit (m_bitmap, id.as_int ());
+    if (!rid.null_p ())
+      bitmap_set_bit (m_bitmap, rid.as_int ());
   }
 
-  bool region_p (id) const
+  bool region_p (region_id rid) const
   {
-    gcc_assert (!id.null_p ());
+    gcc_assert (!rid.null_p ());
     return bitmap_bit_p (const_cast <auto_sbitmap &> (m_bitmap),
-                        id.as_int ());
+                        rid.as_int ());
   }
 
   unsigned int num_regions ()
@@ -400,7 +399,29 @@ private:
   auto_sbitmap m_bitmap;
 };
 
-typedef id_set<region_id> region_id_set;
+/* A set of svalue_ids within a region_model.  */
+
+class svalue_id_set
+{
+public:
+  svalue_id_set ();
+
+  void add_svalue (svalue_id sid)
+  {
+    if (!sid.null_p ())
+      bitmap_set_bit (m_bitmap, sid.as_int ());
+  }
+
+  bool svalue_p (svalue_id sid) const
+  {
+    gcc_assert (!sid.null_p ());
+    return bitmap_bit_p (const_cast <auto_bitmap &> (m_bitmap),
+                        sid.as_int ());
+  }
+
+private:
+  auto_bitmap m_bitmap;
+};
 
 /* Various operations delete information from a region_model.
 
@@ -890,6 +911,7 @@ public:
 
   void add_view (region_id view_rid, region_model *model);
   region_id get_view (tree type, region_model *model) const;
+  region_id get_active_view () const { return m_active_view_rid; }
   bool is_view_p () const { return m_is_view; }
 
   virtual void validate (const region_model &model) const;
@@ -1450,8 +1472,9 @@ public:
 
   void push_frame (region_id frame_rid);
   region_id get_current_frame_id () const;
-  svalue_id pop_frame (region_model *model, bool purge, purge_stats *stats,
-                      region_model_context *ctxt);
+  void pop_frame (region_model *model, region_id result_dst_rid,
+                 bool purge, purge_stats *stats,
+                 region_model_context *ctxt);
 
   void remap_region_ids (const region_id_map &map) FINAL OVERRIDE;
 
@@ -1555,8 +1578,9 @@ public:
                        vec<svalue_id> *arg_sids,
                        region_model_context *ctxt);
   region_id get_current_frame_id (const region_model &model) const;
-  svalue_id pop_frame (region_model *model, bool purge, purge_stats *stats,
-                      region_model_context *ctxt);
+  void pop_frame (region_model *model, region_id result_dst_rid,
+                 bool purge, purge_stats *stats,
+                 region_model_context *ctxt);
 
   region_id ensure_stack_region (region_model *model);
   region_id get_stack_region_id () const { return m_stack_rid; }
@@ -1724,8 +1748,9 @@ class region_model
                        region_model_context *ctxt);
   region_id get_current_frame_id () const;
   function * get_current_function () const;
-  svalue_id pop_frame (bool purge, purge_stats *stats,
-                      region_model_context *ctxt);
+  void pop_frame (region_id result_dst_rid,
+                 bool purge, purge_stats *stats,
+                 region_model_context *ctxt);
   int get_stack_depth () const;
   function *get_function_at_depth (unsigned depth) const;
 
@@ -1781,6 +1806,9 @@ class region_model
   svalue_id set_to_new_unknown_value (region_id dst_rid, tree type,
                                      region_model_context *ctxt);
 
+  void copy_region (region_id dst_rid, region_id src_rid,
+                   region_model_context *ctxt);
+
   tristate eval_condition (svalue_id lhs,
                           enum tree_code op,
                           svalue_id rhs) const;
@@ -1804,7 +1832,7 @@ class region_model
 
   void purge_unused_svalues (purge_stats *out,
                             region_model_context *ctxt,
-                            svalue_id *known_used_sid = NULL);
+                            svalue_id_set *known_used_sids = NULL);
   void remap_svalue_ids (const svalue_id_map &map);
   void remap_region_ids (const region_id_map &map);
 
@@ -1858,6 +1886,13 @@ class region_model
   region_id get_lvalue_1 (path_var pv, region_model_context *ctxt);
   svalue_id get_rvalue_1 (path_var pv, region_model_context *ctxt);
 
+  void copy_struct_region (region_id dst_rid, struct_region *dst_reg,
+                          struct_region *src_reg, region_model_context *ctxt);
+  void copy_union_region (region_id dst_rid, union_region *src_reg,
+                         region_model_context *ctxt);
+  void copy_array_region (region_id dst_rid, array_region *dst_reg,
+                         array_region *src_reg, region_model_context *ctxt);
+
   region_id make_region_for_unexpected_tree_code (region_model_context *ctxt,
                                                  tree t,
                                                  const dump_location_t &loc);
index 11ba84a00e05c95de989be55dd907bdc0259567f..4d617c72c1bad00e1c8a44a5db781479ea0a6e39 100644 (file)
@@ -1,3 +1,10 @@
+2020-04-01  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/94378
+       * gcc.dg/analyzer/compound-assignment-1.c: New test.
+       * gcc.dg/analyzer/compound-assignment-2.c: New test.
+       * gcc.dg/analyzer/compound-assignment-3.c: New test.
+
 2020-04-01  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/94436
diff --git a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c
new file mode 100644 (file)
index 0000000..bb03487
--- /dev/null
@@ -0,0 +1,71 @@
+#include <stdlib.h>
+
+struct ptr_wrapper
+{
+  int *ptr;
+};
+
+struct ptr_wrapper
+test_1 (void)
+{
+  struct ptr_wrapper r;
+  r.ptr = malloc (sizeof (int));
+  return r;
+}
+
+struct ptr_wrapper
+test_2 (void)
+{
+  struct ptr_wrapper r, s;
+  r.ptr = malloc (sizeof (int));
+  s = r;
+  return s;
+}
+
+struct nested
+{
+  struct ptr_wrapper w;
+};
+
+struct nested
+test_3 (void)
+{
+  struct nested n;
+  n.w.ptr = malloc (sizeof (int));
+  return n;
+}
+
+void test_4 (void)
+{
+  struct ptr_wrapper r;
+  r.ptr = malloc (sizeof (int)); /* { dg-message "allocated here" } */
+} /* { dg-warning "leak of 'r.ptr'" } */
+/* { dg-bogus "leak of '<unknown>'" "unknown leak" { xfail *-*-* } .-1 } */
+
+static struct ptr_wrapper __attribute__((noinline))
+called_by_test_5a (void)
+{
+  struct ptr_wrapper r;
+  r.ptr = malloc (sizeof (int));
+  return r;
+}
+
+void test_5a (void)
+{
+  struct ptr_wrapper q = called_by_test_5a ();  
+} /* { dg-warning "leak of 'q.ptr'" } */
+/* TODO: show the allocation point.  */
+
+static struct ptr_wrapper __attribute__((noinline))
+called_by_test_5b (void)
+{
+  struct ptr_wrapper r;
+  r.ptr = malloc (sizeof (int));
+  return r; /* { dg-warning "leak" } */
+  /* TODO: show the allocation point.  */
+}
+
+void test_5b (void)
+{
+  called_by_test_5b ();
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c
new file mode 100644 (file)
index 0000000..ecca9ac
--- /dev/null
@@ -0,0 +1,24 @@
+#include <stdlib.h>
+
+struct array_wrapper
+{
+  void *ptrs[2];
+};
+
+struct array_wrapper
+test_1 (void)
+{
+  struct array_wrapper aw1;
+  aw1.ptrs[0] = malloc (1024);
+  aw1.ptrs[1] = malloc (512);
+  return aw1;
+}
+
+struct array_wrapper
+test_2 (void)
+{
+  struct array_wrapper aw2;
+  aw2.ptrs[0] = malloc (1024);
+  aw2.ptrs[1] = malloc (512);
+} /* { dg-warning "leak of 'aw2.ptrs.0.'" "leak of element 0" } */
+/* { dg-warning "leak of 'aw2.ptrs.1.'" "leak of element 1" { target *-*-* } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c
new file mode 100644 (file)
index 0000000..5083faa
--- /dev/null
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+
+struct union_wrapper
+{
+  union
+  {
+    int i;
+    void *ptr;
+  } u;
+};
+
+struct union_wrapper
+test_1 (void)
+{
+  struct union_wrapper uw1;
+  uw1.u.ptr = malloc (1024);
+  return uw1;
+}
+
+struct union_wrapper
+test_2 (void)
+{
+  struct union_wrapper uw2;
+  uw2.u.ptr = malloc (1024);
+} /* { dg-warning "leak of '\\(void \\*\\)uw2.u'" } */