procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

analyzer: Fix allocation size false positive on conjured svalue [PR109577]

Browse source 
Currently, the analyzer tries to prove that the allocation size is a
multiple of the pointee's type size.  This patch reverses the behavior
to try to prove that the expression is not a multiple of the pointee's
type size.  With this change, each unhandled case should be gracefully
considered as correct.  This fixes the bug reported in PR 109577 by
Paul Eggert.

Regression-tested on Linux x86-64 with -m32 and -m64.

2023-06-09  Tim Lange  <mail@tim-lange.me>

	PR analyzer/109577

gcc/analyzer/ChangeLog:

	* constraint-manager.cc (class sval_finder): Visitor to find
	childs in svalue trees.
	(constraint_manager::sval_constrained_p): Add new function to
	check whether a sval might be part of an constraint.
	* constraint-manager.h: Add sval_constrained_p function.
	* region-model.cc (class size_visitor): Reverse behavior to not
	emit a warning on not explicitly considered cases.
	(region_model::check_region_size):
	Adapt to size_visitor changes.

gcc/testsuite/ChangeLog:

	* gcc.dg/analyzer/allocation-size-2.c: Change expected output
	and add new test case.
	* gcc.dg/analyzer/pr109577.c: New test.
This commit is contained in:
Tim Lange 2023-06-09 20:07:33 +02:00
parent a53a83b63b
commit 1d57a22325
5 changed files with 194 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

131
gcc/analyzer/constraint-manager.cc
View file

