This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

C++ PATCH: PR 28588


This patch fixes PR c++/28588, an accepts-invalid regression.  (This
one was a lot harder than I had hoped!)

The PR indicates that we were failing to check access on uses of
static function members in contexts other than calls.  For example:

+  Foo foo;
+  void (*f)();
+  f = foo.f; // { dg-error "context" }

Here, we cannot always check for the accessibilty of "Foo::f" at the
point the expression "foo.f" occurs, since, if the function is
overloaded, we don't know which function will be selected.  (Overload
resolution is performed when the assignment is made.)  So, we have to
carry around a BASELINK that indicates how we referred to "f".

In the process of fixing this, I gutted instantiate_type, which was
far more general than it needs to be.  Only a relatively fiew
expression forms can ever have an unresolved type.  I also made
really_overloaded_fn use is_overloaded_fn to eliminate duplicate
almost-identical code.

The second test case below is something that I broke while fixing this
bug (and therefore un-broke in this final patch).  The only place that
caught this bug was the libstdc++ testsuite, and it did so in a subtle
way, so I created a stand-alone test.

Tested on x86_64-unknown-linux-gnu, applied to mainline.  I do not
intend to apply this to 4.1, since (a) it fixes an accepts-invalid,
(which means there may be code depending on this) and there's no
semantic problem with accepting the code, (b) it's a relatively big
patch.

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

2006-08-26  Mark Mitchell  <mark@codesourcery.com>

	PR c++/28588
	* class.c (resolve_address_of_overloaded_function): Add
	access_path parameter.  Perform access checks.
	(instantiate_type): Adjust call to
	resolve_address_of_overloaded_function.  Remove unnecessary code.
	* tree.c (is_overloaded_fn): Document.  Return 2 when there are
	acutally multiple functions.
	(really_overloaded_fn): Use is_overloaded_fn.
	* mangle.c (write_expression): Handle BASELINKs.
	* cp-tree.h (really_overloaded_fn): Return bool.
	(baselink_for_fns): Declare.
	* search.c (lookup_member): Check access for single static
	functions here.
	* pt.c (convert_nontype_argument_function): Handle BASELINKs.
	(tsubst_copy_and_build): Generate BASELINKs for template-ids.
	* semantics.c (finish_call_expr): Use baselink_for_fns.
	(baselink_for_fns): New function.
	(finish_id_expression): Use it.
	* parser.c (cp_parser_template_argument): Don't strip BASELINKs.

