analyzer: fix ICEs due to sloppy types in bounds-checking [PR110902,P…

…R110928,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 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, converting
	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 "type"
	param.  Add overload with old signature.
	(region_model_manager::get_or_create_int_cst): Support type being
	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 for
	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<constant_svalue::key_t>): 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 test.
	* c-c++-common/analyzer/out-of-bounds-diagram-pr111305.c: New test.
	* c-c++-common/analyzer/out-of-bounds-diagram-pr111441.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

gcc/analyzer/access-diagram.h

@@ -32,23 +32,24 @@ namespace ana {
 class bit_size_expr
 {
 public:
-  bit_size_expr () : m_num_bits (NULL) {}
-  bit_size_expr (tree num_bits) : m_num_bits (num_bits) {}
-
-  text_art::styled_string
-  get_formatted_str (text_art::style_manager &sm,
-		     const char *concrete_single_bit_fmt,
-		     const char *concrete_plural_bits_fmt,
-		     const char *concrete_single_byte_fmt,
-		     const char *concrete_plural_bytes_fmt,
-		     const char *symbolic_bits_fmt,
-		     const char *symbolic_bytes_fmt) const;
-  void print (pretty_printer *pp) const;
-
-  tree maybe_get_as_bytes () const;
+  bit_size_expr (const svalue &num_bits) : m_num_bits (num_bits) {}
+
+  std::unique_ptr<text_art::styled_string>
+  maybe_get_formatted_str (text_art::style_manager &sm,
+			   const region_model &model,
+			   const char *concrete_single_bit_fmt,
+			   const char *concrete_plural_bits_fmt,
+			   const char *concrete_single_byte_fmt,
+			   const char *concrete_plural_bytes_fmt,
+			   const char *symbolic_bits_fmt,
+			   const char *symbolic_bytes_fmt) const;
+  bool maybe_print_for_user (pretty_printer *pp,
+			     const region_model &model) const;
+
+  const svalue *maybe_get_as_bytes (region_model_manager &mgr) const;
 
 private:
-  tree m_num_bits;
+  const svalue &m_num_bits;
 };
 
 /* A range of bits within a base region, where each endpoint
@@ -60,8 +61,9 @@ struct access_range
   : m_start (), m_next ()
   {
   }
-  access_range (region_offset start, region_offset next)
-  : m_start (start), m_next (next)
+  access_range (region_offset start, region_offset next,
+		region_model_manager &mgr)
+  : m_start (strip_types (start, mgr)), m_next (strip_types (next, mgr))
   {}
   access_range (const region *base_region, const bit_range &bits);
   access_range (const region *base_region, const byte_range &bytes);
@@ -74,7 +76,7 @@ struct access_range
 
   bool empty_p () const;
 
-  bool get_size (const region_model &model, bit_size_expr *out) const;
+  bit_size_expr get_size (region_model_manager *mgr) const;
 
   bool get_size_in_bits (bit_size_t *out) const
   {

gcc/analyzer/analyzer-selftests.cc

@@ -51,6 +51,7 @@ void
 run_analyzer_selftests ()
 {
 #if ENABLE_ANALYZER
+  analyzer_access_diagram_cc_tests ();
   analyzer_constraint_manager_cc_tests ();
   analyzer_function_set_cc_tests ();
   analyzer_program_point_cc_tests ();

gcc/analyzer/analyzer-selftests.h

@@ -33,7 +33,7 @@ extern void run_analyzer_selftests ();
 
 /* Declarations for specific families of tests (by source file), in
    alphabetical order.  */
-extern void analyzer_checker_script_cc_tests ();
+extern void analyzer_access_diagram_cc_tests ();
 extern void analyzer_constraint_manager_cc_tests ();
 extern void analyzer_function_set_cc_tests ();
 extern void analyzer_program_point_cc_tests ();

gcc/analyzer/analyzer.h

@@ -140,6 +140,7 @@ class known_function;
 extern void dump_tree (pretty_printer *pp, tree t);
 extern void dump_quoted_tree (pretty_printer *pp, tree t);
 extern void print_quoted_type (pretty_printer *pp, tree t);
+extern void print_expr_for_user (pretty_printer *pp, tree t);
 extern int readability_comparator (const void *p1, const void *p2);
 extern int tree_cmp (const void *p1, const void *p2);
 extern tree fixup_tree_for_diagnostic (tree);
@@ -236,7 +237,7 @@ class region_offset
     return m_sym_offset;
   }
 
