From: Tim Lange <mail@tim-lange.me>
To: gcc-patches@gcc.gnu.org
Cc: dmalcolm@redhat.com, Tim Lange <mail@tim-lange.me>
Subject: [PATCH v2] analyzer: warn on the use of floating-points operands in the size argument [PR106181]
Date: Thu, 18 Aug 2022 11:44:14 +0200 [thread overview]
Message-ID: <20220818094414.19488-1-mail@tim-lange.me> (raw)
In-Reply-To: <20220815123525.49172-1-mail@tim-lange.me>
Hi,
this is the revised version of my patch. I had trouble to get your
point regarding the float_visitor:
> If the constant is seen first, then the non-constant won't be favored
> (though perhaps binary ops get canonicalized so that constants are on
> the RHS?).
Only the assignment of m_result in visit_constant_svalue is guarded by
!m_result, while the other two are not. So, there are two possibilities:
1. A constant is seen first and then assigned to m_result.
1.1. A non-constant float operand is seen later and
overwrites m_result.
1.2. There's no non-constant float operand, thus the
constant is the actual floating-point operand and
is kept inside m_result.
2. A non-constant is seen first, then m_result might be
overwritten with another non-constant later but never
with a constant.
Do I have a flaw in my thinking? (But they do seem to get canonicalized,
so that shouldn't matter)
> How about:
> -Wanalyzer-imprecise-float-arithmetic
> -Wanalyzer-imprecise-fp-arithmetic
> instead? (ideas welcome)
I've chosen the second. I mostly tried to avoid float because it is also
a reserved keyword in many languages and I wanted to avoid confusion
(might be overthinking that).
- Tim
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 | 4 +
gcc/analyzer/region-model.cc | 179 +++++++++++++++---
gcc/analyzer/region-model.h | 2 +
gcc/doc/invoke.texi | 14 ++
.../gcc.dg/analyzer/allocation-size-1.c | 10 +
.../analyzer/imprecise-floating-point-1.c | 74 ++++++++
gcc/testsuite/gcc.dg/analyzer/pr106181.c | 11 ++
7 files changed, 271 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/imprecise-floating-point-1.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr106181.c
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 61b58c575ff..437ea92e130 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/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.
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index b5bc3efda32..ec29be259b5 100644
--- a/gcc/analyzer/region-model.cc
+++ b/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);
}
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index cdf10872c0f..7ce832f6ce4 100644
--- a/gcc/analyzer/region-model.h
+++ b/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,
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1ac81ad0bb4..f65d351a5fc 100644
--- a/gcc/doc/invoke.texi
+++ b/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
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
index b5bed539250..003914ed96c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
+++ b/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 } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/imprecise-floating-point-1.c b/gcc/testsuite/gcc.dg/analyzer/imprecise-floating-point-1.c
new file mode 100644
index 00000000000..d8a3f4884d6
--- /dev/null
+++ b/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 } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr106181.c b/gcc/testsuite/gcc.dg/analyzer/pr106181.c
new file mode 100644
index 00000000000..6a78b78d352
--- /dev/null
+++ b/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 } */
+}
--
2.37.1
next prev parent reply other threads:[~2022-08-18 9:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-15 12:35 [PATCH] analyzer: warn on the use of floating points " Tim Lange
2022-08-15 18:16 ` David Malcolm
2022-08-18 9:44 ` Tim Lange [this message]
2022-08-18 12:21 ` [PATCH v2] analyzer: warn on the use of floating-points operands " David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220818094414.19488-1-mail@tim-lange.me \
--to=mail@tim-lange.me \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).