analyzer: warn on the use of floating-points operands in the size argument [PR106181]

This patch fixes the ICE reported in PR106181 and adds a new warning to
the analyzer complaining about the use of floating-point operands.

Regrtested on Linux x86_64.

2022-08-17  Tim Lange  <mail@tim-lange.me>

gcc/analyzer/ChangeLog:

	PR analyzer/106181
	* analyzer.opt: Add Wanalyzer-imprecise-floating-point-arithmetic.
	* region-model.cc (is_any_cast_p): Formatting.
	(region_model::check_region_size): Ensure precondition.
	(class imprecise_floating_point_arithmetic): New abstract
	diagnostic class for all floating-point related warnings.
	(class float_as_size_arg): Concrete diagnostic class to complain
	about floating-point operands inside the size argument.
	(class contains_floating_point_visitor):
	New visitor to find floating-point operands inside svalues.
	(region_model::check_dynamic_size_for_floats): New function.
	(region_model::set_dynamic_extents):
	Call to check_dynamic_size_for_floats.
	* region-model.h (class region_model):
	Add region_model::check_dynamic_size_for_floats.

gcc/ChangeLog:

	PR analyzer/106181
	* doc/invoke.texi: Add Wanalyzer-imprecise-fp-arithmetic.

gcc/testsuite/ChangeLog:

	PR analyzer/106181
	* gcc.dg/analyzer/allocation-size-1.c: New test.
	* gcc.dg/analyzer/imprecise-floating-point-1.c: New test.
	* gcc.dg/analyzer/pr106181.c: New test.

gcc/analyzer/analyzer.opt

@@ -98,6 +98,10 @@ Wanalyzer-free-of-non-heap
 Common Var(warn_analyzer_free_of_non_heap) Init(1) Warning
 Warn about code paths in which a non-heap pointer is freed.
 
+Wanalyzer-imprecise-fp-arithmetic
+Common Var(warn_analyzer_imprecise_fp_arithmetic) Init(1) Warning
+Warn about code paths in which floating-point arithmetic is used in locations where precise computation is needed.
+
 Wanalyzer-jump-through-null
 Common Var(warn_analyzer_jump_through_null) Init(1) Warning
 Warn about code paths in which a NULL function pointer is called.

gcc/analyzer/region-model.cc

@@ -3301,7 +3301,8 @@ public:
     m.add_cwe (131);
 
     return warning_meta (rich_loc, m, get_controlling_option (),
-	       "allocated buffer size is not a multiple of the pointee's size");
+			 "allocated buffer size is not a multiple"
+			 " of the pointee's size");
   }
 
   label_text
@@ -3396,21 +3397,20 @@ capacity_compatible_with_type (tree cst, tree pointee_size_tree)
 class size_visitor : public visitor
 {
 public:
-  size_visitor (tree size_cst, const svalue *sval, constraint_manager *cm)
-  : m_size_cst (size_cst), m_sval (sval), m_cm (cm)
+  size_visitor (tree size_cst, const svalue *root_sval, constraint_manager *cm)
+  : m_size_cst (size_cst), m_root_sval (root_sval), m_cm (cm)
   {
-    sval->accept (this);
+    m_root_sval->accept (this);
   }
 
   bool get_result ()
   {
-    return result_set.contains (m_sval);
+    return result_set.contains (m_root_sval);
   }
 
   void visit_constant_svalue (const constant_svalue *sval) final override
   {
-    if (capacity_compatible_with_type (sval->get_constant (), m_size_cst))
-      result_set.add (sval);
+    check_constant (sval->get_constant (), sval);
   }
 
   void visit_unknown_svalue (const unknown_svalue *sval ATTRIBUTE_UNUSED)
@@ -3478,15 +3478,10 @@ public:
     equiv_class_id id (-1);
     if (m_cm->get_equiv_class_by_svalue (sval, &id))
       {
-	if (tree cst_val = id.get_obj (*m_cm).get_any_constant ())
-	  {
-	    if (capacity_compatible_with_type (cst_val, m_size_cst))
-	      result_set.add (sval);
-	  }
+	if (tree cst = id.get_obj (*m_cm).get_any_constant ())
+	  check_constant (cst, sval);
 	else
-	  {
-	    result_set.add (sval);
-	  }
+	  result_set.add (sval);
       }
   }
 
