OpenE2K/gcc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

analyzer: fix false leak involving params [PR98969]

Browse Source 
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.

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.
This commit is contained in:
David Malcolm 2021-02-17 10:37:16 -05:00
parent 366cf1127a
commit e0139b2a91
9 changed files with 69 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gcc/analyzer/constraint-manager.cc
View File

@ -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:

4
gcc/analyzer/program-state.cc
View File

@ -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);
}

2
gcc/analyzer/region-model-reachability.cc
View File

@ -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:

14
gcc/analyzer/region-model.cc
View File

@ -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;
}

52
gcc/analyzer/svalue.cc
View File

@ -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);

20
gcc/analyzer/svalue.h
View File

@ -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; }

2
gcc/testsuite/g++.dg/analyzer/pr99064.C
View File

@ -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" }
}
}

4
gcc/testsuite/gcc.dg/analyzer/pr96841.c
View File

@ -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]);
}
}

9
gcc/testsuite/gcc.dg/analyzer/pr98969.c
View File

@ -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);
}