From d931a4f68574d8ebaaf7469ceb2205af509663e5 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 26 Jun 2020 12:43:27 -0400 Subject: [PATCH 263/315] FIXME: fix missing leaks from sm-file by setting LHS at calls to INIT_VAL(SSA_NAME) and updating detection of implicit reachability to not use stale stack frames --- gcc/analyzer/program-state.cc | 18 ++++-- gcc/analyzer/region-model2-impl-calls.cc | 3 +- gcc/analyzer/region-model2.cc | 74 +++++++++++++++++++++++- gcc/analyzer/region-model2.h | 3 + 4 files changed, 88 insertions(+), 10 deletions(-) diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index d595376dbe7..485d8f0dc53 100644 --- a/gcc/analyzer/program-state.cc +++ b/gcc/analyzer/program-state.cc @@ -2081,16 +2081,22 @@ program_state::detect_leaks2 (const program_state &src_state, if (!dest_svalues.contains (sval)) { /* If src_svalues contains a use of INIT_VAL(REGION) and - dest_svalues doesn't, we don't want to complain if - the region still implicitly has that value. */ + dest_svalues doesn't, we don't want to complain about a leak + if the region still implicitly has that value. */ if (const initial_svalue2 *init_sval = sval->dyn_cast_initial_svalue2 ()) { const region2 *reg = init_sval->get_region (); - const svalue2 *reg_sval - = dest_state.m_region_model2->get_store_value (reg); - if (reg_sval == init_sval) - continue; + /* It must be a region that exists; we don't want to consider + INIT_VAL(R) as still being implicitly reachable if R is in + a popped stack frame. */ + if (dest_state.m_region_model2->region_exists_p (reg)) + { + const svalue2 *reg_sval + = dest_state.m_region_model2->get_store_value (reg); + if (reg_sval == init_sval) + continue; + } } #if 0 pretty_printer pp1, pp2, pp3; diff --git a/gcc/analyzer/region-model2-impl-calls.cc b/gcc/analyzer/region-model2-impl-calls.cc index 415e6efa420..324b07c3b1b 100644 --- a/gcc/analyzer/region-model2-impl-calls.cc +++ b/gcc/analyzer/region-model2-impl-calls.cc @@ -289,7 +289,8 @@ region_model2::impl_call_memset (const call_details &cd) return false; } -/* Handle the on_call_pre part of "strlen". */ +/* Handle the on_call_pre part of "strlen". + Return true if the LHS is updated. */ bool region_model2::impl_call_strlen (const call_details &cd) diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index b4c84f5f279..18e75e04662 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -1212,6 +1212,25 @@ region2::descendent_of_p (const region2 *elder) const return false; } +/* If this region is a frame_region2, or a descendent of one, return it. + Otherwise return NULL. */ + +const frame_region2 * +region2::maybe_get_frame_region () const +{ + const region2 *iter = this; + while (iter) + { + if (const frame_region2 *frame_reg = iter->dyn_cast_frame_region2 ()) + return frame_reg; + if (iter->get_kind () == RK_CAST) + iter = iter->dyn_cast_cast_region2 ()->get_original_region (); + else + iter = iter->get_parent_region (); + } + return NULL; +} + /* FIXME. */ tree @@ -4866,9 +4885,15 @@ region_model2::on_call_pre (const gcall *call, region_model2_context *ctxt) return impl_call_builtin_expect (cd); else if (is_named_call_p (callee_fndecl, "memset", call, 3) || gimple_call_builtin_p (call, BUILT_IN_MEMSET)) - impl_call_memset (cd); + { + impl_call_memset (cd); + return false; + } else if (is_named_call_p (callee_fndecl, "strlen", call, 1)) - impl_call_strlen (cd); + { + if (impl_call_strlen (cd)) + return false; + } else if (is_named_call_p (callee_fndecl, "__analyzer_dump_num_heap_region2s", call, 0)) { @@ -4896,7 +4921,20 @@ region_model2::on_call_pre (const gcall *call, region_model2_context *ctxt) else unknown_side_effects = true; - /* Unknown return value. */ + /* Some of the above cases update the lhs of the call based on the + return value. If we get here, it hasn't been done yet, so do that + now. */ + if (tree lhs = gimple_call_lhs (call)) + { + const region2 *lhs_region = get_lvalue (lhs, ctxt); + // FIXME + if (TREE_CODE (lhs) == SSA_NAME) + { + const svalue2 *sval = m_mgr->get_or_create_initial_value (lhs_region); + set_value (lhs_region, sval, ctxt); + } + } + #if 0 if (lhs_reg) set_to_new_unknown_value (lhs_reg, lhs_type, ctxt); @@ -5672,6 +5710,28 @@ region_model2::get_store_value (const region2 *reg) return m_mgr->get_or_create_initial_value (reg); } +/* Return false if REG does not exist, true if it may do. + This is for detecting regions within the stack that don't exist anymore + after frames are popped. */ + +bool +region_model2::region_exists_p (const region2 *reg) const +{ + /* If within a stack frame, check that the stack frame is live. */ + if (const frame_region2 *enclosing_frame = reg->maybe_get_frame_region ()) + { + /* Check that the current frame is the enclosing frame, or is called + by it. */ + for (const frame_region2 *iter_frame = get_current_frame (); iter_frame; + iter_frame = iter_frame->get_calling_frame ()) + if (iter_frame == enclosing_frame) + return true; + return false; + } + + return true; +} + /* Build a cast of SRC_EXPR to DST_TYPE, or return NULL_TREE. Adapted from gcc::jit::playback::context::build_cast, which in turn is @@ -9011,10 +9071,13 @@ test_stack_frames () = model.push_frame (DECL_STRUCT_FUNCTION (parent_fndecl), NULL, &ctxt); ASSERT_EQ (model.get_current_frame (), parent_frame_reg); + ASSERT_TRUE (model.region_exists_p (parent_frame_reg)); const region2 *a_in_parent_reg = model.get_lvalue (a, &ctxt); model.set_value (a_in_parent_reg, model.get_rvalue (int_42, &ctxt), &ctxt); + ASSERT_EQ (a_in_parent_reg->maybe_get_frame_region (), parent_frame_reg); + #if 0 model.set_to_new_unknown_value (model.get_lvalue (b, &ctxt), integer_type_node, &ctxt); @@ -9027,10 +9090,12 @@ test_stack_frames () const region2 *child_frame_reg = model.push_frame (DECL_STRUCT_FUNCTION (child_fndecl), NULL, &ctxt); ASSERT_EQ (model.get_current_frame (), child_frame_reg); + ASSERT_TRUE (model.region_exists_p (child_frame_reg)); const region2 *x_in_child_reg = model.get_lvalue (x, &ctxt); model.set_value (x_in_child_reg, model.get_rvalue (int_0, &ctxt), &ctxt); + ASSERT_EQ (x_in_child_reg->maybe_get_frame_region (), child_frame_reg); #if 0 model.set_to_new_unknown_value (model.get_lvalue (y, &ctxt), integer_type_node, &ctxt); @@ -9044,6 +9109,7 @@ test_stack_frames () model.set_value (p_in_globals_reg, mgr.get_ptr_svalue2 (ptr_type_node, x_in_child_reg), &ctxt); + ASSERT_EQ (p_in_globals_reg->maybe_get_frame_region (), NULL); /* Point another global pointer at p: q = &p. */ const region2 *q_in_globals_reg = model.get_lvalue (q, &ctxt); @@ -9073,6 +9139,8 @@ test_stack_frames () /* Pop the "child_fn" frame from the stack. */ purge_stats2 purged; model.pop_frame (NULL, true, &purged, NULL, &ctxt); + ASSERT_FALSE (model.region_exists_p (child_frame_reg)); + ASSERT_TRUE (model.region_exists_p (parent_frame_reg)); #if 0 /* We should have purged the unknown values for x and y. */ diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index d441ddc1b2c..fc620f76756 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -1156,6 +1156,7 @@ public: const region2 *get_parent_region () const { return m_parent; } const region2 *get_base_region () const; bool descendent_of_p (const region2 *elder) const; + const frame_region2 *maybe_get_frame_region () const; tree maybe_get_decl () const; @@ -2359,6 +2360,8 @@ class region_model2 const svalue2 *get_store_value (const region2 *reg); + bool region_exists_p (const region2 *reg) const; + private: const region2 *get_lvalue_1 (path_var pv, region_model2_context *ctxt); -- 2.26.2