-  tree calc_symbolic_bit_offset (const region_model &model) const;
+  const svalue &calc_symbolic_bit_offset (region_model_manager *mgr) const;
   const svalue *calc_symbolic_byte_offset (region_model_manager *mgr) const;
 
   bool operator== (const region_offset &other) const
@@ -436,6 +437,14 @@ get_string_cst_size (const_tree string_cst);
 extern tree
 get_ssa_default_def (const function &fun, tree var);
 
+extern const svalue *
+strip_types (const svalue *sval, region_model_manager &mgr);
+
+extern region_offset
+strip_types (const region_offset &offset, region_model_manager &mgr);
+
+extern tree remove_ssa_names (tree expr);
+
 } // namespace ana
 
 extern bool is_special_named_call_p (const gcall *call, const char *funcname,

gcc/analyzer/bounds-checking.cc

@@ -1154,6 +1154,149 @@ class symbolic_buffer_over_read : public symbolic_past_the_end
   enum access_direction get_dir () const final override { return DIR_READ; }
 };
 
+const svalue *
+strip_types (const svalue *sval,
+	     region_model_manager &mgr)
+{
+  switch (sval->get_kind ())
+    {
+    default:
+      gcc_unreachable ();
+    case SK_REGION:
+      {
+	const region_svalue *region_sval = (const region_svalue *)sval;
+	return mgr.get_ptr_svalue (NULL_TREE, region_sval->get_pointee ());
+      }
+    case SK_CONSTANT:
+      return sval;
+    case SK_UNKNOWN:
+      return mgr.get_or_create_unknown_svalue (NULL_TREE);
+    case SK_POISONED:
+      {
+	const poisoned_svalue *poisoned_sval = (const poisoned_svalue *)sval;
+	return mgr.get_or_create_poisoned_svalue
+	  (poisoned_sval->get_poison_kind (),
+	   NULL_TREE);
+      }
+    case SK_SETJMP:
+      return sval;
+    case SK_INITIAL:
+      return sval;
+    case SK_UNARYOP:
+      {
+	const unaryop_svalue *unaryop_sval = (const unaryop_svalue *)sval;
+	const enum tree_code op = unaryop_sval->get_op ();
+	if (op == VIEW_CONVERT_EXPR || op == NOP_EXPR)
+	  return strip_types (unaryop_sval->get_arg (), mgr);
+	return mgr.get_or_create_unaryop
+	  (NULL_TREE,
+	   op,
+	   strip_types (unaryop_sval->get_arg (), mgr));
+      }
+    case SK_BINOP:
+      {
+	const binop_svalue *binop_sval = (const binop_svalue *)sval;
+	const enum tree_code op = binop_sval->get_op ();
+	return mgr.get_or_create_binop
+	  (NULL_TREE,
+	   op,
+	   strip_types (binop_sval->get_arg0 (), mgr),
+	   strip_types (binop_sval->get_arg1 (), mgr));
+      }
+    case SK_SUB:
+      {
+	const sub_svalue *sub_sval = (const sub_svalue *)sval;
+	return mgr.get_or_create_sub_svalue
+	  (NULL_TREE,
+	   strip_types (sub_sval->get_parent (), mgr),
+	   sub_sval->get_subregion ());
+      }
+    case SK_REPEATED:
+      {
+	const repeated_svalue *repeated_sval = (const repeated_svalue *)sval;
+	return mgr.get_or_create_repeated_svalue
+	  (NULL_TREE,
+	   strip_types (repeated_sval->get_outer_size (), mgr),
+	   strip_types (repeated_sval->get_inner_svalue (), mgr));
+      }
+    case SK_BITS_WITHIN:
+      {
+	const bits_within_svalue *bits_within_sval
+	  = (const bits_within_svalue *)sval;
+	return mgr.get_or_create_bits_within
+	  (NULL_TREE,
+	   bits_within_sval->get_bits (),
+	   strip_types (bits_within_sval->get_inner_svalue (), mgr));
+      }
+    case SK_UNMERGEABLE:
+      {
+	const unmergeable_svalue *unmergeable_sval
+	  = (const unmergeable_svalue *)sval;
+	return mgr.get_or_create_unmergeable
+	  (strip_types (unmergeable_sval->get_arg (), mgr));
+      }
+    case SK_PLACEHOLDER:
+      return sval;
+    case SK_WIDENING:
+      {
+	const widening_svalue *widening_sval = (const widening_svalue *)sval;
+	return mgr.get_or_create_widening_svalue
+	  (NULL_TREE,
+	   widening_sval->get_point (),
+	   strip_types (widening_sval->get_base_svalue (), mgr),
+	   strip_types (widening_sval->get_iter_svalue (), mgr));
+      }
+    case SK_COMPOUND:
+      {
+	const compound_svalue *compound_sval = (const compound_svalue *)sval;
+	binding_map typeless_map;
+	for (auto iter : compound_sval->get_map ())
+	  {
+	    const binding_key *key = iter.first;
+	    const svalue *bound_sval = iter.second;
+	    typeless_map.put (key, strip_types (bound_sval, mgr));
+	  }
+	return mgr.get_or_create_compound_svalue (NULL_TREE, typeless_map);
+      }
+    case SK_CONJURED:
+      return sval;
+    case SK_ASM_OUTPUT:
+      {
+	const asm_output_svalue *asm_output_sval
+	  = (const asm_output_svalue *)sval;
+	auto_vec<const svalue *> typeless_inputs
+	  (asm_output_sval->get_num_inputs ());
+	for (unsigned idx = 0; idx < asm_output_sval->get_num_inputs (); idx++)
+	  typeless_inputs.quick_push
+	    (strip_types (asm_output_sval->get_input (idx),
+			  mgr));
+	return mgr.get_or_create_asm_output_svalue
+	  (NULL_TREE,
+	   asm_output_sval->get_asm_string (),
+	   asm_output_sval->get_output_idx (),
+	   asm_output_sval->get_num_outputs (),
+	   typeless_inputs);
+      }
+    case SK_CONST_FN_RESULT:
+      {
+	const const_fn_result_svalue *const_fn_result_sval
+	  = (const const_fn_result_svalue *)sval;
+	auto_vec<const svalue *> typeless_inputs
+	  (const_fn_result_sval->get_num_inputs ());
+	for (unsigned idx = 0;
+	     idx < const_fn_result_sval->get_num_inputs ();
+	     idx++)
+	  typeless_inputs.quick_push
+	    (strip_types (const_fn_result_sval->get_input (idx),
+			  mgr));
+	return mgr.get_or_create_const_fn_result_svalue
+	  (NULL_TREE,
+	   const_fn_result_sval->get_fndecl (),
+	   typeless_inputs);
+      }
+    }
+}
+
 /* Check whether an access is past the end of the BASE_REG.
   Return TRUE if the access was valid, FALSE otherwise.  */
 
@@ -1169,9 +1312,12 @@ region_model::check_symbolic_bounds (const region *base_reg,
   gcc_assert (ctxt);
 
   const svalue *next_byte
-    = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR,
+    = m_mgr->get_or_create_binop (NULL_TREE, PLUS_EXPR,
 				  sym_byte_offset, num_bytes_sval);
 
+  next_byte = strip_types (next_byte, *m_mgr);
+  capacity = strip_types (capacity, *m_mgr);
+
   if (eval_condition (next_byte, GT_EXPR, capacity).is_true ())
     {
       tree diag_arg = get_representative_tree (base_reg);