From patchwork Thu Mar 14 21:26:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1912246 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=S1vlJVFG; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TwgT13TJxz1yWn for ; Fri, 15 Mar 2024 08:27:31 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0A0BF3857BA7 for ; Thu, 14 Mar 2024 21:27:29 +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 F3B1B3857BA2 for ; Thu, 14 Mar 2024 21:27:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F3B1B3857BA2 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 F3B1B3857BA2 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=1710451628; cv=none; b=miulhowfgGRhphhWUoduwvw/LU6xTW0Vm9cv2BTe/7EZOGXfDdG2oI3urkBuBcymykzyzVi8RzBqaD8Lj0wEIK0b94QBOh/r/ecI+NDm6yCrrIX9EdK0TYXFG8L3XIVv3krvv5EkDovu1KdxX9Fq716jLuDz0mpfK1+sAY9MjvI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710451628; c=relaxed/simple; bh=vDcXL2uHygZ/1K4AmL+miiO79bsQQAd8EhuPIs14Dn0=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=qx/0nHtXLZ0Qs4iuZgRfPQISiryYtMAB2CN5cvm0KY7ycUcQgZBhS11e3XLXcgqkQx/uHMxLT0DCARKI2rqkwr2cB++yLJ87V+dlwXyU73qcao2WwQ6YZPyn3YAuCHZAjgeNr/ja8cI+Nk9Jogi9553ZJgPqZLZPSrG5DOWkevQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710451625; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=cTD2SRQIxxqNBqAfepJp0OD67gtTwJCXF1q8Caaeubo=; b=S1vlJVFGyP2PQGb5Dw9cBJwunWZCyTGxV3V7ORT9fz95jkJAtjskySboy6BD170ILK+7j/ p0AomavlaNMiXqvNV1yBr86Jpf32/6ePZ0dp3u/CSWW1SpQ8a5IGsgrgJOsC986oENZSL2 RUYpeROgHbW0Zzoch7aEdDwtwruT4t8= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-590-VTPBVc2OPNqHo6VVvsl5aw-1; Thu, 14 Mar 2024 17:27:04 -0400 X-MC-Unique: VTPBVc2OPNqHo6VVvsl5aw-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-787dea68f58so122217385a.3 for ; Thu, 14 Mar 2024 14:27:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710451623; x=1711056423; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cTD2SRQIxxqNBqAfepJp0OD67gtTwJCXF1q8Caaeubo=; b=w0pELyXY/ApW/Aanw9IjJTdDSBA3AlAPt9N79oTqJksXvQeNHu/wMeUQbyBPJW54aR Sc1wFbIRMlw+AsY2DDcMRoLX8E0EELbqxH/rrCdIr4x5+IZiCwBEyRpg/uaw/g1BwQF0 t4o/ZwTiHYU2qe8Mts9T2odr0wgP5yEBU+Ef1Xds19TufqvEoi/6mODWg3gqx+CrK4bX WjuQRsm38+vXUcLhUN3mu/mXbnwT9DiSQGEDuz2uqT/495n2SjL1mGDmmeOFlZYlYntm 7qi4quqPNLps1lRY46rh/HAdff55fVWLuD84hJXdwoKt3RPkOEYdLHdo4JPaVY59FWTv ZLtg== X-Gm-Message-State: AOJu0Yyo8nZjoUsnj8SqNl1EG5r7/mC3PdMkAFp1MgHK8+q3dzI0PaUj h+9yrix90tK4sMH09ciu5mUi7l2x/3Vr4zkUrRE1Ud1UoN3uyUQ/wrWwFY8W98VG62KdiV/tvol tXu03Nz+YL9TaNziATDUWv3pEz0UuSB+5ccvCp+X8fdtEVvujvyrHljusc0ZeW40= X-Received: by 2002:a05:620a:4415:b0:789:e247:1d7e with SMTP id v21-20020a05620a441500b00789e2471d7emr1083599qkp.43.1710451622962; Thu, 14 Mar 2024 14:27:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEoKqWQYY7B7+Pq+bX2/EthLrmjLecxVxVbj1Qx/nkZK/K/YGJRodEotTP96KWFjLiDBSoufA== X-Received: by 2002:a05:620a:4415:b0:789:e247:1d7e with SMTP id v21-20020a05620a441500b00789e2471d7emr1083581qkp.43.1710451622481; Thu, 14 Mar 2024 14:27:02 -0700 (PDT) Received: from redhat.com (2603-7000-9500-34a5-0000-0000-0000-1db4.res6.spectrum.com. [2603:7000:9500:34a5::1db4]) by smtp.gmail.com with ESMTPSA id 15-20020a05620a04cf00b007885c419f2asm1251987qks.136.2024.03.14.14.27.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Mar 2024 14:27:01 -0700 (PDT) Date: Thu, 14 Mar 2024 17:26:59 -0400 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: [PATCH v3] c++: ICE with temporary of class type in array DMI [PR109966] Message-ID: References: <20240311232758.458729-1-polacek@redhat.com> <2244ac04-8bb6-4030-85c6-7a10aa20c494@redhat.com> MIME-Version: 1.0 In-Reply-To: <2244ac04-8bb6-4030-85c6-7a10aa20c494@redhat.com> User-Agent: Mutt/2.2.12 (2023-09-09) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.8 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_H4, 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+incoming=patchwork.ozlabs.org@gcc.gnu.org On Tue, Mar 12, 2024 at 06:26:14PM -0400, Jason Merrill wrote: > On 3/12/24 11:56, Marek Polacek wrote: > > On Tue, Mar 12, 2024 at 09:57:14AM -0400, Jason Merrill wrote: > > > On 3/11/24 19:27, Marek Polacek wrote: > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13? > > > > > > > > -- >8 -- > > > > This ICE started with the fairly complicated r13-765. We crash in > > > > gimplify_var_or_parm_decl because a stray VAR_DECL leaked there. > > > > The problem is ultimately that potential_prvalue_result_of wasn't > > > > correctly handling arrays and replace_placeholders_for_class_temp_r > > > > replaced a PLACEHOLDER_EXPR in a TARGET_EXPR which is used in the > > > > context of copy elision. If I have > > > > > > > > M m[2] = { M{""}, M{""} }; > > > > > > > > then we don't invoke the M(const M&) copy-ctor. I think the fix is > > > > to detect such a case in potential_prvalue_result_of. > > > > > > > > PR c++/109966 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * typeck2.cc (potential_prvalue_result_of): Add walk_subtrees > > > > parameter. Handle initializing an array from a > > > > brace-enclosed-initializer. > > > > (replace_placeholders_for_class_temp_r): Pass walk_subtrees down to > > > > potential_prvalue_result_of. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/cpp1y/nsdmi-aggr20.C: New test. > > > > * g++.dg/cpp1y/nsdmi-aggr21.C: New test. > > > > --- > > > > gcc/cp/typeck2.cc | 27 ++++++++--- > > > > gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C | 17 +++++++ > > > > gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C | 59 +++++++++++++++++++++++ > > > > 3 files changed, 96 insertions(+), 7 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C > > > > > > > > diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc > > > > index 31198b2f9f5..8b99ce78e9a 100644 > > > > --- a/gcc/cp/typeck2.cc > > > > +++ b/gcc/cp/typeck2.cc > > > > @@ -1406,46 +1406,59 @@ digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) > > > > A a = (A{}); // initializer > > > > A a = (1, A{}); // initializer > > > > A a = true ? A{} : A{}; // initializer > > > > + A arr[1] = { A{} }; // initializer > > > > auto x = A{}.x; // temporary materialization > > > > auto x = foo(A{}); // temporary materialization > > > > FULL_EXPR is the whole expression, SUBOB is its TARGET_EXPR subobject. */ > > > > static bool > > > > -potential_prvalue_result_of (tree subob, tree full_expr) > > > > +potential_prvalue_result_of (tree subob, tree full_expr, int *walk_subtrees) > > > > { > > > > +#define RECUR(t) potential_prvalue_result_of (subob, t, walk_subtrees) > > > > if (subob == full_expr) > > > > return true; > > > > else if (TREE_CODE (full_expr) == TARGET_EXPR) > > > > { > > > > tree init = TARGET_EXPR_INITIAL (full_expr); > > > > if (TREE_CODE (init) == COND_EXPR) > > > > - return (potential_prvalue_result_of (subob, TREE_OPERAND (init, 1)) > > > > - || potential_prvalue_result_of (subob, TREE_OPERAND (init, 2))); > > > > + return (RECUR (TREE_OPERAND (init, 1)) > > > > + || RECUR (TREE_OPERAND (init, 2))); > > > > else if (TREE_CODE (init) == COMPOUND_EXPR) > > > > - return potential_prvalue_result_of (subob, TREE_OPERAND (init, 1)); > > > > + return RECUR (TREE_OPERAND (init, 1)); > > > > /* ??? I don't know if this can be hit. */ > > > > else if (TREE_CODE (init) == PAREN_EXPR) > > > > { > > > > gcc_checking_assert (false); > > > > - return potential_prvalue_result_of (subob, TREE_OPERAND (init, 0)); > > > > + return RECUR (TREE_OPERAND (init, 0)); > > > > } > > > > } > > > > + /* The array case listed above. */ > > > > + else if (TREE_CODE (full_expr) == CONSTRUCTOR > > > > + && TREE_CODE (TREE_TYPE (full_expr)) == ARRAY_TYPE) > > > > + for (constructor_elt &e: CONSTRUCTOR_ELTS (full_expr)) > > > > + if (e.value == subob) > > > > + { > > > > + *walk_subtrees = 0; > > > > > > Why clear walk_subtrees? Won't that mean we fail to replace any > > > placeholders nested within an array element initializer? > > > > Right. I couldn't find a testcase where that would cause a problem > > but I think I just wasn't inventive enough. > > > > Originally, I was checking same_type_ignoring_top_level_qualifiers_p > > but that's not going to work for code like > > > > struct N { N(M); }; > > N arr[2] = { M{""}, M{""} }; > > > > or with operator M(). But I suppose I could just use can_convert > > like below. What do you think about that? > > Basing this on the type seems unreliable, we're looking for where in the > expression the TARGET_EXPR occurs, and there could be others of the same > type elsewhere in the expression. > > Why not loop over CONSTRUCTOR_ELTS like you did above, just without clearing > walk_subtrees? With the test with N(M) we are dealing with: {TARGET_EXPR >> TARGET_EXPR >> (const char *) "" >>>>, .j=NON_LVALUE_EXPR <42>, .i=(&)->j}> >>>>} where the TARGET_EXPR with slot=D.2864 is the problematic one. So looping over CONSTRUCTOR_ELTS of the outer CONSTRUCTOR wouldn't find it. But you're of course right that just checking the type is fragile. In the following patch, I'm taking a different tack. I believe we ought to use TARGET_EXPR_ELIDING_P. The gimplify_arg bit I'm talking about below is this: /* Also strip a TARGET_EXPR that would force an extra copy. */ if (TREE_CODE (*arg_p) == TARGET_EXPR) { tree init = TARGET_EXPR_INITIAL (*arg_p); if (init && !VOID_TYPE_P (TREE_TYPE (init))) *arg_p = init; } Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13? -- >8 -- This ICE started with the fairly complicated r13-765. We crash in gimplify_var_or_parm_decl because a stray VAR_DECL leaked there. The problem is ultimately that potential_prvalue_result_of wasn't correctly handling arrays and replace_placeholders_for_class_temp_r replaced a PLACEHOLDER_EXPR in a TARGET_EXPR which is used in the context of copy elision. If I have M m[2] = { M{""}, M{""} }; then we don't invoke the M(const M&) copy-ctor. One part of the fix is to use TARGET_EXPR_ELIDING_P rather than potential_prvalue_result_of. That unfortunately doesn't handle the case like struct N { N(M); }; N arr[2] = { M{""}, M{""} }; because TARGET_EXPRs that initialize a function argument are not marked TARGET_EXPR_ELIDING_P even though gimplify_arg drops such TARGET_EXPRs on the floor. We can use a pset to avoid replacing placeholders in them. I made an attempt to use set_target_expr_eliding in convert_for_arg_passing but that regressed constexpr-diag1.C, and does not seem like a prudent change in stage 4 anyway. PR c++/109966 gcc/cp/ChangeLog: * typeck2.cc (potential_prvalue_result_of): Remove. (replace_placeholders_for_class_temp_r): Check TARGET_EXPR_ELIDING_P. Use a pset. Don't replace_placeholders in TARGET_EXPRs that initialize a function argument. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/nsdmi-aggr20.C: New test. * g++.dg/cpp1y/nsdmi-aggr21.C: New test. --- gcc/cp/typeck2.cc | 55 ++++++--------------- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C | 17 +++++++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C | 59 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 39 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C base-commit: 6dbf0d252f69ab2924256e6778ba7dc55d5b6915 diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc index 31198b2f9f5..4bf3201eedc 100644 --- a/gcc/cp/typeck2.cc +++ b/gcc/cp/typeck2.cc @@ -1399,41 +1399,6 @@ digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) return digest_init_r (type, init, 0, flags, complain); } -/* Return true if SUBOB initializes the same object as FULL_EXPR. - For instance: - - A a = A{}; // initializer - A a = (A{}); // initializer - A a = (1, A{}); // initializer - A a = true ? A{} : A{}; // initializer - auto x = A{}.x; // temporary materialization - auto x = foo(A{}); // temporary materialization - - FULL_EXPR is the whole expression, SUBOB is its TARGET_EXPR subobject. */ - -static bool -potential_prvalue_result_of (tree subob, tree full_expr) -{ - if (subob == full_expr) - return true; - else if (TREE_CODE (full_expr) == TARGET_EXPR) - { - tree init = TARGET_EXPR_INITIAL (full_expr); - if (TREE_CODE (init) == COND_EXPR) - return (potential_prvalue_result_of (subob, TREE_OPERAND (init, 1)) - || potential_prvalue_result_of (subob, TREE_OPERAND (init, 2))); - else if (TREE_CODE (init) == COMPOUND_EXPR) - return potential_prvalue_result_of (subob, TREE_OPERAND (init, 1)); - /* ??? I don't know if this can be hit. */ - else if (TREE_CODE (init) == PAREN_EXPR) - { - gcc_checking_assert (false); - return potential_prvalue_result_of (subob, TREE_OPERAND (init, 0)); - } - } - return false; -} - /* Callback to replace PLACEHOLDER_EXPRs in a TARGET_EXPR (which isn't used in the context of guaranteed copy elision). */ @@ -1441,11 +1406,13 @@ static tree replace_placeholders_for_class_temp_r (tree *tp, int *, void *data) { tree t = *tp; - tree full_expr = *static_cast(data); + auto pset = static_cast *>(data); /* We're looking for a TARGET_EXPR nested in the whole expression. */ if (TREE_CODE (t) == TARGET_EXPR - && !potential_prvalue_result_of (t, full_expr)) + /* That serves as temporary materialization, not an initializer. */ + && !TARGET_EXPR_ELIDING_P (t) + && !pset->add (t)) { tree init = TARGET_EXPR_INITIAL (t); while (TREE_CODE (init) == COMPOUND_EXPR) @@ -1460,6 +1427,16 @@ replace_placeholders_for_class_temp_r (tree *tp, int *, void *data) gcc_checking_assert (!find_placeholders (init)); } } + /* TARGET_EXPRs initializing function arguments are not marked as eliding, + even though gimplify_arg drops them on the floor. Don't go replacing + placeholders in them. */ + else if (TREE_CODE (t) == CALL_EXPR || TREE_CODE (t) == AGGR_INIT_EXPR) + for (int i = 0; i < call_expr_nargs (t); ++i) + { + tree arg = get_nth_callarg (t, i); + if (TREE_CODE (arg) == TARGET_EXPR) + pset->add (arg); + } return NULL_TREE; } @@ -1507,8 +1484,8 @@ digest_nsdmi_init (tree decl, tree init, tsubst_flags_t complain) temporary materialization does not occur when initializing an object from a prvalue of the same type, therefore we must not replace the placeholder with a temporary object so that it can be elided. */ - cp_walk_tree (&init, replace_placeholders_for_class_temp_r, &init, - nullptr); + hash_set pset; + cp_walk_tree (&init, replace_placeholders_for_class_temp_r, &pset, nullptr); return init; } diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C new file mode 100644 index 00000000000..4796d861e83 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C @@ -0,0 +1,17 @@ +// PR c++/109966 +// { dg-do compile { target c++14 } } + +#define SA(X) static_assert ((X),#X) + +struct A { + int a; + int b = a; +}; + +struct B { + int x = 0; + int y[1]{A{x}.b}; +}; + +constexpr B b = { }; +SA(b.y[0] == 0); diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C new file mode 100644 index 00000000000..efec45bc1a8 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C @@ -0,0 +1,59 @@ +// PR c++/109966 +// { dg-do compile { target c++14 } } + +struct k { + k(const char *); +}; +struct M { + k name; + int j = 42; + int i = j; +}; + +M m = M{""}; + +struct S { + M arr1[1]{M{""}}; + M a1[1] = { (M{""}) }; + M a2[1] = { (true, M{""}) }; + M a3[1] = { true ? M{""} : M{""} }; + M arr2[2]{M{""}, M{""}}; + M arr3[3]{M{""}, M{""}, M{""}}; + + M arr1e[1] = {M{""}}; + M arr2e[2] = {M{""}, M{""}}; + M arr3e[3] = {M{""}, M{""}, M{""}}; + + M arr1l[1] = { m }; + M arr2l[2] = { m, m }; + M arr3l[3] = { m, m, m }; + + M m1 = M{""}; + M m2 = m; + M m3{M{""}}; + M m4 = {M{""}}; +} o; + +struct N { + N(M); +}; + +struct Z { + N arr1[1]{ M{""} }; + N arr2[2]{ M{""}, M{""} }; + N arr1e[1] = { M{""} }; + N arr2e[2] = { M{""}, M{""} }; +} z; + +struct Y { + k name; + int j = 42; + int i = j; + operator M(); +}; + +struct W { + M arr1[1]{ Y{""} }; + M arr2[2]{ Y{""}, Y{""} }; + M arr3[3]{ Y{""}, Y{""}, Y{""} }; +} w;