From patchwork Mon Nov 14 03:19:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 81997 Delivered-To: patch@linaro.org Received: by 10.140.97.165 with SMTP id m34csp823618qge; Sun, 13 Nov 2016 19:19:37 -0800 (PST) X-Received: by 10.99.153.26 with SMTP id d26mr25624608pge.44.1479093577912; Sun, 13 Nov 2016 19:19:37 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id m3si20499501pgc.329.2016.11.13.19.19.37 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 13 Nov 2016 19:19:37 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-441286-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-441286-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-441286-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=D8eGjzklliNDgkXrrckkD+YYkGZSMEW5q3S5Xe0PtR9RU9BbpG //T3+txp47Tv2SzaZZ8KI65d37UmsU9yvf8milT9aA3Xzl66wKna+fNOGLTv0ggc /hHWLUg1UYFuXY4h2mkctewJLa56IZ8rKgr38SLcOKRyLxlkdP6bk4a24= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=iBP7Ri/ekUQ0PJ6NKOu9/Qe/UzE=; b=WFR9RiVax9seSiWJ6OdA y1UZM1yQWrVv6LJssvS+ZaRDRCa5ESUgcPhnogroTM+QLdYCBwBoFF2zODaeEWOr BSinVIpmEoo5ORxJ6gHTuDrCU0AROJfofvrfF4kYVgNVc/Zpv2b+WIjgaHfnc2QW BB6fNqp3KGL8sfeuHhbKfyg= Received: (qmail 22301 invoked by alias); 14 Nov 2016 03:19:25 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 22286 invoked by uid 89); 14 Nov 2016 03:19:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=schar_max, SCHAR_MAX, 1101, sk:attral X-HELO: mail-qk0-f170.google.com Received: from mail-qk0-f170.google.com (HELO mail-qk0-f170.google.com) (209.85.220.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 14 Nov 2016 03:19:19 +0000 Received: by mail-qk0-f170.google.com with SMTP id q130so81934250qke.1 for ; Sun, 13 Nov 2016 19:19:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version; bh=KoV/4vz2Ya9LOnQ61s/mEZHTzjX0ubuKA7QLzPK9XJI=; b=SV4nBsSsjC4rk2xh7Xq9R/i66O3PGnGq09gH86CqkRxghIIkwfRyDyJ+/NU/POiyOR CQzcZokfwH9/acxkQ0/84z62jPgl1nq2PcrP3UI/bgI+CDm/1RYGFq1LMnSfOVidXpTX bF3aT1BEktr2bYuF7jIzgf/B7Nu6Mq/OmMuhwS5t0vnTBUmKtBB/GTYI6yz7P52xjXNW 7o61mAno2MH0+BpXlGAwEPjEHqeWOsxTkW96fsFrGL5E2GfwK1KUcdGCAbqYOnbPin/S LT/KXmSNPDXhhv1BGvbwt3cveamMNZlwKSTP2GLnvVpMpp1HcW746q14dzeMjteac44a 1rQg== X-Gm-Message-State: ABUngvenHWh5yhc2TLew34xD3dJEom+VWm68ODICzEwVThfCRrKL9cPTUS7QvKDXeJO4Tw== X-Received: by 10.55.68.73 with SMTP id r70mr14115515qka.277.1479093557190; Sun, 13 Nov 2016 19:19:17 -0800 (PST) Received: from [192.168.0.26] (75-166-206-79.hlrn.qwest.net. [75.166.206.79]) by smtp.gmail.com with ESMTPSA id w12sm11486525qka.4.2016.11.13.19.19.16 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 13 Nov 2016 19:19:16 -0800 (PST) To: Gcc Patch List From: Martin Sebor Subject: [PATCH] warn on overflow in calls to allocation functions (bugs 77531 and 78284) Message-ID: <204efcec-649f-34a1-22a1-161d1d98ea95@gmail.com> Date: Sun, 13 Nov 2016 20:19:14 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 X-IsSubscribed: yes Bug 77531 requests a new warning for calls to allocation functions (those declared with attribute alloc_size(X, Y)) that overflow the computation X * Z of the size of the allocated object. Bug 78284 suggests that detecting and diagnosing other common errors in calls to allocation functions, such as allocating more space than SIZE_MAX / 2 bytes, would help prevent subsequent buffer overflows. The attached patch adds two new warning options, -Walloc-zero and -Walloc-larger-than=bytes that implement these two enhancements. The patch is not 100% finished because, as it turns out, the GCC allocation built-ins (malloc et al.) do not make use of the attribute and so don't benefit from the warnings. The tests are also incomplete, and there's at least one bug in the implementation I know about. I'm posting the patch while stage 1 is still open and to give a heads up on it and to get early feedback. I expect completing it will be straightforward. Martin PS The alloc_max_size function added in the patch handles sizes specified using suffixes like KB, MB, etc. I added that to make it possible to specify sizes in excess of the maximum of INT_MAX that (AFAIK) options that take integer arguments handle out of the box. It only belatedly occurred to me that the suffixes are unnecessary if the option argument is handled using strtoull. I can remove the suffix (as I suspect it will raise objections) but I think that a general solution along these lines would be useful to let users specify large byte sizes in other options as well (such -Walloca-larger-than, -Wvla-larger-then). Are there any suggestions or preferences? PR c/77531 - __attribute__((alloc_size(1,2))) could also warn on multiplication overflow PR c/78284 - warn on malloc with very large arguments gcc/c-family/ChangeLog: PR c/77531 PR c/78284 * c.opt (-Walloc-zero, -Walloc-larger-than): New options. gcc/testsuite/ChangeLog: PR c/77531 PR c/78284 * gcc.dg/attr-alloc_size-3.c: New test. gcc/ChangeLog: PR c/77531 PR c/78284 * calls.c (alloc_max_size): New function. (maybe_warn_alloc_args_overflow): Same. (initialize_argument_information): Diagnose overflow in functions declared with attaribute alloc_size. * doc/invoke.texi (Warning Options): Document -Walloc-zero and -Walloc-larger-than. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 213353b..72c1e14 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -299,6 +299,15 @@ Walloca C ObjC C++ ObjC++ Var(warn_alloca) Warning Warn on any use of alloca. +Walloc-larger-than= +C ObjC C++ ObjC++ Var(warn_alloc_limit) Warning Joined +-Walloc-larger-than= Warn for calls to allocation functions that attempt +to allocate objects larger than the specified number of bytes. + +Walloc-zero +C ObjC C++ ObjC++ Var(warn_alloc_zero) Warning EnabledBy(Wextra) +-Walloc-zero Warn for calls to allocation functions that specify zero bytes. + Walloca-larger-than= C ObjC C++ ObjC++ Var(warn_alloca_limit) Warning Joined RejectNegative UInteger -Walloca-larger-than= Warn on unbounded uses of diff --git a/gcc/calls.c b/gcc/calls.c index c916e07..dfeb5fe 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -48,6 +48,8 @@ along with GCC; see the file COPYING3. If not see #include "dbgcnt.h" #include "rtl-iter.h" #include "tree-chkp.h" +#include "tree-vrp.h" +#include "tree-ssanames.h" #include "rtl-chkp.h" @@ -1181,6 +1183,192 @@ store_unaligned_arguments_into_pseudos (struct arg_data *args, int num_actuals) } } +static tree alloc_object_size_limit; + +/* Initialize ALLOC_OBJECT_SIZE_LIMIT based on the -Walloc-larger-than=limit + setting if the option is specified, or to the maximum object size if it + is not. Return the initialized value. */ + +static tree +alloc_max_size (void) +{ + if (!alloc_object_size_limit) + { + alloc_object_size_limit = TYPE_MAX_VALUE (ssizetype); + + unsigned HOST_WIDE_INT unit = 1; + + char *end; + errno = 0; + unsigned HOST_WIDE_INT limit + = warn_alloc_limit ? strtoull (warn_alloc_limit, &end, 10) : 0; + + if (limit && !errno) + { + if (end && *end) + { + /* Numeric option arguments are at most INT_MAX. Make it + possible to specify a larger value by accepting common + suffixes. */ + if (!strcmp (end, "kB")) + unit = 1000; + else if (!strcasecmp (end, "KiB") || strcmp (end, "KB")) + unit = 1024; + else if (!strcmp (end, "MB")) + unit = 1000LU * 1000; + else if (!strcasecmp (end, "MiB")) + unit = 1024LU * 1024; + else if (!strcasecmp (end, "GB")) + unit = 1000LU * 1000 * 1000; + else if (!strcasecmp (end, "GiB")) + unit = 1024LU * 1024 * 1024; + else if (!strcasecmp (end, "TB")) + unit = 1000LU * 1000 * 1000 * 1000; + else if (!strcasecmp (end, "TiB")) + unit = 1024LU * 1024 * 1024 * 1024; + else if (!strcasecmp (end, "PB")) + unit = 1000LU * 1000 * 1000 * 1000 * 1000; + else if (!strcasecmp (end, "PiB")) + unit = 1024LU * 1024 * 1024 * 1024 * 1024; + else if (!strcasecmp (end, "EB")) + unit = 1000LU * 1000 * 1000 * 1000 * 1000 * 1000; + else if (!strcasecmp (end, "EiB")) + unit = 1024LU * 1024 * 1024 * 1024 * 1024 * 1024; + else + unit = 0; + } + + if (unit) + alloc_object_size_limit = build_int_cst (ssizetype, limit * unit); + } + } + return alloc_object_size_limit; +} + +/* Diagnose a call EXP to function FN decorated with attribute alloc_size + whose argument numbers given by IDX with values given by ARGS exceed + the maximum object size or cause an unsigned oveflow (wrapping) when + multiplied. When ARGS[0] is null the function does nothing. ARGS[1] + may be null for functions like malloc, and non-null for those like + calloc that are decorated with a two-argument attribute alloc_size. */ + +static void +maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2]) +{ + if (!args[0]) + return; + + bool warned = false; + + if (!args[1]) + args[1] = size_one_node; + + bool range[2] = { false, false }; + + /* Determine the minimum value of the argument. */ + for (unsigned i = 0; i != 2; ++i) + { + if (TREE_CODE (args[i]) == SSA_NAME) + { + tree type = TREE_TYPE (args[i]); + wide_int zero = build_int_cst (type, 0); + signop sgn = TYPE_SIGN (type); + + wide_int min, max; + value_range_type range_type = get_range_info (args[i], &min, &max); + if (range_type == VR_RANGE) + { + if (wi::lt_p (min, zero, sgn) && wi::ge_p (max, zero, sgn)) + continue; + + args[i] = wide_int_to_tree (type, min); + } + else if (range_type == VR_ANTI_RANGE) + { + if (wi::lt_p (min - 1, zero, sgn) && wi::ge_p (max + 1, zero, sgn)) + continue; + args[i] = wide_int_to_tree (type, max + 1); + } + else + args[i] = size_one_node; + + range[i] = true; + + if (integer_zerop (args[i])) + args[i] = size_one_node; + } + } + + tree maxobjsize = alloc_max_size (); + + location_t loc = EXPR_LOCATION (exp); + + if (tree_fits_uhwi_p (args[0]) && tree_fits_uhwi_p (args[1]) + && !integer_zerop (args[0]) && !integer_zerop (args[1]) + && !integer_onep (args[0]) && !integer_onep (args[1])) + { + /* Check for overflow in the product of a function decorated with + attribute alloc_size (X, Y). */ + unsigned szprec = TYPE_PRECISION (size_type_node); + wide_int x = wi::to_wide (args[0], szprec); + wide_int y = wi::to_wide (args[1], szprec); + + bool vflow; + wide_int prod = wi::umul (x, y, &vflow); + + if (vflow) + warned = warning_at (loc, OPT_Walloc_larger_than_, + "product of arguments %i and %i results " + "in unsigned overflow: %<%E * %E%>", + idx[0] + 1, idx[1] + 1, args[0], args[1]); + else if (wi::ltu_p (wi::to_wide (maxobjsize, szprec), prod)) + warned = warning_at (loc, OPT_Walloc_larger_than_, + "product of arguments %i and %i exceeds " + "maximum object size %E: %<%E * %E%>", + idx[0] + 1, idx[1] + 1, + maxobjsize, args[0], args[1]); + } + else + { + /* This is either a function decorated with attribute alloc_size (X), + or one that takes two allocation arguments like realloc and one + of the arguments is equal to 0, or to 1, or it value is unknown. */ + for (unsigned i = 0; i != 2; ++i) + { + if (tree_fits_uhwi_p (args[i])) + if (tree_int_cst_lt (maxobjsize, args[i])) + { + warned = warning_at (loc, OPT_Walloc_larger_than_, + "argument %i value %qE exceeds maximum " + "object size %E", + idx[i] + 1, args[i], maxobjsize); + break; + } + if (!range[i]) + { + if (integer_zerop (args[i])) + { + warned = warning_at (loc, OPT_Walloc_zero, + "argument %i value is zero", + idx[i] + 1); + break; + } + else if (!tree_expr_nonnegative_p (args[i])) + { + warned = warning_at (loc, OPT_Walloc_larger_than_, + "argument %i value %qE is negative", + idx[i] + 1, args[i]); + break; + } + } + } + } + + if (warned) + inform (DECL_SOURCE_LOCATION (fn), + "in a call to allocation function %qD declared here", fn); +} + /* Issue an error if CALL_EXPR was flagged as requiring tall-call optimization. */ @@ -1280,6 +1468,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, j--; } } + argpos = 0; FOR_EACH_CALL_EXPR_ARG (arg, iter, exp) { @@ -1349,6 +1538,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, } else args[j].tree_value = arg; + j--; argpos++; } @@ -1359,6 +1549,24 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, bitmap_obstack_release (NULL); + /* Extract attribute alloc_size and if set, store the indices of + the corresponding arguments in ALLOC_IDX, and then the actual + argument(s) at those indices in ALLOC_ARGS. */ + int alloc_idx[2] = { -1, -1 }; + if (tree alloc_size + = (fndecl ? lookup_attribute ("alloc_size", + TYPE_ATTRIBUTES (TREE_TYPE (fndecl))) + : NULL_TREE)) + { + tree args = TREE_VALUE (alloc_size); + alloc_idx[0] = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1; + if (TREE_CHAIN (args)) + alloc_idx[1] = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (args))) - 1; + } + + /* Array for up to the two attribute alloc_size arguments. */ + tree alloc_args[] = { NULL_TREE, NULL_TREE }; + /* I counts args in order (to be) pushed; ARGPOS counts in order written. */ for (argpos = 0; argpos < num_actuals; i--, argpos++) { @@ -1595,7 +1803,17 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, targetm.calls.function_arg_advance (args_so_far, TYPE_MODE (type), type, argpos < n_named_args); + + /* Store argument values for functions decorated with attribute + alloc_size. */ + if (argpos == alloc_idx[0]) + alloc_args[0] = args[i].tree_value; + else if (argpos == alloc_idx[1]) + alloc_args[1] = args[i].tree_value; } + + /* Check the arguments of functions decorated with attribute alloc_size. */ + maybe_warn_alloc_args_overflow (fndecl, exp, alloc_args, alloc_idx); } /* Update ARGS_SIZE to contain the total size for the argument block. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 76b8540..2ff0b65 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -256,6 +256,7 @@ Objective-C and Objective-C++ Dialects}. @gccoptlist{-fsyntax-only -fmax-errors=@var{n} -Wpedantic @gol -pedantic-errors @gol -w -Wextra -Wall -Waddress -Waggregate-return @gol +-Walloc-zero -Walloc-larger-than=@var{n} -Walloca -Walloca-larger-than=@var{n} @gol -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol -Wno-attributes -Wbool-compare -Wbool-operation @gol @@ -4990,6 +4991,17 @@ annotations. Warn about overriding virtual functions that are not marked with the override keyword. +@item -Walloc-zero +@opindex Wno-alloc-zero +@opindex Walloc-zero +Warn about calls to allocation functions that specify zero bytes. This option +is enabled with @option{-Wextra} + +@item -Walloc-larger-than=@var{n} +Warn about calls to functions decorated with attribute @code{alloc_size} +that attempt to allocate objects larger than the specified number of bytes, +or where the computation of the size would overflow. @xref{Function Attributes}. + @item -Walloca @opindex Wno-alloca @opindex Walloca diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-3.c b/gcc/testsuite/gcc.dg/attr-alloc_size-3.c new file mode 100644 index 0000000..adf9333 --- /dev/null +++ b/gcc/testsuite/gcc.dg/attr-alloc_size-3.c @@ -0,0 +1,101 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall -Walloc-zero" } */ + +#define SCHAR_MAX __SCHAR_MAX__ +#define SCHAR_MIN (-SCHAR_MAX - 1) +#define UCHAR_MAX (SCHAR_MAX * 2 + 1) + +#define SHRT_MAX __SHRT_MAX__ +#define SHRT_MIN (-SHRT_MAX - 1) +#define USHRT_MAX (SHRT_MAX * 2 + 1) + +#define INT_MAX __INT_MAX__ +#define INT_MIN (-INT_MAX - 1) +#define UINT_MAX (INT_MAX * 2 + 1) + +#define LONG_MAX __LONG_MAX__ +#define LONG_MIN (-LONG_MAX - 1L) +#define ULONG_MAX (LONG_MAX * 2 + 1) + +#define LLONG_MAX __LLONG_MAX__ +#define LLONG_MIN (-LLONG_MAX - 1LL) +#define ULLONG_MAX (ULLONG_MAX * 2 + 1) + +#define ALLOC_SIZE(...) __attribute__ ((alloc_size (__VA_ARGS__))) + +void* f_uchar_1 (unsigned char) ALLOC_SIZE (1); +void* f_schar_1 (signed char) ALLOC_SIZE (1); + +void* f_ushrt_1 (unsigned short) ALLOC_SIZE (1); +void* f_shrt_1 (signed short) ALLOC_SIZE (1); + +void* f_uint_1 (unsigned int) ALLOC_SIZE (1); +void* f_sint_1 (signed int) ALLOC_SIZE (1); + +void* f_ulong_1 (unsigned long) ALLOC_SIZE (1); +void* f_slong_1 (signed long) ALLOC_SIZE (1); + +void* f_ullong_1 (unsigned long long) ALLOC_SIZE (1); +void* f_sllong_1 (signed long long) ALLOC_SIZE (1); + +void sink (void*); + +void +test_uchar (unsigned char n) +{ + sink (f_uchar_1 (UCHAR_MAX)); + sink (f_uchar_1 (1)); + sink (f_uchar_1 (0)); /* { dg-warning "argument 1 value is zero" } */ + sink (f_uchar_1 (n)); +} + +void +test_schar (signed char n) +{ + sink (f_schar_1 (SCHAR_MAX)); + sink (f_schar_1 (SCHAR_MIN)); /* { dg-warning "argument 1 value .-\[0-9\]+. is negative" } */ + sink (f_schar_1 (-1)); /* { dg-warning "argument 1 value .-1. is negative" } */ + sink (f_schar_1 (0)); + sink (f_schar_1 (1)); + sink (f_schar_1 (n)); +} + +void +test_ushrt (unsigned short n) +{ + sink (f_ushrt_1 (USHRT_MAX)); + sink (f_ushrt_1 (1)); + sink (f_ushrt_1 (0)); /* { dg-warning "argument 1 value is zero" } */ + sink (f_ushrt_1 (n)); +} + +void +test_sshrt (signed short n) +{ + sink (f_shrt_1 (SHRT_MAX)); + sink (f_shrt_1 (SHRT_MIN)); /* { dg-warning "argument 1 value .-\[0-9\]+. is negative" } */ + sink (f_shrt_1 (-1)); /* { dg-warning "argument 1 value .-1. is negative" } */ + sink (f_shrt_1 (0)); /* { dg-warning "argument 1 value is zero" } */ + sink (f_shrt_1 (1)); + sink (f_shrt_1 (n)); +} + +void +test_uint (unsigned int n) +{ + sink (f_uint_1 (UINT_MAX)); + sink (f_uint_1 (1)); + sink (f_uint_1 (0)); /* { dg-warning "argument 1 value is zero" } */ + sink (f_uint_1 (n)); +} + +void +test_sint (signed short n) +{ + sink (f_int_1 (INT_MAX)); + sink (f_int_1 (INT_MIN)); /* { dg-warning "argument 1 value .-\[0-9\]+. is negative" } */ + sink (f_int_1 (-1)); /* { dg-warning "argument 1 value .-1. is negative" } */ + sink (f_int_1 (0)); /* { dg-warning "argument 1 value is zero" } */ + sink (f_int_1 (1)); + sink (f_int_1 (n)); +}