[Bug analyzer/110902] Missing cast in region_model_manager::maybe_fold_binop on MULT_EXPR by 1

2024-03-18 Thread dmalcolm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110902

David Malcolm  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from David Malcolm  ---
Should be fixed on trunk by the above patch.

[Bug analyzer/110902] Missing cast in region_model_manager::maybe_fold_binop on MULT_EXPR by 1

2024-03-18 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110902

--- Comment #2 from GCC Commits  ---
The master branch has been updated by David Malcolm :

https://gcc.gnu.org/g:1579394c9ecf3d1f678daa54b835c7fc3b76fb6d

commit r14-9527-g1579394c9ecf3d1f678daa54b835c7fc3b76fb6d
Author: David Malcolm 
Date:   Mon Mar 18 18:44:34 2024 -0400

analyzer: fix ICEs due to sloppy types in bounds-checking
[PR110902,PR110928,PR111305,PR111441]

Various analyzer ICEs in our bugzilla relate to sloppy use of types
within bounds-checking.

The bounds-checking code works by comparing symbolic *bit* offsets, and
we don't have a good user-facing type that can represent such an offset
(ptrdiff_type_node is for *byte* offsets).

ana::svalue doesn't enforce valid combinations of types for things like
binary operations.  When I added the access diagrams for GCC 14, this
could lead to attempts to generate trees for such svalues, leading to
trees with invalid combinations of types (e.g. PLUS_EXPR or MULT_EXPR of
incompatible types), leading to ICEs inside the tree folding logic.

I tried two approaches to fixing this.

My first approach was to fix the type-handling throughout the
bounds-checking code to use correct types, using size_type_node for
sizes, ptrdiff_type_node for byte offsets, and trying ptrdiff_type_node
for bit offsets.  I implemented this, and it fixed the crashes, but
unfortunately it led to:
(a) numerous false negatives from the bounds-checking code, due to it
becoming unable to be sure that the accessed offset was beyond the valid
bounds, due to the expressions involved gaining complicated sets of
nested casts.
(b) ugly access diagrams full of nested casts (for capacities, gap
measurements, etc)

So my second approach, implemented in this patch, is to accept that we
don't have a tree type for representing bit offsets.  The patch
represents bit offsets using "typeless" symbolic values i.e. ones for
which get_type () is NULL_TREE, and implements enough support for basic
arithemetic as if these are mathematical integers (albeit ones for which
concrete values within an expression must fit within a signed wide int).
Such values can't be converted to tree, so the patch avoids such
conversions, instead implementing a new svalue::maybe_print_for_user for
printing them to a pretty_printer.  The patch uses ptrdiff_type_node for
byte offsets.

Doing so fixes the crashes, whilst appearing to preserve the behavior of
-Wanalyzer-out-of-bounds in my testing.

gcc/analyzer/ChangeLog:
PR analyzer/110902
PR analyzer/110928
PR analyzer/111305
PR analyzer/111441
* access-diagram.cc: Include "analyzer/analyzer-selftests.h".
(get_access_size_str): Reimplement for conversion of
implmementation of bit_size_expr from tree to const svalue &.  Use
svalue::maybe_print_for_user rather than tree printing routines.
(remove_ssa_names): Make non-static.
(bit_size_expr::get_formatted_str): Rename to...
(bit_size_expr::maybe_get_formatted_str): ...this, adding "model"
param and converting return type to a unique_ptr.  Update for
conversion of implementation of bit_size_expr from tree to
const svalue &.  Use svalue::maybe_print_for_user rather than tree
printing routines.
(bit_size_expr::print): Rename to...
(bit_size_expr::maybe_print_for_user): ...this, adding "model"
param and converting return type to bool.  Update for
conversion of implementation of bit_size_expr from tree to
const svalue &.  Use svalue::maybe_print_for_user rather than tree
printing routines.
(bit_size_expr::maybe_get_as_bytes): Add "mgr" param and convert
return type from tree to const svalue *; reimplement.
(access_range::access_range): Call strip_types when on
region_offset
intializations.
(access_range::get_size): Update for conversion of implementation
of bit_size_expr from tree to const svalue &.
(access_operation::get_valid_bits): Pass manager to access_range
ctor.
(access_operation::maybe_get_invalid_before_bits): Likewise.
(access_operation::maybe_get_invalid_after_bits): Likewise.
(boundaries::add): Likewise.
(bit_to_table_map::populate): Add "mgr" param and pass it to
access_range ctor.
(access_diagram_impl::access_diagram_impl): Pass manager to
bit_to_table_map::populate.
(access_diagram_impl::maybe_add_gap): Use svalue rather than tree
for symbolic bit offsets.  Port to new bit_size_expr
representation.
(access_diagram_impl::add_valid_vs_invalid_ruler): Port 

