This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] handle non-constant offsets in -Wstringop-overflow (PR 77608)
- From: Martin Sebor <msebor at gmail dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 17 Nov 2017 12:36:04 -0700
- Subject: [PATCH] handle non-constant offsets in -Wstringop-overflow (PR 77608)
- Authentication-results: sourceware.org; auth=none
The attached patch enhances -Wstringop-overflow to detect more
instances of buffer overflow at compile time by handling non-
constant offsets into the destination object that are known to
be in some range. The solution could be improved by handling
even more cases (e.g., anti-ranges or offsets relative to
pointers beyond the beginning of an object) but it's a start.
In addition to bootsrapping/regtesting GCC, also tested with
Binutils/GDB, Glibc, and the Linux kernel on x86_64 with no
regressions.
Martin
The top of GDB fails to compile at the moment so the validation
there was incomplete.
PR middle-end/77608 - missing protection on trivially detectable runtime buffer overflow
gcc/ChangeLog:
PR middle-end/77608
* builtins.c (compute_objsize): Handle non-constant offsets.
gcc/testsuite/ChangeLog:
PR middle-end/77608
* gcc.dg/Wstringop-overflow.c: New test.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c (revision 254879)
+++ gcc/builtins.c (working copy)
@@ -3276,11 +3276,6 @@ compute_objsize (tree dest, int ostype)
if (compute_builtin_object_size (dest, ostype, &size))
return build_int_cst (sizetype, size);
- /* Unless computing the largest size (for memcpy and other raw memory
- functions), try to determine the size of the object from its type. */
- if (!ostype)
- return NULL_TREE;
-
if (TREE_CODE (dest) == SSA_NAME)
{
gimple *stmt = SSA_NAME_DEF_STMT (dest);
@@ -3287,13 +3282,47 @@ compute_objsize (tree dest, int ostype)
if (!is_gimple_assign (stmt))
return NULL_TREE;
+ dest = gimple_assign_rhs1 (stmt);
+
tree_code code = gimple_assign_rhs_code (stmt);
- if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR)
+ if (code == POINTER_PLUS_EXPR)
+ {
+ /* compute_builtin_object_size fails for addresses with
+ non-constant offsets. Try to determine the range of
+ such an offset here and use it to adjus the constant
+ size. */
+ tree off = gimple_assign_rhs2 (stmt);
+ if (TREE_CODE (off) == SSA_NAME
+ && INTEGRAL_TYPE_P (TREE_TYPE (off)))
+ {
+ wide_int min, max;
+ enum value_range_type rng = get_range_info (off, &min, &max);
+
+ if (rng == VR_RANGE)
+ {
+ if (tree size = compute_objsize (dest, ostype))
+ {
+ wide_int wisiz = wi::to_wide (size);
+ if (wi::sign_mask (min))
+ /* ignore negative offsets for now */;
+ else if (wi::ltu_p (min, wisiz))
+ return wide_int_to_tree (TREE_TYPE (size),
+ wi::sub (wisiz, min));
+ else
+ return size_zero_node;
+ }
+ }
+ }
+ }
+ else if (code != ADDR_EXPR)
return NULL_TREE;
-
- dest = gimple_assign_rhs1 (stmt);
}
+ /* Unless computing the largest size (for memcpy and other raw memory
+ functions), try to determine the size of the object from its type. */
+ if (!ostype)
+ return NULL_TREE;
+
if (TREE_CODE (dest) != ADDR_EXPR)
return NULL_TREE;
Index: gcc/testsuite/gcc.dg/Wstringop-overflow.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstringop-overflow.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-overflow.c (working copy)
@@ -0,0 +1,132 @@
+/* PR middle-end/77608 - missing protection on trivially detectable runtime
+ buffer overflow
+ { dg-do compile }
+ { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" } */
+
+#define SIZE_MAX __SIZE_MAX__
+#define DIFF_MAX __PTRDIFF_MAX__
+#define DIFF_MIN (-DIFF_MAX - 1)
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void* memcpy (void*, const void*, size_t);
+extern char* strcpy (char*, const char*);
+extern char* strncpy (char*, const char*, size_t);
+
+void sink (void*);
+
+extern size_t unsigned_value (void)
+{
+ extern volatile size_t unsigned_value_source;
+ return unsigned_value_source;
+}
+
+size_t unsigned_range (size_t min, size_t max)
+{
+ size_t val = unsigned_value ();
+ return val < min || max < val ? min : val;
+}
+
+#define UR(min, max) unsigned_range (min, max)
+
+
+char a7[7];
+
+struct MemArray { char a9[9]; char a1[1]; };
+
+void test_memcpy_array (const void *s)
+{
+#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d))
+
+ T (a7 + UR (0, 1), s, 7);
+ T (a7 + UR (0, 7), s, 7);
+ T (a7 + UR (0, 8), s, 7);
+ T (a7 + UR (0, DIFF_MAX), s, 7);
+ T (a7 + UR (0, SIZE_MAX), s, 7);
+
+ T (a7 + UR (1, 2), s, 7); /* { dg-warning "writing 7 bytes into a region of size 6" } */
+ T (a7 + UR (2, 3), s, 7); /* { dg-warning "writing 7 bytes into a region of size 5" } */
+ T (a7 + UR (6, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 1" } */
+ T (a7 + UR (7, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (8, 9), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+ T (a7 + UR (9, 10), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (DIFF_MAX, SIZE_MAX), s, 7); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+ /* This is valid. */
+ char *d = a7 + 7;
+ T (d + UR (-8, -7), s, 7);
+}
+
+/* Verify the absence of warnings for memcpy writing beyond object
+ boundaries. */
+
+void test_memcpy_memarray (struct MemArray *p, const void *s)
+{
+#undef T
+#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d))
+
+ /* The following are valid. */
+ T (p->a9 + UR (0, 1), s, 9);
+ T (p->a9 + UR (0, 7), s, 9);
+ T (p->a9 + UR (0, 8), s, 9);
+ T (p->a9 + UR (0, DIFF_MAX), s, 9);
+ T (p->a9 + UR (0, SIZE_MAX), s, 9);
+
+ /* The following are invalid. Unfortunately, there is apparently enough
+ code out there that abuses memcpy to write past the end of one member
+ and into the members that follow so the following are not diagnosed
+ by design. It sure would be nice not to have to cater to hacks like
+ these... */
+ T (p->a9 + UR (1, 2), s, 9);
+ T (p->a9 + UR (1, 2), s, 123);
+}
+
+
+void test_strcpy_array (void)
+{
+#undef T
+#define T(d, s) (strcpy ((d), (s)), sink (d))
+
+ T (a7 + UR (0, 1), "012345");
+ T (a7 + UR (0, 7), "012345");
+ T (a7 + UR (0, 8), "012345");
+ T (a7 + UR (0, DIFF_MAX), "012345");
+ T (a7 + UR (0, SIZE_MAX), "012345");
+
+ T (a7 + UR (1, 2), "012345"); /* { dg-warning "writing 7 bytes into a region of size 6" } */
+ T (a7 + UR (2, 3), "012345"); /* { dg-warning "writing 7 bytes into a region of size 5" } */
+ T (a7 + UR (6, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 1" } */
+ T (a7 + UR (7, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (8, 9), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+ T (a7 + UR (9, 10), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+ T (a7 + UR (DIFF_MAX, SIZE_MAX), "012345"); /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+ char *d = a7 + 7;
+
+ T (d + UR (-8, -7), "012345");
+}
+
+void test_strncpy_memarray (struct MemArray *p, const void *s)
+{
+#undef T
+#define T(d, s, n) (strncpy ((d), (s), (n)), sink (d))
+
+ T (p->a9 + UR (0, 1), s, 9);
+ T (p->a9 + UR (0, 7), s, 9);
+ T (p->a9 + UR (0, 8), s, 9);
+ T (p->a9 + UR (0, DIFF_MAX), s, 9);
+ T (p->a9 + UR (0, SIZE_MAX), s, 9);
+
+ T (p->a9 + UR (1, 2), s, 9); /* { dg-warning "writing 9 bytes into a region of size 8" } */
+ T (p->a9 + UR (2, 3), s, 9); /* { dg-warning "writing 9 bytes into a region of size 7" } */
+ T (p->a9 + UR (6, 9), s, 9); /* { dg-warning "writing 9 bytes into a region of size 3" } */
+ T (p->a9 + UR (9, 10), s, 9); /* { dg-warning "writing 9 bytes into a region of size 0" } */
+ T (p->a9 + UR (10, 11), s, 9); /* { dg-warning "writing 9 bytes into a region of size 0" } */
+
+ T (p->a9 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 1); /* { dg-warning "writing 1 byte into a region of size 0" } */
+ T (p->a9 + UR (DIFF_MAX, SIZE_MAX), s, 3); /* { dg-warning "writing 3 bytes into a region of size 0" } */
+}