2006-08-26  Mark Mitchell  <mark@codesourcery.com>

	PR c++/28588
	* g++.dg/inherit/access6.C: New test.
	* g++.dg/inherit/access7.C: Likewise.

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 116430)
+++ gcc/cp/class.c	(working copy)
@@ -134,8 +134,6 @@ static int method_name_cmp (const void *
 static int resort_method_name_cmp (const void *, const void *);
 static void add_implicitly_declared_members (tree, int, int);
 static tree fixed_type_or_null (tree, int *, int *);
-static tree resolve_address_of_overloaded_function (tree, tree, tsubst_flags_t,
-						    bool, tree);
 static tree build_simple_base_path (tree expr, tree binfo);
 static tree build_vtbl_ref_1 (tree, tree);
 static tree build_vtbl_initializer (tree, tree, tree, tree, int *);
@@ -5673,18 +5671,21 @@ pop_lang_context (void)
 
 /* Given an OVERLOAD and a TARGET_TYPE, return the function that
    matches the TARGET_TYPE.  If there is no satisfactory match, return
-   error_mark_node, and issue an error & warning messages under control
-   of FLAGS.  Permit pointers to member function if FLAGS permits.  If
-   TEMPLATE_ONLY, the name of the overloaded function was a
-   template-id, and EXPLICIT_TARGS are the explicitly provided
-   template arguments.  */
+   error_mark_node, and issue an error & warning messages under
+   control of FLAGS.  Permit pointers to member function if FLAGS
+   permits.  If TEMPLATE_ONLY, the name of the overloaded function was
+   a template-id, and EXPLICIT_TARGS are the explicitly provided
+   template arguments.  If OVERLOAD is for one or more member
+   functions, then ACCESS_PATH is the base path used to reference
+   those member functions.  */
 
 static tree
 resolve_address_of_overloaded_function (tree target_type,
 					tree overload,
 					tsubst_flags_t flags,
 					bool template_only,
-					tree explicit_targs)
+					tree explicit_targs,
+					tree access_path)
 {
   /* Here's what the standard says:
 
@@ -5935,7 +5936,17 @@ resolve_address_of_overloaded_function (
      function used.  If this conversion sequence is selected, the
      function will be marked as used at this point.  */
   if (!(flags & tf_conv))
-    mark_used (fn);
+    {
+      mark_used (fn);
+      /* We could not check access when this expression was originally
+	 created since we did not know at that time to which function
+	 the expression referred.  */
+      if (DECL_FUNCTION_MEMBER_P (fn))
+	{
+	  gcc_assert (access_path);
+	  perform_or_defer_access_check (access_path, fn);
+	}
+    }
 
   if (TYPE_PTRFN_P (target_type) || TYPE_PTRMEMFUNC_P (target_type))
     return build_unary_op (ADDR_EXPR, fn, 0);
@@ -5964,6 +5975,7 @@ tree
 instantiate_type (tree lhstype, tree rhs, tsubst_flags_t flags)
 {
   tsubst_flags_t flags_in = flags;
+  tree access_path = NULL_TREE;
 
   flags &= ~tf_ptrmem_ok;
 
@@ -5994,7 +6006,10 @@ instantiate_type (tree lhstype, tree rhs
     }
 
   if (TREE_CODE (rhs) == BASELINK)
-    rhs = BASELINK_FUNCTIONS (rhs);
+    {
+      access_path = BASELINK_ACCESS_BINFO (rhs);
+      rhs = BASELINK_FUNCTIONS (rhs);
+    }
 
   /* If we are in a template, and have a NON_DEPENDENT_EXPR, we cannot
      deduce any type information.  */
@@ -6005,6 +6020,13 @@ instantiate_type (tree lhstype, tree rhs
       return error_mark_node;
     }
 
+  /* There only a few kinds of expressions that may have a type
+     dependent on overload resolution.  */
+  gcc_assert (TREE_CODE (rhs) == ADDR_EXPR
+	      || TREE_CODE (rhs) == COMPONENT_REF
+	      || TREE_CODE (rhs) == COMPOUND_EXPR
+	      || really_overloaded_fn (rhs));
+
   /* We don't overwrite rhs if it is an overloaded function.
      Copying it would destroy the tree link.  */
   if (TREE_CODE (rhs) != OVERLOAD)
@@ -6017,32 +6039,6 @@ instantiate_type (tree lhstype, tree rhs
 
   switch (TREE_CODE (rhs))
     {
-    case TYPE_EXPR:
-    case CONVERT_EXPR:
-    case SAVE_EXPR:
-    case CONSTRUCTOR:
-      gcc_unreachable ();
-
-    case INDIRECT_REF:
-    case ARRAY_REF:
-      {
-	tree new_rhs;
-
-	new_rhs = instantiate_type (build_pointer_type (lhstype),
-				    TREE_OPERAND (rhs, 0), flags);
-	if (new_rhs == error_mark_node)
-	  return error_mark_node;
-
-	TREE_TYPE (rhs) = lhstype;
-	TREE_OPERAND (rhs, 0) = new_rhs;
-	return rhs;
-      }
-
-    case NOP_EXPR:
-      rhs = copy_node (TREE_OPERAND (rhs, 0));
-      TREE_TYPE (rhs) = unknown_type_node;
-      return instantiate_type (lhstype, rhs, flags);
-
     case COMPONENT_REF:
       {
 	tree member = TREE_OPERAND (rhs, 1);
@@ -6059,7 +6055,7 @@ instantiate_type (tree lhstype, tree rhs
     case OFFSET_REF:
       rhs = TREE_OPERAND (rhs, 1);
       if (BASELINK_P (rhs))
-	return instantiate_type (lhstype, BASELINK_FUNCTIONS (rhs), flags_in);
+	return instantiate_type (lhstype, rhs, flags_in);
 
       /* This can happen if we are forming a pointer-to-member for a
 	 member template.  */
@@ -6075,7 +6071,7 @@ instantiate_type (tree lhstype, tree rhs
 	return
 	  resolve_address_of_overloaded_function (lhstype, fns, flags_in,
 						  /*template_only=*/true,
-						  args);
+						  args, access_path);
       }
 
     case OVERLOAD:
@@ -6083,14 +6079,9 @@ instantiate_type (tree lhstype, tree rhs
       return
 	resolve_address_of_overloaded_function (lhstype, rhs, flags_in,
 						/*template_only=*/false,
-						/*explicit_targs=*/NULL_TREE);
+						/*explicit_targs=*/NULL_TREE,
+						access_path);
 
-    case CALL_EXPR:
-      /* This is too hard for now.  */
-      gcc_unreachable ();
-
-    case PLUS_EXPR:
-    case MINUS_EXPR:
     case COMPOUND_EXPR:
       TREE_OPERAND (rhs, 0)
 	= instantiate_type (lhstype, TREE_OPERAND (rhs, 0), flags);
@@ -6104,86 +6095,6 @@ instantiate_type (tree lhstype, tree rhs
       TREE_TYPE (rhs) = lhstype;
       return rhs;
 
-    case MULT_EXPR:
-    case TRUNC_DIV_EXPR:
-    case FLOOR_DIV_EXPR:
-    case CEIL_DIV_EXPR:
-    case ROUND_DIV_EXPR:
-    case RDIV_EXPR:
-    case TRUNC_MOD_EXPR:
-    case FLOOR_MOD_EXPR:
-    case CEIL_MOD_EXPR:
-    case ROUND_MOD_EXPR:
-    case FIX_ROUND_EXPR:
-    case FIX_FLOOR_EXPR:
-    case FIX_CEIL_EXPR:
-    case FIX_TRUNC_EXPR:
-    case FLOAT_EXPR:
-    case NEGATE_EXPR:
-    case ABS_EXPR:
-    case MAX_EXPR:
-    case MIN_EXPR:
-
-    case BIT_AND_EXPR:
-    case BIT_IOR_EXPR:
-    case BIT_XOR_EXPR:
-    case LSHIFT_EXPR:
-    case RSHIFT_EXPR:
-    case LROTATE_EXPR:
-    case RROTATE_EXPR:
-
-    case PREINCREMENT_EXPR:
-    case PREDECREMENT_EXPR:
-    case POSTINCREMENT_EXPR:
-    case POSTDECREMENT_EXPR:
-      if (flags & tf_error)
-	error ("invalid operation on uninstantiated type");
-      return error_mark_node;
-
-    case TRUTH_AND_EXPR:
-    case TRUTH_OR_EXPR:
-    case TRUTH_XOR_EXPR:
-    case LT_EXPR:
-    case LE_EXPR:
-    case GT_EXPR:
-    case GE_EXPR:
-    case EQ_EXPR:
-    case NE_EXPR:
-    case TRUTH_ANDIF_EXPR:
-    case TRUTH_ORIF_EXPR:
-    case TRUTH_NOT_EXPR:
-      if (flags & tf_error)
-	error ("not enough type information");
-      return error_mark_node;
-
-    case COND_EXPR:
-      if (type_unknown_p (TREE_OPERAND (rhs, 0)))
-	{
-	  if (flags & tf_error)
-	    error ("not enough type information");
-	  return error_mark_node;
-	}
-      TREE_OPERAND (rhs, 1)
-	= instantiate_type (lhstype, TREE_OPERAND (rhs, 1), flags);
-      if (TREE_OPERAND (rhs, 1) == error_mark_node)
-	return error_mark_node;
-      TREE_OPERAND (rhs, 2)
-	= instantiate_type (lhstype, TREE_OPERAND (rhs, 2), flags);
-      if (TREE_OPERAND (rhs, 2) == error_mark_node)
-	return error_mark_node;
-
-      TREE_TYPE (rhs) = lhstype;
-      return rhs;
-
-    case MODIFY_EXPR:
-      TREE_OPERAND (rhs, 1)
-	= instantiate_type (lhstype, TREE_OPERAND (rhs, 1), flags);
-      if (TREE_OPERAND (rhs, 1) == error_mark_node)
-	return error_mark_node;
-
-      TREE_TYPE (rhs) = lhstype;
-      return rhs;
-
     case ADDR_EXPR:
     {
       if (PTRMEM_OK_P (rhs))
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c	(revision 116430)
+++ gcc/cp/tree.c	(working copy)
@@ -842,6 +842,13 @@ build_qualified_name (tree type, tree sc
   return t;
 }
 
+/* Returns non-zero if X is an expression for a (possibly overloaded)
+   function.  If "f" is a function or function template, "f", "c->f",
+   "c.f", "C::f", and "f<int>" will all be considered possibly
+   overloaded functions.  Returns 2 if the function is actually
+   overloaded, i.e., if it is impossible to know the the type of the
+   function without performing overload resolution.  */
+ 
 int
 is_overloaded_fn (tree x)
 {
@@ -850,24 +857,22 @@ is_overloaded_fn (tree x)
     x = TREE_OPERAND (x, 1);
   if (BASELINK_P (x))
     x = BASELINK_FUNCTIONS (x);
-  return (TREE_CODE (x) == FUNCTION_DECL
-	  || TREE_CODE (x) == TEMPLATE_ID_EXPR
-	  || DECL_FUNCTION_TEMPLATE_P (x)
-	  || TREE_CODE (x) == OVERLOAD);
+  if (TREE_CODE (x) == TEMPLATE_ID_EXPR
+      || DECL_FUNCTION_TEMPLATE_P (OVL_CURRENT (x))
+      || (TREE_CODE (x) == OVERLOAD && OVL_CHAIN (x)))
+    return 2;
+  return  (TREE_CODE (x) == FUNCTION_DECL
+	   || TREE_CODE (x) == OVERLOAD);
 }
 
-int
+/* Returns true iff X is an expression for an overloaded function
+   whose type cannot be known without performing overload
+   resolution.  */
+
+bool
 really_overloaded_fn (tree x)
 {
-  if (TREE_CODE (x) == OFFSET_REF)
-    x = TREE_OPERAND (x, 1);
-  /* A baselink is also considered an overloaded function.  */
-  if (BASELINK_P (x))
-    x = BASELINK_FUNCTIONS (x);
-
-  return ((TREE_CODE (x) == OVERLOAD && OVL_CHAIN (x))
-	  || DECL_FUNCTION_TEMPLATE_P (OVL_CURRENT (x))
-	  || TREE_CODE (x) == TEMPLATE_ID_EXPR);
+  return is_overloaded_fn (x) == 2;
 }
 
 tree
Index: gcc/cp/mangle.c
===================================================================
--- gcc/cp/mangle.c	(revision 116430)
+++ gcc/cp/mangle.c	(working copy)
@@ -2023,6 +2023,12 @@ write_expression (tree expr)
       code = TREE_CODE (expr);
     }
 
+  if (code == BASELINK)
+    {
+      expr = BASELINK_FUNCTIONS (expr);
+      code = TREE_CODE (expr);
+    }
+
   /* Handle pointers-to-members by making them look like expression
      nodes.  */
   if (code == PTRMEM_CST)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 116430)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -4321,6 +4321,7 @@ extern tree cxx_omp_clause_copy_ctor		(t
 extern tree cxx_omp_clause_assign_op		(tree, tree, tree);
 extern tree cxx_omp_clause_dtor			(tree, tree);
 extern bool cxx_omp_privatize_by_reference	(tree);
+extern tree baselink_for_fns                    (tree);
 
 /* in tree.c */
 extern void lang_check_failed			(const char *, int,
@@ -4364,7 +4365,7 @@ extern bool decl_anon_ns_mem_p			(tree);
 extern tree lvalue_type				(tree);
 extern tree error_type				(tree);
 extern int varargs_function_p			(tree);
-extern int really_overloaded_fn			(tree);
+extern bool really_overloaded_fn		(tree);
 extern bool cp_tree_equal			(tree, tree);
 extern tree no_linkage_check			(tree, bool);
 extern void debug_binfo				(tree);
Index: gcc/cp/search.c
===================================================================
--- gcc/cp/search.c	(revision 116430)
+++ gcc/cp/search.c	(working copy)
@@ -1253,8 +1253,26 @@ lookup_member (tree xbasetype, tree name
   /* [class.access]
 
      In the case of overloaded function names, access control is
-     applied to the function selected by overloaded resolution.  */
-  if (rval && protect && !is_overloaded_fn (rval))
+     applied to the function selected by overloaded resolution.  
+
+     We cannot check here, even if RVAL is only a single non-static
+     member function, since we do not know what the "this" pointer
+     will be.  For:
+
+        class A { protected: void f(); };
+        class B : public A { 
+          void g(A *p) {
+            f(); // OK
+            p->f(); // Not OK.
+          }
+        };
+
+    only the first call to "f" is valid.  However, if the function is
+    static, we can check.  */
+  if (rval && protect 
+      && !really_overloaded_fn (rval)
+      && !(TREE_CODE (rval) == FUNCTION_DECL
+	   && DECL_NONSTATIC_MEMBER_FUNCTION_P (rval)))
     perform_or_defer_access_check (basetype_path, rval);
 
   if (errstr && protect)
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 116430)
+++ gcc/cp/pt.c	(working copy)
@@ -3454,7 +3454,9 @@ convert_nontype_argument_function (tree 
   fn_no_ptr = fn;
   if (TREE_CODE (fn_no_ptr) == ADDR_EXPR)
     fn_no_ptr = TREE_OPERAND (fn_no_ptr, 0);
-
+  if (TREE_CODE (fn_no_ptr) == BASELINK)
+    fn_no_ptr = BASELINK_FUNCTIONS (fn_no_ptr);
+ 
   /* [temp.arg.nontype]/1
 
      A template-argument for a non-type, non-template template-parameter
@@ -8823,7 +8833,7 @@ tsubst_copy_and_build (tree t,
 	  return build3 (COMPONENT_REF, TREE_TYPE (template),
 			 object, template, NULL_TREE);
 	else
-	  return template;
+	  return baselink_for_fns (template);
       }
 
     case INDIRECT_REF:
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c	(revision 116430)
+++ gcc/cp/semantics.c	(working copy)
@@ -1774,26 +1774,8 @@ finish_call_expr (tree fn, tree args, bo
       args = build_non_dependent_args (orig_args);
     }
 
-  /* A reference to a member function will appear as an overloaded
-     function (rather than a BASELINK) if an unqualified name was used
-     to refer to it.  */
-  if (!BASELINK_P (fn) && is_overloaded_fn (fn))
-    {
-      tree f = fn;
-
-      if (TREE_CODE (f) == TEMPLATE_ID_EXPR)
-	f = TREE_OPERAND (f, 0);
-      f = get_first_fn (f);
-      if (DECL_FUNCTION_MEMBER_P (f))
-	{
-	  tree type = currently_open_derived_class (DECL_CONTEXT (f));
-	  if (!type)
-	    type = DECL_CONTEXT (f);
-	  fn = build_baselink (TYPE_BINFO (type),
-			       TYPE_BINFO (type),
-			       fn, /*optype=*/NULL_TREE);
-	}
-    }
+  if (is_overloaded_fn (fn))
+    fn = baselink_for_fns (fn);
 
   result = NULL_TREE;
   if (BASELINK_P (fn))
@@ -2405,6 +2387,36 @@ qualified_name_lookup_error (tree scope,
     error ("%<::%D%> has not been declared", name);
 }
 
+/* If FNS is a member function, a set of member functions, or a
+   template-id referring to one or more member functions, return a
+   BASELINK for FNS, incorporating the current access context.
+   Otherwise, return FNS unchanged.  */
+
+tree
+baselink_for_fns (tree fns)
+{
+  tree fn;
+  tree cl;
+
+  if (BASELINK_P (fns) 
+      || processing_template_decl
+      || error_operand_p (fns))
+    return fns;
+  
+  fn = fns;
+  if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
+    fn = TREE_OPERAND (fn, 0);
+  fn = get_first_fn (fn);
+  if (!DECL_FUNCTION_MEMBER_P (fn))
+    return fns;
+
+  cl = currently_open_derived_class (DECL_CONTEXT (fn));
+  if (!cl)
+    cl = DECL_CONTEXT (fn);
+  cl = TYPE_BINFO (cl);
+  return build_baselink (cl, cl, fns, /*optype=*/NULL_TREE);
+}
+
 /* ID_EXPRESSION is a representation of parsed, but unprocessed,
    id-expression.  (See cp_parser_id_expression for details.)  SCOPE,
    if non-NULL, is the type or namespace used to explicitly qualify
@@ -2793,8 +2805,12 @@ finish_id_expression (tree id_expression
 	}
       else if (is_overloaded_fn (decl))
 	{
-	  tree first_fn = OVL_CURRENT (decl);
+	  tree first_fn;
 
+	  first_fn = decl;
+	  if (TREE_CODE (first_fn) == TEMPLATE_ID_EXPR)
+	    first_fn = TREE_OPERAND (first_fn, 0);
+	  first_fn = get_first_fn (first_fn);
 	  if (TREE_CODE (first_fn) == TEMPLATE_DECL)
 	    first_fn = DECL_TEMPLATE_RESULT (first_fn);
 
@@ -2811,6 +2827,8 @@ finish_id_expression (tree id_expression
 	      return finish_class_member_access_expr (decl, id_expression,
 						      /*template_p=*/false);
 	    }
+
+	  decl = baselink_for_fns (decl);
 	}
       else
 	{
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 116430)
+++ gcc/cp/parser.c	(working copy)
@@ -9237,11 +9237,6 @@ cp_parser_template_argument (cp_parser* 
 	      argument = TREE_OPERAND (argument, 0);
 	    }
 
-	  if (TREE_CODE (argument) == BASELINK)
-	    /* We don't need the information about what class was used
-	       to name the overloaded functions.  */
-	    argument = BASELINK_FUNCTIONS (argument);
-
 	  if (TREE_CODE (argument) == VAR_DECL)
 	    {
 	      /* A variable without external linkage might still be a
Index: gcc/testsuite/g++.dg/inherit/access6.C
===================================================================
--- gcc/testsuite/g++.dg/inherit/access6.C	(revision 0)
+++ gcc/testsuite/g++.dg/inherit/access6.C	(revision 0)
@@ -0,0 +1,15 @@
+// PR c++/28588
+
+class Foo { 
+  static void f(); // { dg-error "private" }
+  static void f(int);
+  static void g(); // { dg-error "private" }
+};
+
+void h()
+{
+  Foo foo;
+  void (*f)();
+  f = foo.f; // { dg-error "context" }
+  f = foo.g; // { dg-error "context" }
+}
Index: gcc/testsuite/g++.dg/inherit/access7.C
===================================================================
--- gcc/testsuite/g++.dg/inherit/access7.C	(revision 0)
+++ gcc/testsuite/g++.dg/inherit/access7.C	(revision 0)
@@ -0,0 +1,15 @@
+struct B {
+  static void f();
+};
+
+template <typename T>
+struct D : private B {
+  void g() {
+    f();
+  }
+};
+  
+void h() {
+  D<int> d;
+  d.g();
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]