analyzer: handle compound assignments [PR94378]

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.

gcc/analyzer/ChangeLog

+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

gcc/analyzer/checker-path.cc

@@ -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"

gcc/analyzer/constraint-manager.cc

@@ -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"

gcc/analyzer/diagnostic-manager.cc

@@ -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"

gcc/analyzer/engine.cc

@@ -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.  */

gcc/analyzer/program-point.cc

@@ -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"

gcc/analyzer/program-state.cc

@@ -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"

gcc/analyzer/region-model.h

@@ -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 (T 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 (T 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);

gcc/testsuite/ChangeLog

+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

gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c

+#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 ();
+}

gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c

+#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 } */

gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c

+#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'" } */