From patchwork Sat Jan 11 05:12:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1221533 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-517209-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=UF3zpg/K; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=d1lSSple; dkim-atps=neutral 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 47vp093hCDz9sPJ for ; Sat, 11 Jan 2020 16:13:11 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:content-type :content-transfer-encoding; q=dns; s=default; b=CMDJwq8gCTCUe4z0 7Zv0OL3C75T2OHDybonmamxtzksn4Ye/24mgPI70FVC9sApLNPWMqySDP8fB+a7N svEfRB9ZKeff0r/lV+gEeuo5Tg4vkoAA0MvhOMhqYuroXEktKvpCrHS1/al/Q+5C pwE97HFHbLPcguBu0fISRsotBjw= 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:from :to:subject:date:message-id:content-type :content-transfer-encoding; s=default; bh=hCBQXU7gKGFs/Me1DahJsY uDd74=; b=UF3zpg/K74j5sQWcak4NDzaoV4LA4BRtRhumdngkHH+EoN+HTWnXVw qISAx0yiggO8iGUiWCxU0oR1HqYi5SDm6TPnf59dbMIn7jwTjYC6GLaZH/KOhcx8 jxv1uEB0KQvnN7DXRlNngTaJ5WBwvvGVc4aNpHYiyiI4YP4m2dyYI= Received: (qmail 8443 invoked by alias); 11 Jan 2020 05:13:04 -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 8435 invoked by uid 89); 11 Jan 2020 05:13:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=sk:check_r, 15996, MODIFY_EXPR, modify_expr X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 11 Jan 2020 05:13:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1578719581; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=gEt1xJxTeX+UKoNuwlWZ/SFNHFZTf+qmL7QU7VKCeY0=; b=d1lSSpleW8dWpRe9RtqRskouyTNXZgP2ECeZovFjFypSdJWBmmQowDf3X7wC2S2Ztm0BVb HxQbI5j1aP2BE5cwl1HOubYAQT57QeOVF79s0d/ymJ/pVxduQeqA6gu8/Q0q5R++xTMUhp pN97kiYaTDtf1m039ozyX5vGFAUdcPE= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-226-sVGd_Ii0Owu7pqww5DlL0g-1; Sat, 11 Jan 2020 00:12:58 -0500 Received: by mail-qt1-f197.google.com with SMTP id o18so2761489qtt.19 for ; Fri, 10 Jan 2020 21:12:58 -0800 (PST) Received: from barrymore.redhat.com (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id r12sm1898160qkm.94.2020.01.10.21.12.56 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jan 2020 21:12:56 -0800 (PST) From: Jason Merrill To: gcc-patches@gcc.gnu.org Subject: [RFA (gimplify) PATCH] PR c++/33799 - destroy return value if local cleanup throws. Date: Sat, 11 Jan 2020 00:12:55 -0500 Message-Id: <20200111051255.30271-1-jason@redhat.com> X-Mimecast-Spam-Score: 0 X-IsSubscribed: yes This is a pretty rare situation since the C++11 change to make all destructors default to noexcept, but it is still possible to define throwing destructors, and if a destructor for a local variable throws during the return, we've already constructed the return value, so now we need to destroy it. I handled this somewhat like the new-expression cleanup; as in that case, this cleanup can't properly nest with the cleanups for local variables, so I introduce a cleanup region around the whole function and a flag variable to indicate whether the return value actually needs to be destroyed. Setting the flag requires giving a COMPOUND_EXPR as the operand of a RETURN_EXPR. Simply allowing that in gimplify_return_expr makes the most sense to me, is it OK for trunk? This doesn't currently work with deduced return type because we don't know the type when we're deciding whether to introduce the cleanup region. Tested x86_64-pc-linux-gnu. gcc/ * gimplify.c (gimplify_return_expr): Handle COMPOUND_EXPR. gcc/cp/ * cp-tree.h (current_retval_sentinel): New macro. * decl.c (start_preparsed_function): Set up cleanup for retval. * typeck.c (check_return_expr): Set current_retval_sentinel. --- gcc/cp/cp-tree.h | 7 ++++++ gcc/cp/decl.c | 14 +++++++++++ gcc/cp/typeck.c | 9 +++++++ gcc/gimplify.c | 8 +++++++ gcc/testsuite/g++.dg/eh/return1.C | 40 +++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+) create mode 100644 gcc/testsuite/g++.dg/eh/return1.C base-commit: d3cf980217ce13b1eda01d4c42a7e3afd2bf3217 diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 98572bdbad1..c0f780df685 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1952,6 +1952,13 @@ struct GTY(()) language_function { #define current_vtt_parm cp_function_chain->x_vtt_parm +/* A boolean flag to control whether we need to clean up the return value if a + local destructor throws. Only used in functions that return by value a + class with a destructor. Which 'tors don't, so we can use the same + field as current_vtt_parm. */ + +#define current_retval_sentinel current_vtt_parm + /* Set to 0 at beginning of a function definition, set to 1 if a return statement that specifies a return value is seen. */ diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 094e32edf58..52da0deef40 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -16418,6 +16418,20 @@ start_preparsed_function (tree decl1, tree attrs, int flags) if (!DECL_OMP_DECLARE_REDUCTION_P (decl1)) start_lambda_scope (decl1); + /* If cleaning up locals on return throws an exception, we need to destroy + the return value that we just constructed. */ + if (!processing_template_decl + && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (TREE_TYPE (decl1)))) + { + tree retval = DECL_RESULT (decl1); + tree dtor = build_cleanup (retval); + current_retval_sentinel = get_temp_regvar (boolean_type_node, + boolean_false_node); + dtor = build3 (COND_EXPR, void_type_node, current_retval_sentinel, + dtor, void_node); + push_cleanup (retval, dtor, /*eh-only*/true); + } + return true; } diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 7b653cebca0..8288b662ec0 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -10088,6 +10088,15 @@ check_return_expr (tree retval, bool *no_warning) if (retval && retval != result) retval = build2 (INIT_EXPR, TREE_TYPE (result), result, retval); + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (valtype) + /* FIXME doesn't work with deduced return type. */ + && current_retval_sentinel) + { + tree set = build2 (MODIFY_EXPR, boolean_type_node, + current_retval_sentinel, boolean_true_node); + retval = build2 (COMPOUND_EXPR, void_type_node, retval, set); + } + return retval; } diff --git a/gcc/gimplify.c b/gcc/gimplify.c index fe7236de4c3..05d7922116b 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1599,6 +1599,14 @@ gimplify_return_expr (tree stmt, gimple_seq *pre_p) if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))) result_decl = NULL_TREE; + else if (TREE_CODE (ret_expr) == COMPOUND_EXPR) + { + /* Used in C++ for handling EH cleanup of the return value if a local + cleanup throws. Assume the front-end knows what it's doing. */ + result_decl = DECL_RESULT (current_function_decl); + /* But crash if we end up trying to modify ret_expr below. */ + ret_expr = NULL_TREE; + } else { result_decl = TREE_OPERAND (ret_expr, 0); diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C new file mode 100644 index 00000000000..7469d3128dc --- /dev/null +++ b/gcc/testsuite/g++.dg/eh/return1.C @@ -0,0 +1,40 @@ +// PR c++/33799 +// { dg-do run } + +extern "C" void abort(); + +int c, d; + +#if __cplusplus >= 201103L +#define THROWS noexcept(false) +#else +#define THROWS +#endif + +struct X +{ + X(bool throws) : throws_(throws) { ++c; } + X(const X& x) : throws_(x.throws_) { ++c; } + ~X() THROWS + { + ++d; + if (throws_) { throw 1; } + } +private: + bool throws_; +}; + +X f() +{ + X x(true); + return X(false); +} + +int main() +{ + try { f(); } + catch (...) {} + + if (c != d) + throw; +}