[committed] analyzer: fix false leak involving params [PR98969]
David Malcolm
dmalcolm@redhat.com
Wed Feb 17 15:46:47 GMT 2021
This patch updates the svalue liveness code so that the initial value
of parameters at top-level functions to the analysis are treated as
live (since the values are presumably still live within the
outside-of-the-analysis calling code).
This fixes the false leak in PR analyzer/98969 seen on:
void
test (long int i)
{
struct foo *f = (struct foo *)i;
f->expr = __builtin_malloc (1024);
}
since the calling code can presumably still access the allocated
buffer via:
((struct foo *)i)->expr
The patch also removes the expected leak warnings from
g++.dg/analyzer/pr99064.C and gcc.dg/analyzer/pr96841.c, which now
appear to me to be false positives.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r11-7270-ge0139b2a912585496f23c352f0e2c56895f78fbf.
gcc/analyzer/ChangeLog:
PR analyzer/98969
* constraint-manager.cc (dead_svalue_purger::should_purge_p):
Update for change to svalue::live_p.
* program-state.cc (sm_state_map::on_liveness_change): Likewise.
(program_state::detect_leaks): Likewise.
* region-model-reachability.cc (reachable_regions::init_cluster):
When dealing with a symbolic region, if the underlying pointer is
implicitly live, add the region to the reachable regions.
* region-model.cc (region_model::compare_initial_and_pointer):
Move logic for detecting initial values of params to
initial_svalue::initial_value_of_param_p.
* svalue.cc (svalue::live_p): Convert "live_svalues" from a
reference to a pointer; support it being NULL.
(svalue::implicitly_live_p): Convert first param from a
refererence to a pointer.
(region_svalue::implicitly_live_p): Likewise.
(constant_svalue::implicitly_live_p): Likewise.
(initial_svalue::implicitly_live_p): Likewise. Treat the initial
values of params for the top level frame as still live.
(initial_svalue::initial_value_of_param_p): New function, taken
from a test in region_model::compare_initial_and_pointer.
(unaryop_svalue::implicitly_live_p): Convert first param from a
refererence to a pointer.
(binop_svalue::implicitly_live_p): Likewise.
(sub_svalue::implicitly_live_p): Likewise.
(unmergeable_svalue::implicitly_live_p): Likewise.
* svalue.h (svalue::live_p): Likewise.
(svalue::implicitly_live_p): Likewise.
(region_svalue::implicitly_live_p): Likewise.
(constant_svalue::implicitly_live_p): Likewise.
(initial_svalue::implicitly_live_p): Likewise.
(initial_svalue::initial_value_of_param_p): New decl.
(unaryop_svalue::implicitly_live_p): Convert first param from a
refererence to a pointer.
(binop_svalue::implicitly_live_p): Likewise.
(sub_svalue::implicitly_live_p): Likewise.
(unmergeable_svalue::implicitly_live_p): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/98969
* g++.dg/analyzer/pr99064.C: Convert dg-bogus to dg-warning.
* gcc.dg/analyzer/pr96841.c: Add -Wno-analyzer-too-complex to
options. Remove false leak directive.
* gcc.dg/analyzer/pr98969.c (test_1): Remove xfail from leak
false positive.
(test_3): New.
---
gcc/analyzer/constraint-manager.cc | 2 +-
gcc/analyzer/program-state.cc | 4 +-
gcc/analyzer/region-model-reachability.cc | 2 +
gcc/analyzer/region-model.cc | 14 +-----
gcc/analyzer/svalue.cc | 52 +++++++++++++++++------
gcc/analyzer/svalue.h | 20 +++++----
gcc/testsuite/g++.dg/analyzer/pr99064.C | 2 +-
gcc/testsuite/gcc.dg/analyzer/pr96841.c | 4 +-
gcc/testsuite/gcc.dg/analyzer/pr98969.c | 9 +++-
9 files changed, 69 insertions(+), 40 deletions(-)
diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 23421080fba..4dadd200bee 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -1643,7 +1643,7 @@ public:
bool should_purge_p (const svalue *sval) const
{
- return !sval->live_p (m_live_svalues, m_model);
+ return !sval->live_p (&m_live_svalues, m_model);
}
private:
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 60fed56cd65..e427fff59d6 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -523,7 +523,7 @@ sm_state_map::on_liveness_change (const svalue_set &live_svalues,
++iter)
{
const svalue *iter_sval = (*iter).first;
- if (!iter_sval->live_p (live_svalues, model))
+ if (!iter_sval->live_p (&live_svalues, model))
{
svals_to_unset.add (iter_sval);
entry_t e = (*iter).second;
@@ -1201,7 +1201,7 @@ program_state::detect_leaks (const program_state &src_state,
live in DEST_STATE: either explicitly reachable, or implicitly
live based on the set of explicitly reachable svalues.
Record those that have ceased to be live. */
- if (!sval->live_p (dest_svalues, dest_state.m_region_model))
+ if (!sval->live_p (&dest_svalues, dest_state.m_region_model))
dead_svals.quick_push (sval);
}
diff --git a/gcc/analyzer/region-model-reachability.cc b/gcc/analyzer/region-model-reachability.cc
index a988ffc1439..087185b4e45 100644
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -91,6 +91,8 @@ reachable_regions::init_cluster (const region *base_reg)
if (const symbolic_region *sym_reg = base_reg->dyn_cast_symbolic_region ())
{
const svalue *ptr = sym_reg->get_pointer ();
+ if (ptr->implicitly_live_p (NULL, m_model))
+ add (base_reg, true);
switch (ptr->get_kind ())
{
default:
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 2fc07c49649..159b0f18654 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1980,18 +1980,8 @@ region_model::compare_initial_and_pointer (const initial_svalue *init,
/* If we have a pointer to something within a stack frame, it can't be the
initial value of a param. */
if (pointee->maybe_get_frame_region ())
- {
- const region *reg = init->get_region ();
- if (tree reg_decl = reg->maybe_get_decl ())
- if (TREE_CODE (reg_decl) == SSA_NAME)
- {
- tree ssa_name = reg_decl;
- if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)
- && SSA_NAME_VAR (ssa_name)
- && TREE_CODE (SSA_NAME_VAR (ssa_name)) == PARM_DECL)
- return tristate::TS_FALSE;
- }
- }
+ if (init->initial_value_of_param_p ())
+ return tristate::TS_FALSE;
return tristate::TS_UNKNOWN;
}
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 5bbd05e8327..d6305a37b9a 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -246,15 +246,18 @@ svalue::can_merge_p (const svalue *other,
}
/* Determine if this svalue is either within LIVE_SVALUES, or is implicitly
- live with respect to LIVE_SVALUES and MODEL. */
+ live with respect to LIVE_SVALUES and MODEL.
+ LIVE_SVALUES can be NULL, in which case determine if this svalue is
+ intrinsically live. */
bool
-svalue::live_p (const svalue_set &live_svalues,
+svalue::live_p (const svalue_set *live_svalues,
const region_model *model) const
{
/* Determine if SVAL is explicitly live. */
- if (const_cast<svalue_set &> (live_svalues).contains (this))
- return true;
+ if (live_svalues)
+ if (const_cast<svalue_set *> (live_svalues)->contains (this))
+ return true;
/* Otherwise, determine if SVAL is implicitly live due to being made of
other live svalues. */
@@ -264,7 +267,7 @@ svalue::live_p (const svalue_set &live_svalues,
/* Base implementation of svalue::implicitly_live_p. */
bool
-svalue::implicitly_live_p (const svalue_set &, const region_model *) const
+svalue::implicitly_live_p (const svalue_set *, const region_model *) const
{
return false;
}
@@ -512,7 +515,7 @@ region_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for region_svalue. */
bool
-region_svalue::implicitly_live_p (const svalue_set &,
+region_svalue::implicitly_live_p (const svalue_set *,
const region_model *model) const
{
/* Pointers into clusters that have escaped should be treated as live. */
@@ -609,7 +612,7 @@ constant_svalue::accept (visitor *v) const
Constants are implicitly live. */
bool
-constant_svalue::implicitly_live_p (const svalue_set &,
+constant_svalue::implicitly_live_p (const svalue_set *,
const region_model *) const
{
return true;
@@ -749,7 +752,7 @@ initial_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for initial_svalue. */
bool
-initial_svalue::implicitly_live_p (const svalue_set &,
+initial_svalue::implicitly_live_p (const svalue_set *,
const region_model *model) const
{
/* This svalue may be implicitly live if the region still implicitly
@@ -765,6 +768,31 @@ initial_svalue::implicitly_live_p (const svalue_set &,
return true;
}
+ /* Assume that the initial values of params for the top level frame
+ are still live, because (presumably) they're still
+ live in the external caller. */
+ if (initial_value_of_param_p ())
+ if (const frame_region *frame_reg = m_reg->maybe_get_frame_region ())
+ if (frame_reg->get_calling_frame () == NULL)
+ return true;
+
+ return false;
+}
+
+/* Return true if this is the initial value of a function parameter. */
+
+bool
+initial_svalue::initial_value_of_param_p () const
+{
+ if (tree reg_decl = m_reg->maybe_get_decl ())
+ if (TREE_CODE (reg_decl) == SSA_NAME)
+ {
+ tree ssa_name = reg_decl;
+ if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)
+ && SSA_NAME_VAR (ssa_name)
+ && TREE_CODE (SSA_NAME_VAR (ssa_name)) == PARM_DECL)
+ return true;
+ }
return false;
}
@@ -816,7 +844,7 @@ unaryop_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for unaryop_svalue. */
bool
-unaryop_svalue::implicitly_live_p (const svalue_set &live_svalues,
+unaryop_svalue::implicitly_live_p (const svalue_set *live_svalues,
const region_model *model) const
{
return get_arg ()->live_p (live_svalues, model);
@@ -862,7 +890,7 @@ binop_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for binop_svalue. */
bool
-binop_svalue::implicitly_live_p (const svalue_set &live_svalues,
+binop_svalue::implicitly_live_p (const svalue_set *live_svalues,
const region_model *model) const
{
return (get_arg0 ()->live_p (live_svalues, model)
@@ -919,7 +947,7 @@ sub_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for sub_svalue. */
bool
-sub_svalue::implicitly_live_p (const svalue_set &live_svalues,
+sub_svalue::implicitly_live_p (const svalue_set *live_svalues,
const region_model *model) const
{
return get_parent ()->live_p (live_svalues, model);
@@ -1136,7 +1164,7 @@ unmergeable_svalue::accept (visitor *v) const
/* Implementation of svalue::implicitly_live_p vfunc for unmergeable_svalue. */
bool
-unmergeable_svalue::implicitly_live_p (const svalue_set &live_svalues,
+unmergeable_svalue::implicitly_live_p (const svalue_set *live_svalues,
const region_model *model) const
{
return get_arg ()->live_p (live_svalues, model);
diff --git a/gcc/analyzer/svalue.h b/gcc/analyzer/svalue.h
index 0703cac8bb3..672a89cccff 100644
--- a/gcc/analyzer/svalue.h
+++ b/gcc/analyzer/svalue.h
@@ -128,9 +128,9 @@ public:
virtual void accept (visitor *v) const = 0;
- bool live_p (const svalue_set &live_svalues,
+ bool live_p (const svalue_set *live_svalues,
const region_model *model) const;
- virtual bool implicitly_live_p (const svalue_set &live_svalues,
+ virtual bool implicitly_live_p (const svalue_set *live_svalues,
const region_model *model) const;
static int cmp_ptr (const svalue *, const svalue *);
@@ -194,7 +194,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
const region * get_pointee () const { return m_reg; }
@@ -243,7 +243,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
tree get_constant () const { return m_cst_expr; }
@@ -493,9 +493,11 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
+ bool initial_value_of_param_p () const;
+
const region *get_region () const { return m_reg; }
private:
@@ -564,7 +566,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
enum tree_code get_op () const { return m_op; }
@@ -653,7 +655,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
enum tree_code get_op () const { return m_op; }
@@ -734,7 +736,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
const svalue *get_parent () const { return m_parent_svalue; }
@@ -788,7 +790,7 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE;
void accept (visitor *v) const FINAL OVERRIDE;
- bool implicitly_live_p (const svalue_set &,
+ bool implicitly_live_p (const svalue_set *,
const region_model *) const FINAL OVERRIDE;
const svalue *get_arg () const { return m_arg; }
diff --git a/gcc/testsuite/g++.dg/analyzer/pr99064.C b/gcc/testsuite/g++.dg/analyzer/pr99064.C
index a002219bf58..9fa54f57814 100644
--- a/gcc/testsuite/g++.dg/analyzer/pr99064.C
+++ b/gcc/testsuite/g++.dg/analyzer/pr99064.C
@@ -34,6 +34,6 @@ struct TPkcs11Token {
void list_tokens() {
for (__normal_iterator base = list_tokens_token_list.begin();;) {
int *add_info = new int;
- (*base).add_info = add_info; // { dg-bogus "leak" "PR analyzer/98969" { xfail *-*-* } }
+ (*base).add_info = add_info; // { dg-warning "leak" }
}
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96841.c b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
index d9d35f3dce8..85466617054 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr96841.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-O1 -Wno-builtin-declaration-mismatch" } */
+/* { dg-additional-options "-Wno-analyzer-too-complex -O1 -Wno-builtin-declaration-mismatch" } */
int
l8 (void);
@@ -18,6 +18,6 @@ bv (__SIZE_TYPE__ ny)
{
*mf = 0;
(*mf)[ny] = (int *) malloc (sizeof (int));
- th ((*mf)[ny]); /* { dg-warning "leak" } */
+ th ((*mf)[ny]);
}
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98969.c b/gcc/testsuite/gcc.dg/analyzer/pr98969.c
index 8298f263898..7e1587d7094 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr98969.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr98969.c
@@ -8,7 +8,7 @@ test_1 (long int i)
{
struct foo *f = (struct foo *)i;
f->expr = __builtin_malloc (1024);
-} /* { dg-bogus "leak" "PR analyzer/98969" { xfail *-*-* } } */
+} /* { dg-bogus "leak" } */
void
test_2 (long int i)
@@ -16,3 +16,10 @@ test_2 (long int i)
__builtin_free (((struct foo *)i)->expr);
__builtin_free (((struct foo *)i)->expr); /* { dg-warning "double-'free' of '\\*\\(\\(struct foo \\*\\)i\\)\\.expr'" } */
}
+
+void
+test_3 (void *p)
+{
+ void **q = (void **)p;
+ *q = __builtin_malloc (1024);
+}
--
2.26.2
More information about the Gcc-patches
mailing list