While putting together examples for the GCC 7 changes document
I noticed that a few of the buffer overflow warnings issued by
-Wstringop-overflow are defeated by Glibc's macros for string
manipulation functions like strncat and strncpy.
While testing my fix I also noticed that I had missed a couple
of functions when implementing the warning: memmove and stpcpy.
The attached patch adds handlers for those and fixes the three
bugs below I raised for these omissions.
Is this patch okay for trunk?
PR preprocessor/79214 - -Wno-system-header defeats strncat buffer
overflow warnings
PR middle-end/79222 - missing -Wstringop-overflow= on a stpcpy overflow
PR middle-end/79223 - missing -Wstringop-overflow on a memmove overflow
Martin
PR preprocessor/79214 - -Wno-system-header defeats strncat buffer overflow warnings
PR middle-end/79222 - missing -Wstringop-overflow= on a stpcpy overflow
PR middle-end/79223 - missing -Wstringop-overflow on a memmove overflow
gcc/ChangeLog:
PR preprocessor/79214
PR middle-end/79222
PR middle-end/79223
* builtins.c (check_sizes): Add inlinining context and issue
warnings even when -Wno-system-headers is set.
(check_strncat_sizes): Same.
(expand_builtin_strncat): Same.
(expand_builtin_memmove): New function.
(expand_builtin_stpncpy): Same.
(expand_builtin): Handle memmove and stpncpy.
gcc/testsuite/ChangeLog:
PR preprocessor/79214
PR middle-end/79222
PR middle-end/79223
* gcc.dg/pr79214.c: New test.
* gcc.dg/pr79214.h: New test header.
* gcc.dg/pr79222.c: New test.
* gcc.dg/pr79223.c: New test.
* gcc.dg/pr78138.c: Adjust.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c (revision 244844)
+++ gcc/builtins.c (working copy)
@@ -121,6 +121,7 @@ static rtx builtin_memcpy_read_str (void *, HOST_W
static rtx expand_builtin_memcpy (tree, rtx);
static rtx expand_builtin_memcpy_with_bounds (tree, rtx);
static rtx expand_builtin_memcpy_args (tree, tree, tree, rtx, tree);
+static rtx expand_builtin_memmove (tree, rtx);
static rtx expand_builtin_mempcpy (tree, rtx, machine_mode);
static rtx expand_builtin_mempcpy_with_bounds (tree, rtx, machine_mode);
static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx,
@@ -129,6 +130,7 @@ static rtx expand_builtin_strcat (tree, rtx);
static rtx expand_builtin_strcpy (tree, rtx);
static rtx expand_builtin_strcpy_args (tree, tree, rtx);
static rtx expand_builtin_stpcpy (tree, rtx, machine_mode);
+static rtx expand_builtin_stpncpy (tree, rtx);
static rtx expand_builtin_strncat (tree, rtx);
static rtx expand_builtin_strncpy (tree, rtx);
static rtx builtin_memset_gen_str (void *, HOST_WIDE_INT, machine_mode);
@@ -3123,6 +3125,7 @@ check_sizes (int opt, tree exp, tree size, tree ma
if (range[0] && tree_int_cst_lt (maxobjsize, range[0]))
{
location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
if (range[0] == range[1])
warning_at (loc, opt,
@@ -3155,10 +3158,11 @@ check_sizes (int opt, tree exp, tree size, tree ma
unsigned HOST_WIDE_INT uwir0 = tree_to_uhwi (range[0]);
location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
if (at_least_one)
warning_at (loc, opt,
- "%K%qD: writing at least %wu byte into a region "
+ "%K%qD writing at least %wu byte into a region "
"of size %wu overflows the destination",
exp, get_callee_fndecl (exp), uwir0,
tree_to_uhwi (objsize));
@@ -3165,7 +3169,7 @@ check_sizes (int opt, tree exp, tree size, tree ma
else if (range[0] == range[1])
warning_at (loc, opt,
(uwir0 == 1
- ? G_("%K%qD: writing %wu byte into a region "
+ ? G_("%K%qD writing %wu byte into a region "
"of size %wu overflows the destination")
: G_("%K%qD writing %wu bytes into a region "
"of size %wu overflows the destination")),
@@ -3173,7 +3177,7 @@ check_sizes (int opt, tree exp, tree size, tree ma
tree_to_uhwi (objsize));
else
warning_at (loc, opt,
- "%K%qD: writing between %wu and %wu bytes "
+ "%K%qD writing between %wu and %wu bytes "
"into a region of size %wu overflows "
"the destination",
exp, get_callee_fndecl (exp), uwir0,
@@ -3194,6 +3198,7 @@ check_sizes (int opt, tree exp, tree size, tree ma
if (range[0] && objsize && tree_fits_uhwi_p (objsize))
{
location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
if (tree_int_cst_lt (maxobjsize, range[0]))
{
@@ -3302,6 +3307,24 @@ expand_builtin_memcpy (tree exp, rtx target)
return expand_builtin_memcpy_args (dest, src, len, target, exp);
}
+/* Check a call EXP to the memmove built-in for validity.
+ Return NULL_RTX on both success and failure. */
+
+static rtx
+expand_builtin_memmove (tree exp, rtx)
+{
+ if (!validate_arglist (exp,
+ POINTER_TYPE, POINTER_TYPE, INTEGER_TYPE, VOID_TYPE))
+ return NULL_RTX;
+
+ tree dest = CALL_EXPR_ARG (exp, 0);
+ tree len = CALL_EXPR_ARG (exp, 2);
+
+ check_memop_sizes (exp, dest, len);
+
+ return NULL_RTX;
+}
+
/* Expand an instrumented call EXP to the memcpy builtin.
Return NULL_RTX if we failed, the caller should emit a normal call,
otherwise try to get the result in TARGET, if convenient (and in
@@ -3612,6 +3635,13 @@ expand_builtin_stpcpy (tree exp, rtx target, machi
dst = CALL_EXPR_ARG (exp, 0);
src = CALL_EXPR_ARG (exp, 1);
+ if (warn_stringop_overflow)
+ {
+ tree destsize = compute_dest_size (dst, warn_stringop_overflow - 1);
+ check_sizes (OPT_Wstringop_overflow_,
+ exp, /*size=*/NULL_TREE, /*maxlen=*/NULL_TREE, src, destsize);
+ }
+
/* If return value is ignored, transform stpcpy into strcpy. */
if (target == const0_rtx && builtin_decl_implicit (BUILT_IN_STRCPY))
{
@@ -3672,6 +3702,47 @@ expand_builtin_stpcpy (tree exp, rtx target, machi
}
}
+/* Check a call EXP to the stpncpy built-in for validity.
+ Return NULL_RTX on both success and failure. */
+
+static rtx
+expand_builtin_stpncpy (tree exp, rtx)
+{
+ if (!validate_arglist (exp,
+ POINTER_TYPE, POINTER_TYPE, INTEGER_TYPE, VOID_TYPE)
+ || !warn_stringop_overflow)
+ return NULL_RTX;
+
+ tree dest = CALL_EXPR_ARG (exp, 0);
+ tree src = CALL_EXPR_ARG (exp, 1);
+
+ /* The number of bytes to write (not the maximum). */
+ tree len = CALL_EXPR_ARG (exp, 2);
+ /* The length of the source sequence. */
+ tree slen = c_strlen (src, 1);
+
+ /* Try to determine the range of lengths that the source expression
+ refers to. */
+ tree lenrange[2];
+ if (slen)
+ lenrange[0] = lenrange[1] = slen;
+ else
+ {
+ get_range_strlen (src, lenrange);
+ slen = lenrange[0];
+ }
+
+ tree destsize = compute_dest_size (dest,
+ warn_stringop_overflow - 1);
+
+ /* The number of bytes to write is LEN but check_sizes will also
+ check SLEN if LEN's value isn't known. */
+ check_sizes (OPT_Wstringop_overflow_,
+ exp, len, /*maxlen=*/NULL_TREE, slen, destsize);
+
+ return NULL_RTX;
+}
+
/* Callback routine for store_by_pieces. Read GET_MODE_BITSIZE (MODE)
bytes from constant string DATA + OFFSET and return it as target
constant. */
@@ -3727,9 +3798,13 @@ check_strncat_sizes (tree exp, tree objsize)
if (tree_fits_uhwi_p (maxlen) && tree_fits_uhwi_p (objsize)
&& tree_int_cst_equal (objsize, maxlen))
{
- warning_at (EXPR_LOCATION (exp), OPT_Wstringop_overflow_,
- "specified bound %wu "
+ location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
+
+ warning_at (loc, OPT_Wstringop_overflow_,
+ "%K%qD: specified bound %wu "
"equals the size of the destination",
+ exp, get_callee_fndecl (exp),
tree_to_uhwi (maxlen));
return false;
@@ -3791,9 +3866,13 @@ expand_builtin_strncat (tree exp, rtx)
if (tree_fits_uhwi_p (maxlen) && tree_fits_uhwi_p (destsize)
&& tree_int_cst_equal (destsize, maxlen))
{
- warning_at (EXPR_LOCATION (exp), OPT_Wstringop_overflow_,
- "specified bound %wu "
+ location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
+
+ warning_at (loc, OPT_Wstringop_overflow_,
+ "%K%qD: specified bound %wu "
"equals the size of the destination",
+ exp, get_callee_fndecl (exp),
tree_to_uhwi (maxlen));
return NULL_RTX;
@@ -6709,6 +6788,12 @@ expand_builtin (tree exp, rtx target, rtx subtarge
return target;
break;
+ case BUILT_IN_STPNCPY:
+ target = expand_builtin_stpncpy (exp, target);
+ if (target)
+ return target;
+ break;
+
case BUILT_IN_MEMCPY:
target = expand_builtin_memcpy (exp, target);
if (target)
@@ -6715,6 +6800,12 @@ expand_builtin (tree exp, rtx target, rtx subtarge
return target;
break;
+ case BUILT_IN_MEMMOVE:
+ target = expand_builtin_memmove (exp, target);
+ if (target)
+ return target;
+ break;
+
case BUILT_IN_MEMPCPY:
target = expand_builtin_mempcpy (exp, target, mode);
if (target)
Index: gcc/testsuite/gcc.dg/pr79214.c
===================================================================
--- gcc/testsuite/gcc.dg/pr79214.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr79214.c (working copy)
@@ -0,0 +1,88 @@
+/* PR preprocessor/79214 - -Wno-system-header defeats strncat buffer overflow
+ warnings
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+#include "pr79214.h"
+
+typedef __SIZE_TYPE__ size_t;
+
+char d[3];
+char s[4];
+
+size_t range (void)
+{
+ extern size_t size ();
+ size_t n = size ();
+ if (n <= sizeof d)
+ return sizeof d + 1;
+
+ return n;
+}
+
+void test_bzero (void)
+{
+ bzero (d, range ()); /* { dg-warning ".__builtin_bzero. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+void test_memcpy (void)
+{
+ memcpy (d, s, range ()); /* { dg-warning ".__builtin_memcpy. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+void test_memmove (void)
+{
+ memmove (d, d + 1, range ()); /* { dg-warning ".__builtin_memmove. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+void test_mempcpy (void)
+{
+ mempcpy (d, s, range ()); /* { dg-warning ".__builtin_mempcpy. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+void test_memset (int n)
+{
+ memset (d, n, range ()); /* { dg-warning ".__builtin_memset. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+void test_strcat (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ strcat (d, s); /* { dg-warning ".__builtin_strcat. writing 4 bytes into a region of size 3 overflows the destination" } */
+}
+
+char* test_stpcpy (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ return stpcpy (d, s); /* { dg-warning ".__builtin_stpcpy. writing 4 bytes into a region of size 3 overflows the destination" } */
+}
+
+char* test_stpncpy (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ return stpncpy (d, s, range ()); /* { dg-warning ".__builtin_stpncpy. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+char* test_strcpy (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ return strcpy (d, s); /* { dg-warning ".__builtin_strcpy. writing 4 bytes into a region of size 3 overflows the destination" } */
+}
+
+char* test_strncpy (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ return strncpy (d, s, range ()); /* { dg-warning ".__builtin_strncpy. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
+
+char* test_strncat (int i)
+{
+ const char *s = i < 0 ? "123" : "4567";
+
+ return strncat (d, s, range ()); /* { dg-warning ".__builtin_strncat.: specified bound between 4 and \[0-9\]+" } */
+}
Index: gcc/testsuite/gcc.dg/pr79214.h
===================================================================
--- gcc/testsuite/gcc.dg/pr79214.h (revision 0)
+++ gcc/testsuite/gcc.dg/pr79214.h (working copy)
@@ -0,0 +1,13 @@
+#pragma GCC system_header
+
+#define bzero __builtin_bzero
+#define memcpy __builtin_memcpy
+#define memmove __builtin_memmove
+#define mempcpy __builtin_mempcpy
+#define memset __builtin_memset
+#define strcat __builtin_strcat
+#define stpcpy __builtin_stpcpy
+#define stpncpy __builtin_stpncpy
+#define strcpy __builtin_strcpy
+#define strncpy __builtin_strncpy
+#define strncat __builtin_strncat
Index: gcc/testsuite/gcc.dg/pr79222.c
===================================================================
--- gcc/testsuite/gcc.dg/pr79222.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr79222.c (working copy)
@@ -0,0 +1,13 @@
+/* PR middle-end/79222 - missing -Wstringop-overflow= on a stpcpy overflow
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+extern char* stpcpy (char*, const char*);
+
+char d[3];
+
+char* f (int i)
+{
+ const char *s = i < 0 ? "01234567" : "9876543210";
+ return stpcpy (d, s); /* { dg-warning ".stpcpy. writing 9 bytes into a region of size 3 overflows the destination" } */
+}
Index: gcc/testsuite/gcc.dg/pr79223.c
===================================================================
--- gcc/testsuite/gcc.dg/pr79223.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr79223.c (working copy)
@@ -0,0 +1,25 @@
+/* PR middle-end/79223 - missing -Wstringop-overflow on a memmove overflow
+ { dg-do compile }
+ { dg-additional-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void* memcpy (void*, const void*, size_t);
+
+char d[3];
+char s[4];
+
+size_t range (void)
+{
+ extern size_t size ();
+ size_t n = size ();
+ if (n <= sizeof d)
+ return sizeof d + 1;
+
+ return n;
+}
+
+void test_memcpy (void)
+{
+ memcpy (d, s, range ()); /* { dg-warning ".memcpy. writing between 4 and \[0-9\]+ bytes into a region of size 3 overflows the destination" } */
+}
Index: gcc/testsuite/gcc.dg/pr78138.c
===================================================================
--- gcc/testsuite/gcc.dg/pr78138.c (revision 244844)
+++ gcc/testsuite/gcc.dg/pr78138.c (working copy)
@@ -18,5 +18,5 @@ void g (void *p)
extern unsigned n;
if (n < 17 || 32 < n) n = 7;
- memcpy (d, p, n); /* { dg-warning ".memcpy.: writing between 7 and 32 bytes into a region of size 5" } */
+ memcpy (d, p, n); /* { dg-warning ".memcpy. writing between 7 and 32 bytes into a region of size 5" } */
};
