Overflowing the size of a dynamic allocation (e.g., malloc or VLA) can lead to a subsequent buffer overflow corrupting the heap or stack. The attached patch diagnoses a subset of these cases where the overflow/wraparound is still detectable.
Besides regtesting GCC on x86_64-linux I also verified the warning doesn't introduce any false positives into Glibc or Binutils/GDB builds on the same target. Martin
PR middle-end/96838 - missing warning on integer overflow in calls to allocation functions gcc/ChangeLog: PR middle-end/96838 * calls.c (eval_size_vflow): New function. (get_size_range): Call it. Add argument. (maybe_warn_alloc_args_overflow): Diagnose overflow/wraparound. * calls.h (get_size_range): Add argument. gcc/testsuite/ChangeLog: PR middle-end/96838 * gcc.dg/Walloc-size-larger-than-19.c: New test. * gcc.dg/Walloc-size-larger-than-20.c: New test. diff --git a/gcc/calls.c b/gcc/calls.c index 8ac94db6817..a5acff301e0 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1237,6 +1237,139 @@ alloc_max_size (void) return alloc_object_size_limit; } +/* Try to evaluate the artithmetic EXPresssion representing the size of + an object in the widest possible type and set RANGE[] to the result. + Return the overflow status for the type of the expression (which may + be OVF_UNKNOWN if it cannot be determined from the ranges of its + operands). Used to detect calls to allocation functions with + an argument that either overflows or wraps around zero. */ + +static wi::overflow_type +eval_size_vflow (tree exp, wide_int range[2]) +{ + const int prec = WIDE_INT_MAX_PRECISION; + + if (TREE_CODE (exp) == INTEGER_CST) + { + range[0] = range[1] = wi::to_wide (exp, prec); + return wi::OVF_NONE; + } + + if (TREE_CODE (exp) != SSA_NAME) + return wi::OVF_UNKNOWN; + + gimple *def = SSA_NAME_DEF_STMT (exp); + if (!is_gimple_assign (def)) + return wi::OVF_UNKNOWN; + + tree_code code = gimple_assign_rhs_code (def); + tree optype = NULL_TREE; + wide_int op1r[2], op2r[2]; + if (code == MULT_EXPR + || code == MINUS_EXPR + || code == PLUS_EXPR) + { + /* Ignore the overflow on the operands. */ + tree rhs1 = gimple_assign_rhs1 (def); + wi::overflow_type ovf = eval_size_vflow (rhs1, op1r); + if (ovf == wi::OVF_UNKNOWN) + return ovf; + + optype = TREE_TYPE (rhs1); + tree rhs2 = gimple_assign_rhs2 (def); + ovf = eval_size_vflow (rhs2, op2r); + if (ovf == wi::OVF_UNKNOWN) + return ovf; + + if (code == PLUS_EXPR + && TYPE_UNSIGNED (optype) + && TREE_CODE (rhs2) == INTEGER_CST) + { + /* A - CST is transformed into A + (-CST). Undo that to avoid + false reports of overflow (this means overflow due to very + large constants in the source code isn't detected.) */ + tree sgn_type = signed_type_for (optype); + tree max = TYPE_MAX_VALUE (sgn_type); + wide_int smax = wi::to_wide (max, prec); + if (wi::ltu_p (smax, op2r[0])) + { + op2r[0] = wi::neg (wi::sub (op2r[0], smax, UNSIGNED, &ovf)); + op2r[1] = op2r[0]; + } + } + } + else if (code == NOP_EXPR) + { + /* Strip (implicit) conversions. Explicit conversions are stripped + as well which may result in reporting overflow despite a cast. + Those cases should be rare. */ + tree rhs1 = gimple_assign_rhs1 (def); + wi::overflow_type ovf = eval_size_vflow (rhs1, op1r); + if (ovf == wi::OVF_UNKNOWN) + return ovf; + optype = TREE_TYPE (rhs1); + } + else + { + wide_int min, max; + if (determine_value_range (exp, &min, &max) != VR_RANGE) + return wi::OVF_UNKNOWN; + optype = TREE_TYPE (exp); + op1r[0] = wide_int::from (min, prec, TYPE_SIGN (optype)); + op1r[1] = wide_int::from (max, prec, TYPE_SIGN (optype)); + } + + wide_int umax = wi::to_wide (TYPE_MAX_VALUE (optype), prec); + tree sgn_type = signed_type_for (optype); + wide_int smax = wi::to_wide (TYPE_MAX_VALUE (sgn_type), prec); + + wi::overflow_type ovf = wi::OVF_NONE; + if (code == MULT_EXPR) + { + /* Compute the upper bound of the result first, discarding any + overflow. Only the overflow in the lower bound matters. */ + range[1] = wi::mul (op1r[1], op2r[1], UNSIGNED, &ovf); + range[0] = wi::mul (op1r[0], op2r[0], UNSIGNED, &ovf); + } + else if (code == MINUS_EXPR) + { + range[1] = wi::sub (op1r[1], op2r[1], UNSIGNED, &ovf); + range[0] = wi::sub (op1r[0], op2r[0], UNSIGNED, &ovf); + } + else if (code == PLUS_EXPR) + { + signop sgn = UNSIGNED; + if (op2r[0] == op2r[1] && wi::ltu_p (smax, op2r[0])) + sgn = SIGNED; + + range[1] = wi::add (op1r[1], op2r[1], sgn, &ovf); + range[0] = wi::add (op1r[0], op2r[0], sgn, &ovf); + } + else + { + range[0] = op1r[0]; + range[1] = op1r[1]; + } + + if (ovf != wi::OVF_NONE) + return ovf; + + /* Nothing can be determined from a range that spans zero. + TO DO: Assume a range with a negative lower bound a zero upper + bound overflows? */ + if (wi::sign_mask (range[0]) && !wi::sign_mask (range[1])) + return wi::OVF_UNKNOWN; + + if (wi::ltu_p (umax, range[0])) + return wi::OVF_OVERFLOW; + + umax = wi::to_wide (max_object_size (), prec); + if (wi::ltu_p (umax, range[0])) + return wi::OVF_OVERFLOW; + + return wi::OVF_NONE; +} + /* 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 @@ -1244,11 +1377,13 @@ alloc_max_size (void) 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. */ + allocation functions like malloc but it is a valid argument to functions + like memset. When nonnull, set *VFLOW_RESULT to the overflowing result + if the evaluating EXP results in overflow/wraparound, otherwise to null. */ bool -get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) +get_size_range (tree exp, tree range[2], bool allow_zero /* = false */, + tree vflow_range[2] /* = NULL_TREE */) { if (!exp) return false; @@ -1266,10 +1401,27 @@ get_size_range (tree exp, tree range[2], bool allow_zero /* = false */) wide_int min, max; enum value_range_kind range_type; + if (vflow_range) + { + /* Check for overflow or wraparound first. */ + wide_int vfr[2]; + wi::overflow_type vflow = eval_size_vflow (exp, vfr); + if (vflow == wi::OVF_OVERFLOW || vflow == wi::OVF_UNDERFLOW) + { + tree type = uint128_type_node ? uint128_type_node : sizetype; + vflow_range[0] = wide_int_to_tree (type, vfr[0]); + vflow_range[1] = wide_int_to_tree (type, vfr[1]); + } + } + if (integral) range_type = determine_value_range (exp, &min, &max); else - range_type = VR_VARYING; + { + if (vflow_range) + vflow_range[0] = vflow_range[1] = NULL_TREE; + range_type = VR_VARYING; + } if (range_type == VR_VARYING) { @@ -1373,6 +1525,8 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2]) /* Validate each argument individually. */ for (unsigned i = 0; i != 2 && args[i]; ++i) { + tree vflow_range[2] = { NULL_TREE, NULL_TREE }; + if (TREE_CODE (args[i]) == INTEGER_CST) { argrange[i][0] = args[i]; @@ -1422,12 +1576,12 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2]) } } else if (TREE_CODE (args[i]) == SSA_NAME - && get_size_range (args[i], argrange[i])) + && get_size_range (args[i], argrange[i], false, vflow_range)) { /* Verify that the argument's range is not negative (including upper bound of zero). */ if (tree_int_cst_lt (argrange[i][0], integer_zero_node) - && tree_int_cst_le (argrange[i][1], integer_zero_node)) + && tree_int_cst_le (argrange[i][1], integer_zero_node)) { warned = warning_at (loc, OPT_Walloc_size_larger_than_, "%Kargument %i range [%E, %E] is negative", @@ -1443,6 +1597,23 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2]) argrange[i][0], argrange[i][1], maxobjsize); } + /* Finally, diagnose integer overflow/wraparound. */ + else if (vflow_range[0]) + { + if (tree_int_cst_equal (vflow_range[0], vflow_range[1])) + warned = warning_at (loc, OPT_Walloc_size_larger_than_, + "%Kargument %i value %E is too large " + "to represent in %qT", + exp, idx[i] + 1, vflow_range[0], + TREE_TYPE (args[i])); + else + warned = warning_at (loc, OPT_Walloc_size_larger_than_, + "%Kargument %i range [%E, %E] is too large " + "to represent in %qT", + exp, idx[i] + 1, + vflow_range[0], vflow_range[1], + TREE_TYPE (args[i])); + } } } diff --git a/gcc/calls.h b/gcc/calls.h index dfb951ca45b..cf0ac0c3eee 100644 --- a/gcc/calls.h +++ b/gcc/calls.h @@ -133,7 +133,7 @@ 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 bool maybe_warn_nonstring_arg (tree, tree); -extern bool get_size_range (tree, tree[2], bool = false); +extern bool get_size_range (tree, tree[2], bool = false, tree[2] = NULL); 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/Walloc-size-larger-than-19.c b/gcc/testsuite/gcc.dg/Walloc-size-larger-than-19.c new file mode 100644 index 00000000000..d906354442b --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloc-size-larger-than-19.c @@ -0,0 +1,143 @@ +/* PR middle-end/96838 - missing warning on integer overflow in calls + to allocation functions + { dg-do compile } + { dg-options "-O1 -Wall -ftrack-macro-expansion=0" } */ + +#define INT_MAX __INT_MAX__ +#define UINT_MAX (__INT_MAX__ * 2U + 1) +#define SIZE_MAX __SIZE_MAX__ + +#define ATTR(...) __attribute__ ((__VA_ARGS__)) + +typedef __SIZE_TYPE__ size_t; +typedef __UINT32_TYPE__ uin32_t; + +void* malloc (size_t); + +void sink (void*); + +#define T(call) sink (call) + +ATTR (alloc_size (1)) void* fui1 (unsigned); +ATTR (alloc_size (1, 2)) void* fui1_2 (unsigned, unsigned); + +void alloc_ui_mul_ui (unsigned n) +{ + if (n < UINT_MAX / 2) + n = UINT_MAX / 2; + + T (fui1 (n)); + T (fui1 (n * 2)); + T (fui1 (n * 3)); // { dg-warning "argument 1 range \\\[6442450941, 12884901885] is too large to represent in 'unsigned int'" } + T (fui1 (n * n)); // { dg-warning "argument 1 range \\\[4611686014132420609, 18446744065119617025] is too large to represent in 'unsigned int'" } +} + +void alloc_ui_mul_ui_plus (unsigned n, unsigned p1, int m1) +{ + if (n < 32) + n = 32; + + T (fui1 (n)); + T (fui1 (n * 2)); + T (fui1 (n * 3)); + T (fui1 (n * 4)); + T (fui1 (n * (UINT_MAX / 32))); + T (fui1 (n * (UINT_MAX / 32) + 31)); + T (fui1 (n * (UINT_MAX / 32) + 32)); // { dg-warning "argument 1 range \\\[4294967296, 576460747874238497] is too large to represent in 'unsigned int'" } + + if (n < UINT_MAX / 4) + n = UINT_MAX / 4; + + T (fui1 (n)); + T (fui1 (n * 2)); + T (fui1 (n * 3)); + T (fui1 (n * 4)); + T (fui1 (n * 4 + 1)); + T (fui1 (n * 4 + 2)); + T (fui1 (n * 4 + 3)); + T (fui1 (n * 4 + 4)); // { dg-warning "argument 1 range \\\[4294967296, 17179869184] is too large to represent in 'unsigned int'" } + + if (p1 < 1) + p1 = 1; + + T (fui1 (n * 4 + p1)); + T (fui1 (n * 4 + p1 + 1)); + T (fui1 (n * 4 + p1 + 2)); + T (fui1 (n * 4 + p1 + 3)); // { dg-warning "argument 1 range \\\[4294967296, 21474836478] is too large to represent in 'unsigned int'" } + + T (fui1 (n * 4 - p1 + 1)); + T (fui1 (n * 4 - p1 + 2)); + T (fui1 (n * 4 - p1 + 3)); + T (fui1 (n * 4 - p1 + 4)); + T (fui1 (n * 4 - p1 + 5)); // { dg-warning "argument 1 range \\\[4294967296, 12884901890] is too large to represent in 'unsigned int'" } + + if (m1 > -1) + m1 = -1; + + T (fui1 (n * 1 + m1)); + T (fui1 (n * 2 + m1)); + T (fui1 (n * 3 + m1)); + T (fui1 (n * 4 + m1)); + T (fui1 (n * 4 + m1 + 1)); + T (fui1 (n * 4 + m1 + 2)); + T (fui1 (n * 4 + m1 + 4)); +} + +void alloc_ui_plus_ui (unsigned n) +{ + if (n < UINT_MAX - 2) + n = UINT_MAX - 2; + + T (fui1 (n - 2)); + T (fui1 (n - 1)); + T (fui1 (n)); + T (fui1 (n + 1)); + T (fui1 (n + 2)); + T (fui1 (n + 3)); // { dg-warning "argument 1 range \\\[4294967296, 4294967298] is too large to represent in 'unsigned int'" } + + T (fui1 (n + sizeof (char))); + T (fui1 (n + sizeof (char[2]))); + T (fui1 (n + sizeof (char[3]))); // { dg-warning "argument 1 range \\\[4294967296, 4294967298] is too large to represent in 'unsigned int'" } + T (fui1 (n + sizeof (char[n]))); // { dg-warning "argument 1 range \\\[8589934586, 8589934590] is too large to represent in 'unsigned int'" } +} + +void alloc_ui_plus_sz (size_t n) +{ + if (n < SIZE_MAX - 2) + n = SIZE_MAX - 2; + + T (fui1 (n)); // { dg-warning "argument 1 range \\\[18446744073709551613, 18446744073709551615] is too large to represent in 'unsigned int'" } + T (fui1 (n + 1)); // { dg-warning "argument 1 range \\\[18446744073709551614, 0x10000000000000000] is too large to represent in 'unsigned int'" } + T (fui1 (n + 2)); // { dg-warning "argument 1 range \\\[18446744073709551615, 0x10000000000000001] is too large to represent in 'unsigned int'" } + T (fui1 (n + 3)); // { dg-warning "argument 1 range \\\[0x10000000000000000, 0x10000000000000002] is too large to represent in 'unsigned int'" } + T (fui1 (n + n)); // { dg-warning "argument 1 range \\\[0x1fffffffffffffffa, 0x1fffffffffffffffe] is too large to represent in 'unsigned int'" } +} + +void alloc_ui_mul_sz (size_t n, size_t nx) +{ + if (n < SIZE_MAX / 4) + n = SIZE_MAX / 4; + + T (fui1 (n)); + T (fui1 (n * 2)); // { dg-warning "argument 1 range \\\[9223372036854775806, 0x1fffffffffffffffe] is too large to represent in 'unsigned int'" } + T (fui1 (n * 3)); // { dg-warning "argument 1 range \\\[13835058055282163709, 0x2fffffffffffffffd] is too large to represent in 'unsigned int'" } + + T (fui1 (sizeof (int) * (nx - 2))); + T (fui1 (sizeof (int) * (nx - 1))); + T (fui1 (sizeof (int) * nx)); + T (fui1 (sizeof (int) * (nx + 1))); + T (fui1 (sizeof (int) * (nx + 2))); +} + +void malloc_mul (int n, size_t nx) +{ + if (n > -1) + n = -1; + + T (malloc (n * sizeof (int))); // { dg-warning "argument 1 range \\\[18446744065119617024, 18446744073709551612] exceeds maximum object size" } + + T (malloc (nx)); + T (malloc (nx * 2)); + T (malloc (nx * 3)); + T (malloc (nx * 4)); +} diff --git a/gcc/testsuite/gcc.dg/Walloc-size-larger-than-20.c b/gcc/testsuite/gcc.dg/Walloc-size-larger-than-20.c new file mode 100644 index 00000000000..8353291e4bd --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloc-size-larger-than-20.c @@ -0,0 +1,38 @@ +/* PR middle-end/96838 - missing warning on integer overflow in calls + to allocation functions + Test case reduced from binutils/gas/sb.c. + { dg-do compile } + { dg-options "-O1 -Wall -ftrack-macro-expansion=0" } */ + +typedef __SIZE_TYPE__ size_t; +typedef __PTRDIFF_TYPE__ ssize_t; + +extern void* malloc (size_t); + +typedef struct sb +{ + size_t len; + size_t max; +} +sb; + +static void* +sb_check (sb *ptr, size_t len) +{ + size_t want = ptr->len + len + (2 * sizeof (size_t)) + 1; + if ((ssize_t) want < 0) return 0; + size_t max = (size_t)1 << (8 * sizeof (want) + - (sizeof (want) <= sizeof (long) + ? __builtin_clzl ((long) want) + : __builtin_clzll ((long long) want))); + + max -= (2 * sizeof (size_t)) + 1; + return malloc (max + 1); // { dg-bogus "-Walloc-size-larger-than" } +} + + + +void* sb_add_char (sb *ptr) +{ + return sb_check (ptr, 1); +}