From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x829.google.com (mail-qt1-x829.google.com [IPv6:2607:f8b0:4864:20::829]) by sourceware.org (Postfix) with ESMTPS id F1A1E3857828 for ; Tue, 15 Sep 2020 19:47:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F1A1E3857828 Received: by mail-qt1-x829.google.com with SMTP id 19so4218728qtp.1 for ; Tue, 15 Sep 2020 12:47:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version:content-language; bh=LXlB15xrQbROOohiT7PDRkJyGUphfQtbqrYFAJUxEcs=; b=Rvx+SpE5+Hrq1ra2AYCb3iZb0gd31AT2aKjm6wRWnjhFD1EakMkJvTjFX6sATsDTnv L91tR00Ej1wvHnb0QgBidTB+H8LiMrPc+zmKPyqHso8D/ZhQXLyPpSChttKE2B1cjIiR 6nxPweFmCBuRwPCsFbeIIztFKPbJOq/a/8ej3xcG22jniDn255aBvoyYu6G0NGuSL6F7 bqwGaExUBWzohbhEZhyknPjTuyZ+mxvBsCGYLQAG4T/kvoOtvJ6gDRqvey0lEbaBb5Vd 0vb3Hy/31o73w1j426kKI2wbWo6gzUXJNOnPu9iBHXVP2Kz8GZTmcGMaGrlNMiGLrhlI 08UA== X-Gm-Message-State: AOAM533/7m4T9Gm9jISi/hzmDO55P0CXPn+Ap2jfd+lnxBejQ33Zlt/z 8gkIAYaywyY4Ur5bdKM2fT0= X-Google-Smtp-Source: ABdhPJxPoMJBbibMhvYpxbZyt21q5rXca3cgYQX0gGTaDt6wFuzOOeaZGq9CPUyF6LC5kJUjTQW6Fw== X-Received: by 2002:aed:3aa1:: with SMTP id o30mr19425274qte.152.1600199268644; Tue, 15 Sep 2020 12:47:48 -0700 (PDT) Received: from [192.168.0.41] (174-16-106-113.hlrn.qwest.net. [174.16.106.113]) by smtp.gmail.com with ESMTPSA id m6sm17262423qkh.106.2020.09.15.12.47.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 15 Sep 2020 12:47:48 -0700 (PDT) To: gcc-patches , Jeff Law From: Martin Sebor Subject: [PATCH] warn for integer overflow in allocation calls (PR 96838) Message-ID: <83a00f2c-de4f-60d8-9399-a64671795eda@gmail.com> Date: Tue, 15 Sep 2020 13:47:46 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------79BB5E80B19F5092F7A69FD7" Content-Language: en-US X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Sep 2020 19:47:51 -0000 This is a multi-part message in MIME format. --------------79BB5E80B19F5092F7A69FD7 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 --------------79BB5E80B19F5092F7A69FD7 Content-Type: text/x-patch; name="gcc-96838.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-96838.diff" 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); +} --------------79BB5E80B19F5092F7A69FD7--