From be246ac2d26e1cb072f205bf97d5eac150220f3f Mon Sep 17 00:00:00 2001 From: Anthony Sharp Date: Wed, 10 Mar 2021 20:36:03 +0000 Subject: [PATCH] c++: Private parent access check for using decls [PR19377] This bug was already mostly fixed by the patch for PR17314. This patch continues that by ensuring that where a using decl is used, causing an access failure to a child class because the using decl is private, the compiler correctly points to the using decl as the source of the problem. gcc/cp/ChangeLog: 2021-03-10 Anthony Sharp * semantics.c (get_class_access_diagnostic_decl): New function that examines special cases when a parent class causes a private access failure. (enforce_access): Slightly modified to call function above. gcc/testsuite/ChangeLog: 2021-03-10 Anthony Sharp * g++.dg/cpp1z/using9.C: New using decl test. Co-authored-by: Jason Merrill --- gcc/cp/semantics.c | 103 +++++++++++++++++++++++----- gcc/testsuite/g++.dg/cpp1z/using9.C | 49 +++++++++++++ 2 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1z/using9.C diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 30dd206c899..b02596f73bd 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -256,6 +256,72 @@ pop_to_parent_deferring_access_checks (void) } } +/* Called from enforce_access. A class has attempted (but failed) to access + DECL. It is already established that a baseclass of that class, + PARENT_BINFO, has private access to DECL. Examine certain special cases + to find a decl that accurately describes the source of the problem. If + none of the special cases apply, simply return DECL as the source of the + problem. */ + +static tree +get_class_access_diagnostic_decl (tree parent_binfo, tree decl) +{ + /* When a class is denied access to a decl in a baseclass, most of the + time it is because the decl itself was declared as private at the point + of declaration. + + However, in C++, there are (at least) two situations in which a decl + can be private even though it was not originally defined as such. + These two situations only apply if a baseclass had private access to + DECL (this function is only called if that is the case). */ + + /* We should first check whether the reason the parent had private access + to DECL was simply because DECL was created and declared as private in + the parent. If it was, then DECL is definitively the source of the + problem. */ + if (SAME_BINFO_TYPE_P (context_for_name_lookup (decl), + BINFO_TYPE (parent_binfo))) + return decl; + + /* 1. If the "using" keyword is used to inherit DECL within the parent, + this may cause DECL to be private, so we should return the using + statement as the source of the problem. + + Scan the fields of PARENT_BINFO and see if there are any using decls. If + there are, see if they inherit DECL. If they do, that's where DECL must + have been declared private. */ + + for (tree parent_field = TYPE_FIELDS (BINFO_TYPE (parent_binfo)); + parent_field; + parent_field = DECL_CHAIN (parent_field)) + /* Not necessary, but also check TREE_PRIVATE for the sake of + eliminating obviously non-relevant using decls. */ + if (TREE_CODE (parent_field) == USING_DECL + && TREE_PRIVATE (parent_field)) + { + tree decl_stripped = strip_using_decl (parent_field); + + /* The using statement might be overloaded. If so, we need to + check all of the overloads. */ + for (ovl_iterator iter (decl_stripped); iter; ++iter) + /* If equal, the using statement inherits DECL, and so is the + source of the access failure, so return it. */ + if (*iter == decl) + return parent_field; + } + + /* 2. If DECL was privately inherited by the parent class, then DECL will + be inaccessible, even though it may originally have been accessible to + deriving classes. In that case, the fault lies with the parent, since it + used a private inheritance, so we return the parent as the source of the + problem. + + Since this is the last check, we just assume it's true. At worst, it + will simply point to the class that failed to give access, which is + technically true. */ + return TYPE_NAME (BINFO_TYPE (parent_binfo)); +} + /* If the current scope isn't allowed to access DECL along BASETYPE_PATH, give an error, or if we're parsing a function or class template, defer the access check to be performed at instantiation time. @@ -317,34 +383,33 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl, diag_decl = strip_inheriting_ctors (diag_decl); if (complain & tf_error) { - /* 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; + access_kind access_failure_reason = 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. */ + /* By default, using the decl as the source of the problem will + usually give correct results. */ + tree diag_location = diag_decl; + + /* However, if a parent of BASETYPE_PATH had private access to decl, + then it actually might be the case that the source of the problem + is not DECL. */ 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))) + /* So if a parent did have private access, then we need to do + special checks to obtain the best diagnostic location decl. */ + if (parent_binfo != NULL_TREE) { - diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo)); - parent_access = ak_private; + diag_location = get_class_access_diagnostic_decl (parent_binfo, + diag_decl); + + /* We also at this point know that the reason access failed was + because decl was private. */ + access_failure_reason = ak_private; } /* Finally, generate an error message. */ complain_about_access (decl, diag_decl, diag_location, true, - parent_access); + access_failure_reason); } if (afi) afi->record_access_failure (basetype_path, decl, diag_decl); diff --git a/gcc/testsuite/g++.dg/cpp1z/using9.C b/gcc/testsuite/g++.dg/cpp1z/using9.C new file mode 100644 index 00000000000..98e36babb01 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/using9.C @@ -0,0 +1,49 @@ +/* { dg-do compile { target c++17 } } */ +// Created for c++ PR19377 + +class A2 +{ + protected: + int separate(int a); + int separate(int a, int b); + int separate(int a, int b, int c); + int comma(int a); + int alone; +}; + +class A1 +{ + protected: + int separate(); + int comma(); +}; + +class A3 +{ + protected: + int comma(int a, int b); +}; + +class B:private A3, private A1, private A2 +{ + // Using decls in a comma-separated list. + using A2::comma, A3::comma, A1::comma; // { dg-message "declared" } + // Separate using statements. + using A2::separate; // { dg-message "declared" } + using A1::separate; // { dg-message "declared" } + // No ambiguity, just for the sake of it. + using A2::alone; // { dg-message "declared" } +}; + +class C:public B +{ + void f() + { + comma(); // { dg-error "private" } + separate(); // { dg-error "private" } + separate(1); // { dg-error "private" } + separate(1, 2); // { dg-error "private" } + separate(1, 2, 3); // { dg-error "private" } + alone = 5; // { dg-error "private" } + } +};