From patchwork Thu Aug 20 01:44:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 508893 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 7510A1401F6 for ; Thu, 20 Aug 2015 11:44:58 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=MghGqYqe; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=S9M45zcEpBHSWONRW1h0ozGx1GNnhJghWf0ItzKT2gGzCdIOiH f9Q7Odj8mgTCK+fCQtRt30LbRNJ3uZOgJ5k6ZFS92Aqau+OkRcL17pU6NL9JJxBy HkVFRXi0s93H1KI0+6HEIxK5WxPL88DGGaWG09DaRVcTQg4gnVezAYoFI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=+b3ddQH6vZbLWh+VAE2IlMmkUl0=; b=MghGqYqexO9COdNe6xTf o0OfxlCEZb6OQX3CqX+v4o4Ea+Yko6hhXCBWk/DEXZZcnQUqUZqpAsjz1a9o2yod ooGGswk1oLi7RkNH1UjtGs1DOGZCxRvk1VoWIE9dbRQbGzGlSEB43YElYwa+ilJ/ 9CsfF0a4TyaBKN67e/Nk7k8= Received: (qmail 42600 invoked by alias); 20 Aug 2015 01:44:45 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 42590 invoked by uid 89); 20 Aug 2015 01:44:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 20 Aug 2015 01:44:41 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 61BDF8E243 for ; Thu, 20 Aug 2015 01:44:40 +0000 (UTC) Received: from [10.10.116.45] (ovpn-116-45.rdu2.redhat.com [10.10.116.45]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7K1idnr006505 for ; Wed, 19 Aug 2015 21:44:39 -0400 To: gcc-patches List From: Jason Merrill Subject: C++ PATCH for c++/66957 (protected access) Message-ID: <55D53107.202@redhat.com> Date: Wed, 19 Aug 2015 21:44:39 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 The fix for bug 38579 was correct, but due to other bugs with our handling of protected access, it introduced bug 66957. The basic problem here was that [class.access.base] says, A member m is accessible at the point R when named in class N if -- m as a member of N is public, or -- m as a member of N is private, and R occurs in a member or friend of class N, or -- m as a member of N is protected, and R occurs in a member or friend of class N, or in a member or friend of a class P derived from N, where m as a member of P is public, private, or protected, or -- there exists a base class B of N that is accessible at R, and m is accessible at R when named in class B. And the last bullet indicates that we need to check the other bullets for all of N's accessible bases. But in our base walk we were only checking the private bullet, so we missed that DerivedB is a suitable class P derived from BaseClass, allowing the access. When I fixed dfs_accessible_post to check the protected access bullet, I discovered that friend_accessible_p was also significantly broken: we weren't handling class templates, and add_friend was failing to set up DECL_BEFRIENDING_CLASSES for multiple friend functions of the same name. I also needed to stop walking up when we reach an access declaration for the member to avoid incorrectly seeing more permissive access higher up. So, this needed a surprising amount of overhaul for code as old as it was. Tested x86_64-pc-linux-gnu, applying to trunk. commit 6e7040b215a8906787609e1f0fefb70d5b23edac Author: Jason Merrill Date: Wed Aug 19 17:45:52 2015 -0400 PR c++/66957 * search.c (protected_accessible_p): Remove redundant access_in_type. Add otype parm instead of walking binfo. (friend_accessible_p): Check SCOPE itself. Handle class templates. Pass through otype. (dfs_accessible_post): Handle all accessibility cases. (dfs_accessible_pre): New. (accessible_p): Use it. Don't check protected access here. Pass decl and otype to dfs_walk. (member_declared_in_type, dfs_access_in_type_pre): New. (access_in_type): Use dfs_access_in_type_pre. * friend.c (add_friend): Fix multiple friends with the same name. diff --git a/gcc/cp/friend.c b/gcc/cp/friend.c index e107cee..f53ce27 100644 --- a/gcc/cp/friend.c +++ b/gcc/cp/friend.c @@ -156,11 +156,9 @@ add_friend (tree type, tree decl, bool complain) } } - maybe_add_class_template_decl_list (type, decl, /*friend_p=*/1); - TREE_VALUE (list) = tree_cons (NULL_TREE, decl, TREE_VALUE (list)); - return; + break; } list = TREE_CHAIN (list); } @@ -172,9 +170,10 @@ add_friend (tree type, tree decl, bool complain) maybe_add_class_template_decl_list (type, decl, /*friend_p=*/1); - DECL_FRIENDLIST (typedecl) - = tree_cons (DECL_NAME (decl), build_tree_list (NULL_TREE, decl), - DECL_FRIENDLIST (typedecl)); + if (!list) + DECL_FRIENDLIST (typedecl) + = tree_cons (DECL_NAME (decl), build_tree_list (NULL_TREE, decl), + DECL_FRIENDLIST (typedecl)); if (!uses_template_parms (type)) DECL_BEFRIENDING_CLASSES (decl) = tree_cons (NULL_TREE, type, diff --git a/gcc/cp/search.c b/gcc/cp/search.c index 90cd243..42db122 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -58,8 +58,6 @@ static tree dfs_walk_once_accessible (tree, bool, void *data); static tree dfs_access_in_type (tree, void *); static access_kind access_in_type (tree, tree); -static int protected_accessible_p (tree, tree, tree); -static int friend_accessible_p (tree, tree, tree); static tree dfs_get_pure_virtuals (tree, void *); @@ -582,9 +580,36 @@ context_for_name_lookup (tree decl) return context; } +/* Returns true iff DECL is declared in TYPE. */ + +static bool +member_declared_in_type (tree decl, tree type) +{ + /* A normal declaration obviously counts. */ + if (context_for_name_lookup (decl) == type) + return true; + /* So does a using or access declaration. */ + if (DECL_LANG_SPECIFIC (decl) && !DECL_DISCRIMINATOR_P (decl) + && purpose_member (type, DECL_ACCESS (decl))) + return true; + return false; +} + /* The accessibility routines use BINFO_ACCESS for scratch space during the computation of the accessibility of some declaration. */ +/* Avoid walking up past a declaration of the member. */ + +static tree +dfs_access_in_type_pre (tree binfo, void *data) +{ + tree decl = (tree) data; + tree type = BINFO_TYPE (binfo); + if (member_declared_in_type (decl, type)) + return dfs_skip_bases; + return NULL_TREE; +} + #define BINFO_ACCESS(NODE) \ ((access_kind) ((TREE_PUBLIC (NODE) << 1) | TREE_PRIVATE (NODE))) @@ -705,19 +730,17 @@ access_in_type (tree type, tree decl) The algorithm we use is to make a post-order depth-first traversal of the base-class hierarchy. As we come up the tree, we annotate each node with the most lenient access. */ - dfs_walk_once (binfo, NULL, dfs_access_in_type, decl); + dfs_walk_once (binfo, dfs_access_in_type_pre, dfs_access_in_type, decl); return BINFO_ACCESS (binfo); } -/* Returns nonzero if it is OK to access DECL through an object - indicated by BINFO in the context of DERIVED. */ +/* Returns nonzero if it is OK to access DECL named in TYPE through an object + of OTYPE in the context of DERIVED. */ static int -protected_accessible_p (tree decl, tree derived, tree binfo) +protected_accessible_p (tree decl, tree derived, tree type, tree otype) { - access_kind access; - /* We're checking this clause from [class.access.base] m as a member of N is protected, and the reference occurs in a @@ -725,16 +748,10 @@ protected_accessible_p (tree decl, tree derived, tree binfo) class P derived from N, where m as a member of P is public, private or protected. - Here DERIVED is a possible P, DECL is m and BINFO_TYPE (binfo) is N. */ + Here DERIVED is a possible P, DECL is m and TYPE is N. */ /* If DERIVED isn't derived from N, then it can't be a P. */ - if (!DERIVED_FROM_P (BINFO_TYPE (binfo), derived)) - return 0; - - access = access_in_type (derived, decl); - - /* If m is inaccessible in DERIVED, then it's not a P. */ - if (access == ak_none) + if (!DERIVED_FROM_P (type, derived)) return 0; /* [class.protected] @@ -748,33 +765,38 @@ protected_accessible_p (tree decl, tree derived, tree binfo) derived from that class) (_expr.ref_). If the access is to form a pointer to member, the nested-name-specifier shall name the derived class (or any class derived from that class). */ - if (DECL_NONSTATIC_MEMBER_P (decl)) - { - /* We can tell through what the reference is occurring by - chasing BINFO up to the root. */ - tree t = binfo; - while (BINFO_INHERITANCE_CHAIN (t)) - t = BINFO_INHERITANCE_CHAIN (t); - - if (!DERIVED_FROM_P (derived, BINFO_TYPE (t))) - return 0; - } + if (DECL_NONSTATIC_MEMBER_P (decl) + && !DERIVED_FROM_P (derived, otype)) + return 0; return 1; } -/* Returns nonzero if SCOPE is a friend of a type which would be able - to access DECL through the object indicated by BINFO. */ +/* Returns nonzero if SCOPE is a type or a friend of a type which would be able + to access DECL through TYPE. OTYPE is the type of the object. */ static int -friend_accessible_p (tree scope, tree decl, tree binfo) +friend_accessible_p (tree scope, tree decl, tree type, tree otype) { + /* We're checking this clause from [class.access.base] + + m as a member of N is protected, and the reference occurs in a + member or friend of class N, or in a member or friend of a + class P derived from N, where m as a member of P is public, private + or protected. + + Here DECL is m and TYPE is N. SCOPE is the current context, + and we check all its possible Ps. */ tree befriending_classes; tree t; if (!scope) return 0; + /* Is SCOPE itself a suitable P? */ + if (TYPE_P (scope) && protected_accessible_p (decl, scope, type, otype)) + return 1; + if (DECL_DECLARES_FUNCTION_P (scope)) befriending_classes = DECL_BEFRIENDING_CLASSES (scope); else if (TYPE_P (scope)) @@ -783,54 +805,113 @@ friend_accessible_p (tree scope, tree decl, tree binfo) return 0; for (t = befriending_classes; t; t = TREE_CHAIN (t)) - if (protected_accessible_p (decl, TREE_VALUE (t), binfo)) + if (protected_accessible_p (decl, TREE_VALUE (t), type, otype)) return 1; /* Nested classes have the same access as their enclosing types, as - per DR 45 (this is a change from the standard). */ + per DR 45 (this is a change from C++98). */ if (TYPE_P (scope)) - for (t = TYPE_CONTEXT (scope); t && TYPE_P (t); t = TYPE_CONTEXT (t)) - if (protected_accessible_p (decl, t, binfo)) - return 1; + if (friend_accessible_p (TYPE_CONTEXT (scope), decl, type, otype)) + return 1; if (DECL_DECLARES_FUNCTION_P (scope)) { /* Perhaps this SCOPE is a member of a class which is a friend. */ if (DECL_CLASS_SCOPE_P (scope) - && friend_accessible_p (DECL_CONTEXT (scope), decl, binfo)) + && friend_accessible_p (DECL_CONTEXT (scope), decl, type, otype)) return 1; + } - /* Or an instantiation of something which is a friend. */ - if (DECL_TEMPLATE_INFO (scope)) + /* Maybe scope's template is a friend. */ + if (tree tinfo = get_template_info (scope)) + { + tree tmpl = TI_TEMPLATE (tinfo); + if (DECL_CLASS_TEMPLATE_P (tmpl)) + tmpl = TREE_TYPE (tmpl); + else + tmpl = DECL_TEMPLATE_RESULT (tmpl); + if (tmpl != scope) { - int ret; /* Increment processing_template_decl to make sure that dependent_type_p works correctly. */ ++processing_template_decl; - ret = friend_accessible_p (DECL_TI_TEMPLATE (scope), decl, binfo); + int ret = friend_accessible_p (tmpl, decl, type, otype); --processing_template_decl; - return ret; + if (ret) + return 1; } } + /* If is_friend is true, we should have found a befriending class. */ + gcc_checking_assert (!is_friend (type, scope)); + return 0; } +struct dfs_accessible_data +{ + tree decl; + tree object_type; +}; + +/* Avoid walking up past a declaration of the member. */ + +static tree +dfs_accessible_pre (tree binfo, void *data) +{ + dfs_accessible_data *d = (dfs_accessible_data *)data; + tree type = BINFO_TYPE (binfo); + if (member_declared_in_type (d->decl, type)) + return dfs_skip_bases; + return NULL_TREE; +} + /* Called via dfs_walk_once_accessible from accessible_p */ static tree -dfs_accessible_post (tree binfo, void * /*data*/) +dfs_accessible_post (tree binfo, void *data) { - if (BINFO_ACCESS (binfo) != ak_none) + /* access_in_type already set BINFO_ACCESS for us. */ + access_kind access = BINFO_ACCESS (binfo); + tree N = BINFO_TYPE (binfo); + dfs_accessible_data *d = (dfs_accessible_data *)data; + tree decl = d->decl; + tree scope = current_nonlambda_scope (); + + /* A member m is accessible at the point R when named in class N if */ + switch (access) { - tree scope = current_scope (); - if (scope && TREE_CODE (scope) != NAMESPACE_DECL - && is_friend (BINFO_TYPE (binfo), scope)) - return binfo; - } + case ak_none: + return NULL_TREE; - return NULL_TREE; + case ak_public: + /* m as a member of N is public, or */ + return binfo; + + case ak_private: + { + /* m as a member of N is private, and R occurs in a member or friend of + class N, or */ + if (scope && TREE_CODE (scope) != NAMESPACE_DECL + && is_friend (N, scope)) + return binfo; + return NULL_TREE; + } + + case ak_protected: + { + /* m as a member of N is protected, and R occurs in a member or friend + of class N, or in a member or friend of a class P derived from N, + where m as a member of P is public, private, or protected */ + if (friend_accessible_p (scope, decl, N, d->object_type)) + return binfo; + return NULL_TREE; + } + + default: + gcc_unreachable (); + } } /* Like accessible_p below, but within a template returns true iff DECL is @@ -858,21 +939,15 @@ int accessible_p (tree type, tree decl, bool consider_local_p) { tree binfo; - tree scope; access_kind access; - /* Nonzero if it's OK to access DECL if it has protected - accessibility in TYPE. */ - int protected_ok = 0; - /* If this declaration is in a block or namespace scope, there's no access control. */ if (!TYPE_P (context_for_name_lookup (decl))) return 1; /* There is no need to perform access checks inside a thunk. */ - scope = current_scope (); - if (scope && DECL_THUNK_P (scope)) + if (current_function_decl && DECL_THUNK_P (current_function_decl)) return 1; /* In a template declaration, we cannot be sure whether the @@ -886,13 +961,18 @@ accessible_p (tree type, tree decl, bool consider_local_p) && (!processing_template_parmlist || processing_template_decl > 1)) return 1; + tree otype; if (!TYPE_P (type)) { - binfo = type; + /* When accessing a non-static member, the most derived type in the + binfo chain is the type of the object; remember that type for + protected_accessible_p. */ + for (tree b = type; b; b = BINFO_INHERITANCE_CHAIN (b)) + otype = BINFO_TYPE (b); type = BINFO_TYPE (type); } else - binfo = TYPE_BINFO (type); + otype = type; /* [class.access.base] @@ -905,7 +985,7 @@ accessible_p (tree type, tree decl, bool consider_local_p) --m as a member of N is protected, and the reference occurs in a member or friend of class N, or in a member or friend of a - class P derived from N, where m as a member of P is private or + class P derived from N, where m as a member of P is public, private or protected, or --there exists a base class B of N that is accessible at the point @@ -913,40 +993,28 @@ accessible_p (tree type, tree decl, bool consider_local_p) We walk the base class hierarchy, checking these conditions. */ - if (consider_local_p) - { - /* Figure out where the reference is occurring. Check to see if - DECL is private or protected in this scope, since that will - determine whether protected access is allowed. */ - tree ct = current_nonlambda_class_type (); - if (ct) - protected_ok = protected_accessible_p (decl, - ct, - binfo); - - /* Now, loop through the classes of which we are a friend. */ - if (!protected_ok) - protected_ok = friend_accessible_p (scope, decl, binfo); - } - - /* Standardize the binfo that access_in_type will use. We don't - need to know what path was chosen from this point onwards. */ + /* We walk using TYPE_BINFO (type) because access_in_type will set + BINFO_ACCESS on it and its bases. */ binfo = TYPE_BINFO (type); /* Compute the accessibility of DECL in the class hierarchy dominated by type. */ access = access_in_type (type, decl); - if (access == ak_public - || (access == ak_protected && protected_ok)) + if (access == ak_public) return 1; + /* If we aren't considering the point of reference, only the first bullet + applies. */ if (!consider_local_p) return 0; + dfs_accessible_data d = { decl, otype }; + /* Walk the hierarchy again, looking for a base class that allows access. */ return dfs_walk_once_accessible (binfo, /*friends=*/true, - NULL, dfs_accessible_post, NULL) + dfs_accessible_pre, + dfs_accessible_post, &d) != NULL_TREE; } diff --git a/gcc/testsuite/g++.dg/inherit/access9.C b/gcc/testsuite/g++.dg/inherit/access9.C new file mode 100644 index 0000000..cdbc640 --- /dev/null +++ b/gcc/testsuite/g++.dg/inherit/access9.C @@ -0,0 +1,14 @@ +// PR c++/66957 + +class BaseClass { +protected: + static int x; +}; + +struct DerivedA : BaseClass { }; + +struct DerivedB : BaseClass { + DerivedB() { + (void) DerivedA::x; + } +}; commit f99ba899fa437c837755e9ad300890f296900930 Author: Jason Merrill Date: Wed Aug 19 17:45:01 2015 -0400 * lambda.c (current_nonlambda_scope): New. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 7cf5278..4dee60c 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6308,6 +6308,7 @@ extern tree lambda_expr_this_capture (tree, bool); extern tree maybe_resolve_dummy (tree, bool); extern tree current_nonlambda_function (void); extern tree nonlambda_method_basetype (void); +extern tree current_nonlambda_scope (void); extern void maybe_add_lambda_conv_op (tree); extern bool is_lambda_ignored_entity (tree); diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index dcd3bb9..ea9dba0 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -817,6 +817,30 @@ nonlambda_method_basetype (void) return TYPE_METHOD_BASETYPE (TREE_TYPE (fn)); } +/* Like current_scope, but looking through lambdas. */ + +tree +current_nonlambda_scope (void) +{ + tree scope = current_scope (); + for (;;) + { + if (TREE_CODE (scope) == FUNCTION_DECL + && LAMBDA_FUNCTION_P (scope)) + { + scope = CP_TYPE_CONTEXT (DECL_CONTEXT (scope)); + continue; + } + else if (LAMBDA_TYPE_P (scope)) + { + scope = CP_TYPE_CONTEXT (scope); + continue; + } + break; + } + return scope; +} + /* Helper function for maybe_add_lambda_conv_op; build a CALL_EXPR with indicated FN and NARGS, but do not initialize the return type or any of the argument slots. */