From patchwork Mon Jun 11 16:11:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 164225 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]) by ozlabs.org (Postfix) with SMTP id 854A2B704B for ; Tue, 12 Jun 2012 02:12:12 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1340035933; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=GnnL6+oxgNaVU5A6KKcc2T32L8s=; b=GL0oebf3kQdYM2Z +km+hByyn+AKYumSGm3bQYUwZFlRrlLjjsTlQATRi/OVAfYVLWgHawq3YnfRekBP lgNIqTeQC0ifokRk8v3BDSg7KXpAOeZB+huadX4xZjS1tYdKRzRx1dG3us9l+LXP w/03HeammFSnc8TZrLmNsc7RVzuc= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=Z+xsoe+aqzJgYsof0hYmMVc/3HY1Vl6hC51IKmuFkGa1p52hqo/OeZ23kPAcpV qT2Qs/+AQYud8AfIMEXIL2KhV5hsVgSa1a57hjHt+O+/nQPn+wNrA1wKMiTuK9vs A7/XiyKabs7XwxJuJidijR903a6yx+I8Zj6ubpOLh+psQ=; Received: (qmail 29434 invoked by alias); 11 Jun 2012 16:12:01 -0000 Received: (qmail 29196 invoked by uid 22791); 11 Jun 2012 16:11:55 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Jun 2012 16:11:40 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5BGBdoO024080 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 11 Jun 2012 12:11:39 -0400 Received: from dhcp-5-241.str.redhat.com (dhcp-5-241.str.redhat.com [10.32.5.241]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5BGBbLi009868 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Mon, 11 Jun 2012 12:11:38 -0400 Message-ID: <4FD618B8.1020806@redhat.com> Date: Mon, 11 Jun 2012 18:11:37 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org, Jason Merrill , Gabriel Dos Reis Subject: Re: [C++] Reject variably modified types in operator new References: <4FC4F2AE.2090106@redhat.com> <4FC88495.60607@redhat.com> <4FC8B116.6000607@redhat.com> In-Reply-To: <4FC8B116.6000607@redhat.com> X-IsSubscribed: yes 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 On 06/01/2012 02:09 PM, Florian Weimer wrote: > On 06/01/2012 11:00 AM, Florian Weimer wrote: > >> I'll try to warn about this case and make the transformation to the >> proper operator new[] call. > > Here's the version. I've added a warning for the ill-formed code. > > The only remaining glitch is in g++.dg/cpp0x/regress/debug-debug7.C, > specifically (b is not a constant): > > int (*x)[b] = new int[a][b]; // { dg-error "not usable" } > > The new warning I've added fires on this line, too. How can I check for > the pending error and suppress the warning? As discussed in the other thread with Jason and Gaby, I have changed cp/parser.c to accept non-constant values and produce the error in build_new_1. This is arguably the right place because this check must be performed after template instantiation. (I will submit a follow-up patch for the remaining type check in the parser once this is accepted.) testsuite/g++.dg/cpp0x/regress/debug-debug7.C remains problematic. The diagnostic at parse time was definitely better, but I see no way to keep that. I have reworded the diagnostics because the code no longer has access to the original syntax, so it cannot tell whether the operator new call involved a type-id in parens. Bootstrapped and tested on x86_64-linux-gnu, with no new regressions. 2012-06-11 Florian Weimer * init.c (build_new_1): Warn about (T[N]) for variable N, and reject T[M][N]. * parser.c (cp_parser_direct_new_declarator): Accept non-constant expressions. Handled now in build_new_1. 2012-06-11 Florian Weimer * g++.dg/init/new35.C: New. * g++.dg/init/new36.C: New. * g++.dg/ext/vla5.C: New warning. * g++.dg/ext/vla8.C: New warning. * g++.dg/cpp0x/regress/debug-debug7.C: Update diagnostics. Index: cp/init.c =================================================================== --- cp/init.c (revision 188384) +++ cp/init.c (working copy) @@ -2175,6 +2175,7 @@ tree pointer_type; tree non_const_pointer_type; tree outer_nelts = NULL_TREE; + bool outer_nelts_from_type = false; tree alloc_call, alloc_expr; /* The address returned by the call to "operator new". This node is a VAR_DECL and is therefore reusable. */ @@ -2209,10 +2210,14 @@ } else if (TREE_CODE (type) == ARRAY_TYPE) { + /* Transforms new (T[N]) to new T[N]. The former is a GNU + extension. (This also covers new T where T is a VLA + typedef.) */ array_p = true; nelts = array_type_nelts_top (type); outer_nelts = nelts; type = TREE_TYPE (type); + outer_nelts_from_type = true; } /* If our base type is an array, then make sure we know how many elements @@ -2220,11 +2225,40 @@ for (elt_type = type; TREE_CODE (elt_type) == ARRAY_TYPE; elt_type = TREE_TYPE (elt_type)) - nelts = cp_build_binary_op (input_location, - MULT_EXPR, nelts, - array_type_nelts_top (elt_type), - complain); + { + tree inner_nelts = array_type_nelts_top (elt_type); + tree inner_nelts_cst = maybe_constant_value (inner_nelts); + if (!TREE_CONSTANT (inner_nelts_cst)) + { + if (complain & tf_error) + error_at (EXPR_LOC_OR_HERE (inner_nelts), + "array size in operator new must be constant"); + nelts = error_mark_node; + } + if (nelts != error_mark_node) + nelts = cp_build_binary_op (input_location, + MULT_EXPR, nelts, + inner_nelts_cst, + complain); + } + if (variably_modified_type_p (elt_type, NULL_TREE) && (complain & tf_error)) + { + error ("variably modified type not allowed in operator new"); + return error_mark_node; + } + + if (nelts == error_mark_node) + return error_mark_node; + + /* Warn if we performed the (T[N]) to T[N] transformation and N is + variable. */ + if (outer_nelts_from_type + && !TREE_CONSTANT (maybe_constant_value (outer_nelts)) + && (complain & tf_warning_or_error)) + pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla, + "ISO C++ does not support variable-length array types"); + if (TREE_CODE (elt_type) == VOID_TYPE) { if (complain & tf_error) Index: cp/parser.c =================================================================== --- cp/parser.c (revision 188384) +++ cp/parser.c (working copy) @@ -6845,41 +6845,34 @@ while (true) { tree expression; + cp_token *token; /* Look for the opening `['. */ cp_parser_require (parser, CPP_OPEN_SQUARE, RT_OPEN_SQUARE); - /* The first expression is not required to be constant. */ - if (!declarator) + + token = cp_lexer_peek_token (parser->lexer); + expression = cp_parser_expression (parser, /*cast_p=*/false, NULL); + /* The standard requires that the expression have integral + type. DR 74 adds enumeration types. We believe that the + real intent is that these expressions be handled like the + expression in a `switch' condition, which also allows + classes with a single conversion to integral or + enumeration type. */ + if (!processing_template_decl) { - cp_token *token = cp_lexer_peek_token (parser->lexer); - expression = cp_parser_expression (parser, /*cast_p=*/false, NULL); - /* The standard requires that the expression have integral - type. DR 74 adds enumeration types. We believe that the - real intent is that these expressions be handled like the - expression in a `switch' condition, which also allows - classes with a single conversion to integral or - enumeration type. */ - if (!processing_template_decl) + expression + = build_expr_type_conversion (WANT_INT | WANT_ENUM, + expression, + /*complain=*/true); + if (!expression) { - expression - = build_expr_type_conversion (WANT_INT | WANT_ENUM, - expression, - /*complain=*/true); - if (!expression) - { - error_at (token->location, - "expression in new-declarator must have integral " - "or enumeration type"); - expression = error_mark_node; - } + error_at (token->location, + "expression in new-declarator must have integral " + "or enumeration type"); + expression = error_mark_node; } } - /* But all the other expressions must be. */ - else - expression - = cp_parser_constant_expression (parser, - /*allow_non_constant=*/false, - NULL); + /* Look for the closing `]'. */ cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE); Index: testsuite/g++.dg/cpp0x/regress/debug-debug7.C =================================================================== --- testsuite/g++.dg/cpp0x/regress/debug-debug7.C (revision 188384) +++ testsuite/g++.dg/cpp0x/regress/debug-debug7.C (working copy) @@ -7,8 +7,8 @@ main() { int a = 4; - int b = 5; // { dg-message "not const" } - int (*x)[b] = new int[a][b]; // { dg-error "not usable" } + int b = 5; + int (*x)[b] = new int[a][b]; // { dg-error "array size.*must be constant" } x[2][1] = 7; Index: testsuite/g++.dg/ext/vla5.C =================================================================== --- testsuite/g++.dg/ext/vla5.C (revision 188384) +++ testsuite/g++.dg/ext/vla5.C (working copy) @@ -6,5 +6,5 @@ void test (int a) { - new (char[a]); + new (char[a]); // { dg-warning "variable-length array" } } Index: testsuite/g++.dg/ext/vla8.C =================================================================== --- testsuite/g++.dg/ext/vla8.C (revision 188384) +++ testsuite/g++.dg/ext/vla8.C (working copy) @@ -8,8 +8,8 @@ AvlTreeIter() { - new (void* [Num()]); + new (void* [Num()]); // { dg-warning "variable-length array" } } }; -AvlTreeIter a; +AvlTreeIter a; // { dg-message "from here" } Index: testsuite/g++.dg/init/new35.C =================================================================== --- testsuite/g++.dg/init/new35.C (revision 0) +++ testsuite/g++.dg/init/new35.C (revision 0) @@ -0,0 +1,13 @@ +// { dg-do compile } +// { dg-options "" } + +int +main (int argc, char **argv) +{ + typedef char A[argc]; + new A; // { dg-warning "variable-length array types" } + new A[0]; // { dg-error "must be constant" } + new A[5]; // { dg-error "must be constant" } + new (A[0]); // { dg-error "must be constant" } + new (A[5]); // { dg-error "must be constant" } +} Index: testsuite/g++.dg/init/new36.C =================================================================== --- testsuite/g++.dg/init/new36.C (revision 0) +++ testsuite/g++.dg/init/new36.C (revision 0) @@ -0,0 +1,153 @@ +// Testcase for invocation of constructors/destructors in operator new[]. +// { dg-do run } + +#include + +struct E { + virtual ~E() { } +}; + +struct S { + S(); + ~S(); +}; + +static int count; +static int max; +static int throwAfter = -1; +static S *pS; + +S::S() +{ + if (throwAfter >= 0 && count >= throwAfter) + throw E(); + if (pS) + { + ++pS; + if (this != pS) + abort(); + } + else + pS = this; + ++count; + max = count; +} + +S::~S() +{ + if (count > 1) + { + if (this != pS) + abort(); + --pS; + } + else + pS = 0; + --count; +} + +void __attribute__((noinline)) doit(int n) +{ + { + S *s = new S[n]; + if (count != n) + abort(); + if (pS != s + n - 1) + abort(); + delete [] s; + if (count != 0) + abort(); + } + throwAfter = 2; + max = 0; + try + { + new S[n]; + abort(); + } + catch (E) + { + if (max != 2) + abort(); + } + throwAfter = -1; +} + +int main() +{ + { + S s; + if (count != 1) + abort(); + if (pS != &s) + abort(); + } + if (count != 0) + abort(); + { + S *s = new S; + if (count != 1) + abort(); + if (pS != s) + abort(); + delete s; + if (count != 0) + abort(); + } + { + S *s = new S[1]; + if (count != 1) + abort(); + if (pS != s) + abort(); + delete [] s; + if (count != 0) + abort(); + } + { + S *s = new S[5]; + if (count != 5) + abort(); + if (pS != s + 4) + abort(); + delete [] s; + if (count != 0) + abort(); + } + typedef S A[5]; + { + S *s = new A; + if (count != 5) + abort(); + if (pS != s + 4) + abort(); + delete [] s; + if (count != 0) + abort(); + } + throwAfter = 2; + max = 0; + try + { + new S[5]; + abort(); + } + catch (E) + { + if (max != 2) + abort(); + } + max = 0; + try + { + new A; + abort(); + } + catch (E) + { + if (max != 2) + abort(); + } + throwAfter = -1; + doit(5); +}