C++ PATCH for c++/66957 (protected access)

C++ PATCH for c++/66957 (protected access)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

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.

[-- Attachment #2: 66957.patch --]
[-- Type: text/x-patch, Size: 16223 bytes --]

commit 6e7040b215a8906787609e1f0fefb70d5b23edac
Author: Jason Merrill <jason@redhat.com>
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 *);
 
 \f
@@ -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 <jason@redhat.com>
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.  */