From patchwork Thu Dec 14 21:01:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 178901 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:3b04:b0:fb:cd0c:d3e with SMTP id c4csp8848899dys; Thu, 14 Dec 2023 13:02:23 -0800 (PST) X-Google-Smtp-Source: AGHT+IEI8ZOmOTK1svfHzOKBLl5c8PJUlS710XfNQRKrXT80IZpKgyz3Me2gEV96tJ3lIAd0rvyJ X-Received: by 2002:a05:6830:148a:b0:6d9:f64e:ed5 with SMTP id s10-20020a056830148a00b006d9f64e0ed5mr10461041otq.2.1702587742963; Thu, 14 Dec 2023 13:02:22 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1702587742; cv=pass; d=google.com; s=arc-20160816; b=RfxyBpZqrP/SfGwJTDJji2RSba0MlQpqrdmXYJYxxXK3eK4zAjotQSKCuHHMvYeLJB r8rZUh+cmGb0cTv6MqmrpCpTRI+OFNiOh/GnC354oKRZRDENNtnB2ip06eOgy4ciG32q uPJbxFqAtZfE3paKjI1EqGJQMbDHdrhFGedIocR34JAj1TxS9YZxljVo0GwGrz26SeWt NHOqc3IOuSlvbomJVbAR9GLgfQpUtsT1P2IC3PPyRvLfcM9FZHA8GelkAJJ5nu0E1cB5 IvWxQmcmglBehQM+KlFHYxFma+U98vhMeFawxyu0yOzGbMPFBSznFriuQPOR6fXueimh Xvzg== 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-disposition:user-agent :in-reply-to:mime-version:references:message-id:subject:cc:to:from :date:dkim-signature:arc-filter:dmarc-filter:delivered-to; bh=gWJ8qAkKUPHZNwVYb7pTpNeTrBjl+r5S8oJxTdfuOmg=; fh=rsfJLMIL/7j+58qzkgIXBlfQSah3iXypQc76SN5PqTs=; b=N4+1+2teHo8e577aIlhL3iBfwneF79lXzBOadd1i1G3HBi6aYhWtI+KN4cV/a+pxSD sF1jDMn3zyvwBddnSozm/kYvguAcEBdqSjrxmByKmaFtuvAhI7cxw16ANPwv9ghTgiGI P9WPF1s1oFQSB2/M+9nLa8qURyMsbV0VMHXg7BesMm5Ad5wfR62uUW3nMr+Ipka8+2MV CWy21Uv1TP+r5ywXB+myg5UENgqyw0oA+Kn72wuqAE57zQpvUJf63mDUxEQv/owR3DRd 6kEBi4/Mrsv/6oveWVirVqu2ZokgqtKGNQNHvP+/GC9VfVv1WGNTYIwyVFo35pM32vVD hq9Q== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=FLJvTn+7; 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 p3-20020ab03b83000000b007c5af20659csi3003986uaw.66.2023.12.14.13.02.22 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Dec 2023 13:02:22 -0800 (PST) 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=FLJvTn+7; 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 B7441384CB98 for ; Thu, 14 Dec 2023 21:02:18 +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 DD4393858D20 for ; Thu, 14 Dec 2023 21:01:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DD4393858D20 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 DD4393858D20 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=1702587713; cv=none; b=AKX5m/DSv/VSr8zKvaw3EoR7QJeuzHwUGHM1bvgQHwR7u04pxUoP4EINNtutoZBjVqfRPOnz2UI3zdNUI19YlCh7NRg+RxVYoaWupYByrV2MfNlZwRSLxKIXN6HMQJk1YVdNqIkfjCZJL+BH1tgMyR1nYQucwHBcDLc0aWLNJ8Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702587713; c=relaxed/simple; bh=izfElwW49m8JWltVPsLKY6+PesWZ5O0+ldZU3jctuaw=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=v32eoHc1rloeaPpGpn4GqRwTkvecwyJRpQTWeAm9hgAmgahzEBEYoWdDXUgZZRSvASLSX3Shwa/hHYl8NV+eyowT1Z5Le3+mBKCowAk6Yi03gxTyFslLPc0byT++Ln45neQ02Syp1Ho1jXten2L7Da2Zf4nL7RQ4lMWcoCGC2ew= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1702587711; 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=gWJ8qAkKUPHZNwVYb7pTpNeTrBjl+r5S8oJxTdfuOmg=; b=FLJvTn+77hqk5hKZRcDk6P+0lnu26d9h0zRyHUeEcmFbNUpPvFlas9kgWxTEZ/a7frF816 xf4carhrGA1G2MFajn4AZ5WJOTIqQLDBK9APk4h86KVV9ERPdw4ZwovrJZc9a8Q29+c1lJ IEkrp3JCeRX+VHHCXTKq+p3StxtfZUo= Received: from mail-oo1-f69.google.com (mail-oo1-f69.google.com [209.85.161.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-32-mKKLma76PDGHKKzCN5P9BQ-1; Thu, 14 Dec 2023 16:01:47 -0500 X-MC-Unique: mKKLma76PDGHKKzCN5P9BQ-1 Received: by mail-oo1-f69.google.com with SMTP id 006d021491bc7-58d6987da39so3203eaf.2 for ; Thu, 14 Dec 2023 13:01:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702587706; x=1703192506; 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=gWJ8qAkKUPHZNwVYb7pTpNeTrBjl+r5S8oJxTdfuOmg=; b=EiM0KcN/l6Wanoh6H98V1tOcOOAgMRxe4JNKBNetPexoZ0H+m96apLMCAsWRm7Hy78 vOnFtCTLcAhmdtqfvxcds2yfRxUyDnL/WqXzZrDCuLW/qcye1sP460WN6RE3o5VIYVWv WesuvGL9dTFAxp3m9kPw+z06JzZ0iHdLbUCZo3SS6mBhjVQAQQ9IAOVzzQkq1ltz5BS0 ZR5J/gIkN+Mk5UjZy+fdRP8teYIj8TfZTN81S1XpKY+vfbifnofcQt1TlwZUWy31gq7z TZTNI6qyjdgltG3eO7IIyWbQJ0eLnLMzTPVU59zW0PuHC4hEt19UzN8Vv82iyPdkQm+n v1BA== X-Gm-Message-State: AOJu0YyZWzSOHBXLxVcIKWOJLZWPqFk7Vitj4pJHLQN++/MEaCAks6HX ZFJzsWzY5TAn8tbOV/bEpiCjLvxFL92jfuy5xZElbtCZg/zw+0S9hfU8BtYCbmd1DuF9pMDon3Q otjtKssEIB37dr8kLpQ== X-Received: by 2002:a05:6358:3a1b:b0:170:c15a:8004 with SMTP id g27-20020a0563583a1b00b00170c15a8004mr10859900rwe.53.1702587706475; Thu, 14 Dec 2023 13:01:46 -0800 (PST) X-Received: by 2002:a05:6358:3a1b:b0:170:c15a:8004 with SMTP id g27-20020a0563583a1b00b00170c15a8004mr10859880rwe.53.1702587705994; Thu, 14 Dec 2023 13:01:45 -0800 (PST) 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 z8-20020ad44148000000b0067f084053afsm907607qvp.55.2023.12.14.13.01.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Dec 2023 13:01:45 -0800 (PST) Date: Thu, 14 Dec 2023 16:01:43 -0500 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: [PATCH v4] c++: fix ICE with sizeof in a template [PR112869] Message-ID: References: <20231205203136.94832-1-polacek@redhat.com> <9c08e101-0392-4c01-b336-6558e80a4193@redhat.com> <9ca5d651-17ce-464a-8cc3-e8f5ab27c61a@redhat.com> <1c9a2965-13e1-4b7b-9275-ad2e309d6b58@redhat.com> MIME-Version: 1.0 In-Reply-To: <1c9a2965-13e1-4b7b-9275-ad2e309d6b58@redhat.com> User-Agent: Mutt/2.2.10 (2023-03-25) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.2 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: 1784475376066982984 X-GMAIL-MSGID: 1785292644923327351 On Wed, Dec 13, 2023 at 03:28:38PM -0500, Jason Merrill wrote: > On 12/12/23 17:48, Marek Polacek wrote: > > On Fri, Dec 08, 2023 at 11:09:15PM -0500, Jason Merrill wrote: > > > On 12/8/23 16:15, Marek Polacek wrote: > > > > On Fri, Dec 08, 2023 at 12:09:18PM -0500, Jason Merrill wrote: > > > > > On 12/5/23 15:31, Marek Polacek wrote: > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > > > > > -- >8 -- > > > > > > This test shows that we cannot clear *walk_subtrees in > > > > > > cp_fold_immediate_r when we're in_immediate_context, because that, > > > > > > as the comment says, affects cp_fold_r as well. Here we had an > > > > > > expression with > > > > > > > > > > > > min ((long int) VIEW_CONVERT_EXPR(bytecount), (long int) <<< Unknown tree: sizeof_expr > > > > > > (int) <<< error >>> >>>) > > > > > > > > > > > > as its sub-expression, and we never evaluated that into > > > > > > > > > > > > min ((long int) bytecount, 4) > > > > > > > > > > > > so the SIZEOF_EXPR leaked into the middle end. > > > > > > > > > > > > (There's still one *walk_subtrees = 0; in cp_fold_immediate_r, but that > > > > > > one should be OK.) > > > > > > > > > > > > PR c++/112869 > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > * cp-gimplify.cc (cp_fold_immediate_r): Don't clear *walk_subtrees > > > > > > for unevaluated operands. > > > > > > > > > > I agree that we want this change for in_immediate_context (), but I don't > > > > > see why we want it for TYPE_P or unevaluated_p (code) or > > > > > cp_unevaluated_operand? > > > > > > > > No particular reason, just paranoia. How's this? > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > -- >8 -- > > > > This test shows that we cannot clear *walk_subtrees in > > > > cp_fold_immediate_r when we're in_immediate_context, because that, > > > > as the comment says, affects cp_fold_r as well. Here we had an > > > > expression with > > > > > > > > min ((long int) VIEW_CONVERT_EXPR(bytecount), (long int) <<< Unknown tree: sizeof_expr > > > > (int) <<< error >>> >>>) > > > > > > > > as its sub-expression, and we never evaluated that into > > > > > > > > min ((long int) bytecount, 4) > > > > > > > > so the SIZEOF_EXPR leaked into the middle end. > > > > > > > > (There's still one *walk_subtrees = 0; in cp_fold_immediate_r, but that > > > > one should be OK.) > > > > > > > > PR c++/112869 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * cp-gimplify.cc (cp_fold_immediate_r): Don't clear *walk_subtrees > > > > for in_immediate_context. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/template/sizeof18.C: New test. > > > > --- > > > > gcc/cp/cp-gimplify.cc | 6 +++++- > > > > gcc/testsuite/g++.dg/template/sizeof18.C | 8 ++++++++ > > > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > create mode 100644 gcc/testsuite/g++.dg/template/sizeof18.C > > > > > > > > diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc > > > > index 5abb91bbdd3..6af7c787372 100644 > > > > --- a/gcc/cp/cp-gimplify.cc > > > > +++ b/gcc/cp/cp-gimplify.cc > > > > @@ -1179,11 +1179,15 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, void *data_) > > > > /* No need to look into types or unevaluated operands. > > > > NB: This affects cp_fold_r as well. */ > > > > - if (TYPE_P (stmt) || unevaluated_p (code) || in_immediate_context ()) > > > > + if (TYPE_P (stmt) || unevaluated_p (code)) > > > > { > > > > *walk_subtrees = 0; > > > > return NULL_TREE; > > > > } > > > > + else if (in_immediate_context ()) > > > > + /* Don't clear *walk_subtrees here: we still need to walk the subtrees > > > > + of SIZEOF_EXPR and similar. */ > > > > + return NULL_TREE; > > > > tree decl = NULL_TREE; > > > > bool call_p = false; > > > > diff --git a/gcc/testsuite/g++.dg/template/sizeof18.C b/gcc/testsuite/g++.dg/template/sizeof18.C > > > > new file mode 100644 > > > > index 00000000000..afba9946258 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/g++.dg/template/sizeof18.C > > > > @@ -0,0 +1,8 @@ > > > > +// PR c++/112869 > > > > +// { dg-do compile } > > > > + > > > > +void min(long, long); > > > > +template void Binaryread(int &, T, unsigned long); > > > > +template <> void Binaryread(int &, float, unsigned long bytecount) { > > > > + min(bytecount, sizeof(int)); > > > > +} > > > > > > Hmm, actually, why does the above make a difference for this testcase? > > > > > > ... > > > > > > It seems that in_immediate_context always returns true in cp_fold_function > > > because current_binding_level->kind == sk_template_parms. That seems like a > > > problem. Maybe for cp_fold_immediate_r we only want to check > > > cp_unevaluated_operand or DECL_IMMEDIATE_CONTEXT (current_function_decl)? > > > > Yeah, I suppose that could become an issue. How about this, then? > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > > This test shows that we cannot clear *walk_subtrees in > > cp_fold_immediate_r when we're in_immediate_context, because that, > > as the comment says, affects cp_fold_r as well. Here we had an > > expression with > > > > min ((long int) VIEW_CONVERT_EXPR(bytecount), (long int) <<< Unknown tree: sizeof_expr > > (int) <<< error >>> >>>) > > > > as its sub-expression, and we never evaluated that into > > > > min ((long int) bytecount, 4) > > > > so the SIZEOF_EXPR leaked into the middle end. > > > > (There's still one *walk_subtrees = 0; in cp_fold_immediate_r, but that > > one should be OK.) > > > > PR c++/112869 > > > > gcc/cp/ChangeLog: > > > > * cp-gimplify.cc (cp_fold_immediate_r): Don't clear *walk_subtrees > > in an unevaluated operand or immediate function. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/template/sizeof18.C: New test. > > --- > > gcc/cp/cp-gimplify.cc | 8 +++++++- > > gcc/testsuite/g++.dg/template/sizeof18.C | 8 ++++++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/template/sizeof18.C > > > > diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc > > index c307e1b62db..413ebafbd1a 100644 > > --- a/gcc/cp/cp-gimplify.cc > > +++ b/gcc/cp/cp-gimplify.cc > > @@ -1179,11 +1179,17 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, void *data_) > > /* No need to look into types or unevaluated operands. > > NB: This affects cp_fold_r as well. */ > > - if (TYPE_P (stmt) || unevaluated_p (code) || in_immediate_context ()) > > + if (TYPE_P (stmt) || unevaluated_p (code)) > > { > > *walk_subtrees = 0; > > return NULL_TREE; > > } > > + else if (cp_unevaluated_operand > > + || (current_function_decl > > + && DECL_IMMEDIATE_FUNCTION_P (current_function_decl))) > > It should still be fine to clear *walk_subtrees in these cases; the problem > I was talking about above was that in_immediate_context was returning true > for all functions, not just consteval functions. > > I think the fix is not adding an else, but rather replacing the > in_immediate_context call with "unevaluated or consteval cfun". Aha, sorry, I misunderstood what you meant. > > + /* Don't clear *walk_subtrees here: we still need to walk the subtrees > > + of SIZEOF_EXPR and similar. */ > > We shouldn't need to walk subtrees of SIZEOF_EXPR or other unevaluated > operands, they'll all get cp_folded away. The bug was that we weren't > calling cp_fold on the SIZEOF_EXPR itself. Yes. I suppose the comment should have read "because of SIZEOF_EXPR". Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- This test shows that we cannot clear *walk_subtrees in cp_fold_immediate_r when we're in_immediate_context, because that checks even e.g. sk_template_parms, and, as the comment says, affects cp_fold_r as well. Here we had an expression with min ((long int) VIEW_CONVERT_EXPR(bytecount), (long int) <<< Unknown tree: sizeof_expr (int) <<< error >>> >>>) as its sub-expression, and we never evaluated that into min ((long int) bytecount, 4) so the SIZEOF_EXPR leaked into the middle end. We need to make sure we are calling cp_fold on the SIZEOF_EXPR. PR c++/112869 gcc/cp/ChangeLog: * cp-gimplify.cc (cp_fold_immediate_r): Check cp_unevaluated_operand and DECL_IMMEDIATE_FUNCTION_P rather than in_immediate_context. gcc/testsuite/ChangeLog: * g++.dg/template/sizeof18.C: New test. --- gcc/cp/cp-gimplify.cc | 8 +++++++- gcc/testsuite/g++.dg/template/sizeof18.C | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/template/sizeof18.C base-commit: 83088b331cde0843d65d316e554873ef6d7b6bca diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index c307e1b62db..64049f4154e 100644 --- a/gcc/cp/cp-gimplify.cc +++ b/gcc/cp/cp-gimplify.cc @@ -1179,7 +1179,13 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, void *data_) /* No need to look into types or unevaluated operands. NB: This affects cp_fold_r as well. */ - if (TYPE_P (stmt) || unevaluated_p (code) || in_immediate_context ()) + if (TYPE_P (stmt) + || unevaluated_p (code) + /* We do not use in_immediate_context here because it checks + more than is desirable, e.g., sk_template_parms. */ + || cp_unevaluated_operand + || (current_function_decl + && DECL_IMMEDIATE_FUNCTION_P (current_function_decl))) { *walk_subtrees = 0; return NULL_TREE; diff --git a/gcc/testsuite/g++.dg/template/sizeof18.C b/gcc/testsuite/g++.dg/template/sizeof18.C new file mode 100644 index 00000000000..afba9946258 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/sizeof18.C @@ -0,0 +1,8 @@ +// PR c++/112869 +// { dg-do compile } + +void min(long, long); +template void Binaryread(int &, T, unsigned long); +template <> void Binaryread(int &, float, unsigned long bytecount) { + min(bytecount, sizeof(int)); +}