From 3734527dfa0d10a50aee2f088d37320000fd65bf Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 28 Mar 2022 20:41:23 -0400 Subject: analyzer: ensure that we purge state when reusing a conjured_svalue [PR105087] PR analyzer/105087 describes a false positive from -Wanalyzer-double-free in which the analyzer erroneously considers two successive inlined vasprintf calls to have allocated the same pointer. The root cause is that the result written back from vasprintf is a conjured_svalue, and that we normally purge state when reusing a conjured_svalue, but various places in the code were calling region_model_manager::get_or_create_conjured_svalue but failing to then call region_model::purge_state_involving on the result. This patch fixes things by moving responsibility for calling region_model::purge_state_involving into region_model_manager::get_or_create_conjured_svalue, so that it is always called when reusing a conjured_svalue, fixing the false positive. gcc/analyzer/ChangeLog: PR analyzer/105087 * analyzer.h (class conjured_purge): New forward decl. * region-model-asm.cc (region_model::on_asm_stmt): Add conjured_purge param to calls binding_cluster::on_asm and region_model_manager::get_or_create_conjured_svalue. * region-model-impl-calls.cc (call_details::get_or_create_conjured_svalue): Likewise for call to region_model_manager::get_or_create_conjured_svalue. (region_model::impl_call_fgets): Remove call to region_model::purge_state_involving, as this is now done implicitly by call_details::get_or_create_conjured_svalue. (region_model::impl_call_fread): Likewise. (region_model::impl_call_strchr): Pass conjured_purge param to call to region_model_manager::get_or_create_conjured_svalue. * region-model-manager.cc (conjured_purge::purge): New. (region_model_manager::get_or_create_conjured_svalue): Add param "p". Use it to purge state when reusing an existing conjured_svalue. * region-model.cc (region_model::on_call_pre): Replace call to region_model::purge_state_involving with passing conjured_purge to region_model_manager::get_or_create_conjured_svalue. (region_model::handle_unrecognized_call): Pass conjured_purge to store::on_unknown_fncall. * region-model.h (region_model_manager::get_or_create_conjured_svalue): Add param "p". * store.cc (binding_cluster::on_unknown_fncall): Likewise. Pass it on to region_model_manager::get_or_create_conjured_svalue. (binding_cluster::on_asm): Likewise. (store::on_unknown_fncall): Add param "p" and pass it on to binding_cluster::on_unknown_fncall. * store.h (binding_cluster::on_unknown_fncall): Add param p. (binding_cluster::on_asm): Likewise. (store::on_unknown_fncall): Likewise. * svalue.h (class conjured_purge): New. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/pr105087-1.c: New test. * gcc.dg/analyzer/pr105087-2.c: New test. * gcc.dg/analyzer/vasprintf-1.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/store.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'gcc/analyzer/store.h') diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index ee084dd..89bb352 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -609,8 +609,10 @@ public: store_manager *mgr); void mark_as_escaped (); - void on_unknown_fncall (const gcall *call, store_manager *mgr); - void on_asm (const gasm *stmt, store_manager *mgr); + void on_unknown_fncall (const gcall *call, store_manager *mgr, + const conjured_purge &p); + void on_asm (const gasm *stmt, store_manager *mgr, + const conjured_purge &p); bool escaped_p () const { return m_escaped; } bool touched_p () const { return m_touched; } @@ -735,7 +737,8 @@ public: model_merger *merger); void mark_as_escaped (const region *base_reg); - void on_unknown_fncall (const gcall *call, store_manager *mgr); + void on_unknown_fncall (const gcall *call, store_manager *mgr, + const conjured_purge &p); bool escaped_p (const region *reg) const; void get_representative_path_vars (const region_model *model, -- cgit v1.1