From patchwork Fri Jan 7 00:21:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 49676 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 06270385803F for ; Fri, 7 Jan 2022 00:24:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 06270385803F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1641515068; bh=/bSBrf4ZyuerdAGCNihZJFEqtemMBM/EEhp9S5Z5Igw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=cLQpac/R1j/NjNVqwYjP5sze+jaDngytz84OqLDvBGXGQdLWYol8Cqx53v9X4/sy1 rXk+DViFVPSj+LBbd89s8Zfo7eftOtU/hgT2yz+K1RErWAbZGcvDh6f9BPxwohhDCU ZmWaSBobuvMHdgNNphNrT51LbzxcCDp1mbLX0I4E= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id E7C883858416 for ; Fri, 7 Jan 2022 00:22:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E7C883858416 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-108-SaMYjkEvP6Kd_xSXOeJyRg-1; Thu, 06 Jan 2022 19:22:02 -0500 X-MC-Unique: SaMYjkEvP6Kd_xSXOeJyRg-1 Received: by mail-qk1-f197.google.com with SMTP id v13-20020a05620a440d00b00468380f4407so3034314qkp.17 for ; Thu, 06 Jan 2022 16:22:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=/bSBrf4ZyuerdAGCNihZJFEqtemMBM/EEhp9S5Z5Igw=; b=KPIum7IGSRjQRucxOKPXd//O49dfYn2FagV6WLDytKM8/z92iF6RbYV6OEPZ8MaAds WqqjoR+UrV2x34sKKEpfxqjNHCp0iIaWbwwqNJY+9ky0RuNrYkqN6LX63nXofnb8V2K+ o51CEpUDDbzimMvO8Dz9SEHpr2VsLkirQOoQmTh6Yyk+25JuNNM3V7Abar2gDcWGgmeh KWMjv91xQ/aRp7ui1QgXDAygB+b6hPh7v5WKLwQU/VnvD3oHjCj9Z9pK2MhRoJ4CPW8c 4NgR14kJsCL5T4quC8z1C+ulzJPE2sgO6ZRE17Ca0Mh3Qoqg3z6zvsmACz9kp+mF+XkZ wgYA== X-Gm-Message-State: AOAM533hjf3KwH7R2N4tVMldQaNi4DEt/oCO99zLCTn5Tn+g9+7RJjPK LNb3rvwWDlETDYIRDJ41NtGiOezA7TigxsDm7lDsnjARFlk/aknVOcuLawI6h+UZO+gqkIi/xqp ZMWgCgp1gQrl1WzHEYjo904h2M8LxGPmSfVG0a38Cof+L7TSO25PhqqSxMiZ9UA0hgg== X-Received: by 2002:a05:6214:29e7:: with SMTP id jv7mr56981622qvb.122.1641514921328; Thu, 06 Jan 2022 16:22:01 -0800 (PST) X-Google-Smtp-Source: ABdhPJx1VMRalHQ4HOIByV9oJSBzqMqD/YSYzZd678trvm6Rl5Cc/3BUbeWp3nfugNE1sY2xBa1g7w== X-Received: by 2002:a05:6214:29e7:: with SMTP id jv7mr56981607qvb.122.1641514920853; Thu, 06 Jan 2022 16:22:00 -0800 (PST) Received: from barrymore.redhat.com (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id s3sm2323785qkp.93.2022.01.06.16.22.00 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jan 2022 16:22:00 -0800 (PST) To: gcc-patches@gcc.gnu.org Subject: [pushed 03/11] c++: temporary lifetime with aggregate init [PR94041] Date: Thu, 6 Jan 2022 19:21:48 -0500 Message-Id: <20220107002156.2992278-3-jason@redhat.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20220107002156.2992278-1-jason@redhat.com> References: <20220107002156.2992278-1-jason@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-27.0 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" In C++98 the lifetime of temporaries in aggregate initialization was unclear, but C++11 DR201 clarified that only temporaries created for default-initialization of an array element with no corresponding initializer-clause are destroyed immediately; all others persist until the end of the full-expression. But we never implemented that, and continued treating individual element initializations as full-expressions, such as in my patch for PR50866 in r180442. This blocked my attempted fix for PR66139, which extended the use of split_nonconstant_init, and thus the bug, to aggregate initialization of temporaries within an expression. The longer temporary lifetime creates further EH region overlap problems like the ones that wrap_temporary_cleanups addresses: in aggr7.C, we start out with a normal nesting of A1 c.b1 A2 c.b2 ... ~A2 ~A1 where on the way in, throwing from one of the inits will clean up from the previous inits. But once c.b2 is initialized, throwing from ~A2 must not clean up c.b1; instead it needs to clean up c. So as in build_new_1, we deal with this by guarding the B cleanups with flags that are cleared once c is fully constructed; throwing from one of the ~A still hits that region, but does not call ~B. And then wrap_temporary_cleanups deals with calling ~C, but we need to tell it not to wrap the subobject cleanups. The change from expressing the subobject cleanups with CLEANUP_STMT to TARGET_EXPR was also necessary because we need them to collate with the ~A in gimplify_cleanup_point_expr; the CLEANUP_STMT representation only worked with treating subobject initializations as full-expressions. PR c++/94041 gcc/cp/ChangeLog: * decl.c (check_initializer): Remove obsolete comment. (wrap_cleanups_r): Don't wrap CLEANUP_EH_ONLY. (initialize_local_var): Change assert to test. * typeck2.c (maybe_push_temp_cleanup): New. (split_nonconstant_init_1): Use it. (split_nonconstant_init): Clear cleanup flags. gcc/testsuite/ChangeLog: * g++.dg/init/aggr7-eh.C: New test. * g++.dg/cpp0x/initlist122.C: Also test aggregate variable. --- gcc/cp/decl.c | 12 ++--- gcc/cp/typeck2.c | 55 +++++++++++++++++---- gcc/testsuite/g++.dg/cpp0x/initlist122.C | 12 ++++- gcc/testsuite/g++.dg/init/aggr7-eh.C | 62 ++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/init/aggr7-eh.C diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 0b71c00f5ab..9f759ceb1c8 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7257,11 +7257,6 @@ check_initializer (tree decl, tree init, int flags, vec **cleanups) if (init && TREE_CODE (init) != TREE_VEC) { - /* In aggregate initialization of a variable, each element - initialization is a full-expression because there is no - enclosing expression. */ - gcc_assert (stmts_are_full_exprs_p ()); - init_code = store_init_value (decl, init, cleanups, flags); if (DECL_INITIAL (decl) @@ -7428,7 +7423,8 @@ wrap_cleanups_r (tree *stmt_p, int *walk_subtrees, void *data) tree guard = (tree)data; tree tcleanup = TARGET_EXPR_CLEANUP (*stmt_p); - if (tcleanup && !expr_noexcept_p (tcleanup, tf_none)) + if (tcleanup && !CLEANUP_EH_ONLY (*stmt_p) + && !expr_noexcept_p (tcleanup, tf_none)) { tcleanup = build2 (TRY_CATCH_EXPR, void_type_node, tcleanup, guard); /* Tell honor_protect_cleanup_actions to handle this as a separate @@ -7500,11 +7496,11 @@ initialize_local_var (tree decl, tree init) { tree rinit = (TREE_CODE (init) == INIT_EXPR ? TREE_OPERAND (init, 1) : NULL_TREE); - if (rinit && !TREE_SIDE_EFFECTS (rinit)) + if (rinit && !TREE_SIDE_EFFECTS (rinit) + && TREE_OPERAND (init, 0) == decl) { /* Stick simple initializers in DECL_INITIAL so that -Wno-init-self works (c++/34772). */ - gcc_assert (TREE_OPERAND (init, 0) == decl); DECL_INITIAL (decl) = rinit; if (warn_init_self && TYPE_REF_P (type)) diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index 3d4d35e13c6..54b1d0da129 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -459,13 +459,34 @@ cxx_incomplete_type_error (location_t loc, const_tree value, const_tree type) } +/* We've just initialized subobject SUB; also insert a TARGET_EXPR with an + EH-only cleanup for SUB. Because of EH region nesting issues, we need to + make the cleanup conditional on a flag that we will clear once the object is + fully initialized, so push a new flag onto FLAGS. */ + +static void +maybe_push_temp_cleanup (tree sub, vec **flags) +{ + if (tree cleanup + = cxx_maybe_build_cleanup (sub, tf_warning_or_error)) + { + tree tx = get_target_expr (boolean_true_node); + tree flag = TARGET_EXPR_SLOT (tx); + CLEANUP_EH_ONLY (tx) = true; + TARGET_EXPR_CLEANUP (tx) = build3 (COND_EXPR, void_type_node, + flag, cleanup, void_node); + add_stmt (tx); + vec_safe_push (*flags, flag); + } +} + /* The recursive part of split_nonconstant_init. DEST is an lvalue expression to which INIT should be assigned. INIT is a CONSTRUCTOR. Return true if the whole of the value was initialized by the generated statements. */ static bool -split_nonconstant_init_1 (tree dest, tree init, bool nested) +split_nonconstant_init_1 (tree dest, tree init, bool nested, vec **flags) { unsigned HOST_WIDE_INT idx, tidx = HOST_WIDE_INT_M1U; tree field_index, value; @@ -501,9 +522,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested) if (nested) /* Also clean up the whole array if something later in an enclosing init-list throws. */ - if (tree cleanup = cxx_maybe_build_cleanup (dest, - tf_warning_or_error)) - finish_eh_cleanup (cleanup); + maybe_push_temp_cleanup (dest, flags); return true; } /* FALLTHRU */ @@ -533,7 +552,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested) sub = build3 (COMPONENT_REF, inner_type, dest, field_index, NULL_TREE); - if (!split_nonconstant_init_1 (sub, value, true) + if (!split_nonconstant_init_1 (sub, value, true, flags) /* For flexible array member with initializer we can't remove the initializer, because only the initializer determines how many elements the @@ -616,11 +635,8 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested) code = build2 (INIT_EXPR, inner_type, sub, value); } code = build_stmt (input_location, EXPR_STMT, code); - code = maybe_cleanup_point_expr_void (code); add_stmt (code); - if (tree cleanup - = cxx_maybe_build_cleanup (sub, tf_warning_or_error)) - finish_eh_cleanup (cleanup); + maybe_push_temp_cleanup (sub, flags); } num_split_elts++; @@ -687,10 +703,29 @@ split_nonconstant_init (tree dest, tree init) init = TARGET_EXPR_INITIAL (init); if (TREE_CODE (init) == CONSTRUCTOR) { + /* Subobject initializers are not full-expressions. */ + auto fe = (make_temp_override + (current_stmt_tree ()->stmts_are_full_exprs_p, 0)); + init = cp_fully_fold_init (init); code = push_stmt_list (); - if (split_nonconstant_init_1 (dest, init, false)) + + /* Collect flags for disabling subobject cleanups once the complete + object is fully constructed. */ + vec *flags = make_tree_vector (); + + if (split_nonconstant_init_1 (dest, init, false, &flags)) init = NULL_TREE; + + for (tree f : flags) + { + /* See maybe_push_temp_cleanup. */ + tree d = f; + tree i = boolean_false_node; + add_stmt (build2 (MODIFY_EXPR, TREE_TYPE (d), d, i)); + } + release_tree_vector (flags); + code = pop_stmt_list (code); if (VAR_P (dest) && !is_local_temp (dest)) { diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist122.C b/gcc/testsuite/g++.dg/cpp0x/initlist122.C index 002bc1eaf3c..81953a44e12 100644 --- a/gcc/testsuite/g++.dg/cpp0x/initlist122.C +++ b/gcc/testsuite/g++.dg/cpp0x/initlist122.C @@ -6,11 +6,19 @@ struct Temp { ~Temp() { gone = true; } }; struct A{ A() {}; A(const Temp&) noexcept {}; }; struct B{ ~B() {}; }; struct Pair{ A a; B b; }; - void foo(const Pair&) noexcept { if (gone) __builtin_abort(); } +B bar() { if (gone) __builtin_abort(); return {}; } + int main() { - foo({A(Temp{}), B()}); + Pair p{A(Temp{}), bar()}; + + if (!gone) __builtin_abort (); + + gone = false; + + foo({A(Temp{})}); + if (!gone) __builtin_abort (); } diff --git a/gcc/testsuite/g++.dg/init/aggr7-eh.C b/gcc/testsuite/g++.dg/init/aggr7-eh.C new file mode 100644 index 00000000000..db45e15d9af --- /dev/null +++ b/gcc/testsuite/g++.dg/init/aggr7-eh.C @@ -0,0 +1,62 @@ +// PR c++/50866, adjusted +// { dg-do run } + +#if __cplusplus > 201100L +#define THROWING noexcept(false) +#else +#define THROWING +#endif + +extern "C" void abort (); + +int a; +int d = -1; +struct A { + A() { ++a; } + A(const A&); + ~A() THROWING { + --a; + if (a == d) + throw (short)a; + } +}; +int b; +int t; +struct B { + B(const char *, const A& = A()) + { + if (b == t) + throw b; + ++b; + if (a != b) abort (); + } + B(const B&); + ~B() + { + --b; + } +}; +struct C { + B b1, b2, b3; +}; +void f() +{ + try + { + C c = { "a","b","c" }; + if (a != 0) abort (); + if (b != 3) abort (); + } + catch (int i) { } + catch (short s) { } + if (a != 0) abort (); + if (b != 0) abort (); +} + +int main() +{ + for (t = 0; t <= 3; ++t) + f(); + for (d = 0; d <= 2; ++d) + f(); +}