[Bug analyzer/110902] Missing cast in region_model_manager::maybe_fold_binop on MULT_EXPR by 1

2023-08-04 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110902

--- Comment #1 from CVS Commits  ---
The master branch has been updated by David Malcolm :

https://gcc.gnu.org/g:021077b94741c9300dfff3a24e95b3ffa3f508a7

commit r14-3001-g021077b94741c9300dfff3a24e95b3ffa3f508a7
Author: David Malcolm 
Date:   Fri Aug 4 16:18:40 2023 -0400

analyzer: handle function attribute "alloc_size" [PR110426]

This patch makes -fanalyzer make use of the function attribute
"alloc_size", allowing -fanalyzer to emit -Wanalyzer-allocation-size,
-Wanalyzer-out-of-bounds, and -Wanalyzer-tainted-allocation-size on
execution paths involving allocations using such functions.

gcc/analyzer/ChangeLog:
PR analyzer/110426
* bounds-checking.cc (region_model::check_region_bounds): Handle
symbolic base regions.
* call-details.cc: Include "stringpool.h" and "attribs.h".
(call_details::lookup_function_attribute): New function.
* call-details.h (call_details::lookup_function_attribute): New
function decl.
* region-model-manager.cc
(region_model_manager::maybe_fold_binop): Add reference to
PR analyzer/110902.
* region-model-reachability.cc (reachable_regions::handle_sval):
Add symbolic regions for pointers that are conjured svalues for
the LHS of a stmt.
* region-model.cc (region_model::canonicalize): Purge dynamic
extents for regions that aren't referenced.
(get_result_size_in_bytes): New function.
(region_model::on_call_pre): Use get_result_size_in_bytes and
potentially set the dynamic extents of the region pointed to by
the return value.
(region_model::deref_rvalue): Add param "add_nonnull_constraint"
and use it to conditionalize adding the constraint.
(pending_diagnostic_subclass::dubious_allocation_size): Add "stmt"
param to both ctors and use it to initialize new "m_stmt" field.
(pending_diagnostic_subclass::operator==): Use m_stmt; don't use
m_lhs or m_rhs.
(pending_diagnostic_subclass::m_stmt): New field.
(region_model::check_region_size): Generalize to any kind of
pointer svalue by using deref_rvalue rather than checking for
region_svalue.  Pass stmt to dubious_allocation_size ctor.
* region-model.h (region_model::deref_rvalue): Add param
"add_nonnull_constraint".
* svalue.cc (conjured_svalue::lhs_value_p): New function.
* svalue.h (conjured_svalue::lhs_value_p): New decl.

gcc/testsuite/ChangeLog:
PR analyzer/110426
* gcc.dg/analyzer/allocation-size-1.c: Update expected message to
reflect consolidation of size and assignment into a single event.
* gcc.dg/analyzer/allocation-size-2.c: Likewise.
* gcc.dg/analyzer/allocation-size-3.c: Likewise.
* gcc.dg/analyzer/allocation-size-4.c: Likewise.
* gcc.dg/analyzer/allocation-size-multiline-1.c: Likewise.
* gcc.dg/analyzer/allocation-size-multiline-2.c: Likewise.
* gcc.dg/analyzer/allocation-size-multiline-3.c: Likewise.
* gcc.dg/analyzer/attr-alloc_size-1.c: New test.
* gcc.dg/analyzer/attr-alloc_size-2.c: New test.
* gcc.dg/analyzer/attr-alloc_size-3.c: New test.
* gcc.dg/analyzer/explode-4.c: New test.
* gcc.dg/analyzer/taint-size-1.c: Add test coverage for
__attribute__ alloc_size.

Signed-off-by: David Malcolm