From: Anthony Sharp <anthonysharp15@gmail.com>
Date: Fri, 22 Jan 2021 22:36:06 +0000 (+0000)
Subject: c++: private inheritance access diagnostics fix [PR17314]
X-Git-Tag: basepoints/gcc-12~1377
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7e0f147a29f42d6149585573650bd4827f3b2b93;p=thirdparty%2Fgcc.git

c++: private inheritance access diagnostics fix [PR17314]

This patch fixes PR17314. Previously, when class C attempted
to access member a declared in class A through class B, where class B
privately inherits from A and class C inherits from B, GCC would correctly
report an access violation, but would erroneously report that the reason was
because a was "protected", when in fact, from the point of view of class C,
it was really "private". This patch updates the diagnostics code to generate
more correct errors in cases of failed inheritance such as these.

The reason this bug happened was because GCC was examining the
declared access of decl, instead of looking at it in the
context of class inheritance.

gcc/cp/ChangeLog:

2021-01-21  Anthony Sharp  <anthonysharp15@gmail.com>

	* call.c (complain_about_access): Altered function.
	* cp-tree.h (complain_about_access): Changed parameters of function.
	(get_parent_with_private_access): Declared new function.
	* search.c (get_parent_with_private_access): Defined new function.
	* semantics.c (enforce_access): Modified function.
	* typeck.c (complain_about_unrecognized_member): Updated function
	arguments in complain_about_access.

gcc/testsuite/ChangeLog:

2021-01-21  Anthony Sharp  <anthonysharp15@gmail.com>

	* g++.dg/lookup/scoped1.C: Modified testcase to run successfully
	with changes.
	* g++.dg/tc1/dr142.C: Same as above.
	* g++.dg/tc1/dr52.C: Same as above.
	* g++.old-deja/g++.brendan/visibility6.C: Same as above.
	* g++.old-deja/g++.brendan/visibility8.C: Same as above.
	* g++.old-deja/g++.jason/access8.C: Same as above.
	* g++.old-deja/g++.law/access4.C: Same as above.
	* g++.old-deja/g++.law/visibility12.C: Same as above.
	* g++.old-deja/g++.law/visibility4.C: Same as above.
	* g++.old-deja/g++.law/visibility8.C: Same as above.
	* g++.old-deja/g++.other/access4.C: Same as above.
