From a96f1c38a787fbc847cb014d4b094e2787d539a7 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 30 Mar 2020 17:02:59 -0400 Subject: [PATCH] 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::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 | 57 +++ gcc/analyzer/checker-path.cc | 1 + gcc/analyzer/constraint-manager.cc | 1 + gcc/analyzer/diagnostic-manager.cc | 1 + gcc/analyzer/engine.cc | 4 +- gcc/analyzer/program-point.cc | 1 + gcc/analyzer/program-state.cc | 1 + gcc/analyzer/region-model.cc | 342 ++++++++++++++---- gcc/analyzer/region-model.h | 71 +++- gcc/testsuite/ChangeLog | 7 + .../gcc.dg/analyzer/compound-assignment-1.c | 71 ++++ .../gcc.dg/analyzer/compound-assignment-2.c | 24 ++ .../gcc.dg/analyzer/compound-assignment-3.c | 25 ++ 13 files changed, 522 insertions(+), 84 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 98093cd4f35..3f136f45c20 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,60 @@ +2020-04-01 David Malcolm + + 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::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 * program-state.cc (selftest::test_program_state_dumping): Update diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc index c781cd8dbeb..c07273460d5 100644 --- a/gcc/analyzer/checker-path.cc +++ b/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" diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc index a6235435681..786bd6ccf85 100644 --- a/gcc/analyzer/constraint-manager.cc +++ b/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" diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 4b884c7a452..e132a5625de 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/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" diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index befd4836cb4..880e70fb2ba 100644 --- a/gcc/analyzer/engine.cc +++ b/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. */ diff --git a/gcc/analyzer/program-point.cc b/gcc/analyzer/program-point.cc index 2e7f7694111..7bf2de4e3bf 100644 --- a/gcc/analyzer/program-point.cc +++ b/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" diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index a6049604ca8..43396c65827 100644 --- a/gcc/analyzer/program-state.cc +++ b/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" diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 68152526486..30fe72b90e9 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -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's ctor. */ - -template<> -id_set::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 (dst_reg); + struct_region *src_sub = as_a (src_reg); + copy_struct_region (dst_rid, dst_sub, src_sub, ctxt); + } + return; + case RK_UNION: + { + union_region *src_sub = as_a (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 (dst_reg); + array_region *src_sub = as_a (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_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 (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_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 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 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 (); diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 235db72141e..2c9ee39a38d 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -370,25 +370,24 @@ one_way_id_map::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 -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 (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_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 (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 *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); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 11ba84a00e0..4d617c72c1b 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2020-04-01 David Malcolm + + 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 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 index 00000000000..bb03487f23e --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-1.c @@ -0,0 +1,71 @@ +#include + +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 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 index 00000000000..ecca9aca83b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c @@ -0,0 +1,24 @@ +#include + +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 index 00000000000..5083faa9ce2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c @@ -0,0 +1,25 @@ +#include + +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'" } */ -- 2.30.2