From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id A14073858289; Mon, 18 Mar 2024 22:47:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A14073858289 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1710802029; bh=ZmuMGk6J/zleMzBm5g/bJMZiVaJuUTeoR2XVKwQpkZs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=FYjWoZ93yXzw2shInya2JGcmUdXP6WIEnKPc49npsNIxdSbVQsJm9np/GZ7YDQMd6 nc42BVcJfShhsdeKJpdsW72jIS1UpOH8RpENyYJrAp5LeHG5WkcU1xiag79OsA9Hts VamCxH1YaRQHCvUpcSqNQtlBXbsevez+YJc/vSqs= From: "cvs-commit at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug analyzer/111305] [13/14 Regression] GCC Static Analyzer -Wanalyzer-out-of-bounds FP and ICE problem Date: Mon, 18 Mar 2024 22:47:07 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: analyzer X-Bugzilla-Version: 14.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: dmalcolm at gcc dot gnu.org X-Bugzilla-Target-Milestone: 13.3 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D111305 --- Comment #3 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 "mode= l" 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 t= ree 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 t= ree 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 implementati= on 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 tr= ee for symbolic bit offsets. Port to new bit_size_expr representation. (access_diagram_impl::add_valid_vs_invalid_ruler): Port to new bit_size_expr representation. (selftest::assert_eq_typeless_integer): New. (ASSERT_EQ_TYPELESS_INTEGER): New. (selftest::test_bit_size_expr_to_bytes): New. (selftest::analyzer_access_diagram_cc_tests): New. * access-diagram.h (class bit_size_expr): Reimplement, converti= ng implementation from tree to const svalue &. (access_range::access_range): Add "mgr" param. Call strip_types on region_offset initializations. (access_range::get_size): Update decl for reimplementation. * analyzer-selftests.cc (selftest::run_analyzer_selftests): Call selftest::analyzer_access_diagram_cc_tests. * analyzer-selftests.h (selftest::analyzer_checker_script_cc_tests): Delete this stray typo. (selftest::analyzer_access_diagram_cc_tests): New decl. * analyzer.h (print_expr_for_user): New decl. (calc_symbolic_bit_offset): Update decl for reimplementation. (strip_types): New decls. (remove_ssa_names): New decl. * bounds-checking.cc (strip_types): New. (region_model::check_symbolic_bounds): Use typeless svalues. * region-model-manager.cc (region_model_manager::get_or_create_constant_svalue): Add "typ= e" param. Add overload with old signature. (region_model_manager::get_or_create_int_cst): Support type bei= ng NULL_TREE. (region_model_manager::maybe_fold_unaryop): Gracefully reject folding of casts to NULL_TREE type. (get_code_for_cast): Use NOP_EXPR for "casting" svalues to NULL_TREE type. (region_model_manager::get_or_create_cast): Support "casting" svalues to NULL_TREE type. (region_model_manager::maybe_fold_binop): Don't crash on inputs with NULL_TREE type. Handle folding of binops on constants with NULL_TREE type. Add missing cast from PR analyzer/110902. Support enough folding of other ops on NULL_TREE type to support bounds checking. (region_model_manager::get_or_create_const_fn_result_svalue): Remove assertion that type is nonnull. * region-model-manager.h (region_model_manager::get_or_create_constant_svalue): Add overloaded decl taking a type. (region_model_manager::maybe_fold_binop): Make public. (region_model_manager::constants_map_t): Use constant_svalue::key_t for the key, rather than just tree. * region-model.cc (print_expr_for_user): New. (selftest::test_array_2): Handle casts. * region.cc (region_offset::calc_symbolic_bit_offset): Return const svalue & rather than tree, and reimplement accordingly. (region::calc_offset): Use ptrdiff_type_node for types of byte offsets. (region::maybe_print_for_user): New. (element_region::get_relative_symbolic_offset): Use NULL_TREE f= or types of bit offsets. (offset_region::get_bit_offset): Likewise. (sized_region::get_bit_size_sval): Likewise for bit sizes. * region.h (region::maybe_print_for_user): New decl. * svalue.cc (class auto_add_parens): New. (svalue::maybe_print_for_user): New. (svalue::cmp_ptr): Support typeless constant svalues. (tristate_from_boolean_tree_node): New, taken from... (constant_svalue::eval_condition): ...here. Handle comparison = of typeless integer svalue constants. * svalue.h (svalue::maybe_print_for_user): New decl. (class constant_svalue): Support the type of the svalue being NULL_TREE. (struct default_hash_traits): New. gcc/ChangeLog: PR analyzer/110902 PR analyzer/110928 PR analyzer/111305 PR analyzer/111441 * selftest.h (ASSERT_NE_AT): New macro. gcc/testsuite/ChangeLog: PR analyzer/110902 PR analyzer/110928 PR analyzer/111305 PR analyzer/111441 * c-c++-common/analyzer/out-of-bounds-const-fn.c: New test. * c-c++-common/analyzer/out-of-bounds-diagram-11.c: Update expected diagram output. * c-c++-common/analyzer/out-of-bounds-diagram-pr110928.c: New t= est. * c-c++-common/analyzer/out-of-bounds-diagram-pr111305.c: New t= est. * c-c++-common/analyzer/out-of-bounds-diagram-pr111441.c: New t= est. Signed-off-by: David Malcolm =