diff options
author | David Malcolm <dmalcolm@redhat.com> | 2021-04-08 09:46:03 -0400 |
---|---|---|
committer | David Malcolm <dmalcolm@redhat.com> | 2021-04-08 09:46:03 -0400 |
commit | 3a66c289a3f395e50de79424e1e6f401a4dc1ab7 (patch) | |
tree | 3172c6eddc63beb32dc40b78df987ecb11766421 /gcc/analyzer/store.h | |
parent | 8e84a142913a1f3084d415462024964f97356bee (diff) | |
download | gcc-3a66c289a3f395e50de79424e1e6f401a4dc1ab7.zip gcc-3a66c289a3f395e50de79424e1e6f401a4dc1ab7.tar.gz gcc-3a66c289a3f395e50de79424e1e6f401a4dc1ab7.tar.bz2 |
analyzer: fix leak false +ves due to maybe-clobbered regions [PR99042,PR99774]
Prior to this patch, program_state::detect_leaks worked by finding all
live svalues in the old state and in the new state, and calling
on_svalue_leak for each svalue that has changed from being live to
not being live.
PR analyzer/99042 and PR analyzer/99774 both describe false leak
diagnostics from -fanalyzer (a false FILE * leak in git, and a false
malloc leak in qemu, respectively).
In both cases the root cause of the false leak diagnostic relates to
svalues no longer being explicitly bound in the store due to regions
being conservatively clobbered, due to an unknown function being
called, or due to a write through a pointer that could alias the
region, respectively.
We have a transition from an svalue being explicitly live to not
being explicitly live - but only because the store is being
conservative, clobbering the binding. The leak detection is looking
for transitions from "definitely live" to "not definitely live",
when it should be looking for transitions from "definitely live"
to "definitely not live".
This patch introduces a new class to temporarily capture information
about svalues that were explicitly live, but for which a region bound
to them got clobbered for conservative reasons. This new
"uncertainty_t" class is passed around to capture the data long enough
for use in program_state::detect_leaks, where it is used to only
complain about svalues that were definitely live and are now both
not definitely live *or* possibly-live i.e. definitely not-live.
The class also captures for which svalues we can't meaningfully track
sm-state anymore, and resets the svalues back to the "start" state.
Together, these changes fix the false leak reports.
gcc/analyzer/ChangeLog:
PR analyzer/99042
PR analyzer/99774
* engine.cc
(impl_region_model_context::impl_region_model_context): Add
uncertainty param and use it to initialize m_uncertainty.
(impl_region_model_context::get_uncertainty): New.
(impl_sm_context::get_fndecl_for_call): Add NULL for new
uncertainty param when constructing impl_region_model_context.
(impl_sm_context::get_state): Likewise.
(impl_sm_context::set_next_state): Likewise.
(impl_sm_context::warn): Likewise.
(exploded_node::on_stmt): Add uncertainty param
and use it when constructing impl_region_model_context.
(exploded_node::on_edge): Add uncertainty param and pass
to on_edge call.
(exploded_node::detect_leaks): Create uncertainty_t and pass to
impl_region_model_context.
(exploded_graph::get_or_create_node): Create uncertainty_t and
pass to prune_for_point.
(maybe_process_run_of_before_supernode_enodes): Create
uncertainty_t and pass to impl_region_model_context.
(exploded_graph::process_node): Create uncertainty_t instances and
pass around as needed.
* exploded-graph.h
(impl_region_model_context::impl_region_model_context): Add
uncertainty param.
(impl_region_model_context::get_uncertainty): New decl.
(impl_region_model_context::m_uncertainty): New field.
(exploded_node::on_stmt): Add uncertainty param.
(exploded_node::on_edge): Likewise.
* program-state.cc (sm_state_map::on_liveness_change): Get
uncertainty from context and use it to unset sm-state from
svalues as appropriate.
(program_state::on_edge): Add uncertainty param and use it when
constructing impl_region_model_context. Fix indentation.
(program_state::prune_for_point): Add uncertainty param and use it
when constructing impl_region_model_context.
(program_state::detect_leaks): Get any uncertainty from ctxt and
use it to get maybe-live svalues for dest_state, rather than
definitely-live ones; use this when determining which svalues
have leaked.
(selftest::test_program_state_merging): Create uncertainty_t and
pass to impl_region_model_context.
* program-state.h (program_state::on_edge): Add uncertainty param.
(program_state::prune_for_point): Likewise.
* region-model-impl-calls.cc (call_details::get_uncertainty): New.
(region_model::impl_call_memcpy): Pass uncertainty to
mark_region_as_unknown call.
(region_model::impl_call_memset): Likewise.
(region_model::impl_call_strcpy): Likewise.
* region-model-reachability.cc (reachable_regions::handle_sval):
Also add sval to m_mutable_svals.
* region-model.cc (region_model::on_assignment): Pass any
uncertainty from ctxt to the store::set_value call.
(region_model::handle_unrecognized_call): Get any uncertainty from
ctxt and use it to record mutable svalues at the unknown call.
(region_model::get_reachable_svalues): Add uncertainty param and
use it to mark any maybe-bound svalues as being reachable.
(region_model::set_value): Pass any uncertainty from ctxt to the
store::set_value call.
(region_model::mark_region_as_unknown): Add uncertainty param and
pass it on to the store::mark_region_as_unknown call.
(region_model::update_for_call_summary): Add uncertainty param and
pass it on to the region_model::mark_region_as_unknown call.
* region-model.h (call_details::get_uncertainty): New decl.
(region_model::get_reachable_svalues): Add uncertainty param.
(region_model::mark_region_as_unknown): Add uncertainty param.
(region_model_context::get_uncertainty): New vfunc.
(noop_region_model_context::get_uncertainty): New vfunc
implementation.
* store.cc (dump_svalue_set): New.
(uncertainty_t::dump_to_pp): New.
(uncertainty_t::dump): New.
(binding_cluster::clobber_region): Pass NULL for uncertainty to
remove_overlapping_bindings.
(binding_cluster::mark_region_as_unknown): Add uncertainty param
and pass it to remove_overlapping_bindings.
(binding_cluster::remove_overlapping_bindings): Add uncertainty param.
Use it to record any svalues that were in clobbered bindings.
(store::set_value): Add uncertainty param. Pass it to
binding_cluster::mark_region_as_unknown when handling symbolic
regions.
(store::mark_region_as_unknown): Add uncertainty param and pass it
to binding_cluster::mark_region_as_unknown.
(store::remove_overlapping_bindings): Add uncertainty param and
pass it to binding_cluster::remove_overlapping_bindings.
* store.h (binding_cluster::mark_region_as_unknown): Add
uncertainty param.
(binding_cluster::remove_overlapping_bindings): Likewise.
(store::set_value): Likewise.
(store::mark_region_as_unknown): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/99042
PR analyzer/99774
* gcc.dg/analyzer/pr99042.c: New test.
* gcc.dg/analyzer/pr99774-1.c: New test.
* gcc.dg/analyzer/pr99774-2.c: New test.
Diffstat (limited to 'gcc/analyzer/store.h')
-rw-r--r-- | gcc/analyzer/store.h | 89 |
1 files changed, 85 insertions, 4 deletions
diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index 2bcef6c..dc22d96 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -119,6 +119,83 @@ along with GCC; see the file COPYING3. If not see namespace ana { +/* A class for keeping track of aspects of a program_state that we don't + know about, to avoid false positives about leaks. + + Consider: + + p->field = malloc (1024); + q->field = NULL; + + where we don't know whether or not p and q point to the same memory, + and: + + p->field = malloc (1024); + unknown_fn (p); + + In both cases, the svalue for the address of the allocated buffer + goes from being bound to p->field to not having anything explicitly bound + to it. + + Given that we conservatively discard bindings due to possible aliasing or + calls to unknown function, the store loses references to svalues, + but these svalues could still be live. We don't want to warn about + them leaking - they're effectively in a "maybe live" state. + + This "maybe live" information is somewhat transient. + + We don't want to store this "maybe live" information in the program_state, + region_model, or store, since we don't want to bloat these objects (and + potentially bloat the exploded_graph with more nodes). + However, we can't store it in the region_model_context, as these context + objects sometimes don't last long enough to be around when comparing the + old vs the new state. + + This class is a way to track a set of such svalues, so that we can + temporarily capture that they are in a "maybe live" state whilst + comparing old and new states. */ + +class uncertainty_t +{ +public: + typedef hash_set<const svalue *>::iterator iterator; + + void on_maybe_bound_sval (const svalue *sval) + { + m_maybe_bound_svals.add (sval); + } + void on_mutable_sval_at_unknown_call (const svalue *sval) + { + m_mutable_at_unknown_call_svals.add (sval); + } + + bool unknown_sm_state_p (const svalue *sval) + { + return (m_maybe_bound_svals.contains (sval) + || m_mutable_at_unknown_call_svals.contains (sval)); + } + + void dump_to_pp (pretty_printer *pp, bool simple) const; + void dump (bool simple) const; + + iterator begin_maybe_bound_svals () const + { + return m_maybe_bound_svals.begin (); + } + iterator end_maybe_bound_svals () const + { + return m_maybe_bound_svals.end (); + } + +private: + + /* svalues that might or might not still be bound. */ + hash_set<const svalue *> m_maybe_bound_svals; + + /* svalues that have mutable sm-state at unknown calls. */ + hash_set<const svalue *> m_mutable_at_unknown_call_svals; +}; + class concrete_binding; /* An enum for discriminating between "direct" vs "default" levels of @@ -409,7 +486,8 @@ public: void clobber_region (store_manager *mgr, const region *reg); void purge_region (store_manager *mgr, const region *reg); void zero_fill_region (store_manager *mgr, const region *reg); - void mark_region_as_unknown (store_manager *mgr, const region *reg); + void mark_region_as_unknown (store_manager *mgr, const region *reg, + uncertainty_t *uncertainty); const svalue *get_binding (store_manager *mgr, const region *reg, binding_kind kind) const; @@ -421,7 +499,8 @@ public: const svalue *maybe_get_compound_binding (store_manager *mgr, const region *reg) const; - void remove_overlapping_bindings (store_manager *mgr, const region *reg); + void remove_overlapping_bindings (store_manager *mgr, const region *reg, + uncertainty_t *uncertainty); template <typename T> void for_each_value (void (*cb) (const svalue *sval, T user_data), @@ -539,11 +618,13 @@ public: bool called_unknown_fn_p () const { return m_called_unknown_fn; } void set_value (store_manager *mgr, const region *lhs_reg, - const svalue *rhs_sval, enum binding_kind kind); + const svalue *rhs_sval, enum binding_kind kind, + uncertainty_t *uncertainty); void clobber_region (store_manager *mgr, const region *reg); void purge_region (store_manager *mgr, const region *reg); void zero_fill_region (store_manager *mgr, const region *reg); - void mark_region_as_unknown (store_manager *mgr, const region *reg); + void mark_region_as_unknown (store_manager *mgr, const region *reg, + uncertainty_t *uncertainty); const binding_cluster *get_cluster (const region *base_reg) const; binding_cluster *get_cluster (const region *base_reg); |