From patchwork Tue Aug 2 12:55:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 654779 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3s3bln3wH4z9t45 for ; Tue, 2 Aug 2016 22:55:32 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Z/fXVXLI; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=cFmp2b/ev4JCJ70ei eMp9uwGNaflh2sSj/Brwp0hFZ+Kof+ehoWJ4X78i7k8Xl1rsOlPe5V5rkZ5Oom/v 99kzKj7jn0A/InJGoFcW5jCXJj1gR0I6K6+G8GMYb1o4aNzZY4vNcfhHz2WgrYPv tgPIbkfE8xU2Jp51NnqezD8CgM= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=k8yBeoAA1Y53emuBnKYeX9+ vMeo=; b=Z/fXVXLI7x09d2/xPLRVjuuWA311eflKmPVmwS8usVpgWiK4Z9xjtUU ZAByngz8zhW06kAzxXcl9FBVTVKCcfpliy/mx26NoJeXPEiK6axeRkbeA2bMICAl //vYyx/w64aAYeehpwPoM6jFYOjpfAOFDEYJx3TxCa5MEO7rgLPw= Received: (qmail 51363 invoked by alias); 2 Aug 2016 12:55:21 -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 51320 invoked by uid 89); 2 Aug 2016 12:55:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=warned, andrews, Andrews, 7000 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 02 Aug 2016 12:55:08 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3C8FD4E022; Tue, 2 Aug 2016 12:55:06 +0000 (UTC) Received: from reynosa.quesejoda.com (vpn-228-97.phx2.redhat.com [10.3.228.97]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u72Ct2WA011786; Tue, 2 Aug 2016 08:55:03 -0400 Subject: Re: RFA: new pass to warn on questionable uses of alloca() and VLAs To: Joseph Myers References: <577F9301.10205@redhat.com> <5782C7A3.9050308@gmail.com> <578917C0.4000809@redhat.com> <578AA22C.4050307@gmail.com> <578CB32B.9080602@redhat.com> <578D9B10.6010600@gmail.com> <578E02C7.5050400@redhat.com> Cc: Martin Sebor , gcc-patches , Martin Sebor , Jeff Law , Andrew MacLeod From: Aldy Hernandez Message-ID: <57A09826.3040703@redhat.com> Date: Tue, 2 Aug 2016 08:55:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: On 08/01/2016 04:35 PM, Joseph Myers wrote: > On Tue, 19 Jul 2016, Aldy Hernandez wrote: > >> + // Do not warn on VLAs occurring in a loop, since VLAs are >> + // guaranteed to be cleaned up when they go out of scope. >> + // That is, there is a corresponding __builtin_stack_restore >> + // at the end of the scope in which the VLA occurs. > > Given this ... > >> + case ALLOCA_IN_LOOP: >> + warning_at (loc, wcode, >> + is_vla ? "use of variable-length array " >> + "within a loop" >> + : "use of % within a loop"); >> + break; > > ... why is there a VLA case for this diagnostic at all? I'd expect an > assertion that only the alloca case can reach this diagnostic. > Indeed. Fixed. > Also, if the format string for a diagnostic function is a ? : conditional > expression you need to mark up each half with G_() so that both halves are > properly extracted for translation. This applies to lots of diagnostics > in this patch. Hugh, I didn't know about the G_() magic for marking format strings. Fixed everywhere. OK? gcc/ * Makefile.in (OBJS): Add gimple-ssa-warn-alloca.o. * passes.def: Add two instances of pass_walloca. * tree-pass.h (make_pass_walloca): New. * gimple-ssa-warn-walloca.c: New file. * doc/invoke.texi: Document -Walloca, -Walloca-larger-than=, and -Wvla-larger-than= options. gcc/c-family/ * c.opt (Walloca): New. (Walloca-larger-than=): New. (Wvla-larger-than=): New. diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 7a0160f..210153a 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1296,6 +1296,7 @@ OBJS = \ gimple-ssa-nonnull-compare.o \ gimple-ssa-split-paths.o \ gimple-ssa-strength-reduction.o \ + gimple-ssa-warn-alloca.o \ gimple-streamer-in.o \ gimple-streamer-out.o \ gimple-walk.o \ diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c index c11e7e7..6e82fc8 100644 --- a/gcc/c-family/c-opts.c +++ b/gcc/c-family/c-opts.c @@ -376,6 +376,16 @@ c_common_handle_option (size_t scode, const char *arg, int value, cpp_opts->warn_num_sign_change = value; break; + case OPT_Walloca_larger_than_: + if (!value) + inform (loc, "-Walloca-larger-than=0 is meaningless"); + break; + + case OPT_Wvla_larger_than_: + if (!value) + inform (loc, "-Wvla-larger-than=0 is meaningless"); + break; + case OPT_Wunknown_pragmas: /* Set to greater than 1, so that even unknown pragmas in system headers will be warned about. */ diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index a5358ed..c9caffe 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -275,6 +275,16 @@ Wall C ObjC C++ ObjC++ Warning Enable most warning messages. +Walloca +C ObjC C++ ObjC++ Var(warn_alloca) Warning +Warn on any use of alloca. + +Walloca-larger-than= +C ObjC C++ ObjC++ Var(warn_alloca_limit) Warning Joined RejectNegative UInteger +-Walloca-larger-than= Warn on unbounded uses of +alloca, and on bounded uses of alloca whose bound can be larger than + bytes. + Warray-bounds LangEnabledBy(C ObjC C++ ObjC++,Wall) ; in common.opt @@ -980,6 +990,12 @@ Wvla C ObjC C++ ObjC++ Var(warn_vla) Init(-1) Warning Warn if a variable length array is used. +Wvla-larger-than= +C ObjC C++ ObjC++ Var(warn_vla_limit) Warning Joined RejectNegative UInteger +-Wvla-larger-than= Warn on unbounded uses of variable-length arrays, and +on bounded uses of variable-length arrays whose bound can be +larger than bytes. + Wvolatile-register-var C ObjC C++ ObjC++ Var(warn_volatile_register_var) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn when a register variable is declared volatile. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 22001f9..82fb89e 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -255,6 +255,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 +-Walloca -Walloca-larger-than=@var{n} @gol -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol -Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol -Wc90-c99-compat -Wc99-c11-compat @gol @@ -311,7 +312,7 @@ Objective-C and Objective-C++ Dialects}. -Wunused-const-variable -Wunused-const-variable=@var{n} @gol -Wunused-but-set-parameter -Wunused-but-set-variable @gol -Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol --Wvla -Wvolatile-register-var -Wwrite-strings @gol +-Wvla -Wvla-larger-than=@var{n} -Wvolatile-register-var -Wwrite-strings @gol -Wzero-as-null-pointer-constant -Whsa} @item C and Objective-C-only Warning Options @@ -4666,6 +4667,61 @@ annotations. Warn about overriding virtual functions that are not marked with the override keyword. +@item -Walloca +@opindex Wno-alloca +@opindex Walloca +This option warns on all uses of @code{alloca} in the source. + +@item -Walloca-larger-than=@var{n} +This option warns on calls to @code{alloca} that are not bounded by a +controlling predicate limiting its size to @var{n} bytes, or calls to +@code{alloca} where the bound is unknown. + +For example, a bounded case of @code{alloca} could be: + +@smallexample +unsigned int n; +... +if (n <= 1000) + alloca (n); +@end smallexample + +In the above example, passing @code{-Walloca=1000} would not issue a +warning because the call to @code{alloca} is known to be at most 1000 +bytes. However, if @code{-Walloca=500} was passed, the compiler would +have emitted a warning. + +Unbounded uses, on the other hand, are uses of @code{alloca} with no +controlling predicate verifying its size. For example: + +@smallexample +stuff (); +alloca (n); +@end smallexample + +If @code{-Walloca=500} was passed, the above would trigger a warning, +but this time because of the lack of bounds checking. + +Note, that even seemingly correct code involving signed integers could +cause a warning: + +@smallexample +signed int n; +... +if (n < 500) + alloca (n); +@end smallexample + +In the above example, @var{n} could be negative, causing a larger than +expected argument to be implicitly casted into the @code{alloca} call. + +This option also warns when @code{alloca} is used in a loop. + +This warning is not enabled by @option{-Wall}, and is only active when +@option{-ftree-vrp} is active (default for @option{-O2} and above). + +See also @option{-Wvla-larger-than=@var{n}}. + @item -Warray-bounds @itemx -Warray-bounds=@var{n} @opindex Wno-array-bounds @@ -5830,9 +5886,25 @@ moving from a moved-from object, this warning can be disabled. @item -Wvla @opindex Wvla @opindex Wno-vla -Warn if variable length array is used in the code. +Warn if a variable-length array is used in the code. @option{-Wno-vla} prevents the @option{-Wpedantic} warning of -the variable length array. +the variable-length array. + +@item -Wvla-larger-than=@var{n} +If this option is used, the compiler will warn on uses of +variable-length arrays where the size is either unbounded, or bounded +by an argument that can be larger than @var{n} bytes. This is similar +to how @option{-Walloca-larger-than=@var{n}} works, but with +variable-length arrays. + +Note that GCC may optimize small variable-length arrays of a known +value into plain arrays, so this warning may not get triggered for +such arrays. + +This warning is not enabled by @option{-Wall}, and is only active when +@option{-ftree-vrp} is active (default for @option{-O2} and above). + +See also @option{-Walloca-larger-than=@var{n}}. @item -Wvolatile-register-var @opindex Wvolatile-register-var diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c new file mode 100644 index 0000000..2be0ed2 --- /dev/null +++ b/gcc/gimple-ssa-warn-alloca.c @@ -0,0 +1,517 @@ +/* Warn on problematic uses of alloca and variable length arrays. + Copyright (C) 2016 Free Software Foundation, Inc. + Contributed by Aldy Hernandez . + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "backend.h" +#include "tree.h" +#include "gimple.h" +#include "tree-pass.h" +#include "ssa.h" +#include "gimple-pretty-print.h" +#include "diagnostic-core.h" +#include "fold-const.h" +#include "gimple-iterator.h" +#include "tree-ssa.h" +#include "params.h" +#include "tree-cfg.h" +#include "calls.h" +#include "cfgloop.h" +#include "intl.h" + +const pass_data pass_data_walloca = { + GIMPLE_PASS, + "walloca", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + 0, // properties_finish +}; + +class pass_walloca : public gimple_opt_pass +{ +public: + pass_walloca (gcc::context *ctxt) + : gimple_opt_pass(pass_data_walloca, ctxt), first_time_p (false) + {} + opt_pass *clone () { return new pass_walloca (m_ctxt); } + void set_pass_param (unsigned int n, bool param) + { + gcc_assert (n == 0); + first_time_p = param; + } + virtual bool gate (function *); + virtual unsigned int execute (function *); + + private: + // Set to TRUE the first time we run this pass on a function. + bool first_time_p; +}; + +bool +pass_walloca::gate (function *fun ATTRIBUTE_UNUSED) +{ + // The first time this pass is called, it is called before + // optimizations have been run and range information is unavailable, + // so we can only perform strict alloca checking. + if (first_time_p) + return warn_alloca != 0; + + return warn_alloca_limit > 0 || warn_vla_limit > 0; +} + +// Possible problematic uses of alloca. +enum alloca_type { + // Alloca argument is within known bounds that are appropriate. + ALLOCA_OK, + + // Alloca argument is KNOWN to have a value that is too large. + ALLOCA_BOUND_DEFINITELY_LARGE, + + // Alloca argument may be too large. + ALLOCA_BOUND_MAYBE_LARGE, + + // Alloca argument is bounded but of an indeterminate size. + ALLOCA_BOUND_UNKNOWN, + + // Alloca argument was casted from a signed integer. + ALLOCA_CAST_FROM_SIGNED, + + // Alloca appears in a loop. + ALLOCA_IN_LOOP, + + // Alloca argument is 0. + ALLOCA_ARG_IS_ZERO, + + // Alloca call is unbounded. That is, there is no controlling + // predicate for its argument. + ALLOCA_UNBOUNDED +}; + +// We have a few heuristics up our sleeve to determine if a call to +// alloca() is within bounds. Try them out and return the type of +// alloca call this is based on its argument. +// +// Given a known argument (ARG) to alloca() and an EDGE (E) +// calculating said argument, verify that the last statement in the BB +// in E->SRC is a gate comparing ARG to an acceptable bound for +// alloca(). See examples below. +// +// MAX_SIZE is WARN_ALLOCA= adjusted for VLAs. It is the maximum size +// in bytes we allow for arg. +// +// If the alloca bound is determined to be too large, ASSUMED_LIMIT is +// set to the bound used to determine this. ASSUMED_LIMIT is only set +// for ALLOCA_BOUND_MAYBE_LARGE and ALLOCA_BOUND_DEFINITELY_LARGE. +// +// Returns the alloca type. + +static enum alloca_type +alloca_call_type_by_arg (tree arg, edge e, unsigned max_size, + wide_int *assumed_limit) +{ + // All the tests bellow depend on the jump being on the TRUE path. + if (!(e->flags & EDGE_TRUE_VALUE)) + return ALLOCA_UNBOUNDED; + + basic_block bb = e->src; + gimple_stmt_iterator gsi = gsi_last_bb (bb); + gimple *last = gsi_stmt (gsi); + if (!last || gimple_code (last) != GIMPLE_COND) + return ALLOCA_UNBOUNDED; + + /* Check for: + if (ARG <= N) + goto ; + else + goto ; + : + alloca(ARG); + */ + if (gimple_cond_code (last) == LE_EXPR + && gimple_cond_lhs (last) == arg) + { + if (TREE_CODE (gimple_cond_rhs (last)) == INTEGER_CST) + { + tree rhs = gimple_cond_rhs (last); + if (tree_to_uhwi (rhs) > max_size) + { + *assumed_limit = rhs; + return ALLOCA_BOUND_MAYBE_LARGE; + } + return ALLOCA_OK; + } + else + return ALLOCA_BOUND_UNKNOWN; + } + + /* Check for: + if (arg .cond. LIMIT) -or- if (LIMIT .cond. arg) + alloca(arg); + + Where LIMIT has a bound of unknown range. */ + tree limit = NULL; + if (gimple_cond_lhs (last) == arg) + limit = gimple_cond_rhs (last); + else if (gimple_cond_rhs (last) == arg) + limit = gimple_cond_lhs (last); + if (limit && TREE_CODE (limit) == SSA_NAME) + { + wide_int min, max; + value_range_type range_type = get_range_info (limit, &min, &max); + if (range_type == VR_UNDEFINED || range_type == VR_VARYING) + return ALLOCA_BOUND_UNKNOWN; + // FIXME: We could try harder here and handle a possible range + // or anti-range. Hopefully the upcoming changes to range info + // will give us finer grained info, and we can avoid somersaults + // here. + } + + return ALLOCA_UNBOUNDED; +} + +// Return TRUE if SSA's definition is a cast from a signed type. +// If so, set *INVALID_CASTED_TYPE to the signed type. + +static bool +cast_from_signed_p (tree ssa, tree *invalid_casted_type) +{ + gimple *def = SSA_NAME_DEF_STMT (ssa); + if (def + && !gimple_nop_p (def) + && gimple_assign_cast_p (def) + && !TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (def)))) + { + *invalid_casted_type = TREE_TYPE (gimple_assign_rhs1 (def)); + return true; + } + return false; +} + +// Return TURE if X has a maximum range of MAX, basically covering the +// entire domain, in which case it's no range at all. + +static bool +is_max (tree x, wide_int max) +{ + return wi::max_value (TREE_TYPE (x)) == max; +} + +// Analyze the alloca call in STMT and return an `enum alloca_type' +// explaining what type of alloca it is. IS_VLA is set if the alloca +// call is really a BUILT_IN_ALLOCA_WITH_ALIGN, signifying a VLA. +// +// If the alloca bound is determined to be too large, ASSUMED_LIMIT is +// set to the bound used to determine this. ASSUMED_LIMIT is only set +// for ALLOCA_BOUND_MAYBE_LARGE and ALLOCA_BOUND_DEFINITELY_LARGE. +// +// If the alloca call may be too large because of a cast from a signed +// type to an unsigned type, set *INVALID_CASTED_TYPE to the +// problematic signed type. + +static enum alloca_type +alloca_call_type (gimple *stmt, bool is_vla, wide_int *assumed_limit, + tree *invalid_casted_type) +{ + gcc_assert (gimple_alloca_call_p (stmt)); + tree len = gimple_call_arg (stmt, 0); + enum alloca_type w = ALLOCA_UNBOUNDED; + wide_int min, max; + + gcc_assert (!is_vla || warn_vla_limit > 0); + gcc_assert (is_vla || warn_alloca_limit > 0); + + // Adjust warn_alloca_max_size for VLAs, by taking the underlying + // type into account. + unsigned HOST_WIDE_INT max_size; + if (is_vla) + max_size = (unsigned HOST_WIDE_INT) warn_vla_limit; + else + max_size = (unsigned HOST_WIDE_INT) warn_alloca_limit; + + // Check for the obviously bounded case. + if (TREE_CODE (len) == INTEGER_CST) + { + if (tree_to_uhwi (len) > max_size) + { + *assumed_limit = len; + return ALLOCA_BOUND_DEFINITELY_LARGE; + } + if (integer_zerop (len)) + return ALLOCA_ARG_IS_ZERO; + w = ALLOCA_OK; + } + else if (TREE_CODE (len) != SSA_NAME) + return ALLOCA_UNBOUNDED; + // Check the range info if available. + else + { + if (value_range_type range_type = get_range_info (len, &min, &max)) + { + if (range_type == VR_RANGE) + { + if (wi::leu_p (max, max_size)) + w = ALLOCA_OK; + else if (is_max (len, max)) + { + // A cast may have created a range we don't care + // about. For instance, a cast from 16-bit to + // 32-bit creates a range of 0..65535, even if there + // is not really a determinable range in the + // underlying code. In this case, look through the + // cast at the original argument, and fall through + // to look at other alternatives. + gimple *def = SSA_NAME_DEF_STMT (len); + if (gimple_assign_cast_p (def)) + len = gimple_assign_rhs1 (def); + } + else + { + /* If `len' is merely a cast that is being + calculated right before the call to alloca, look + at the range for the original value. + + This avoids the cast creating a range where the + original expression did not have a range: + + # RANGE [0, 18446744073709551615] NONZERO 4294967295 + _2 = (long unsigned int) n_7(D); + p_9 = __builtin_alloca (_2); + + The correct thing would've been for the user to + use size_t, which in the case above would've been + 'long unsigned int', and everything would've + worked. But we have to catch cases where the + user is using some other compatible type for the + call argument to alloca (say unsigned short). */ + gimple *def = SSA_NAME_DEF_STMT (len); + if (gimple_assign_cast_p (def)) + { + len = gimple_assign_rhs1 (def); + range_type = get_range_info (len, &min, &max); + } + + if (range_type != VR_VARYING && is_max (len, max)) + { + // Treat a max of the entire domain as if it had no + // range info, and fall through the try other + // alternatives. + } + else + { + *assumed_limit = max; + return ALLOCA_BOUND_MAYBE_LARGE; + } + } + } + else if (range_type == VR_ANTI_RANGE) + { + // There may be some wrapping around going on. Catch it + // with this heuristic. Hopefully, this VR_ANTI_RANGE + // nonsense will go away, and we won't have to catch the + // sign conversion problems with this crap. + if (cast_from_signed_p (len, invalid_casted_type)) + return ALLOCA_CAST_FROM_SIGNED; + + // Fall thru and try other things. + } + else if (range_type == VR_VARYING) + { + // No easily determined range. Try other things. + } + } + } + + // If we couldn't find anything, try a few heuristics for things we + // can easily determine. Check these misc cases but only accept + // them if all predecessors have a known bound. + basic_block bb = gimple_bb (stmt); + if (w == ALLOCA_UNBOUNDED) + { + w = ALLOCA_OK; + for (unsigned ix = 0; ix < EDGE_COUNT (bb->preds); ix++) + { + enum alloca_type w + = alloca_call_type_by_arg (len, EDGE_PRED (bb, ix), max_size, + assumed_limit); + if (w != ALLOCA_OK) + return w; + } + } + + return w; +} + +// Return TRUE if the alloca call in STMT is in a loop, otherwise +// return FALSE. As an exception, ignore alloca calls for VLAs that +// occur in a loop since those will be cleaned up when they go out of +// scope. + +static bool +in_loop_p (bool is_vla, gimple *stmt) +{ + basic_block bb = gimple_bb (stmt); + if (bb->loop_father + // ?? Functions with no loops get a loop_father? I + // don't get it. The following conditional seems to do + // the trick to exclude such nonsense. + && bb->loop_father->header != ENTRY_BLOCK_PTR_FOR_FN (cfun)) + { + // Do not warn on VLAs occurring in a loop, since VLAs are + // guaranteed to be cleaned up when they go out of scope. + // That is, there is a corresponding __builtin_stack_restore + // at the end of the scope in which the VLA occurs. + tree fndecl = gimple_call_fn (stmt); + while (TREE_CODE (fndecl) == ADDR_EXPR) + fndecl = TREE_OPERAND (fndecl, 0); + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL + && is_vla + && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_ALLOCA_WITH_ALIGN) + return false; + + return true; + } + return false; +} + +unsigned int +pass_walloca::execute (function *fun) +{ + basic_block bb; + FOR_EACH_BB_FN (bb, fun) + { + for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si); + gsi_next (&si)) + { + gimple *stmt = gsi_stmt (si); + location_t loc = gimple_location (stmt); + + if (!gimple_alloca_call_p (stmt)) + continue; + gcc_assert (gimple_call_num_args (stmt) >= 1); + + bool is_vla = gimple_alloca_call_p (stmt) + && gimple_call_alloca_for_var_p (as_a (stmt)); + + // Strict mode whining for VLAs is handled by the front-end, + // so we can safely ignore this case. Also, ignore VLAs if + // the user doesn't care about them. + if (is_vla + && (warn_vla > 0 || !warn_vla_limit)) + continue; + + if (!is_vla && (warn_alloca || !warn_alloca_limit)) + { + if (warn_alloca) + warning_at (loc, OPT_Walloca, G_("use of %")); + continue; + } + + wide_int assumed_limit + = wi::to_wide (integer_zero_node, + TYPE_PRECISION (size_type_node)); + tree invalid_casted_type = NULL; + enum alloca_type w = alloca_call_type (stmt, is_vla, &assumed_limit, + &invalid_casted_type); + + // Even if we think the alloca call is OK, make sure it's + // not in a loop. + if (w == ALLOCA_OK && in_loop_p (is_vla, stmt)) + w = ALLOCA_IN_LOOP; + + enum opt_code wcode + = is_vla ? OPT_Wvla_larger_than_ : OPT_Walloca_larger_than_; + char buff[WIDE_INT_MAX_PRECISION / 4 + 4]; + switch (w) + { + case ALLOCA_OK: + break; + case ALLOCA_BOUND_MAYBE_LARGE: + gcc_assert (assumed_limit != 0); + if (warning_at (loc, wcode, + is_vla ? G_("argument to variable-length array " + "may be too large") + : G_("argument to % may be too large"))) + { + print_decu (assumed_limit, buff); + inform (loc, G_("limit is %u bytes, but argument " + "may be as large as %s"), + is_vla ? warn_vla_limit : warn_alloca_limit, buff); + } + break; + case ALLOCA_BOUND_DEFINITELY_LARGE: + gcc_assert (assumed_limit != 0); + if (warning_at (loc, wcode, + is_vla ? G_("argument to variable-length array " + "is too large") + : G_("argument to % is too large"))) + { + print_decu (assumed_limit, buff); + inform (loc, G_("limit is %u bytes, but argument is %s"), + is_vla ? warn_vla_limit : warn_alloca_limit, buff); + } + break; + case ALLOCA_BOUND_UNKNOWN: + warning_at (loc, wcode, + is_vla ? G_("variable-length array bound is unknown") + : G_("% bound is unknown")); + break; + case ALLOCA_UNBOUNDED: + warning_at (loc, wcode, + is_vla ? G_("unbounded use of variable-length array") + : G_("unbounded use of %")); + break; + case ALLOCA_IN_LOOP: + gcc_assert (!is_vla); + warning_at (loc, wcode, G_("use of % within a loop")); + break; + case ALLOCA_CAST_FROM_SIGNED: + gcc_assert (invalid_casted_type != NULL_TREE); + warning_at (loc, wcode, + is_vla ? G_("argument to variable-length array " + "may be too large due to " + "conversion from %qT to %qT") + : G_("argument to % may be too large " + "due to conversion from %qT to %qT"), + invalid_casted_type, size_type_node); + break; + case ALLOCA_ARG_IS_ZERO: + warning_at (loc, wcode, + is_vla ? G_("argument to variable-length array " + "is zero") + : G_("argument to % is zero")); + break; + default: + gcc_unreachable (); + } + } + } + return 0; +} + +gimple_opt_pass * +make_pass_walloca (gcc::context *ctxt) +{ + return new pass_walloca (ctxt); +} diff --git a/gcc/passes.def b/gcc/passes.def index 3647e90..298b455 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_warn_function_return); NEXT_PASS (pass_expand_omp); NEXT_PASS (pass_build_cgraph_edges); + NEXT_PASS (pass_walloca, /*strict_mode_p=*/true); TERMINATE_PASS_LIST (all_lowering_passes) /* Interprocedural optimization passes. */ @@ -247,6 +248,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_laddress); NEXT_PASS (pass_lim); NEXT_PASS (pass_split_crit_edges); + NEXT_PASS (pass_walloca, /*strict_mode_p=*/false); NEXT_PASS (pass_pre); NEXT_PASS (pass_sink_code); NEXT_PASS (pass_sancov); diff --git a/gcc/testsuite/gcc.dg/Walloca-1.c b/gcc/testsuite/gcc.dg/Walloca-1.c new file mode 100644 index 0000000..34a20c3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-1.c @@ -0,0 +1,63 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=2000 -O2" } */ + +#define alloca __builtin_alloca + +typedef __SIZE_TYPE__ size_t; +extern size_t strlen(const char *); + +extern void useit (char *); + +int num; + +void foo1 (size_t len, size_t len2, size_t len3) +{ + int i; + + for (i=0; i < 123; ++i) + { + char *s = alloca (566); /* { dg-warning "'alloca' within a loop" } */ + useit (s); + } + + char *s = alloca (123); + useit (s); // OK, constant argument to alloca + + s = alloca (num); // { dg-warning "large due to conversion" } + useit (s); + + s = alloca(90000); /* { dg-warning "is too large" } */ + useit (s); + + if (len < 2000) + { + s = alloca(len); // OK, bounded + useit (s); + } + + if (len + len2 < 2000) // OK, bounded + { + s = alloca(len + len2); + useit (s); + } + + if (len3 <= 2001) + { + s = alloca(len3); /* { dg-warning "may be too large" } */ + useit(s); + } +} + +void foo2 (__SIZE_TYPE__ len) +{ + // Test that a direct call to __builtin_alloca_with_align is not confused + // with a VLA. + void *p = __builtin_alloca_with_align (len, 8); // { dg-warning "unbounded use of 'alloca'" } + useit (p); +} + +void foo3 (unsigned char a) +{ + if (a == 0) + useit (__builtin_alloca (a)); // { dg-warning "argument to 'alloca' is zero" } +} diff --git a/gcc/testsuite/gcc.dg/Walloca-2.c b/gcc/testsuite/gcc.dg/Walloca-2.c new file mode 100644 index 0000000..284b34e --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-2.c @@ -0,0 +1,40 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=2000 -O2" } */ + +void f (void *); + +void +g1 (int n) +{ + void *p; + if (n > 0 && n < 2000) + p = __builtin_alloca (n); + else + p = __builtin_malloc (n); + f (p); +} + +void +g2 (int n) +{ + void *p; + if (n < 2000) + p = __builtin_alloca (n); // { dg-warning "large due to conversion" } + else + p = __builtin_malloc (n); + f (p); +} + +void +g3 (int n) +{ + void *p; + if (n > 0 && n < 3000) + { + p = __builtin_alloca (n); // { dg-warning "'alloca' may be too large" } + // { dg-message "note:.*argument may be as large as 2999" "note" { target *-*-* } 34 } + } + else + p = __builtin_malloc (n); + f (p); +} diff --git a/gcc/testsuite/gcc.dg/Walloca-3.c b/gcc/testsuite/gcc.dg/Walloca-3.c new file mode 100644 index 0000000..5345197 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-3.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=2000 -O2" } */ + +void f (void *); + +__SIZE_TYPE__ LIMIT; + +// Warn when there is an alloca bound, but it is an unknown bound. + +void +g1 (__SIZE_TYPE__ n) +{ + void *p; + if (n < LIMIT) + p = __builtin_alloca (n); // { dg-warning "'alloca' bound is unknown" } + else + p = __builtin_malloc (n); + f (p); +} + +// Similar to the above, but do not get confused by the upcast. + +unsigned short SHORT_LIMIT; +void +g2 (unsigned short n) +{ + void *p; + if (n < SHORT_LIMIT) + p = __builtin_alloca (n); // { dg-warning "'alloca' bound is unknown" } + else + p = __builtin_malloc (n); + f (p); +} diff --git a/gcc/testsuite/gcc.dg/Walloca-4.c b/gcc/testsuite/gcc.dg/Walloca-4.c new file mode 100644 index 0000000..d96cc4e --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-4.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=5000 -O2" } */ +/* { dg-xfail-if "Currently broken but Andrew's work should fix this" { *-*-* } } */ + +// Should be another variant of Walloca-7.c. +// This should not warn, as we have a known bound within limits. + + char * + _i18n_number_rewrite (char *w, char *rear_ptr) +{ + + char *src; + _Bool + use_alloca = (((rear_ptr - w) * sizeof (char)) < 4096U); + if (use_alloca) + src = (char *) __builtin_alloca ((rear_ptr - w) * sizeof (char)); + else + src = (char *) __builtin_malloc ((rear_ptr - w) * sizeof (char)); + return src; +} diff --git a/gcc/testsuite/gcc.dg/Walloca-5.c b/gcc/testsuite/gcc.dg/Walloca-5.c new file mode 100644 index 0000000..5ed1171 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-5.c @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=123 -O2" } */ +/* { dg-xfail-if "Currently broken but Andrew's work should fix this" { *-*-* } } */ + +/* The argument to alloca ends up having a range of 0..MAXINT(32bits), + so we think we have a range because of the upcast. Consequently, + we warn with "alloca may be too large", but we should technically + warn with "unbounded use of alloca". + + We currently drill through casts to figure this stuff out, but we + get confused because it's not just a cast. It's a cast, plus a + multiply. + + : + # RANGE [0, 4294967295] NONZERO 4294967295 + _1 = (long unsigned int) something_4(D); + # RANGE [0, 34359738360] NONZERO 34359738360 + _2 = _1 * 8; + _3 = __builtin_alloca (_2); + + I don't know whether it's even worth such fine-grained warnings. + Perhaps we should generically warn everywhere with "alloca may be + too large". + + I'm hoping that this particular case will be easier to diagnose with + Andrew's work. */ + +void useit(void *); +void foobar(unsigned int something) +{ + useit(__builtin_alloca (something * sizeof (const char *))); // { dg-warning "unbounded use of alloca" "" { xfail *-*-* } } +} diff --git a/gcc/testsuite/gcc.dg/Walloca-6.c b/gcc/testsuite/gcc.dg/Walloca-6.c new file mode 100644 index 0000000..b4d8d41 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-6.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=256 -O2" } */ +/* { dg-xfail-if "Currently broken but Andrew's work should fix this" { *-*-* } } */ + +void f (void*); +void g (__SIZE_TYPE__ n) +{ + // No warning on this case. Range is easily determinable. + if (n > 0 && n < 256) + f (__builtin_alloca (n)); +} diff --git a/gcc/testsuite/gcc.dg/Walloca-7.c b/gcc/testsuite/gcc.dg/Walloca-7.c new file mode 100644 index 0000000..d6581a5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-7.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca -O0" } */ + +extern void f(void *); + +void foo(void) +{ + // Test that strict -Walloca works even without optimization. + f (__builtin_alloca(500)); // { dg-warning "use of 'alloca'" } +} + +void bar(void) +{ + // Test that we warn on alloca() calls, not just __builtin_alloca calls. + extern void *alloca(__SIZE_TYPE__); + f (alloca (123)); // { dg-warning "use of 'alloca'" } +} diff --git a/gcc/testsuite/gcc.dg/Walloca-8.c b/gcc/testsuite/gcc.dg/Walloca-8.c new file mode 100644 index 0000000..a4a1204 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Walloca-8.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca-larger-than=2000 -O2" } */ + +void *p; +void +foo (__SIZE_TYPE__ len) +{ + if (len < 2000 / sizeof (void *)) + p = __builtin_alloca (len * sizeof (void *)); + else + p = __builtin_malloc (len * sizeof (void *)); +} diff --git a/gcc/testsuite/gcc.dg/Wvla-1.c b/gcc/testsuite/gcc.dg/Wvla-1.c new file mode 100644 index 0000000..384c930 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wvla-1.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-Wvla-larger-than=100 -O2" } */ + +typedef __SIZE_TYPE__ size_t; + +extern void useit (char *); + +int num; + +void test_vlas (size_t num) +{ + char str2[num]; /* { dg-warning "unbounded use" } */ + useit(str2); + + num = 98; + for (int i=0; i < 1234; ++i) { + char str[num]; // OK, VLA in a loop, but it is a + // known size *AND* the compiler takes + // care of cleaning up between + // iterations with + // __builtin_stack_restore. + useit(str); + } +} diff --git a/gcc/testsuite/gcc.dg/Wvla-2.c b/gcc/testsuite/gcc.dg/Wvla-2.c new file mode 100644 index 0000000..96814dc --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wvla-2.c @@ -0,0 +1,70 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target stdint_types } */ +/* { dg-options "-O2 -Wvla-larger-than=40" } */ + +#include + +void f0 (void *); +void +f1 (__SIZE_TYPE__ a) +{ + if (a <= 10) + { + // 10 * 4 bytes = 40: OK! + uint32_t x[a]; + f0 (x); + } +} + +void +f2 (__SIZE_TYPE__ a) +{ + if (a <= 11) + { + // 11 * 4 bytes = 44: Not OK. + uint32_t x[a]; // { dg-warning "array may be too large" } + // { dg-message "note:.*argument may be as large as 44" "note" { target *-*-* } 25 } + f0 (x); + } +} + +void +f3 (__SIZE_TYPE__ a, __SIZE_TYPE__ b) +{ + if (a <= 5 && b <= 3) + { + // 5 * 3 * 4 bytes = 60: Not OK. + uint32_t x[a][b]; // { dg-warning "array may be too large" } + f0 (x); + } +} + +void +f4 (__SIZE_TYPE__ a, __SIZE_TYPE__ b) +{ + if (a <= 5 && b <= 2) + { + // 5 * 2 * 4 bytes = 40 bytes: OK! + uint32_t x[a][b]; + f0 (x); + } +} + +void +f5 (__SIZE_TYPE__ len) +{ + // Test that a direct call to __builtin_alloca_with_align is not + // confused with a VLA. + void *p = __builtin_alloca_with_align (len, 8); + f0 (p); +} + +void +f6 (unsigned stuff) +{ + int n = 7000; + do { + char a[n]; // { dg-warning "variable-length array is too large" } + f0 (a); + } while (stuff--); +} diff --git a/gcc/testsuite/gcc.dg/Wvla-3.c b/gcc/testsuite/gcc.dg/Wvla-3.c new file mode 100644 index 0000000..5124476 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wvla-3.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-Walloca -O2" } */ + +// Make sure we don't warn on VLA with -Walloca. + +void f (void*); + +void h1 (unsigned n) +{ + int a [n]; + f (a); +} diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 36299a6..57b61f4 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -469,6 +469,7 @@ extern simple_ipa_opt_pass *make_pass_ipa_oacc (gcc::context *ctxt); extern simple_ipa_opt_pass *make_pass_ipa_oacc_kernels (gcc::context *ctxt); extern gimple_opt_pass *make_pass_gen_hsail (gcc::context *ctxt); extern gimple_opt_pass *make_pass_warn_nonnull_compare (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_walloca (gcc::context *ctxt); /* IPA Passes */ extern simple_ipa_opt_pass *make_pass_ipa_lower_emutls (gcc::context *ctxt);