From patchwork Mon Feb 3 13:29:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Martin X-Patchwork-Id: 105924 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 9ADE93858C33 for ; Mon, 3 Feb 2025 13:30:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9ADE93858C33 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=nasilyan.com header.i=@nasilyan.com header.a=rsa-sha256 header.s=tey23rxsjton5kop5bydp3vc5ylkyjkg header.b=Ov23Ds/V; dkim=pass (1024-bit key, unprotected) header.d=amazonses.com header.i=@amazonses.com header.a=rsa-sha256 header.s=uku4taia5b5tsbglxyj6zym32efj7xqv header.b=HKLm0m1I X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from a2-88.smtp-out.eu-west-1.amazonses.com (a2-88.smtp-out.eu-west-1.amazonses.com [54.240.2.88]) by sourceware.org (Postfix) with ESMTPS id 1B5E2385841D for ; Mon, 3 Feb 2025 13:29:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1B5E2385841D Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=nasilyan.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eu-west-1.amazonses.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1B5E2385841D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=54.240.2.88 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738589349; cv=none; b=ap5M9t/tHyIZjZBnu9AHqNzwATxSwnELPjNk24OMnhIMVcHzuOyHXLSRClrjgapbnrRNsUzQ7e1QmC/3PqSlB0tUAbMTAlc/OnZwQcUMnAfQzixYsSuVFE53SbtqYDxAsvC9deaCpxrE2/srda8wgzn0HOEr1C+ydAjFG1k9kgQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1738589349; c=relaxed/simple; bh=14CH+zp7P9QDa8o2HXADNFZajkaoGSieqfPu4oYBR2I=; h=DKIM-Signature:DKIM-Signature:Subject:From:To:Date:Mime-Version: Message-ID; b=XKdZd9De78B+KsNUsQj/5cDIfbiIYg5NYpAZU3R4LIQ5LtnkMVPzBKUDk06KD2Jtmn8eu6zKdKgXUk+vr/TIuJux7ys8Tj9wQBzSO24j3MSQy8bK7CgXYdfWnPGp6EkX4rOAmj72RV/usnOhrsTYFG7ocVEEq29peFee+HTpAgU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1B5E2385841D DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=tey23rxsjton5kop5bydp3vc5ylkyjkg; d=nasilyan.com; t=1738589347; h=Subject:From:To:Cc:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id; bh=14CH+zp7P9QDa8o2HXADNFZajkaoGSieqfPu4oYBR2I=; b=Ov23Ds/VW56mJRQoAYN0PslBjKT0JigUH6No64yIdG6Q06LbS7X/qRdW54jMQTOf hjpfJMvmABCZh9mwC0MvdW/KtRhNl9keI35mMPVAMiVkrtOc4r098YI0YvG7Fqap9Vx ooiICSGUqG5T1Xl4huc1k6rnGYCLE1O07HZ7pnbw= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=uku4taia5b5tsbglxyj6zym32efj7xqv; d=amazonses.com; t=1738589347; h=Subject:From:To:Cc:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id:Feedback-ID; bh=14CH+zp7P9QDa8o2HXADNFZajkaoGSieqfPu4oYBR2I=; b=HKLm0m1INSsFHMe73XIJiI0o2/UESECLTPAw49pQS0Z8BFjwkGEWWALOSArtODGH 2DPKfkyv+lhsfBR8j86cT8Kd0alDNHZXgUa8RxjtOdZE4z3syxV9RpNxX87hYkHCz7r Q42dEzBuco2pVJGFi9oGKzl+YttBDSagzoaHWXLw= Subject: [PATCH v2] c++: Properly detect calls to digest_init in build_vec_init [PR114619] From: =?utf-8?q?Simon_Martin?= To: =?utf-8?q?Jason_Merrill?= Cc: =?utf-8?q?gcc-patches=40gcc=2Egnu=2Eorg?= Date: Mon, 3 Feb 2025 13:29:07 +0000 Mime-Version: 1.0 In-Reply-To: <9a31eadf-3d6f-4dfd-89f5-0ceae433de94@redhat.com> References: <20241019090956.90129-1-simon@nasilyan.com> <01020192a40a6da2-b98a9365-88fe-42d1-92a1-4c58c9d88909-000000@eu-west-1.amazonses.com> <9a31eadf-3d6f-4dfd-89f5-0ceae433de94@redhat.com> <172DA5E9-6C47-4AD1-BC8B-A295423A8C8D@nasilyan.com> X-Mailer: Amazon WorkMail Thread-Index: AQHbIgar8Fm6SL89SN+VVtdslKI5oBGX6whUFQ47eDA= Thread-Topic: [PATCH v2] c++: Properly detect calls to digest_init in build_vec_init [PR114619] X-Original-Mailer: MailMate (1.13.2r5673) X-Wm-Sent-Timestamp: 1738589346 Message-ID: <01020194cc001f33-5b7f7c43-e784-49bd-bcfb-099c22eefe95-000000@eu-west-1.amazonses.com> Feedback-ID: ::1.eu-west-1.b24dn6frgCi6dh20skzbuMRr7UL8M6Soir/3ogtEjHQ=:AmazonSES X-SES-Outgoing: 2025.02.03-54.240.2.88 X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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~patchwork=sourceware.org@gcc.gnu.org Hi Jason, On 16 Jan 2025, at 23:28, Jason Merrill wrote: > On 10/19/24 5:09 AM, Simon Martin wrote: >> We currently ICE in checking mode with cxx_dialect < 17 on the >> following >> valid code >> >> === cut here === >> struct X { >> X(const X&) {} >> }; >> extern X x; >> void foo () { >> new X[1]{x}; >> } >> === cut here === >> >> The problem is that cp_gimplify_expr gcc_checking_asserts that a >> TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while >> in >> this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we >> have >> not even tried to elide. >> >> This patch relaxes that gcc_checking_assert to not fail when using >> cxx_dialect < 17 and -fno-elide-constructors (I considered being more >> clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks >> more >> risky and not worth the extra complexity for a checking assert). > > The problem is that in that case we end up with two copy constructor > calls instead of one: one built in massage_init_elt, and the other in > expand_default_init. The result of the first copy is marked > TARGET_EXPR_ELIDING_P, so when we try to pass it to the second copy we > hit the assert. I think the assert is catching a real bug: even with > -fno-elide-constructors we should only copy once, not twice. That’s right, thanks for pointing me in the right direction. > This seems to be because 'digested' has the wrong value in > build_vec_init; we did just call digest_init in build_new_1, but > build_vec_init doesn't understand that. The test to determine whether digest_init has been called is indeed incorrect, in that it will work if BASE is a reference to the array but not if it’s a pointer to its first element. The attached updated patch fixes this. Successfully tested on x86_64-pc-linux-gnu. OK for trunk? Simon From 578ac1a022ff039cdca45cdfca31bdfe8b571b79 Mon Sep 17 00:00:00 2001 From: Simon Martin Date: Mon, 3 Feb 2025 11:43:14 +0100 Subject: [PATCH] c++: Properly detect calls to digest_init in build_vec_init [PR114619] We currently ICE in checking mode with cxx_dialect < 17 on the following valid code === cut here === struct X { X(const X&) {} }; extern X x; void foo () { new X[1]{x}; } === cut here === We trip on a gcc_checking_assert in cp_gimplify_expr due to a TARGET_EXPR that is not TARGET_EXPR_ELIDING_P. As pointed by Jason, the problem is that build_vec_init does not recognize that digest_init has been called, and we end up calling the copy constructor twice. This happens because the detection in build_vec_init assumes that BASE is a reference to the array, while it's a pointer to its first element here. This patch makes sure that the detection works in both cases. Successfully tested on x86_64-pc-linux-gnu. PR c++/114619 gcc/cp/ChangeLog: * init.cc (build_vec_init): Properly determine whether digest_init has been called. gcc/testsuite/ChangeLog: * g++.dg/init/no-elide4.C: New test. --- gcc/cp/init.cc | 3 ++- gcc/testsuite/g++.dg/init/no-elide4.C | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/init/no-elide4.C -- 2.44.0 diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc index 3ab7f96335c..613775c5a7c 100644 --- a/gcc/cp/init.cc +++ b/gcc/cp/init.cc @@ -4786,7 +4786,8 @@ build_vec_init (tree base, tree maxindex, tree init, tree field, elt; /* If the constructor already has the array type, it's been through digest_init, so we shouldn't try to do anything more. */ - bool digested = same_type_p (atype, TREE_TYPE (init)); + bool digested = (TREE_CODE (TREE_TYPE (init)) == ARRAY_TYPE + && same_type_p (type, TREE_TYPE (TREE_TYPE (init)))); from_array = 0; if (length_check) diff --git a/gcc/testsuite/g++.dg/init/no-elide4.C b/gcc/testsuite/g++.dg/init/no-elide4.C new file mode 100644 index 00000000000..9377d9f0161 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/no-elide4.C @@ -0,0 +1,11 @@ +// PR c++/114619 +// { dg-do "compile" { target c++11 } } +// { dg-options "-fno-elide-constructors" } + +struct X { + X(const X&) {} +}; +extern X x; +void foo () { + new X[1]{x}; +}