From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1208 invoked by alias); 6 Dec 2018 01:10:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 1189 invoked by uid 89); 6 Dec 2018 01:10:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=pushes, substitute, collapse, uncomfortable X-HELO: mail-qk1-f193.google.com Received: from mail-qk1-f193.google.com (HELO mail-qk1-f193.google.com) (209.85.222.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Dec 2018 01:10:52 +0000 Received: by mail-qk1-f193.google.com with SMTP id d19so13110481qkg.5 for ; Wed, 05 Dec 2018 17:10:52 -0800 (PST) Return-Path: Received: from [192.168.1.132] (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 94sm23141201qks.8.2018.12.05.17.10.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Dec 2018 17:10:50 -0800 (PST) Subject: Re: [PATCH] [PR86823] retain deferred access checks from outside firewall To: Alexandre Oliva , gcc-patches@gcc.gnu.org Cc: nathan@acm.org References: From: Jason Merrill Message-ID: Date: Thu, 06 Dec 2018 01:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------C817AAFCAE7CF11D2F757E07" X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00330.txt.bz2 This is a multi-part message in MIME format. --------------C817AAFCAE7CF11D2F757E07 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2740 On 11/16/18 9:16 PM, Alexandre Oliva wrote: > We used to preserve deferred access check along with resolved template > ids, but a tentative parsing firewall introduced additional layers of > deferred access checks, so that we don't preserve the checks we > want to any more. > > This patch collapses the access check levels introduced by the > firewall with those we wanted to preserve to begin with, saves them, > pushes them one more layer up so that the firewall doesn't drop them, > and then pushes new empty levels so balance out with the upcoming > popping. > > We may want to avoid the pop_to_parent and one push after the save, and > instead leave the firewall's scope before the final pop_to_parent. > However, this patch is what I regression-tested successfully with > check-g++ on x86_64-linux-gnu. Regstrapping on i686- and > x86_64-linux-gnu now. Ok to install? > > > for gcc/cp/ChangeLog > > PR c++/86823 > * parser.c (cp_parser_template_id): Merge access checks from > outside the tentative parsing firewall with those from inside, > and save them all with the template id token. > > for gcc/testsuite/ChangeLog > > PR c++/86823 > * g++.dg/pr86823.C: New. > --- > gcc/cp/parser.c | 12 ++++++++++++ > gcc/testsuite/g++.dg/pr86823.C | 15 +++++++++++++++ > 2 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/pr86823.C > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index db0f0338179e..6a08b09715a7 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -16185,7 +16185,19 @@ cp_parser_template_id (cp_parser *parser, > so the memory will not be reclaimed during token replacing below. */ > token->u.tree_check_value = ggc_cleared_alloc (); > token->u.tree_check_value->value = template_id; > + /* Collapse the access check levels introduced by > + tentative_firewall with the one we introduced ourselves. */ > + pop_to_parent_deferring_access_checks (); > + pop_to_parent_deferring_access_checks (); > token->u.tree_check_value->checks = get_deferred_access_checks (); > + /* Push the checks up one more level, so that the firewall > + doesn't drop them on the floor when we return. Then, push > + empty levels back in place so that they are popped > + properly. */ > + pop_to_parent_deferring_access_checks (); > + push_deferring_access_checks (dk_deferred); > + push_deferring_access_checks (dk_deferred); > + push_deferring_access_checks (dk_deferred); Hmm, I'm uncomfortable with how this depends on the specific implementation of tentative_firewall. What do you think of this alternate approach (untested other than with the testcase)? Jason --------------C817AAFCAE7CF11D2F757E07 Content-Type: text/x-patch; name="checks.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="checks.diff" Content-length: 1029 diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index ac19cb4b9bb..c7ec7dcf413 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -16182,16 +16182,18 @@ cp_parser_template_id (cp_parser *parser, is_declaration, tag_type, &is_identifier); + + /* Push any access checks inside the firewall we're about to create. */ + vec *checks = get_deferred_access_checks (); + pop_deferring_access_checks (); if (templ == error_mark_node || is_identifier) - { - pop_deferring_access_checks (); - return templ; - } + return templ; /* Since we're going to preserve any side-effects from this parse, set up a firewall to protect our callers from cp_parser_commit_to_tentative_parse in the template arguments. */ tentative_firewall firewall (parser); + reopen_deferring_access_checks (checks); /* If we find the sequence `[:' after a template-name, it's probably a digraph-typo for `< ::'. Substitute the tokens and check if we can --------------C817AAFCAE7CF11D2F757E07--