---

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index a2c5ef736a26..5977582e2797 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7142,27 +7142,45 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 /* Issue diagnostics about a disallowed access of DECL, using DIAG_DECL
    in the diagnostics.
 
-   If ISSUE_ERROR is true, then issue an error about the
-   access, followed by a note showing the declaration.
-   Otherwise, just show the note.  */
+   If ISSUE_ERROR is true, then issue an error about the access, followed
+   by a note showing the declaration.  Otherwise, just show the note.
+
+   DIAG_DECL and DIAG_LOCATION will almost always be the same.
+   DIAG_LOCATION is just another DECL.  NO_ACCESS_REASON is an optional
+   parameter used to specify why DECL wasn't accessible (e.g. ak_private
+   would be because DECL was private).  If not using NO_ACCESS_REASON,
+   then it must be ak_none, and the access failure reason will be
+   figured out by looking at the protection of DECL.  */
 
 void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+		       bool issue_error, access_kind no_access_reason)
 {
-  if (TREE_PRIVATE (decl))
+  /* If we have not already figured out why DECL is inaccessible...  */
+  if (no_access_reason == ak_none)
+    {
+      /* Examine the access of DECL to find out why.  */
+      if (TREE_PRIVATE (decl))
+	no_access_reason = ak_private;
+      else if (TREE_PROTECTED (decl))
+	no_access_reason = ak_protected;
+    }
+
+  /* Now generate an error message depending on calculated access.  */
+  if (no_access_reason == ak_private)
     {
       if (issue_error)
 	error ("%q#D is private within this context", diag_decl);
-      inform (DECL_SOURCE_LOCATION (diag_decl),
-	      "declared private here");
+      inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
     }
-  else if (TREE_PROTECTED (decl))
+  else if (no_access_reason == ak_protected)
     {
       if (issue_error)
 	error ("%q#D is protected within this context", diag_decl);
-      inform (DECL_SOURCE_LOCATION (diag_decl),
-	      "declared protected here");
+      inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
     }
+  /* Couldn't figure out why DECL is inaccesible, so just say it's
+     inaccessible.  */
   else
     {
       if (issue_error)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3046c8386305..bef452f592a4 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6434,7 +6434,8 @@ class access_failure_info
   tree m_diag_decl;
 };
 
-extern void complain_about_access		(tree, tree, bool);
+extern void complain_about_access		(tree, tree, tree, bool,
+						 access_kind);
 extern void push_defarg_context			(tree);
 extern void pop_defarg_context			(void);
 extern tree convert_default_arg			(tree, tree, tree, int,
@@ -7274,6 +7275,7 @@ extern unsigned get_pseudo_tinfo_index		(tree);
 extern tree get_pseudo_tinfo_type		(unsigned);
 
 /* in search.c */
+extern tree get_parent_with_private_access 	(tree decl, tree binfo);
 extern bool accessible_base_p			(tree, tree, bool);
 extern tree lookup_base                         (tree, tree, base_access,
 						 base_kind *, tsubst_flags_t);
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 81bdd45e40dd..7b1836854767 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -122,6 +122,41 @@ dfs_lookup_base (tree binfo, void *data_)
   return NULL_TREE;
 }
 
+/* This deals with bug PR17314.
+
+   DECL is a declaration and BINFO represents a class that has attempted (but
+   failed) to access DECL.
+
+   Examine the parent binfos of BINFO and determine whether any of them had
+   private access to DECL.  If they did, return the parent binfo.  This helps
+   in figuring out the correct error message to show (if the parents had
+   access, it's their fault for not giving sufficient access to BINFO).
+
+   If no parents had access, return NULL_TREE.  */
+
+tree
+get_parent_with_private_access (tree decl, tree binfo)
+{
+  /* Only BINFOs should come through here.  */
+  gcc_assert (TREE_CODE (binfo) == TREE_BINFO);
+
+  tree base_binfo = NULL_TREE;
+
+  /* Iterate through immediate parent classes.  */
+  for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+    {
+      /* This parent had private access.  Therefore that's why BINFO can't
+	  access DECL.  */
+      if (access_in_type (BINFO_TYPE (base_binfo), decl) == ak_private)
+	return base_binfo;
+    }
+
+  /* None of the parents had access.  Note: it's impossible for one of the
+     parents to have had public or protected access to DECL, since then
+     BINFO would have been able to access DECL too.  */
+  return NULL_TREE;
+}
+
 /* Returns true if type BASE is accessible in T.  (BASE is known to be
    a (possibly non-proper) base class of T.)  If CONSIDER_LOCAL_P is
    true, consider any special access of the current scope, or access
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 51841dcf24a0..73834467fcae 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -316,7 +316,36 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
       if (flag_new_inheriting_ctors)
 	diag_decl = strip_inheriting_ctors (diag_decl);
       if (complain & tf_error)
-	complain_about_access (decl, diag_decl, true);
+	{
+	  /* We will usually want to point to the same place as
+	     diag_decl but not always.  */
+	  tree diag_location = diag_decl;
+	  access_kind parent_access = ak_none;
+
+	  /* See if any of BASETYPE_PATH's parents had private access
+	     to DECL.  If they did, that will tell us why we don't.  */
+	  tree parent_binfo = get_parent_with_private_access (decl,
+							      basetype_path);
+
+	  /* If a parent had private access, then the diagnostic
+	     location DECL should be that of the parent class, since it
+	     failed to give suitable access by using a private
+	     inheritance.  But if DECL was actually defined in the parent,
+	     it wasn't privately inherited, and so we don't need to do
+	     this, and complain_about_access will figure out what to
+	     do.  */
+	  if (parent_binfo != NULL_TREE
+	      && (context_for_name_lookup (decl)
+		  != BINFO_TYPE (parent_binfo)))
+	    {
+	      diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
+	      parent_access = ak_private;
+	    }
+
+	  /* Finally, generate an error message.  */
+	  complain_about_access (decl, diag_decl, diag_location, true,
+				 parent_access);
+	}
       if (afi)
 	afi->record_access_failure (basetype_path, decl, diag_decl);
       return false;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cd592fd558e8..315f706de733 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2980,7 +2980,8 @@ complain_about_unrecognized_member (tree access_path, tree name,
 		    TREE_CODE (access_path) == TREE_BINFO
 		    ? TREE_TYPE (access_path) : object_type,
 		    name, afi.get_diag_decl ());
-	  complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+	  complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+				 afi.get_diag_decl (), false, ak_none);
 	}
     }
   else