@@ -3503,8 +3498,23 @@ public:
   }
 
 private:
+  void check_constant (tree cst, const svalue *sval)
+  {
+    switch (TREE_CODE (cst))
+      {
+      default:
+	/* Assume all unhandled operands are compatible.  */
+	result_set.add (sval);
+	break;
+      case INTEGER_CST:
+	if (capacity_compatible_with_type (cst, m_size_cst))
+	  result_set.add (sval);
+	break;
+      }
+  }
+
   tree m_size_cst;
-  const svalue *m_sval;
+  const svalue *m_root_sval;
   constraint_manager *m_cm;
   svalue_set result_set; /* Used as a mapping of svalue*->bool.  */
 };
@@ -3541,12 +3551,12 @@ struct_or_union_with_inheritance_p (tree struc)
 static bool
 is_any_cast_p (const gimple *stmt)
 {
-  if (const gassign *assign = dyn_cast<const gassign *>(stmt))
+  if (const gassign *assign = dyn_cast <const gassign *> (stmt))
     return gimple_assign_cast_p (assign)
 	   || !pending_diagnostic::same_tree_p (
 		  TREE_TYPE (gimple_assign_lhs (assign)),
 		  TREE_TYPE (gimple_assign_rhs1 (assign)));
-  else if (const gcall *call = dyn_cast<const gcall *>(stmt))
+  else if (const gcall *call = dyn_cast <const gcall *> (stmt))
     {
       tree lhs = gimple_call_lhs (call);
       return lhs != NULL_TREE && !pending_diagnostic::same_tree_p (
@@ -3606,10 +3616,11 @@ region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
     case svalue_kind::SK_CONSTANT:
       {
 	const constant_svalue *cst_cap_sval
-		= as_a <const constant_svalue *> (capacity);
+	  = as_a <const constant_svalue *> (capacity);
 	tree cst_cap = cst_cap_sval->get_constant ();
-	if (!capacity_compatible_with_type (cst_cap, pointee_size_tree,
-					    is_struct))
+	if (TREE_CODE (cst_cap) == INTEGER_CST
+	    && !capacity_compatible_with_type (cst_cap, pointee_size_tree,
+					       is_struct))
 	  ctxt->warn (new dubious_allocation_size (lhs_reg, rhs_reg,
 						   cst_cap));
       }
@@ -5055,6 +5066,125 @@ region_model::append_regions_cb (const region *base_reg,
     cb_data->out->safe_push (decl_reg);
 }
 
+
+/* Abstract class for diagnostics related to the use of
+   floating-point arithmetic where precision is needed.  */
+
+class imprecise_floating_point_arithmetic : public pending_diagnostic
+{
+public:
+  int get_controlling_option () const final override
+  {
+    return OPT_Wanalyzer_imprecise_fp_arithmetic;
+  }
+};
+
+/* Concrete diagnostic to complain about uses of floating-point arithmetic
+   in the size argument of malloc etc.  */
+
+class float_as_size_arg : public imprecise_floating_point_arithmetic
+{
+public:
+  float_as_size_arg (tree arg) : m_arg (arg)
+  {}
+
+  const char *get_kind () const final override
+  {
+    return "float_as_size_arg_diagnostic";
+  }
+
+  bool subclass_equal_p (const pending_diagnostic &other) const
+  {
+    return same_tree_p (m_arg, ((const float_as_size_arg &) other).m_arg);
+  }
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
+				"use of floating-point arithmetic here might"
+				" yield unexpected results");
+    if (warned)
+      inform (rich_loc->get_loc (), "only use operands of an integer type"
+				    " inside the size argument");
+    return warned;
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) final
+  override
+  {
+    if (m_arg)
+      return ev.formatted_print ("operand %qE is of type %qT",
+				 m_arg, TREE_TYPE (m_arg));
+    return ev.formatted_print ("at least one operand of the size argument is"
+			       " of a floating-point type");
+  }
+
+private:
+  tree m_arg;
+};
+
+/* Visitor to find uses of floating-point variables/constants in an svalue.  */
+
+class contains_floating_point_visitor : public visitor
+{
+public:
+  contains_floating_point_visitor (const svalue *root_sval) : m_result (NULL)
+  {
+    root_sval->accept (this);
+  }
+
+  const svalue *get_svalue_to_report ()
+  {
+    return m_result;
+  }
+
+  void visit_constant_svalue (const constant_svalue *sval) final override
+  {
+    /* At the point the analyzer runs, constant integer operands in a floating
+       point expression are already implictly converted to floating-points.
+       Thus, we do prefer to report non-constants such that the diagnostic
+       always reports a floating-point operand.  */
+    tree type = sval->get_type ();
+    if (type && FLOAT_TYPE_P (type) && !m_result)
+      m_result = sval;
+  }
+
+  void visit_conjured_svalue (const conjured_svalue *sval) final override
+  {
+    tree type = sval->get_type ();
+    if (type && FLOAT_TYPE_P (type))
+      m_result = sval;
+  }
+
+  void visit_initial_svalue (const initial_svalue *sval) final override
+  {
+    tree type = sval->get_type ();
+    if (type && FLOAT_TYPE_P (type))
+      m_result = sval;
+  }
+
+private:
+  /* Non-null if at least one floating-point operand was found.  */
+  const svalue *m_result;
+};
+
+/* May complain about uses of floating-point operands in SIZE_IN_BYTES.  */
+
+void
+region_model::check_dynamic_size_for_floats (const svalue *size_in_bytes,
+					     region_model_context *ctxt) const
+{
+  gcc_assert (ctxt);
+
+  contains_floating_point_visitor v (size_in_bytes);
+  if (const svalue *float_sval = v.get_svalue_to_report ())
+	{
+	  tree diag_arg = get_representative_tree (float_sval);
+	  ctxt->warn (new float_as_size_arg (diag_arg));
+	}
+}
+
 /* Return a new region describing a heap-allocated block of memory.
    Use CTXT to complain about tainted sizes.  */
 
@@ -5092,8 +5222,11 @@ region_model::set_dynamic_extents (const region *reg,
 {
   assert_compat_types (size_in_bytes->get_type (), size_type_node);
   if (ctxt)
-    check_dynamic_size_for_taint (reg->get_memory_space (), size_in_bytes,
-				  ctxt);
+    {
+      check_dynamic_size_for_taint (reg->get_memory_space (), size_in_bytes,
+				    ctxt);
+      check_dynamic_size_for_floats (size_in_bytes, ctxt);
+    }
   m_dynamic_extents.put (reg, size_in_bytes);
 }
 

gcc/analyzer/region-model.h

@@ -853,6 +853,8 @@ class region_model
   void check_dynamic_size_for_taint (enum memory_space mem_space,
 				     const svalue *size_in_bytes,
 				     region_model_context *ctxt) const;
+  void check_dynamic_size_for_floats (const svalue *size_in_bytes,
+				      region_model_context *ctxt) const;
 
   void check_region_for_taint (const region *reg,
 			       enum access_direction dir,

gcc/doc/invoke.texi

@@ -453,6 +453,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-analyzer-fd-use-without-check @gol
 -Wno-analyzer-file-leak @gol
 -Wno-analyzer-free-of-non-heap @gol
+-Wno-analyzer-imprecise-fp-arithmetic @gol
 -Wno-analyzer-jump-through-null @gol
 -Wno-analyzer-malloc-leak @gol
 -Wno-analyzer-mismatching-deallocation @gol
@@ -9758,6 +9759,7 @@ Enabling this option effectively enables the following warnings:
 -Wanalyzer-fd-use-without-check @gol
 -Wanalyzer-file-leak @gol
 -Wanalyzer-free-of-non-heap @gol
+-Wanalyzer-imprecise-fp-arithmetic @gol
 -Wanalyzer-jump-through-null @gol
 -Wanalyzer-malloc-leak @gol
 -Wanalyzer-mismatching-deallocation @gol
@@ -9946,6 +9948,18 @@ is called on a non-heap pointer (e.g. an on-stack buffer, or a global).
 
 See @uref{https://cwe.mitre.org/data/definitions/590.html, CWE-590: Free of Memory not on the Heap}.
 
+@item -Wno-analyzer-imprecise-fp-arithmetic
+@opindex Wanalyzer-imprecise-fp-arithmetic
+@opindex Wno-analyzer-imprecise-fp-arithmetic
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-imprecise-fp-arithmetic}
+to disable it.
+
+This diagnostic warns for paths through the code in which floating-point
+arithmetic is used in locations where precise computation is needed.  This
+diagnostic only warns on use of floating-point operands inside the
+calculation of an allocation size at the moment.
+
 @item -Wno-analyzer-jump-through-null
 @opindex Wanalyzer-jump-through-null
 @opindex Wno-analyzer-jump-through-null

gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c

@@ -115,3 +115,13 @@ void test_10 (int32_t n)
   char *ptr = malloc (7 * n);
   free (ptr);
 }
+
+void test_11 ()
+{
+  /* 3.0 is folded to an int before the analyzer runs.  */
+  int32_t *ptr = malloc (3.0); /* { dg-line malloc11 } */
+  free (ptr);
+
+  /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc11 } */
+  /* { dg-message "'int32_t \\*' (\\\{aka '(long )?int \\*'\\\})? here; 'sizeof \\(int32_t (\\\{aka (long )?int\\\})?\\)' is '4'" "note" { target *-*-* } malloc11 } */
+}

gcc/testsuite/gcc.dg/analyzer/imprecise-floating-point-1.c

@@ -0,0 +1,74 @@
+#include <stdlib.h>
+
+/* Tests warn on use of floating-point operands inside the calculation
+   of an allocation size.
+
+   The test cases here only test for warnings.  The test cases inside
+   allocation-size-X.c should be plently enough to test for false positives.  */
+
+void test_1 (float f)
+{
+  int *ptr = malloc (sizeof (int) * f); /* { dg-line test_1 } */
+  free (ptr);
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_1 } */
+  /* { dg-message "operand 'f' is of type 'float'" "note" { target *-*-* } test_1 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_1 } */
+}
+
+void test_2 (int n)
+{
+  int *ptr = malloc (n * 3.1); /* { dg-line test_2 } */
+  free (ptr);
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_2 } */
+  /* { dg-message "operand '\(\\d|e|f|\\.|\\+|\)+' is of type 'double'" "note" { target *-*-* } test_2 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_2 } */
+}
+
+void *alloc_me (size_t size)
+{
+  return malloc (size); /* { dg-line test_3 } */
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_3 } */
+  /* { dg-message "operand 'f' is of type 'float'" "note" { target *-*-* } test_3 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_3 } */
+}
+
+void test_3 (float f)
+{
+  void *ptr = alloc_me (f); /* { dg-message "calling 'alloc_me' from 'test_3'" } */
+  free (ptr);
+}
+
+void test_4 (int n)
+{
+  int *ptr = calloc(1.7 * n, sizeof (int)); /* { dg-line test_4 } */
+  free (ptr);
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_4 } */
+  /* { dg-message "operand '\(\\d|e|f|\\.|\\+|\)+' is of type 'double'" "note" { target *-*-* } test_4 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_4 } */
+}
+
+int test_5 (float f)
+{
+  int *ptr = __builtin_alloca (sizeof (int) * f); /* { dg-line test_5 } */
+  *ptr = 4;
+  return *ptr;
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_5 } */
+  /* { dg-message "operand 'f' is of type 'float'" "note" { target *-*-* } test_5 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_5 } */
+}
+
+int test_6 (float f)
+{
+  int *ptr = __builtin_alloca (1.7f * f * 2.3f); /* { dg-line test_6 } */
+  *ptr = 4;
+  return *ptr;
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } test_6 } */
+  /* { dg-message "operand 'f' is of type 'float'" "note" { target *-*-* } test_6 } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } test_6 } */
+}

gcc/testsuite/gcc.dg/analyzer/pr106181.c

@@ -0,0 +1,11 @@
+#include <stdint.h>
+
+void *
+foo (int x)
+{
+  return __builtin_calloc (x * 1.1, 1); /* { dg-line calloc } */
+
+  /* { dg-warning "use of floating-point arithmetic here might yield unexpected results" "warning" { target *-*-* } calloc } */
+  /* { dg-message "operand '\(\\d|e|f|\\.|\\+|\)+' is of type 'double'" "note" { target *-*-* } calloc } */
+  /* { dg-message "only use operands of an integer type inside the size argument" "note" { target *-*-* } calloc } */
+}