https://gcc.gnu.org/g:13dcaf1bb6d4f15665a47b14ac0c12cf454e38a2
commit r15-1107-g13dcaf1bb6d4f15665a47b14ac0c12cf454e38a2
Author: David Malcolm <dmalc...@redhat.com>
Date: Fri Jun 7 16:14:28 2024 -0400
analyzer: new warning: -Wanalyzer-undefined-behavior-ptrdiff (PR
analyzer/105892)
Add a new warning to complain about pointer subtraction involving
different chunks of memory.
For example, given:
#include <stddef.h>
int arr[42];
int sentinel;
ptrdiff_t
test_invalid_calc_of_array_size (void)
{
return &sentinel - arr;
}
this emits:
demo.c: In function ‘test_invalid_calc_of_array_size’:
demo.c:9:20: warning: undefined behavior when subtracting pointers
[CWE-469] [-Wanalyzer-undefined-behavior-ptrdiff]
9 | return &sentinel - arr;
| ^
events 1-2
│
│ 3 | int arr[42];
│ | ~~~
│ | |
│ | (2) underlying object for right-hand side of subtraction
created here
│ 4 | int sentinel;
│ | ^~~~~~~~
│ | |
│ | (1) underlying object for left-hand side of subtraction
created here
│
└──> ‘test_invalid_calc_of_array_size’: event 3
│
│ 9 | return &sentinel - arr;
│ | ^
│ | |
│ | (3) ⚠️ subtraction of pointers has
undefined behavior if they do not point into the same array object
│
gcc/analyzer/ChangeLog:
PR analyzer/105892
* analyzer.opt (Wanalyzer-undefined-behavior-ptrdiff): New option.
* analyzer.opt.urls: Regenerate.
* region-model.cc (class undefined_ptrdiff_diagnostic): New.
(check_for_invalid_ptrdiff): New.
(region_model::get_gassign_result): Call it for POINTER_DIFF_EXPR.
gcc/ChangeLog:
* doc/invoke.texi: Add -Wanalyzer-undefined-behavior-ptrdiff.
gcc/testsuite/ChangeLog:
PR analyzer/105892
* c-c++-common/analyzer/out-of-bounds-pr110387.c: Add
expected warnings about pointer subtraction.
* c-c++-common/analyzer/ptr-subtraction-1.c: New test.
* c-c++-common/analyzer/ptr-subtraction-CWE-469-example.c: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
Diff:
---
gcc/analyzer/analyzer.opt | 4 +
gcc/analyzer/analyzer.opt.urls | 3 +
gcc/analyzer/region-model.cc | 141 +++++++++++++++++++++
gcc/doc/invoke.texi | 16 +++
.../c-c++-common/analyzer/out-of-bounds-pr110387.c | 4 +-
.../c-c++-common/analyzer/ptr-subtraction-1.c | 46 +++++++
.../analyzer/ptr-subtraction-CWE-469-example.c | 81 ++++++++++++
7 files changed, 293 insertions(+), 2 deletions(-)
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index bbf2ba670d8..5335f7e1999 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -222,6 +222,10 @@ Wanalyzer-tainted-size
Common Var(warn_analyzer_tainted_size) Init(1) Warning
Warn about code paths in which an unsanitized value is used as a size.
+Wanalyzer-undefined-behavior-ptrdiff
+Common Var(warn_analyzer_undefined_behavior_ptrdiff) Init(1) Warning
+Warn about code paths in which pointer subtraction involves undefined behavior.
+
Wanalyzer-undefined-behavior-strtok
Common Var(warn_analyzer_undefined_behavior_strtok) Init(1) Warning
Warn about code paths in which a call is made to strtok with undefined
behavior.
diff --git a/gcc/analyzer/analyzer.opt.urls b/gcc/analyzer/analyzer.opt.urls
index 5fcab720582..18a0d6926de 100644
--- a/gcc/analyzer/analyzer.opt.urls
+++ b/gcc/analyzer/analyzer.opt.urls
@@ -114,6 +114,9 @@
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-tainted-offset)
Wanalyzer-tainted-size
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-tainted-size)
+Wanalyzer-undefined-behavior-ptrdiff
+UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-undefined-behavior-ptrdiff)
+
Wanalyzer-undefined-behavior-strtok
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-undefined-behavior-strtok)
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index d142d851a26..d6bcb8630cd 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -841,6 +841,144 @@ private:
tree m_count_cst;
};
+/* A subclass of pending_diagnostic for complaining about pointer
+ subtractions involving unrelated buffers. */
+
+class undefined_ptrdiff_diagnostic
+: public pending_diagnostic_subclass<undefined_ptrdiff_diagnostic>
+{
+public:
+ /* Region_creation_event subclass to give a custom wording when
+ talking about creation of buffers for LHS and RHS of the
+ subtraction. */
+ class ptrdiff_region_creation_event : public region_creation_event
+ {
+ public:
+ ptrdiff_region_creation_event (const event_loc_info &loc_info,
+ bool is_lhs)
+ : region_creation_event (loc_info),
+ m_is_lhs (is_lhs)
+ {
+ }
+
+ label_text get_desc (bool) const
+ {
+ if (m_is_lhs)
+ return label_text::borrow ("underlying object for left-hand side"
+ " of subtraction created here");
+ else
+ return label_text::borrow ("underlying object for right-hand side"
+ " of subtraction created here");
+ }
+
+ private:
+ bool m_is_lhs;
+ };
+
+ undefined_ptrdiff_diagnostic (const gassign *assign,
+ const svalue *sval_a,
+ const svalue *sval_b,
+ const region *base_reg_a,
+ const region *base_reg_b)
+ : m_assign (assign),
+ m_sval_a (sval_a),
+ m_sval_b (sval_b),
+ m_base_reg_a (base_reg_a),
+ m_base_reg_b (base_reg_b)
+ {
+ gcc_assert (m_base_reg_a != m_base_reg_b);
+ }
+
+ const char *get_kind () const final override
+ {
+ return "undefined_ptrdiff_diagnostic";
+ }
+
+ bool operator== (const undefined_ptrdiff_diagnostic &other) const
+ {
+ return (m_assign == other.m_assign
+ && m_sval_a == other.m_sval_a
+ && m_sval_b == other.m_sval_b
+ && m_base_reg_a == other.m_base_reg_a
+ && m_base_reg_b == other.m_base_reg_b);
+ }
+
+ int get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_undefined_behavior_ptrdiff;
+ }
+
+ bool emit (diagnostic_emission_context &ctxt) final override
+ {
+ /* CWE-469: Use of Pointer Subtraction to Determine Size. */
+ ctxt.add_cwe (469);
+ return ctxt.warn ("undefined behavior when subtracting pointers");
+ }
+
+ void add_region_creation_events (const region *reg,
+ tree /*capacity*/,
+ const event_loc_info &loc_info,
+ checker_path &emission_path) final override
+ {
+ if (reg == m_base_reg_a)
+ emission_path.add_event
+ (make_unique<ptrdiff_region_creation_event> (loc_info, true));
+ else if (reg == m_base_reg_b)
+ emission_path.add_event
+ (make_unique<ptrdiff_region_creation_event> (loc_info, false));
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) final
override
+ {
+ return ev.formatted_print
+ ("subtraction of pointers has undefined behavior if"
+ " they do not point into the same array object");
+ }
+
+ void mark_interesting_stuff (interesting_t *interesting) final override
+ {
+ interesting->add_region_creation (m_base_reg_a);
+ interesting->add_region_creation (m_base_reg_b);
+ }
+
+private:
+ const gassign *m_assign;
+ const svalue *m_sval_a;
+ const svalue *m_sval_b;
+ const region *m_base_reg_a;
+ const region *m_base_reg_b;
+};
+
+/* Check the pointer subtraction SVAL_A - SVAL_B at ASSIGN and add
+ a warning to CTXT if they're not within the same base region. */
+
+static void
+check_for_invalid_ptrdiff (const gassign *assign,
+ region_model_context &ctxt,
+ const svalue *sval_a, const svalue *sval_b)
+{
+ const region *base_reg_a = sval_a->maybe_get_deref_base_region ();
+ if (!base_reg_a)
+ return;
+ const region *base_reg_b = sval_b->maybe_get_deref_base_region ();
+ if (!base_reg_b)
+ return;
+
+ if (base_reg_a == base_reg_b)
+ return;
+
+ if (base_reg_a->get_kind () == RK_SYMBOLIC)
+ return;
+ if (base_reg_b->get_kind () == RK_SYMBOLIC)
+ return;
+
+ ctxt.warn (make_unique<undefined_ptrdiff_diagnostic> (assign,
+ sval_a,
+ sval_b,
+ base_reg_a,
+ base_reg_b));
+}
+
/* If ASSIGN is a stmt that can be modelled via
set_value (lhs_reg, SVALUE, CTXT)
for some SVALUE, get the SVALUE.
@@ -897,6 +1035,9 @@ region_model::get_gassign_result (const gassign *assign,
// TODO: perhaps fold to zero if they're known to be equal?
+ if (ctxt)
+ check_for_invalid_ptrdiff (assign, *ctxt, rhs1_sval, rhs2_sval);
+
const svalue *sval_binop
= m_mgr->get_or_create_binop (TREE_TYPE (lhs), op,
rhs1_sval, rhs2_sval);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ca2591ce2c3..a0b37564646 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -496,6 +496,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-tainted-size
-Wanalyzer-symbol-too-complex
-Wanalyzer-too-complex
+-Wno-analyzer-undefined-behavior-ptrdiff
-Wno-analyzer-undefined-behavior-strtok
-Wno-analyzer-unsafe-call-within-signal-handler
-Wno-analyzer-use-after-free
@@ -10766,6 +10767,7 @@ Enabling this option effectively enables the following
warnings:
-Wanalyzer-tainted-divisor
-Wanalyzer-tainted-offset
-Wanalyzer-tainted-size
+-Wanalyzer-undefined-behavior-ptrdiff
-Wanalyzer-undefined-behavior-strtok
-Wanalyzer-unsafe-call-within-signal-handler
-Wanalyzer-use-after-free
@@ -11416,6 +11418,20 @@ attacker could inject an out-of-bounds access.
See @uref{https://cwe.mitre.org/data/definitions/129.html, CWE-129: Improper
Validation of Array Index}.
+@opindex Wanalyzer-undefined-behavior-ptrdiff
+@opindex Wno-analyzer-undefined-behavior-ptrdiff
+@item -Wno-analyzer-undefined-behavior-ptrdiff
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-undefined-behavior-ptrdiff} to disable it.
+
+This diagnostic warns for paths through the code in which a pointer
+subtraction occurs where the pointers refer to different chunks of
+memory. Such code relies on undefined behavior, as pointer subtraction
+is only defined for cases where both pointers point to within (or just
+after) the same array.
+
+See @uref{https://cwe.mitre.org/data/definitions/469.html, CWE-469: Use of
Pointer Subtraction to Determine Size}.
+
@opindex Wanalyzer-undefined-behavior-strtok
@opindex Wno-analyzer-undefined-behavior-strtok
@item -Wno-analyzer-undefined-behavior-strtok
diff --git a/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr110387.c
b/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr110387.c
index a046659c83e..b4454805eb5 100644
--- a/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr110387.c
+++ b/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr110387.c
@@ -12,8 +12,8 @@ _S_copy (long __n)
void
_M_construct ()
{
- x = &c - &b;
+ x = &c - &b; /* { dg-warning "undefined behavior when subtracting pointers"
} */
unsigned long __dnew = x;
if (__dnew > 1)
- _S_copy (&c - &b);
+ _S_copy (&c - &b); /* { dg-warning "undefined behavior when subtracting
pointers" } */
}
diff --git a/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-1.c
b/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-1.c
new file mode 100644
index 00000000000..73684c115fc
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-1.c
@@ -0,0 +1,46 @@
+#include <stddef.h>
+
+ptrdiff_t
+test_invalid_sub_addrs_of_locals (void)
+{
+ int a; /* { dg-message "underlying object for left-hand side of subtraction
created here" } */
+ int b; /* { dg-message "underlying object for right-hand side of subtraction
created here" } */
+ return &a - &b; /* { dg-warning "undefined behavior when subtracting
pointers \\\[CWE-469\\\] \\\[-Wanalyzer-undefined-behavior-ptrdiff\\\]" } */
+ /* { dg-message "subtraction of pointers has undefined behavior if they do
not point into the same array object" "final event" { target *-*-* } .-1 } */
+}
+
+ptrdiff_t
+test_valid_sub_addrs_within_array (void)
+{
+ int a[10];
+ return &a[7] - &a[3];
+}
+
+ptrdiff_t
+test_invalid_sub_addrs_within_arrays (void)
+{
+ int a[10]; /* { dg-message "left-hand side" } */
+ int b[10]; /* { dg-message "right-hand side" } */
+ return &a[7] - &b[3]; /* { dg-warning "undefined behavior when subtracting
pointers" } */
+}
+
+
+ptrdiff_t
+test_invalid_sub_addrs_between_heap_allocs (size_t n)
+{
+ char *p = (char *)__builtin_malloc (n); /* { dg-message "left-hand side" } */
+ char *q = (char *)__builtin_malloc (n); /* { dg-message "right-hand side" }
*/
+ ptrdiff_t d = p - q; /* { dg-warning "undefined behavior when subtracting
pointers" } */
+ __builtin_free (p);
+ __builtin_free (q);
+ return d;
+}
+
+int arr[42]; /* { dg-message "right-hand side" } */
+int sentinel; /* { dg-message "left-hand side" } */
+
+ptrdiff_t
+test_invalid_calc_of_array_size (void)
+{
+ return &sentinel - arr; /* { dg-warning "undefined behavior when subtracting
pointers" } */
+}
diff --git
a/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-CWE-469-example.c
b/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-CWE-469-example.c
new file mode 100644
index 00000000000..7f8bd342acb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/ptr-subtraction-CWE-469-example.c
@@ -0,0 +1,81 @@
+/* Example heavily adapted from https://cwe.mitre.org/data/definitions/469.html
+ which states "Copyright © 2006–2024, The MITRE Corporation. CWE, CWSS,
CWRAF, and the CWE logo are trademarks of The MITRE Corporation."
+ and which has this on:
+ https://cwe.mitre.org/about/termsofuse.html
+
+ Terms of Use
+
+ CWE™ is free to use by any organization or individual for any research,
development, and/or commercial purposes, per these CWE Terms of Use.
Accordingly, The MITRE Corporation hereby grants you a non-exclusive,
royalty-free license to use CWE for research, development, and commercial
purposes. Any copy you make for such purposes is authorized on the condition
that you reproduce MITRE’s copyright designation and this license in any such
copy. CWE is a trademark of The MITRE Corporation. Please contact
c...@mitre.org if you require further clarification on this issue.
+
+ DISCLAIMERS
+
+ By accessing information through this site you (as “the user”) hereby
agrees the site and the information is provided on an “as is” basis only
without warranty of any kind, express or implied, including but not limited to
implied warranties of merchantability, availability, accuracy, noninfringement,
or fitness for a particular purpose. Use of this site and the information is at
the user’s own risk. The user shall comply with all applicable laws, rules, and
regulations, and the data source’s restrictions, when using the site.
+
+ By contributing information to this site you (as “the contributor”)
hereby represents and warrants the contributor has obtained all necessary
permissions from copyright holders and other third parties to allow the
contributor to contribute, and this site to host and display, the information
and any such contribution, hosting, and displaying will not violate any law,
rule, or regulation. Additionally, the contributor hereby grants all users of
such information a perpetual, worldwide, non-exclusive, no-charge,
royalty-free, irrevocable license to reproduce, prepare derivative works of,
publicly display, publicly perform, sublicense, and distribute such information
and all derivative works.
+
+ The MITRE Corporation expressly disclaims any liability for any damages
arising from the contributor’s contribution of such information, the user’s use
of the site or such information, and The MITRE Corporation’s hosting the tool
and displaying the information. The foregoing disclaimer specifically includes
but is not limited to general, consequential, indirect, incidental, exemplary,
or special or punitive damages (including but not limited to loss of income,
program interruption, loss of information, or other pecuniary loss) arising out
of use of this information, no matter the cause of action, even if The MITRE
Corporation has been advised of the possibility of such damages. */
+
+/* We need this for the issue to be found, or the loop analysis
+ is too simplistic. */
+/* { dg-additional-options "-fno-analyzer-state-merge" } */
+/* { dg-additional-options "-Wno-analyzer-too-complex" } */
+/* { dg-additional-options "-Wno-analyzer-symbol-too-complex" } */
+
+#include <stddef.h>
+
+struct node {
+ int data;
+ struct node* next;
+};
+
+/* The example fails to initialize "tail" for the head == NULL case. */
+
+int example_1_with_uninit (struct node* head) {
+ struct node* current = head;
+ struct node* tail;
+ while (current != NULL) {
+ tail = current;
+ current = current->next;
+ }
+ return tail - head; /* { dg-warning "use of uninitialized value 'tail'" } */
+}
+
+int example_1_bad (struct node* head) {
+ struct node* current = head;
+ struct node* tail = head; /* initialization added */
+ while (current != NULL) {
+ tail = current;
+ current = current->next;
+ }
+ return tail - head; /* { dg-warning "undefined behavior when subtracting
pointers" } */
+}
+
+int example_1_good (struct node* head) {
+ struct node* current = head;
+ int count = 0;
+ while (current != NULL) {
+ count++;
+ current = current->next;
+ }
+ return count;
+}
+
+/* We need to add usage of the function to detect the issue. */
+
+int usage_of_example_1_bad (void)
+{
+ struct node p; /* { dg-message "right-hand side" } */
+ struct node q; /* { dg-message "left-hand side" } */
+ p.next = &q;
+ q.next = NULL;
+ return example_1_bad (&p);
+}
+
+int usage_of_example_1_good (void)
+{
+ struct node p;
+ struct node q;
+ p.next = &q;
+ q.next = NULL;
+ return example_1_good (&p);
+}