diff --git a/gcc/testsuite/g++.dg/lookup/scoped1.C b/gcc/testsuite/g++.dg/lookup/scoped1.C
index 663f718b734d..2f7f603b49f7 100644
--- a/gcc/testsuite/g++.dg/lookup/scoped1.C
+++ b/gcc/testsuite/g++.dg/lookup/scoped1.C
@@ -4,12 +4,12 @@
 struct A
 {
   static int i1;
-  int i2; // { dg-message "declared" }
+  int i2; 
   static void f1 ();
   void f2 ();
 };
 
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
 struct C: public B
 {
   void g ()
diff --git a/gcc/testsuite/g++.dg/tc1/dr142.C b/gcc/testsuite/g++.dg/tc1/dr142.C
index 2f0370233e67..6e216da89f08 100644
--- a/gcc/testsuite/g++.dg/tc1/dr142.C
+++ b/gcc/testsuite/g++.dg/tc1/dr142.C
@@ -2,13 +2,13 @@
 // Origin: Giovanni Bajo <giovannibajo at gcc dot gnu dot org>
 // DR142: Injection-related errors in access example 
 
-class B {                 // { dg-message "declared" }
+class B {                 
 public:
-  int mi;                 // { dg-message "declared" }
-  static int si;          // { dg-message "declared" }
+  int mi;                 
+  static int si;          
 };
 
-class D: private B {
+class D: private B { // { dg-message "declared" }
 };
 
 class DD: public D {
diff --git a/gcc/testsuite/g++.dg/tc1/dr52.C b/gcc/testsuite/g++.dg/tc1/dr52.C
index 7cff847023fe..17b6496916cf 100644
--- a/gcc/testsuite/g++.dg/tc1/dr52.C
+++ b/gcc/testsuite/g++.dg/tc1/dr52.C
@@ -17,11 +17,11 @@ struct B1 : B {};
 struct B2 : B {};
 
 struct C
-{ // { dg-message "declared" }
-  void foo(void);
+{ 
+  void foo(void); 
 };
 
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
 
 struct X: A, B1, B2, D
 {
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
index 3dfaf7fd0ea2..8d6c6f10806f 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
@@ -3,9 +3,9 @@
 class bottom
 {
 public:
-  int b; // { dg-message "" } private
+  int b; 
 };
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
 {
 public:
   void foo () { b; }
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
index 3c443afe6785..c165b0874a05 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
@@ -5,9 +5,9 @@
 class foo
 {
 public:
-  static int y; // { dg-message "" } private
+  static int y; 
 };
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
 { };
 class foo2 : public foo1
 { public:
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/access8.C b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
index 4404d8ad95d1..0aa85d0457c0 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/access8.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
@@ -3,13 +3,14 @@
 // Date: 25 Jan 1994 23:41:33 -0500
 // Bug: g++ forgets access decls after the definition.
 
-class inh { // { dg-message "" } inaccessible
+class inh { 
         int a;
 protected:
         void myf(int);
 };
 
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
 protected:
         int t;
 	inh::myf;  // { dg-warning "deprecated" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/access4.C b/gcc/testsuite/g++.old-deja/g++.law/access4.C
index 54072ce3b32a..57fa24a0dde7 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/access4.C
@@ -6,12 +6,13 @@
 // Subject:  g++ 2.5.5 doesn't warn about inaccessible virtual base ctor
 // Message-ID: <9403030024.AA04534@ses.com>
 
-class ForceLeafSterile { // { dg-message "" } 
+class ForceLeafSterile { 
     friend class Sterile;
       ForceLeafSterile() {} // { dg-message "" } 
 };
 
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" } 
+{
 public:
     Sterile() {}
     Sterile(const char* /*blah*/) {}
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
index 59467ba7d803..6b7ff75d2caf 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
@@ -6,11 +6,12 @@
 // Subject:  member access rule bug
 // Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
 struct a {
-  int aa; // { dg-message "" } private
+  int aa; 
         };
 
-class b : private a {
-        };
+class b : private a // { dg-message "" } private
+{
+};
 
 class c : public b {
         int xx(void) { return (aa); }  // aa should be invisible// { dg-error "" } .*
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
index 1cdec1c2b554..644154e66fb4 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
@@ -8,10 +8,11 @@
 
 class A {
 public:
-     int b; // { dg-message "" } private
+     int b; 
 };
 
-class C : private A {                   // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{                   
 public:
         int d;
 };
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
index 5242dfc804f4..4457ddf46c77 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
@@ -7,11 +7,12 @@
 // Message-ID: <m0nof3E-0021ifC@jts.com
 class t1 {
 protected:
-    int a; // { dg-message "" } protected
+    int a; 
 };
 
 
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{ 
 public:
     int b;
 };
diff --git a/gcc/testsuite/g++.old-deja/g++.other/access4.C b/gcc/testsuite/g++.old-deja/g++.other/access4.C
index d3c8d85c3f50..6c47700db19a 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/access4.C
@@ -1,10 +1,10 @@
 // { dg-do assemble  }
 
-struct A { // { dg-message "" } inaccessible
+struct A { 
   static int i;
 };
 
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
 
 struct C : public B {
   int f () { return A::i; } // { dg-error "" } context