The gimple_call_alloc_size() function that determines the range of sizes of allocated objects and constrains the bounds in calls to functions like memcpy calls get_range() instead of get_size_range() to obtain its result. The latter is the right function to call because it has the necessary logic to constrain the range to just the values that are valid for object sizes. This is especially useful when the range is the result of a conversion from a signed to a wider unsigned integer where the upper subrange is excessive and can be eliminated such as in:
char* f (int n) { if (n > 8) n = 8; char *p = malloc (n); strcpy (p, "0123456789"); // buffer overflow ... } Attached is a fix that lets -Wstringop-overflow diagnose the buffer overflow above. Besides with GCC I have also tested the change by building Binutils/GDB and Glibc and verifying that it doesn't introduce any false positives. Martin
PR middle-end/92942 - missing -Wstringop-overflow for allocations with a negative lower bound size gcc/ChangeLog: PR middle-end/92942 * builtins.c (gimple_call_alloc_size): Call get_size_range instead of get_range. * calls.c (get_size_range): Define new overload. Handle anti-ranges whose upper part is with the valid size range. * calls.h (get_size_range): Declare new overload. gcc/testsuite/ChangeLog: PR middle-end/92942 * gcc.dg/Wstringop-overflow-40.c: New test. * gcc.dg/Wstringop-overflow-41.c: New test. * gcc.dg/attr-alloc_size-10.c: Disable macro tracking. diff --git a/gcc/builtins.c b/gcc/builtins.c index 8845816aebd..4caa02f9a3b 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3884,7 +3884,7 @@ check_access (tree exp, tree, tree, tree dstwrite, tree gimple_call_alloc_size (gimple *stmt, wide_int rng1[2] /* = NULL */, - const vr_values *rvals /* = NULL */) + const vr_values * /* = NULL */) { if (!stmt) return NULL_TREE; @@ -3936,7 +3936,7 @@ gimple_call_alloc_size (gimple *stmt, wide_int rng1[2] /* = NULL */, if (!rng1) rng1 = rng1_buf; - if (!get_range (size, rng1, rvals)) + if (!get_size_range (size, rng1)) return NULL_TREE; if (argidx2 > nargs && TREE_CODE (size) == INTEGER_CST) @@ -3946,7 +3946,7 @@ gimple_call_alloc_size (gimple *stmt, wide_int rng1[2] /* = NULL */, of the upper bounds as a constant. Ignore anti-ranges. */ tree n = argidx2 < nargs ? gimple_call_arg (stmt, argidx2) : integer_one_node; wide_int rng2[2]; - if (!get_range (n, rng2, rvals)) + if (!get_size_range (n, rng2)) return NULL_TREE; /* Extend to the maximum precision to avoid overflow. */ diff --git a/gcc/calls.c b/gcc/calls.c index a11da66492d..6c59b82dd78 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1238,17 +1238,17 @@ alloc_max_size (void) } /* Return true when EXP's range can be determined and set RANGE[] to it - after adjusting it if necessary to make EXP a represents a valid size - of object, or a valid size argument to an allocation function declared - with attribute alloc_size (whose argument may be signed), or to a string - manipulation function like memset. When ALLOW_ZERO is true, allow - returning a range of [0, 0] for a size in an anti-range [1, N] where - N > PTRDIFF_MAX. A zero range is a (nearly) invalid argument to - allocation functions like malloc but it is a valid argument to - functions like memset. */ + after adjusting it if necessary to make EXP represent a valid size + of an object, or a valid size argument to an allocation function + declared with attribute alloc_size (whose argument may be signed), + or to a string manipulation function like memset. When ALLOW_ZERO + is true, allow returning a range of [0, 0] for a size in an anti-range + [1, N] where N >= PTRDIFF_MAX. A zero range is a (nearly) invalid + argument to allocation functions like malloc but it is a valid argument + to functions like memset. */ bool -get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) +get_size_range (tree exp, wide_int range[2], bool allow_zero /* = false */) { if (!exp) return false; @@ -1256,7 +1256,7 @@ get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) if (tree_fits_uhwi_p (exp)) { /* EXP is a constant. */ - range[0] = range[1] = exp; + range[0] = range[1] = wi::to_wide (exp); return true; } @@ -1277,13 +1277,11 @@ get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) { /* Use the full range of the type of the expression when no value range information is available. */ - range[0] = TYPE_MIN_VALUE (exptype); - range[1] = TYPE_MAX_VALUE (exptype); + range[0] = wi::to_wide (TYPE_MIN_VALUE (exptype)); + range[1] = wi::to_wide (TYPE_MAX_VALUE (exptype)); return true; } - range[0] = NULL_TREE; - range[1] = NULL_TREE; return false; } @@ -1337,14 +1335,46 @@ get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) } else { - max = min - 1; - min = wi::zero (expprec); + /* Make sure both operands have the same precision to keep + wide_int from helpfully aborting. */ + wide_int maxsz = wi::to_wide (max_object_size ()); + max = wide_int::from (max, maxsz.get_precision (), UNSIGNED); + if (wi::geu_p (max, maxsz)) + { + /* EXP is unsigned and the upper part of the range is + in excess of maximum object size. */ + max = min - 1; + min = wi::zero (expprec); + } + else + { + min = wi::zero (expprec); + max = wi::to_wide (TYPE_MAX_VALUE (exptype)); + } } } - range[0] = wide_int_to_tree (exptype, min); - range[1] = wide_int_to_tree (exptype, max); + range[0] = min; + range[1] = max; + return true; +} + + +/* Convenience overload for the get_size_range above. */ + +bool +get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) +{ + wide_int rng[2]; + if (!get_size_range (exp, rng, allow_zero)) + { + range[0] = range[1] = NULL_TREE; + return false; + } + tree type = TREE_TYPE (exp); + range[0] = wide_int_to_tree (type, rng[0]); + range[1] = wide_int_to_tree (type, rng[1]); return true; } diff --git a/gcc/calls.h b/gcc/calls.h index 4ee49360777..913f879bb42 100644 --- a/gcc/calls.h +++ b/gcc/calls.h @@ -133,7 +133,10 @@ extern bool reference_callee_copied (CUMULATIVE_ARGS *, extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]); extern tree get_attr_nonstring_decl (tree, tree * = NULL); extern void maybe_warn_nonstring_arg (tree, tree); -extern bool get_size_range (tree, tree[2], bool = false); +extern bool get_size_range (tree, wide_int[2], bool = false) + ATTRIBUTE_NONNULL (2); +extern bool get_size_range (tree, tree[2], bool = false) + ATTRIBUTE_NONNULL (2); extern rtx rtx_for_static_chain (const_tree, bool); extern bool cxx17_empty_base_field_p (const_tree); diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c new file mode 100644 index 00000000000..b3e598ca30e --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-40.c @@ -0,0 +1,163 @@ +/* PR middle-end/92942 - missing -Wstringop-overflow for allocations with + a negative lower bound size + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +#define SIZE_MAX __SIZE_MAX__ +#define UINT8_MAX __UINT8_MAX__ +#define UINT16_MAX __UINT16_MAX__ + +typedef __SIZE_TYPE__ size_t; +typedef __UINT8_TYPE__ uint8_t; +typedef __UINT16_TYPE__ uint16_t; + +void* usr_alloc1 (size_t) __attribute__ ((alloc_size (1))); +void* usr_alloc2 (size_t, size_t) __attribute__ ((alloc_size (1, 2))); + +void* malloc (size_t); +void* memcpy (void*, const void*, size_t); +void* memset (void*, int, size_t); +char* strcpy (char*, const char*); + +void sink (void*); + +void malloc_uint_range_strcpy (unsigned n) +{ + void *p = malloc (5 < n ? 5 : n); + + strcpy (p, "01234"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + strcpy (p, "0123"); + sink (p); +} + +void malloc_uint16_anti_range_memset (uint16_t n) +{ + if (5 <= n && n <= 9) return; + void *p = malloc (n); + + if (UINT16_MAX < SIZE_MAX) + { + size_t sz = (uint16_t)-1 + (size_t)1; + memset (p, 0, sz); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + } + + memset (p, 0, 1); + sink (p); + memset (p, 0, 5); + sink (p); + memset (p, 0, 6); + sink (p); + memset (p, 0, UINT16_MAX - 1); + sink (p); + memset (p, 0, UINT16_MAX); + sink (p); +} + +void malloc_int_strcpy (int n) +{ + void *p = malloc (7 < n ? 7 : n); + + strcpy (p, "0123456"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + strcpy (p, "012345"); + sink (p); +} + +void vla_int_strcpy (int n) +{ + char a[9 < n ? 9 : n]; + + strcpy (a, "012345678"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (a); + + strcpy (a, "01234567"); + sink (a); +} + +void usr_alloc1_int_strcpy (int n) +{ + void *p = usr_alloc1 (7 < n ? 7 : n); + + strcpy (p, "0123456"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + strcpy (p, "012345"); + sink (p); +} + +void usr_alloc2_cst_ir_strcpy (int n) +{ + void *p = usr_alloc2 (1, 5 < n ? 5 : n); + + strcpy (p, "01234"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + strcpy (p, "0123"); + sink (p); +} + +void usr_alloc2_ir_ir_strcpy (int m, int n) +{ + void *p = usr_alloc2 (3 < n ? 3 : n, 5 < n ? 5 : n); + + strcpy (p, "0123456789abcde"); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + strcpy (p, "0123456789abcd"); + sink (p); +} + +void usr_alloc2_uint8_memset (uint8_t m, uint8_t n) +{ + if (3 <= m && m <= 7) return; + if (5 <= n && n <= 9) return; + void *p = usr_alloc2 (m, n); + + size_t sz = UINT8_MAX * UINT8_MAX + 1; + memset (p, 0, sz); // { dg-warning "\\\[-Wstringop-overflow" "" { xfail *-*-* } } + // { dg-warning "\\\[-Warray-bounds" "pr?????" { target *-*-* } .-1 } + sink (p); + + memset (p, 0, sz - 1); + sink (p); + memset (p, 0, 64); + sink (p); + memset (p, 0, 63); + sink (p); + memset (p, 0, 16); + sink (p); + memset (p, 0, 15); + sink (p); + memset (p, 0, 14); + sink (p); + memset (p, 0, 3); + sink (p); +} + + + +void malloc_int_memset (int n) +{ + void *p = malloc (11 < n ? 11 : n); + + memset (p, 0, 12); // { dg-warning "\\\[-Wstringop-overflow" } + sink (p); + + memset (p, 0, 11); + sink (p); +} + +void vla_int_memset (int n) +{ + char a[13 < n ? 13 : n]; + + memset (a, 0, 14); // { dg-warning "\\\[-Wstringop-overflow" } + sink (a); + + memset (a, 0, 13); + sink (a); +} diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-41.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-41.c new file mode 100644 index 00000000000..173aa164646 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-41.c @@ -0,0 +1,91 @@ +/* Verify that an anti-range ~[A, B] with small positive A and B + is handled correctly and doesn't trigger warnings. + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +typedef __typeof__ (sizeof 0) size_t; + +int f (void*, size_t); +int g (void*); + +// Test case distilled from gcc/cp/semantics.c + +int omp_reduction_id (int i, int j, const char *mm) +{ + const char *p = 0; + const char *m = 0; + + switch (i) + { + case 1: + p = "min"; + break; + case 2: + p = "max"; + break; + default: + break; + } + + if (j) + m = mm; + + const char prefix[] = "omp declare reduction "; + size_t lenp = sizeof (prefix); + + if (__builtin_strncmp (p, prefix, lenp - 1) == 0) + lenp = 1; + + size_t len = __builtin_strlen (p); + size_t lenm = m ? __builtin_strlen (m) + 1 : 0; + char *name = ((char *) __builtin_alloca(lenp + len + lenm)); + + if (lenp > 1) + __builtin_memcpy (name, prefix, lenp - 1); + + __builtin_memcpy (name + lenp - 1, p, len + 1); + if (m) + { + name[lenp + len - 1] = '~'; + __builtin_memcpy (name + lenp + len, m, lenm); + } + return (__builtin_constant_p (name) + ? f (name, __builtin_strlen (name)) : g (name)); +} + +// Test case derived from gcc/d/dmd/root/filename.c. + +const char *ext (const char *str) +{ + size_t len = __builtin_strlen(str); + + const char *e = str + len; + for (;;) + { + switch (*e) + { + case '.': return e + 1; + case '/': break; + default: + if (e == str) + break; + e--; + continue; + } + return 0; + } +} + +const char *removeExt (const char *str) +{ + const char *e = ext (str); + if (e) + { + size_t len = (e - str) - 1; + char *n = (char *)__builtin_malloc (len + 1); + __builtin_memcpy(n, str, len); + n[len] = 0; + return n; + } + return 0; +} diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-10.c b/gcc/testsuite/gcc.dg/attr-alloc_size-10.c index 071c6aa1e3b..dac6ca1e147 100644 --- a/gcc/testsuite/gcc.dg/attr-alloc_size-10.c +++ b/gcc/testsuite/gcc.dg/attr-alloc_size-10.c @@ -4,7 +4,7 @@ range. { dg-do compile } - { dg-options "-O2 -Walloc-size-larger-than=12" } + { dg-options "-O2 -Walloc-size-larger-than=12 -ftrack-macro-expansion=0" } { dg-options "-Wno-overflow" { target { ! int32plus } } } */ #define SCHAR_MAX __SCHAR_MAX__