From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id D9EFF385BF81 for ; Thu, 9 Apr 2020 07:00:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D9EFF385BF81 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C9431AC64; Thu, 9 Apr 2020 06:59:59 +0000 (UTC) Subject: Re: [PATCH] Allow new/delete operator deletion only for replaceable. To: Richard Biener Cc: Jason Merrill , Jakub Jelinek , Jonathan Wakely , Jan Hubicka , GCC Patches , Marc Glisse , Nathan Sidwell References: <20200403152609.GA35629@kam.mff.cuni.cz> <0dbc191e-66f7-9878-956d-96149f20f5bf@suse.cz> <20200408133252.GG2212@tucnak> <20d175a6-23df-43e5-7027-d11fc660abd1@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <150a514d-f460-78de-fd53-43c3eb2f6d4c@suse.cz> Date: Thu, 9 Apr 2020 08:59:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------746EA15ED5303E6927D8BAE6" Content-Language: en-US X-Spam-Status: No, score=-28.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-List-Received-Date: Thu, 09 Apr 2020 07:00:04 -0000 This is a multi-part message in MIME format. --------------746EA15ED5303E6927D8BAE6 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 4/9/20 8:45 AM, Richard Biener wrote: > On Thu, Apr 9, 2020 at 7:06 AM Martin Liška wrote: >> >> Hi. >> >> We've got one another sneaky test-case (thank you Marc ;) ): >> >> $ cat pr94314-array.C >> #include >> #include >> >> int count = 0; >> >> __attribute__((malloc, noinline)) void* operator new[](unsigned long sz) { >> ++count; >> return ::operator new(sz); >> } >> >> void operator delete[](void* ptr) noexcept { >> --count; >> ::operator delete(ptr); >> } >> >> void operator delete[](void* ptr, std::size_t sz) noexcept { >> --count; >> ::operator delete(ptr, sz); >> } >> >> int main() { >> delete[] new int[1]; >> if (count != 0) >> __builtin_abort (); >> } >> >> I bet we need to include the Honza's fix for inline stacks. >> Or it the test-case invalid? > > I don't see how inline stacking helps here when you consider > > void *foo(unsigned long sz) { return ::operator new(sz); } > void operator delete[](void* ptr) noexcept { > --count; > ::operator delete(ptr); > } > > thus regular functions inlining where definitely the inline > stack depth does not need to match. I was considering quite strict rules: - inline stack can contain only up to 1 replaceable operator new (or delete) - no non-replaceable operators are allowed - number of repl. operator much match. > > I guess the testcase asks for us to match the exact > operator form (in the testcase we match ::delete and ::new[]), > for example by instead of looking at the decl flags > simply match the assembler names (the mangled names) > of the operator? What do you mean by 'decl flags'. We can't compare ASM names as one is ctor and the second one is dtor. It's about argument types that much match, right? Thanks, Martin > > Richard. > >> Thanks, >> Martin --------------746EA15ED5303E6927D8BAE6 Content-Type: text/x-patch; charset=UTF-8; name="0001-Compare-inline-stacks-for-new-delete-repl.-operators.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Compare-inline-stacks-for-new-delete-repl.-operators.pa"; filename*1="tch" >From 16c77b6b65e31be657f91dcf0308f1638a784d7a Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 9 Apr 2020 07:36:56 +0200 Subject: [PATCH] Compare inline stacks for new/delete repl. operators. gcc/ChangeLog: 2020-04-09 Martin Liska PR c++/94314 * tree-ssa-dce.c (compare_new_delete_inline_contexts): (propagate_necessity): * cgraphclones.c (set_new_clone_decl_and_node_flags): Drop DECL_IS_REPLACEABLE_OPERATOR to false. gcc/testsuite/ChangeLog: 2020-04-09 Martin Liska PR c++/94314 * g++.dg/pr94314-4.C: New test. * g++.dg/pr94314.C: Do not expect a dtor deletion. --- gcc/cgraphclones.c | 2 + gcc/testsuite/g++.dg/pr94314-4.C | 33 ++++++++++++++ gcc/testsuite/g++.dg/pr94314.C | 2 +- gcc/tree-ssa-dce.c | 76 +++++++++++++++++++++++++++----- 4 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr94314-4.C diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index c73b8f810f0..8f541a28b6e 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -165,6 +165,7 @@ set_new_clone_decl_and_node_flags (cgraph_node *new_node) DECL_STATIC_DESTRUCTOR (new_node->decl) = 0; DECL_SET_IS_OPERATOR_NEW (new_node->decl, 0); DECL_SET_IS_OPERATOR_DELETE (new_node->decl, 0); + DECL_IS_REPLACEABLE_OPERATOR (new_node->decl) = 0; new_node->externally_visible = 0; new_node->local = 1; @@ -1030,6 +1031,7 @@ cgraph_node::create_version_clone_with_body DECL_STATIC_DESTRUCTOR (new_decl) = 0; DECL_SET_IS_OPERATOR_NEW (new_decl, 0); DECL_SET_IS_OPERATOR_DELETE (new_decl, 0); + DECL_IS_REPLACEABLE_OPERATOR (new_decl) = 0; /* Create the new version's call-graph node. and update the edges of the new node. */ diff --git a/gcc/testsuite/g++.dg/pr94314-4.C b/gcc/testsuite/g++.dg/pr94314-4.C new file mode 100644 index 00000000000..afa2a443dc4 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94314-4.C @@ -0,0 +1,33 @@ +/* PR c++/94314. */ +/* { dg-do run } */ +/* { dg-options "-O2 -fdump-tree-cddce-details -std=c++14" } */ +/* { dg-additional-options "-fdelete-null-pointer-checks" } */ + +#include + +int count = 0; + +__attribute__((malloc, noinline)) void* operator new[](__SIZE_TYPE__ sz) { + ++count; + return ::operator new(sz); +} + +void operator delete[](void* ptr) noexcept { + --count; + ::operator delete(ptr); +} + +void operator delete[](void* ptr, __SIZE_TYPE__ sz) noexcept { + --count; + ::operator delete(ptr, sz); +} + +int main() { + delete[] new int[1]; + if (count != 0) + __builtin_abort (); + + return 0; +} + +/* { dg-final { scan-tree-dump-not "Deleting : operator delete" "cddce1"} } */ diff --git a/gcc/testsuite/g++.dg/pr94314.C b/gcc/testsuite/g++.dg/pr94314.C index 86e651d10ba..da293a7dd9b 100644 --- a/gcc/testsuite/g++.dg/pr94314.C +++ b/gcc/testsuite/g++.dg/pr94314.C @@ -81,5 +81,5 @@ int main(){ return 0; } -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 1 "cddce1"} } */ +/* { dg-final { scan-tree-dump-not "Deleting : operator delete" "cddce1"} } */ /* { dg-final { scan-tree-dump-not "Deleting : B::operator delete" "cddce1"} } */ diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index fd5f24c746c..e1085d6b049 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -646,6 +646,53 @@ degenerate_phi_p (gimple *phi) return true; } +/* Compare new/delete inline contexts and return false if + either number of replaceable operators is different or we meet + a non-replaceable operator. */ + +static bool +compare_new_delete_inline_contexts (gimple *new_call, gimple *delete_call) +{ + unsigned int replaceable_new_ops = 0; + unsigned int replaceable_delete_ops = 0; + + for (tree block = gimple_block (new_call); + block && TREE_CODE (block) == BLOCK; block = BLOCK_SUPERCONTEXT (block)) + { + tree fn = block_ultimate_origin (block); + if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL) + { + if (DECL_IS_OPERATOR_NEW_P (fn)) + { + if (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fn)) + ++replaceable_new_ops; + else + return false; + } + } + } + + for (tree block = gimple_block (delete_call); + block && TREE_CODE (block) == BLOCK; block = BLOCK_SUPERCONTEXT (block)) + { + tree fn = block_ultimate_origin (block); + if (fn != NULL && TREE_CODE (fn) == FUNCTION_DECL) + { + if (DECL_IS_OPERATOR_DELETE_P (fn)) + { + if (DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fn)) + ++replaceable_delete_ops; + else + return false; + } + } + } + + return (replaceable_new_ops <= 1 + && replaceable_delete_ops <= 1 + && replaceable_new_ops == replaceable_delete_ops); +} + /* Propagate necessity using the operands of necessary statements. Process the uses on each statement in the worklist, and add all feeding statements which contribute to the calculation of this @@ -824,16 +871,25 @@ propagate_necessity (bool aggressive) || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC)) || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee))) { - /* Delete operators can have alignment and (or) size as next - arguments. When being a SSA_NAME, they must be marked - as necessary. */ - if (is_delete_operator && gimple_call_num_args (stmt) >= 2) - for (unsigned i = 1; i < gimple_call_num_args (stmt); i++) - { - tree arg = gimple_call_arg (stmt, i); - if (TREE_CODE (arg) == SSA_NAME) - mark_operand_necessary (arg); - } + if (is_delete_operator) + { + /* Verify that new and delete operators have the same + context. */ + if (!compare_new_delete_inline_contexts (def_stmt, stmt)) + mark_operand_necessary (gimple_call_arg (stmt, 0)); + + /* Delete operators can have alignment and (or) size + as next arguments. When being a SSA_NAME, they + must be marked as necessary. */ + if (gimple_call_num_args (stmt) >= 2) + for (unsigned i = 1; i < gimple_call_num_args (stmt); + i++) + { + tree arg = gimple_call_arg (stmt, i); + if (TREE_CODE (arg) == SSA_NAME) + mark_operand_necessary (arg); + } + } continue; } -- 2.26.0 --------------746EA15ED5303E6927D8BAE6--