From patchwork Thu Nov 2 20:01:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 161111 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:8f47:0:b0:403:3b70:6f57 with SMTP id j7csp607717vqu; Thu, 2 Nov 2023 13:01:47 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGCkNHs2DqXbL6V97kMe1h78LdWAE2fHVujzCs5SZTUVfh2W3Bqqf6+34k4/z4hsujgc6S7 X-Received: by 2002:a05:6870:12d1:b0:1ef:f14e:6f50 with SMTP id 17-20020a05687012d100b001eff14e6f50mr10013054oam.8.1698955307361; Thu, 02 Nov 2023 13:01:47 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1698955307; cv=pass; d=google.com; s=arc-20160816; b=kkKlK7sopbQlxLUyXzIhghsL/Cm7Mf4mAUV2/XmkxNr6SkM6SaPphjo9B/Nfrgnb0y CNdfCEYLvlF080BkVqpLG4DbOGopsPGaCsl1WUmM1oU/GV97/sHNGX0xtfqeK8lTdkgR vzcui3+mvpEsjTa82KtHL02xt4XZO8oQAqV1fuvyJfqUswSm5i/tcyHOpv95819khHDB XTPqI3VVMqCuTlxVlqSTu73YMRN61UD5NawojlUmlfnUo8Nt6U2n1UAyDvD4xar62SLT cPS0zwJHhmT14LbUX5UMYn0b2szBnm5yR1XftVnElCMy1xQwpNEUmCjMWRjomChGsTus Y0WA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :arc-filter:dmarc-filter:delivered-to; bh=rpcikKOxTdCbKb80x08tZ/125jLIjAKMzcOVdHglrys=; fh=hPrbWPhweUx4V0GV9uXJqbyAzg2ABmTz7kczrAQqMmM=; b=mTfH7ORKzmeB9aEwkA3jRlAlr9Xe7qVP6PekH1NHaCrWz41bdMa1HoqpfLL98aAuou VdFp6WOjzJ2FPSIx9ipG1RCgnK1ZG1qPA2XN5eBkKpdanIdhgOnPcH8JIoRkI668OnEd hdx78+OSV4EeDtn8IjNuDbm1FahrYcpDiploxueCAZz1N/dMCl3/3piTNEhNxmoNc67E GbNX6MmiJSf3RFZSmroOhQQi3YVOuZlCYKwBsRT65zow9Bimy/zSAXEhXnBi66hIrzP4 kd/SQma9FBG7Z74NRErRnJoGtIT7BFunAos+Fv94ZuSlrMb1xbop9i1t0+ZvrFtPYvhe 0o+g== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iWgT9uDU; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id p7-20020ab06487000000b0079d9de084bdsi55221uam.81.2023.11.02.13.01.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Nov 2023 13:01:47 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=iWgT9uDU; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1506E3858C2F for ; Thu, 2 Nov 2023 20:01:47 +0000 (GMT) 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 9BCEE3858D32 for ; Thu, 2 Nov 2023 20:01:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9BCEE3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9BCEE3858D32 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698955283; cv=none; b=lJKdliVbnvPEk7SDBq/ZNsj10A9qS/CUlD3fTtYD7Ojpcno3+S6radUjzVF6SkeT9i88s+VjZupglascR3TOO0J7E6p4DnkuDqqAVgQyRQUpw0RtgJ8cQTtfTSjsTl8SmxwWaaFDakSQXszCiYXUPgB9nled+DYK3HqkgT13gA8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698955283; c=relaxed/simple; bh=ZEHxpfV2Tju6NtpEgCMzxdvnDGTd4SOJSurqAApLVII=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=jvQp2E6VwMpsJ9ZGFIlpdJveI5Tg7ndV5rYV0Qx+cvTFKxYUfXplXSKdB5EepBLi0DZkrWda6LbYNuJcsynW1Z2fm/jGpWAE06S9fGywQPMfdb1xCBsLpMPxX46A9qGeLRh+hNpYWQK4y8A7M715avMHkb9A9Ivx0TKgXnPbJWU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1698955273; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=rpcikKOxTdCbKb80x08tZ/125jLIjAKMzcOVdHglrys=; b=iWgT9uDUF0T1ZMzkLYF29e8xnRx5QYOayarHcNXsgw0jbcbrrscwM1SS/8VdElQb4Trc+c LPAgvTn7VX4ZynYeLysXdBptXlEdlcBf2QUgo2kA1Cb2eDymNVd3g1cR85spx3FSXzmzo5 Zu6+NRTI/9QR5NSs085o+kSQA7jghVE= Received: from mail-oo1-f70.google.com (mail-oo1-f70.google.com [209.85.161.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-586-nQXJSBSKPX6WLrH3NSpYnQ-1; Thu, 02 Nov 2023 16:01:11 -0400 X-MC-Unique: nQXJSBSKPX6WLrH3NSpYnQ-1 Received: by mail-oo1-f70.google.com with SMTP id 006d021491bc7-581e1d59302so1544743eaf.0 for ; Thu, 02 Nov 2023 13:01:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698955269; x=1699560069; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rpcikKOxTdCbKb80x08tZ/125jLIjAKMzcOVdHglrys=; b=BNlxnmYgE5o7JPFTPf8o+A7KpmKpC+a4ZBOVZ47d1PBJjdmZuosByHE1IbP3U+ft65 np5A2p8IguDbkNh0Y99mRIgu2q2ClCFRnLuswmCTvJUB6tb39immTtZyOWf6RoNMbiQQ HaYizYLO/4Tfr8IWlzvFvL/BiZr+eEoOJNVQZ+xE1auKUSH2U5S2Td8auDS0xVBReEi8 fQ5iQ3LgNYF4nNZOYpqshTyFiYj3+bwMpowTx76ngC+CMBgr1nmRTJeDzA/b9RW587eP BscPavjSuwYm7d83GfugbBz7+K0VsJDUgULCVyRslxW38QFKvc6MZOv6wKb8it8ai/Zx 5Osg== X-Gm-Message-State: AOJu0Yw5tUXrMzZQKITjtXr5ThIw+qW62tgcZjyBDwPcf8682VIrxgNN vWMpAk7eE0GA2MqAZp37Z2CuWdm/1ez/zaAT+JGsupHKoduJGha7um5rCX1HaFGQbRIulEVk4CM 9F3lYf0v+zBYoYHEtJ622cwwfjaPp7rcBUAsoZXhGcnHpyjkQn3vMdrdeVyNNEgwdZL7Kzg2nSw == X-Received: by 2002:a05:6359:6c12:b0:168:e6cf:de8e with SMTP id tc18-20020a0563596c1200b00168e6cfde8emr10157544rwb.14.1698955269011; Thu, 02 Nov 2023 13:01:09 -0700 (PDT) X-Received: by 2002:a05:6359:6c12:b0:168:e6cf:de8e with SMTP id tc18-20020a0563596c1200b00168e6cfde8emr10157520rwb.14.1698955268434; Thu, 02 Nov 2023 13:01:08 -0700 (PDT) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id t2-20020a0cef02000000b0064f53943626sm46458qvr.89.2023.11.02.13.01.07 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Nov 2023 13:01:07 -0700 (PDT) From: Jason Merrill To: gcc-patches@gcc.gnu.org Subject: [pushed] c++: retval dtor on rethrow [PR112301] Date: Thu, 2 Nov 2023 16:01:04 -0400 Message-Id: <20231102200104.723881-1-jason@redhat.com> X-Mailer: git-send-email 2.39.3 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781483760239678593 X-GMAIL-MSGID: 1781483760239678593 Tested x86_64-pc-linux-gnu, applying to trunk. -- 8< -- In r12-6333 for PR33799, I fixed the example in [except.ctor]/2. In that testcase, the exception is caught and the function returns again, successfully. In this testcase, however, the exception is rethrown, and hits two separate cleanups: one in the try block and the other in the function body. So we destroy twice an object that was only constructed once. Fortunately, the fix for the normal case is easy: we just need to clear the "return value constructed by return" flag when we do it the first time. This gets more complicated with the named return value optimization, since we don't want to destroy the return value while the NRV variable is still in scope. PR c++/112301 PR c++/102191 PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Clear current_retval_sentinel when destroying retval. * semantics.cc (nrv_data): Add in_nrv_cleanup. (finalize_nrv): Set it. (finalize_nrv_r): Fix handling of throwing cleanups. gcc/testsuite/ChangeLog: * g++.dg/eh/return1.C: Add more cases. --- gcc/cp/except.cc | 18 ++++++- gcc/cp/semantics.cc | 47 +++++++++++++++++- gcc/testsuite/g++.dg/eh/return1.C | 81 ++++++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 4 deletions(-) base-commit: 36a26298ec7dfca615d4ba411a3508d1287d6ce5 diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc index e32efb30457..d966725db9b 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -1284,7 +1284,15 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) current_retval_sentinel so that we know that the return value needs to be destroyed on throw. Do the same if the current function might use the named return value optimization, so we don't destroy it on return. - Otherwise, returns NULL_TREE. */ + Otherwise, returns NULL_TREE. + + The sentinel is set to indicate that we're in the process of returning, and + therefore should destroy a normal return value on throw, and shouldn't + destroy a named return value variable on normal scope exit. It is set on + return, and cleared either by maybe_splice_retval_cleanup, or when an + exception reaches the NRV scope (finalize_nrv_r). Note that once return + passes the NRV scope, it's effectively a normal return value, so cleanup + past that point is handled by maybe_splice_retval_cleanup. */ tree maybe_set_retval_sentinel () @@ -1361,6 +1369,14 @@ maybe_splice_retval_cleanup (tree compound_stmt, bool is_try) tsi_delink (&iter); } tree dtor = build_cleanup (retval); + if (!function_body) + { + /* Clear the sentinel so we don't try to destroy the retval again on + rethrow (c++/112301). */ + tree clear = build2 (MODIFY_EXPR, boolean_type_node, + current_retval_sentinel, boolean_false_node); + dtor = build2 (COMPOUND_EXPR, void_type_node, clear, dtor); + } tree cond = build3 (COND_EXPR, void_type_node, current_retval_sentinel, dtor, void_node); tree cleanup = build_stmt (loc, CLEANUP_STMT, diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 52044be7af8..a0f2edcf117 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -4982,6 +4982,7 @@ public: tree result; hash_table > visited; bool simple; + bool in_nrv_cleanup; }; /* Helper function for walk_tree, used by finalize_nrv below. */ @@ -4997,7 +4998,7 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) if (TYPE_P (*tp)) *walk_subtrees = 0; /* If there's a label, we might need to destroy the NRV on goto (92407). */ - else if (TREE_CODE (*tp) == LABEL_EXPR) + else if (TREE_CODE (*tp) == LABEL_EXPR && !dp->in_nrv_cleanup) dp->simple = false; /* Change NRV returns to just refer to the RESULT_DECL; this is a nop, but differs from using NULL_TREE in that it indicates that we care @@ -5016,16 +5017,59 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) else if (TREE_CODE (*tp) == CLEANUP_STMT && CLEANUP_DECL (*tp) == dp->var) { + dp->in_nrv_cleanup = true; + cp_walk_tree (&CLEANUP_BODY (*tp), finalize_nrv_r, data, 0); + dp->in_nrv_cleanup = false; + cp_walk_tree (&CLEANUP_EXPR (*tp), finalize_nrv_r, data, 0); + *walk_subtrees = 0; + if (dp->simple) + /* For a simple NRV, just run it on the EH path. */ CLEANUP_EH_ONLY (*tp) = true; else { + /* Not simple, we need to check current_retval_sentinel to decide + whether to run it. If it's set, we're returning normally and + don't want to destroy the NRV. If the sentinel is not set, we're + leaving scope some other way, either by flowing off the end of its + scope or throwing an exception. */ tree cond = build3 (COND_EXPR, void_type_node, current_retval_sentinel, void_node, CLEANUP_EXPR (*tp)); CLEANUP_EXPR (*tp) = cond; } + + /* If a cleanup might throw, we need to clear current_retval_sentinel on + the exception path, both so the check above succeeds and so an outer + cleanup added by maybe_splice_retval_cleanup doesn't run. */ + if (cp_function_chain->throwing_cleanup) + { + tree clear = build2 (MODIFY_EXPR, boolean_type_node, + current_retval_sentinel, + boolean_false_node); + if (dp->simple) + { + /* We're already only on the EH path, just prepend it. */ + tree &exp = CLEANUP_EXPR (*tp); + exp = build2 (COMPOUND_EXPR, void_type_node, clear, exp); + } + else + { + /* The cleanup runs on both normal and EH paths, we need another + CLEANUP_STMT to clear the flag only on the EH path. */ + tree &bod = CLEANUP_BODY (*tp); + bod = build_stmt (EXPR_LOCATION (*tp), CLEANUP_STMT, + bod, clear, current_retval_sentinel); + CLEANUP_EH_ONLY (bod) = true; + } + } } + /* Disable maybe_splice_retval_cleanup within the NRV cleanup scope, we don't + want to destroy the retval before the variable goes out of scope. */ + else if (TREE_CODE (*tp) == CLEANUP_STMT + && dp->in_nrv_cleanup + && CLEANUP_DECL (*tp) == dp->result) + CLEANUP_EXPR (*tp) = void_node; /* Replace the DECL_EXPR for the NRV with an initialization of the RESULT_DECL, if needed. */ else if (TREE_CODE (*tp) == DECL_EXPR @@ -5082,6 +5126,7 @@ finalize_nrv (tree fndecl, tree var) data.var = var; data.result = result; + data.in_nrv_cleanup = false; /* This is simpler for variables declared in the outer scope of the function so we know that their lifetime always ends with a diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C index e22d674ae9a..c3d4ebe596b 100644 --- a/gcc/testsuite/g++.dg/eh/return1.C +++ b/gcc/testsuite/g++.dg/eh/return1.C @@ -16,13 +16,14 @@ extern "C" int printf (const char *, ...); struct X { - X(bool throws) : throws_(throws) { ++c; DEBUG; } + X(bool throws) : i(-42), throws_(throws) { ++c; DEBUG; } X(const X& x); // not defined ~X() THROWS { - ++d; DEBUG; + i = ++d; DEBUG; if (throws_) { throw 1; } } + int i; private: bool throws_; }; @@ -71,6 +72,75 @@ X i2() return X(false); } +// c++/112301 +X i3() +{ + try { + X x(true); + return X(false); + } catch(...) { throw; } +} + +X i4() +{ + try { + X x(true); + X x2(false); + return x2; + } catch(...) { throw; } +} + +X i4a() +{ + try { + X x2(false); + X x(true); + return x2; + } catch(...) { throw; } +} + +X i4b() +{ + X x(true); + X x2(false); + return x2; +} + +X i4c() +{ + X x2(false); + X x(true); + return x2; +} + +X i5() +{ + X x2(false); + + try { + X x(true); + return x2; + } catch(...) { + if (x2.i != -42) + d += 42; + throw; + } +} + +X i6() +{ + X x2(false); + + try { + X x(true); + return x2; + } catch(...) { + if (x2.i != -42) + d += 42; + } + return x2; +} + X j() { try { @@ -113,6 +183,13 @@ int main() catch (...) {} try { i2(); } catch (...) {} + try { i3(); } catch (...) {} + try { i4(); } catch (...) {} + try { i4a(); } catch (...) {} + try { i4b(); } catch (...) {} + try { i4c(); } catch (...) {} + try { i5(); } catch (...) {} + try { i6(); } catch (...) {} try { j(); } catch (...) {}