From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id E3ECB3854813 for ; Fri, 8 Jan 2021 01:12:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E3ECB3854813 Received: from mail-io1-f71.google.com (mail-io1-f71.google.com [209.85.166.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-128-NveQdxJdO46vXkyzYZAUug-1; Thu, 07 Jan 2021 20:12:18 -0500 X-MC-Unique: NveQdxJdO46vXkyzYZAUug-1 Received: by mail-io1-f71.google.com with SMTP id c7so6502016iob.10 for ; Thu, 07 Jan 2021 17:12:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=F9GKYeP4XEHWGfa3DNmIAt+Fcqy03w5F7HE80x8GetA=; b=Qogo8Ixaar0XK47RbQvkpbC8S2MAe+D3TxSAxms2Y8UAbTrpeMPYG28PpkFgJBg77J XbAURnMqGyrM9YsAFXvXjt9erckX9j0LRDx/r8GlNXsgbBEgx5sIRhHq7kP/THSvAyPK nPDWNojqDGgAVy3+68zB8udcPBeVAV9U/t92YeLZykVkOM/owE1CAfcYqkQlS8eplVCL xdNX0MVw/kehEAXk671uMbr0OTenmhP4//pxkcxpqn+YXKs4hru6ilU80V+l7mfK0cMQ LKYDplIUsICQc90hL5HihJ9LJrG2osJZ6ejwPtRBJ8ZgmXHgLowacRWeGTDTUTGnYkkx Sb6A== X-Gm-Message-State: AOAM533LLoFHvtmkvoOe/i4xjchLmVS45Te1PZ3TjTnkUtV3VkcJPveM ctyg3GeJTOhzzUXdwvS2YOlkcHoVRQ+2B99lzFee5S5HJcYPqW+TLJ/WMU3+JugNNFRlR3E3fyD qaQj6e8ZBPKqvJbOK5V3DNGrNMwrLYUHDZ9URSa73ol35HCTWLoRUn8LJsL6G7qcE5g== X-Received: by 2002:a5e:de08:: with SMTP id e8mr3439854iok.203.1610068337156; Thu, 07 Jan 2021 17:12:17 -0800 (PST) X-Google-Smtp-Source: ABdhPJzkEAiNUgdCAexBWKdw/yp+QMb5NbUR1OIz3C9f8+kcdIER8CG2kpjPZd0PjEMcJmURoQ2OjA== X-Received: by 2002:a5e:de08:: with SMTP id e8mr3439831iok.203.1610068336729; Thu, 07 Jan 2021 17:12:16 -0800 (PST) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id e25sm3988184iom.40.2021.01.07.17.12.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Jan 2021 17:12:15 -0800 (PST) Subject: Re: [PATCH] c++: Fix access checking of scoped non-static member [PR98515] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20210106181932.1393432-1-ppalka@redhat.com> <30ad8ec-9fd-fe-b5b1-3fac7e3e6fb@idea> From: Jason Merrill Message-ID: <6f105a0f-c591-5995-82c3-a1d4a9d4109e@redhat.com> Date: Thu, 7 Jan 2021 20:12:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.3 MIME-Version: 1.0 In-Reply-To: <30ad8ec-9fd-fe-b5b1-3fac7e3e6fb@idea> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-16.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, 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: Fri, 08 Jan 2021 01:12:21 -0000 On 1/7/21 5:47 PM, Patrick Palka wrote: > On Thu, 7 Jan 2021, Jason Merrill wrote: > >> On 1/6/21 1:19 PM, Patrick Palka wrote: >>> In the first testcase below, we incorrectly reject the use of the >>> protected non-static member A::var0 from C::g() because >>> check_accessibility_of_qualified_id, at template parse time, determines >>> that the access doesn't go through 'this'. (This happens because the >>> dependent base B of C doesn't have a binfo object, so it appears >>> to DERIVED_FROM_P that A is not an indirect base of C.) From there >>> we create the corresponding deferred access check, which we then >>> perform at instantiation time and which (unsurprisingly) fails. >>> >>> The problem ultimately seems to be that we can't, in general, know >>> whether a use of a scoped non-static member goes through 'this' until >>> instantiation time, as the second testcase below demonstrates. So this >>> patch makes check_accessibility_of_qualified_id punt in this situation. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to >>> commit? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/98515 >>> * semantics.c (check_accessibility_of_qualified_id): Punt if >>> we're checking the access of a scoped non-static member at >>> class template parse time. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/98515 >>> * g++.dg/template/access32.C: New test. >>> * g++.dg/template/access33.C: New test. >>> --- >>> gcc/cp/semantics.c | 20 +++++++++++++++----- >>> gcc/testsuite/g++.dg/template/access32.C | 8 ++++++++ >>> gcc/testsuite/g++.dg/template/access33.C | 9 +++++++++ >>> 3 files changed, 32 insertions(+), 5 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/template/access32.C >>> create mode 100644 gcc/testsuite/g++.dg/template/access33.C >>> >>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c >>> index b448efe024a..f52b2e4d1e7 100644 >>> --- a/gcc/cp/semantics.c >>> +++ b/gcc/cp/semantics.c >>> @@ -2107,14 +2107,24 @@ check_accessibility_of_qualified_id (tree decl, >>> /* If the reference is to a non-static member of the >>> current class, treat it as if it were referenced through >>> `this'. */ >>> - tree ct; >>> if (DECL_NONSTATIC_MEMBER_P (decl) >>> - && current_class_ptr >>> - && DERIVED_FROM_P (scope, ct = current_nonlambda_class_type ())) >>> - qualifying_type = ct; >>> + && current_class_ptr) >>> + { >>> + if (dependent_type_p (TREE_TYPE (current_class_ptr))) >> >> This should also look at current_nonlambda_class_type. > > Ah, ack. But it seems to me we really only need to be checking > dependence of current_nonlambda_class_type here. Yes, that's what I meant, sorry about the ambiguous use of "also". :) > IIUC, dependence of > these two types should coincide except in the case where we're inside a > generic lambda within a non-template class (in which case > current_class_ptr will dependent and current_nonlambda_class_type won't). > But in this case we have enough information to be able to resolve the > access check at parse time, I think (and so we shouldn't punt). > > The below patch, which seems to pass 'make check-c++', checks the > dependence of current_nonlambda_class_type instead of that of > current_class_ptr. Does this approach seem right? OK. > -- >8 -- > > Subject: [PATCH] c++: Fix access checking of scoped non-static member > [PR98515] > > In the first testcase below, we incorrectly reject the use of the > protected non-static member A::var0 from C::g() because > check_accessibility_of_qualified_id, at template parse time, determines > that the access doesn't go through 'this'. (This happens because the > dependent base B of C doesn't have a binfo object, so it appears > to DERIVED_FROM_P that A is not an indirect base of C.) From there > we create the corresponding deferred access check, which we then > perform at instantiation time and which (unsurprisingly) fails. > > The problem ultimately seems to be that we can't in general determine > whether a use of a scoped non-static member goes through 'this' until > instantiation time, as the second testcase below illustrates. So this > patch makes check_accessibility_of_qualified_id punt in such situations > to avoid creating a bogus deferred access check. > > gcc/cp/ChangeLog: > > PR c++/98515 > * semantics.c (check_accessibility_of_qualified_id): Punt if > we're checking access of a scoped non-static member inside a > class template. > > gcc/testsuite/ChangeLog: > > PR c++/98515 > * g++.dg/template/access32.C: New test. > * g++.dg/template/access33.C: New test. > --- > gcc/cp/semantics.c | 22 +++++++++++++++++----- > gcc/testsuite/g++.dg/template/access32.C | 8 ++++++++ > gcc/testsuite/g++.dg/template/access33.C | 9 +++++++++ > 3 files changed, 34 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/access32.C > create mode 100644 gcc/testsuite/g++.dg/template/access33.C > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > index b448efe024a..51f7c114b03 100644 > --- a/gcc/cp/semantics.c > +++ b/gcc/cp/semantics.c > @@ -2107,14 +2107,26 @@ check_accessibility_of_qualified_id (tree decl, > /* If the reference is to a non-static member of the > current class, treat it as if it were referenced through > `this'. */ > - tree ct; > if (DECL_NONSTATIC_MEMBER_P (decl) > - && current_class_ptr > - && DERIVED_FROM_P (scope, ct = current_nonlambda_class_type ())) > - qualifying_type = ct; > + && current_class_ptr) > + { > + if (tree current = current_nonlambda_class_type ()) > + { > + if (dependent_type_p (current)) > + /* In general we can't know whether this access goes through > + `this' until instantiation time. Punt now, or else we might > + create a deferred access check that's not relative to 'this' > + when it ought to be. We'll check this access again after > + substitution, e.g. from tsubst_qualified_id. */ > + return true; > + > + if (DERIVED_FROM_P (scope, current)) > + qualifying_type = current; > + } > + } > /* Otherwise, use the type indicated by the > nested-name-specifier. */ > - else > + if (!qualifying_type) > qualifying_type = nested_name_specifier; > } > else > diff --git a/gcc/testsuite/g++.dg/template/access32.C b/gcc/testsuite/g++.dg/template/access32.C > new file mode 100644 > index 00000000000..08faa9f0f97 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/access32.C > @@ -0,0 +1,8 @@ > +// PR c++/98515 > +// { dg-do compile } > + > +struct A { protected: int var0; }; > +template struct B : public A { }; > +template struct C : public B { void g(); }; > +template void C::g() { A::var0++; } > +template class C; > diff --git a/gcc/testsuite/g++.dg/template/access33.C b/gcc/testsuite/g++.dg/template/access33.C > new file mode 100644 > index 00000000000..9fb9b9a1236 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/access33.C > @@ -0,0 +1,9 @@ > +// PR c++/98515 > +// { dg-do compile } > + > +struct A { protected: int var0; }; > +template struct B : public A { }; > +template struct C : public B { void g(); }; > +template void C::g() { A::var0++; } // { dg-error "protected|invalid" } > +template <> struct B { }; > +template class C; >