@ -2218,6 +2218,137 @@ constraint_manager::get_equiv_class_by_svalue (const svalue *sval,
return false;
}
/* Tries to find a svalue inside another svalue. */
class sval_finder : public visitor
{
public:
sval_finder (const svalue *query) : m_query (query), m_found (false)
{
}
bool found_query_p ()
{
return m_found;
}
void visit_region_svalue (const region_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_constant_svalue (const constant_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_unknown_svalue (const unknown_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_poisoned_svalue (const poisoned_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_setjmp_svalue (const setjmp_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_initial_svalue (const initial_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_unaryop_svalue (const unaryop_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_binop_svalue (const binop_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_sub_svalue (const sub_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_repeated_svalue (const repeated_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_bits_within_svalue (const bits_within_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_unmergeable_svalue (const unmergeable_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_placeholder_svalue (const placeholder_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_widening_svalue (const widening_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_compound_svalue (const compound_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_conjured_svalue (const conjured_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_asm_output_svalue (const asm_output_svalue *sval)
{
m_found |= m_query == sval;
}
void visit_const_fn_result_svalue (const const_fn_result_svalue *sval)
{
m_found |= m_query == sval;
}
private:
const svalue *m_query;
bool m_found;
};
/* Returns true if SVAL is constrained. */
bool
constraint_manager::sval_constrained_p (const svalue *sval) const
{
int i;
equiv_class *ec;
sval_finder finder (sval);
FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
{
int j;
const svalue *iv;
FOR_EACH_VEC_ELT (ec->m_vars, j, iv)
{
iv->accept (&finder);
if (finder.found_query_p ())
return true;
}
}
return false;
}
/* Ensure that SVAL has an equivalence class within this constraint_manager;
return the ID of the class. */

1
gcc/analyzer/constraint-manager.h
View file

@ -459,6 +459,7 @@ public:
bool get_equiv_class_by_svalue (const svalue *sval,
equiv_class_id *out) const;
bool sval_constrained_p (const svalue *sval) const;
equiv_class_id get_or_add_equiv_class (const svalue *sval);
tristate eval_condition (equiv_class_id lhs,
enum tree_code op,

80
gcc/analyzer/region-model.cc
View file

@ -2955,7 +2955,7 @@ capacity_compatible_with_type (tree cst, tree pointee_size_tree)
It works by visiting all svalues inside SVAL until it reaches
atomic nodes. From those, it goes back up again and adds each
node that might be a multiple of SIZE_CST to the RESULT_SET. */
node that is not a multiple of SIZE_CST to the RESULT_SET. */
class size_visitor : public visitor
{
@ -2966,7 +2966,7 @@ public:
m_root_sval->accept (this);
}
bool get_result ()
bool is_dubious_capacity ()
{
return result_set.contains (m_root_sval);
}
@ -2976,22 +2976,10 @@ public:
check_constant (sval->get_constant (), sval);
}
void visit_unknown_svalue (const unknown_svalue *sval ATTRIBUTE_UNUSED)
final override
{
result_set.add (sval);
}
void visit_poisoned_svalue (const poisoned_svalue *sval ATTRIBUTE_UNUSED)
final override
{
result_set.add (sval);
}
void visit_unaryop_svalue (const unaryop_svalue *sval) final override
{
const svalue *arg = sval->get_arg ();
if (result_set.contains (arg))
if (CONVERT_EXPR_CODE_P (sval->get_op ())
&& result_set.contains (sval->get_arg ()))
result_set.add (sval);
}
@ -3000,28 +2988,24 @@ public:
const svalue *arg0 = sval->get_arg0 ();
const svalue *arg1 = sval->get_arg1 ();
if (sval->get_op () == MULT_EXPR)
switch (sval->get_op ())
{
if (result_set.contains (arg0) || result_set.contains (arg1))
result_set.add (sval);
case MULT_EXPR:
if (result_set.contains (arg0) && result_set.contains (arg1))
result_set.add (sval);
break;
case PLUS_EXPR:
case MINUS_EXPR:
if (result_set.contains (arg0) || result_set.contains (arg1))
result_set.add (sval);
break;
default:
break;
}
else
{
if (result_set.contains (arg0) && result_set.contains (arg1))
result_set.add (sval);
}
}
void visit_repeated_svalue (const repeated_svalue *sval) final override
{
sval->get_inner_svalue ()->accept (this);
if (result_set.contains (sval->get_inner_svalue ()))
result_set.add (sval);
}
void visit_unmergeable_svalue (const unmergeable_svalue *sval) final override
{
sval->get_arg ()->accept (this);
if (result_set.contains (sval->get_arg ()))
result_set.add (sval);
}
@ -3031,33 +3015,30 @@ public:
const svalue *base = sval->get_base_svalue ();
const svalue *iter = sval->get_iter_svalue ();
if (result_set.contains (base) && result_set.contains (iter))
if (result_set.contains (base) || result_set.contains (iter))
result_set.add (sval);
}
void visit_conjured_svalue (const conjured_svalue *sval ATTRIBUTE_UNUSED)
final override
void visit_initial_svalue (const initial_svalue *sval) final override
{
equiv_class_id id (-1);
equiv_class_id id = equiv_class_id::null ();
if (m_cm->get_equiv_class_by_svalue (sval, &id))
{
if (tree cst = id.get_obj (*m_cm).get_any_constant ())
check_constant (cst, sval);
else
result_set.add (sval);
}
else if (!m_cm->sval_constrained_p (sval))
{
result_set.add (sval);
}
}
void visit_asm_output_svalue (const asm_output_svalue *sval ATTRIBUTE_UNUSED)
final override
void visit_conjured_svalue (const conjured_svalue *sval) final override
{
result_set.add (sval);
}
void visit_const_fn_result_svalue (const const_fn_result_svalue
*sval ATTRIBUTE_UNUSED) final override
{
result_set.add (sval);
equiv_class_id id = equiv_class_id::null ();
if (m_cm->get_equiv_class_by_svalue (sval, &id))
if (tree cst = id.get_obj (*m_cm).get_any_constant ())
check_constant (cst, sval);
}
private:
@ -3067,10 +3048,9 @@ private:
{
default:
/* Assume all unhandled operands are compatible. */
result_set.add (sval);
break;
case INTEGER_CST:
if (capacity_compatible_with_type (cst, m_size_cst))
if (!capacity_compatible_with_type (cst, m_size_cst))
result_set.add (sval);
break;
}
@ -3193,7 +3173,7 @@ region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
if (!is_struct)
{
size_visitor v (pointee_size_tree, capacity, m_constraints);
if (!v.get_result ())
if (v.is_dubious_capacity ())
{
tree expr = get_representative_tree (capacity);
ctxt->warn (make_unique <dubious_allocation_size> (lhs_reg,

24
gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c
View file

@ -76,13 +76,13 @@ void *create_buffer(int32_t n)
return malloc(n);
}
void test_7(int32_t n)
void test_7(int32_t n)
{
int32_t *buf = create_buffer(n * sizeof (int32_t));
free (buf);
}
void test_8(int32_t n)
void test_8(int32_t n)
{
/* FIXME: At the moment, region_model::set_value (lhs, <return_value>)
is called at the src_node of the return edge. This edge has no stmts
@ -98,13 +98,11 @@ void test_9 (void)
{
int32_t n;
scanf("%i", &n);
/* n is a conjured_svalue. */
void *ptr = malloc (n); /* { dg-message "'n' bytes" "note" } */
int32_t *iptr = (int32_t *)ptr; /* { dg-line assign9 } */
/* n is a conjured_svalue without any constraint. We have to assume
that is a multiple of sizeof (int32_t *); see PR analyzer/110014. */
void *ptr = malloc (n);
int32_t *iptr = (int32_t *)ptr;
free (iptr);
/* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign9 } */
/* { dg-message "'int32_t \\*' (\\\{aka '(long )?int \\*'\\\})? here; 'sizeof \\(int32_t (\\\{aka (long )?int\\\})?\\)' is '4'" "note" { target *-*-* } assign9 } */
}
void test_11 (void)
@ -157,3 +155,13 @@ void test_13 (void)
else
free (ptr);
}
int *test_14 (size_t n)
{
int *ptr = NULL;
/* n is an initial_svalue and guarded such that there is no equiv_class
for n itself but only for a binop_svalue containing n. */
if (n % sizeof (int) == 0)
ptr = malloc (n);
return ptr;
}

16
gcc/testsuite/gcc.dg/analyzer/pr109577.c Normal file
View file

@ -0,0 +1,16 @@
void *malloc (unsigned long);
double *
unsafe (unsigned long n)
{
return malloc (n * sizeof (double));
}
double *
safer (unsigned long n)
{
unsigned long nbytes;
if (__builtin_mul_overflow (n, sizeof (double), &nbytes))
return 0;
return malloc (nbytes);
}