From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22398 invoked by alias); 28 Jan 2016 20:33:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 22380 invoked by uid 89); 28 Jan 2016 20:33:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=iterator, !found, sk:constru, sk:array_t 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; Thu, 28 Jan 2016 20:33:11 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 25E0FC0A9CC6 for ; Thu, 28 Jan 2016 20:33:10 +0000 (UTC) Received: from redhat.com (ovpn-204-33.brq.redhat.com [10.40.204.33]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0SKX6dA020926 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 28 Jan 2016 15:33:08 -0500 Date: Thu, 28 Jan 2016 20:33:00 -0000 From: Marek Polacek To: GCC Patches , Jason Merrill Subject: C++ PATCH for c++/69509 and c++/69516 (infinite recursion with invalid VLAs in constexpr functions) Message-ID: <20160128203305.GE25193@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-SW-Source: 2016-01/txt/msg02252.txt.bz2 This patch fixes some problems with VLAs and constexprs. (The runtime aspect of this matter is being tracked in PR69517.) It does two things: 1) Changes build_vec_init slightly to prevent the compiler from getting into infinite recursion. Currently, we emit the following sequence when we're initializing e.g. n = 1; int a[n] = { 1, 2 }; (cleanup_point junk omitted): int a[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)]; (void) (int[0:(sizetype) (SAVE_EXPR <(ssizetype) n + -1>)] *) int * D.2256; (void) (D.2256 = (int *) &a) int * D.2257; (void) (D.2257 = D.2256) long int D.2258; (void) (D.2258 = (long int) (SAVE_EXPR <(ssizetype) n + -1>)) // == 0 (void) (*D.2257 = 1) (void) ++D.2257 (void) --D.2258 // == -1 (void) (*D.2257 = 2) (void) ++D.2257 (void) --D.2258 // == -2 while (1) { if (D.2258 == -1) // never triggers goto ; (void) (*D.2257 = 0) (void) ++D.2257 (void) --D.2258; } :; ... So we first write the elements from the initializer and then we default-initialize any remaining elements. But the iterator == -1 check is never true for invalid code, which means the compiler will get stuck in the while loop forever, leading to crash -- it tries to cxx_eval_* the body of the loop over and over again. Fixed by changing == -1 into <= -1; this should only ever happen for invalid code, but we don't want to ICE in any case. This also "fixes" the problem in PR69517 -- the program could get into infinite recursion as well, because it was evaluating the sequence above at runtime. But since it's invoking UB, it doesn't matter much. 2) Moves the check for "array subscript out of bound" a tad above, because for invalid VLAs we can't rely on the bool "found" -- e.g. for the example above it will find all indexes in the initializer, so "found" is true, which means we wouldn't get to the out-of-bounds check below. Hopefully I'm making sense. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2016-01-28 Marek Polacek PR c++/69509 PR c++/69516 * constexpr.c (cxx_eval_array_reference): Give the "array subscript out of bound" error earlier. * init.c (build_vec_init): Change NE_EXPR into GT_EXPR. Update the commentary. * g++.dg/ext/constexpr-vla2.C: New test. * g++.dg/ext/constexpr-vla3.C: New test. * g++.dg/ubsan/vla-1.C: Remove dg-shouldfail. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 57595a4..b076991 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1833,6 +1833,19 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t, return t; } + tree nelts = array_type_nelts_top (TREE_TYPE (ary)); + /* For VLAs, the number of elements won't be an integer constant. */ + nelts = cxx_eval_constant_expression (ctx, nelts, false, non_constant_p, + overflow_p); + VERIFY_CONSTANT (nelts); + if (!tree_int_cst_lt (index, nelts)) + { + if (!ctx->quiet) + error ("array subscript out of bound"); + *non_constant_p = true; + return t; + } + bool found; if (TREE_CODE (ary) == CONSTRUCTOR) { @@ -1846,37 +1859,23 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t, if (!found) { - tree nelts = array_type_nelts_top (TREE_TYPE (ary)); - /* For VLAs, the number of elements won't be an integer constant. */ - nelts = cxx_eval_constant_expression (ctx, nelts, false, non_constant_p, - overflow_p); - VERIFY_CONSTANT (nelts); - if (tree_int_cst_lt (index, nelts)) + if (TREE_CODE (ary) == CONSTRUCTOR + && CONSTRUCTOR_NO_IMPLICIT_ZERO (ary)) { - if (TREE_CODE (ary) == CONSTRUCTOR - && CONSTRUCTOR_NO_IMPLICIT_ZERO (ary)) - { - /* 'ary' is part of the aggregate initializer we're currently - building; if there's no initializer for this element yet, - that's an error. */ - if (!ctx->quiet) - error ("accessing uninitialized array element"); - *non_constant_p = true; - return t; - } - - /* If it's within the array bounds but doesn't have an explicit - initializer, it's value-initialized. */ - tree val = build_value_init (elem_type, tf_warning_or_error); - return cxx_eval_constant_expression (ctx, val, - lval, - non_constant_p, overflow_p); + /* 'ary' is part of the aggregate initializer we're currently + building; if there's no initializer for this element yet, + that's an error. */ + if (!ctx->quiet) + error ("accessing uninitialized array element"); + *non_constant_p = true; + return t; } - if (!ctx->quiet) - error ("array subscript out of bound"); - *non_constant_p = true; - return t; + /* If it's within the array bounds but doesn't have an explicit + initializer, it's value-initialized. */ + tree val = build_value_init (elem_type, tf_warning_or_error); + return cxx_eval_constant_expression (ctx, val, lval, non_constant_p, + overflow_p); } if (TREE_CODE (ary) == CONSTRUCTOR) diff --git gcc/cp/init.c gcc/cp/init.c index 9f7b614..976ada8 100644 --- gcc/cp/init.c +++ gcc/cp/init.c @@ -4008,7 +4008,7 @@ build_vec_init (tree base, tree maxindex, tree init, && (num_initialized_elts == tree_to_shwi (maxindex) + 1)))) { - /* If the ITERATOR is equal to -1, then we don't have to loop; + /* If the ITERATOR is lesser or equal to -1, then we don't have to loop; we've already initialized all the elements. */ tree for_stmt; tree elt_init; @@ -4016,7 +4016,7 @@ build_vec_init (tree base, tree maxindex, tree init, for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE); finish_for_init_stmt (for_stmt); - finish_for_cond (build2 (NE_EXPR, boolean_type_node, iterator, + finish_for_cond (build2 (GT_EXPR, boolean_type_node, iterator, build_int_cst (TREE_TYPE (iterator), -1)), for_stmt, false); elt_init = cp_build_unary_op (PREDECREMENT_EXPR, iterator, 0, diff --git gcc/testsuite/g++.dg/ext/constexpr-vla2.C gcc/testsuite/g++.dg/ext/constexpr-vla2.C index e69de29..6cb1f70 100644 --- gcc/testsuite/g++.dg/ext/constexpr-vla2.C +++ gcc/testsuite/g++.dg/ext/constexpr-vla2.C @@ -0,0 +1,21 @@ +// PR c++/69509 +// { dg-do compile { target c++14 } } + +constexpr int +fn_bad (int n) +{ + __extension__ int a [n] = { 0 }; + int z = a [0] + (n ? fn_bad (n - 1) : 0); + return z; +} + +constexpr int +fn_ok (int n) +{ + __extension__ int a [n] = { 0 }; + int z = a [0] + (n > 1 ? fn_ok (n - 1) : 0); + return z; +} + +constexpr int i1 = fn_ok (3); +constexpr int i2 = fn_bad (3); // { dg-error "array subscript out of bound" } diff --git gcc/testsuite/g++.dg/ext/constexpr-vla3.C gcc/testsuite/g++.dg/ext/constexpr-vla3.C index e69de29..ba4eb50 100644 --- gcc/testsuite/g++.dg/ext/constexpr-vla3.C +++ gcc/testsuite/g++.dg/ext/constexpr-vla3.C @@ -0,0 +1,14 @@ +// PR c++/69516 +// { dg-do compile { target c++14 } } + +constexpr int +foo (int n) +{ + __extension__ int a[n] = { 1, 2, 3, 4, 5, 6 }; + int z = 0; + for (int i = 0; i <= n; ++i) + z += a[i]; + return z; +} + +constexpr int n = foo (3); // { dg-error "array subscript out of bound" } diff --git gcc/testsuite/g++.dg/ubsan/vla-1.C gcc/testsuite/g++.dg/ubsan/vla-1.C index e7f2494..311cdb1 100644 --- gcc/testsuite/g++.dg/ubsan/vla-1.C +++ gcc/testsuite/g++.dg/ubsan/vla-1.C @@ -1,6 +1,5 @@ // { dg-do run } // { dg-options "-Wno-vla -fsanitize=undefined" } -// { dg-shouldfail "ubsan" } // { dg-output "index 1 out of bounds" } void f(int i) { Marek