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 82221396ECAC for ; Thu, 7 Jan 2021 21:37:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 82221396ECAC Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-CBejW-O9PP63BSo-U3PACQ-1; Thu, 07 Jan 2021 16:37:49 -0500 X-MC-Unique: CBejW-O9PP63BSo-U3PACQ-1 Received: by mail-qk1-f197.google.com with SMTP id l138so7403235qke.4 for ; Thu, 07 Jan 2021 13:37:49 -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:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dsZ7Yhzos/lykav34f2Yy0zcqiwyhbaK365GU6fC9r8=; b=QjkQX7e5Ud4xB94DL5OetCRW2aHkCDNta/+wGIMxpaxZqhAdLKCdiEdS3Vpc03bhK/ aE8z8YQvtlX/1QSUv/RRDBkjebFaJ8knhpkFhcT/tVQcqRSVNH6c7qDQw3hAq6ggHwyo Gv8v4wM0lfl/ErJZUzEY/Xx/x24HXl/lXvLW4TzugCLKmbFhnQ8xDmFdLh9TVeVI7Lu+ f3kl5mTggIC/vy/nLToh/0oIZ1RkUS9wuZntHVE6SK7/bQ6/XjCS6+Q0kqvfjbThmPy1 Bjo8TTK6qHRx/d2qWxyGnwyI+JF95qPz4sB8Hea6wn6596DZKnBFbiCDnzggJ9DvYGUR 6BXQ== X-Gm-Message-State: AOAM5332UYWlxwVuVROl1JUBLRcGOlPDQi/CnCn5Q1qoteFZCU4rwfDd YlHhffN6EXVK9t0XQ5NZ7eS9u9GBZMZhp9Qlny0HEfhMfE13If+SsG1LULdhjPg3H9GSXiZPSV8 OIHZy2gyRFvG5u659p1zJnoDpySBcC5CVaiV3J8zhMxZo55OMaqG25GIz5FjNkqcumA== X-Received: by 2002:a37:45cf:: with SMTP id s198mr1059377qka.57.1610055468745; Thu, 07 Jan 2021 13:37:48 -0800 (PST) X-Google-Smtp-Source: ABdhPJxAGItuHvPEh1WabRBfdRzp74RQCh7fInthOSdzVWb/KfW8vC5RllyEVjfBVSeyjYOOOq5HWw== X-Received: by 2002:a37:45cf:: with SMTP id s198mr1059339qka.57.1610055468284; Thu, 07 Jan 2021 13:37:48 -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 v145sm4039179qka.27.2021.01.07.13.37.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Jan 2021 13:37:47 -0800 (PST) Subject: Re: [PATCH] c++: Fix access checking of scoped non-static member [PR98515] To: Patrick Palka , gcc-patches@gcc.gnu.org References: <20210106181932.1393432-1-ppalka@redhat.com> From: Jason Merrill Message-ID: Date: Thu, 7 Jan 2021 16:37:47 -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: <20210106181932.1393432-1-ppalka@redhat.com> 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: Thu, 07 Jan 2021 21:37:52 -0000 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. > + /* In general we can't know whether this access goes through `this' > + until instantiation of the current class. Punt now, or else > + we might create a deferred access check that's relative to the > + wrong class. We'll check this access again after substitution, > + e.g. from tsubst_qualified_id. */ > + return true; > + > + if (tree current = current_nonlambda_class_type ()) > + 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; >