[PATCH] c++: class-head parsing and CPP_TEMPLATE_ID access [PR108275]

[PATCH] c++: class-head parsing and CPP_TEMPLATE_ID access [PR108275]

[Patrick Palka]

When tentatively parsing what is really an elaborated-type-specifier
first as a class-specifier, we may form a CPP_TEMPLATE_ID token that
later gets reused in the fallback parse if the tentative parse fails.
These special tokens also capture the access checks that have been
deferred while parsing the template-id.  But here, we form such a token
when the access check state is dk_no_check, and so the token captures
no access checks.  This effectively bypasses access checking for the
template-id during the subsequent parse as an elaborated-type-specifier.

This patch fixes this by using dk_deferred instead of dk_no_check when
parsing the class name.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

	PR c++/108275

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_class_head): Use dk_deferred instead of
	dk_no_check when parsing the class name.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/access14.C: New test.
---
 gcc/cp/parser.cc                      | 23 +++++++++++++++++------
 gcc/testsuite/g++.dg/parse/access14.C | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/parse/access14.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index bfd8aeae39f..8b1658decba 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -26559,7 +26559,23 @@ cp_parser_class_head (cp_parser* parser,
   if (cp_parser_global_scope_opt (parser, /*current_scope_valid_p=*/false))
     qualified_p = true;
 
-  push_deferring_access_checks (dk_no_check);
+  /* It is OK to define an inaccessible class; for example:
+
+       class A { class B; };
+       class A::B {};
+
+     So we want to ignore access when parsing the class name.
+     However, we might be tentatively parsing what is really an
+     elaborated-type-specifier naming a template-id, e.g.
+
+       struct C<&D::m> c;
+
+     In this case the tentative parse as a class-head will fail, but not
+     before cp_parser_template_id splices in a CPP_TEMPLATE_ID token.
+     Since dk_no_check is sticky, we must instead use dk_deferred so that
+     any such CPP_TEMPLATE_ID token created during this tentative parse
+     will correctly capture the access checks imposed by the template-id . */
+  push_deferring_access_checks (dk_deferred);
 
   /* Determine the name of the class.  Begin by looking for an
      optional nested-name-specifier.  */
@@ -26586,11 +26602,6 @@ cp_parser_class_head (cp_parser* parser,
 	 The proposed resolution for Core Issue 180 says that wherever
 	 you see `class T::X' you should treat `X' as a type-name.
 
-	 It is OK to define an inaccessible class; for example:
-
-	   class A { class B; };
-	   class A::B {};
-
 	 We do not know if we will see a class-name, or a
 	 template-name.  We look for a class-name first, in case the
 	 class-name is a template-id; if we looked for the
diff --git a/gcc/testsuite/g++.dg/parse/access14.C b/gcc/testsuite/g++.dg/parse/access14.C
new file mode 100644
index 00000000000..bdbc7f6ee2b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/access14.C
@@ -0,0 +1,18 @@
+// PR c++/108275
+
+struct A {
+  int i;
+private:
+  int j;
+};
+
+template<int A::* V>
+struct B {
+  struct C { };
+private:
+  template<int N> struct D { };
+};
+
+struct B<&A::j> b;       // { dg-error "private" }
+struct B<&A::j>::C c;    // { dg-error "private" }
+struct B<&A::i>::D<0> d; // { dg-error "private" }
-- 
2.39.0.189.g4dbebc36b0

Re: [PATCH] c++: class-head parsing and CPP_TEMPLATE_ID access [PR108275]

[Patrick Palka]

On Thu, 5 Jan 2023, Patrick Palka wrote:

> When tentatively parsing what is really an elaborated-type-specifier
> first as a class-specifier, we may form a CPP_TEMPLATE_ID token that
> later gets reused in the fallback parse if the tentative parse fails.
> These special tokens also capture the access checks that have been
> deferred while parsing the template-id.  But here, we form such a token
> when the access check state is dk_no_check, and so the token captures
> no access checks.  This effectively bypasses access checking for the
> template-id during the subsequent parse as an elaborated-type-specifier.
> 
> This patch fixes this by using dk_deferred instead of dk_no_check when
> parsing the class name.

Looks like this issue isn't specific to the CPP_TEMPLATE_ID mechanism --
using dk_no_check also means we bypass access checking during satisfaction too:

  template<typename T>
    requires T::value
  struct A { };

  struct B {
  private:
    static constexpr bool value = true;
  };

  struct A<B> a; // incorrectly accepted

> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/108275
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_class_head): Use dk_deferred instead of
> 	dk_no_check when parsing the class name.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/parse/access14.C: New test.
> ---
>  gcc/cp/parser.cc                      | 23 +++++++++++++++++------
>  gcc/testsuite/g++.dg/parse/access14.C | 18 ++++++++++++++++++
>  2 files changed, 35 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/parse/access14.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index bfd8aeae39f..8b1658decba 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -26559,7 +26559,23 @@ cp_parser_class_head (cp_parser* parser,
>    if (cp_parser_global_scope_opt (parser, /*current_scope_valid_p=*/false))
>      qualified_p = true;
>  
> -  push_deferring_access_checks (dk_no_check);
> +  /* It is OK to define an inaccessible class; for example:
> +
> +       class A { class B; };
> +       class A::B {};
> +
> +     So we want to ignore access when parsing the class name.
> +     However, we might be tentatively parsing what is really an
> +     elaborated-type-specifier naming a template-id, e.g.
> +
> +       struct C<&D::m> c;
> +
> +     In this case the tentative parse as a class-head will fail, but not
> +     before cp_parser_template_id splices in a CPP_TEMPLATE_ID token.
> +     Since dk_no_check is sticky, we must instead use dk_deferred so that
> +     any such CPP_TEMPLATE_ID token created during this tentative parse
> +     will correctly capture the access checks imposed by the template-id . */
> +  push_deferring_access_checks (dk_deferred);
>  
>    /* Determine the name of the class.  Begin by looking for an
>       optional nested-name-specifier.  */
> @@ -26586,11 +26602,6 @@ cp_parser_class_head (cp_parser* parser,
>  	 The proposed resolution for Core Issue 180 says that wherever
>  	 you see `class T::X' you should treat `X' as a type-name.
>  
> -	 It is OK to define an inaccessible class; for example:
> -
> -	   class A { class B; };
> -	   class A::B {};
> -
>  	 We do not know if we will see a class-name, or a
>  	 template-name.  We look for a class-name first, in case the
>  	 class-name is a template-id; if we looked for the
> diff --git a/gcc/testsuite/g++.dg/parse/access14.C b/gcc/testsuite/g++.dg/parse/access14.C
> new file mode 100644
> index 00000000000..bdbc7f6ee2b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/access14.C
> @@ -0,0 +1,18 @@
> +// PR c++/108275
> +
> +struct A {
> +  int i;
> +private:
> +  int j;
> +};
> +
> +template<int A::* V>
> +struct B {
> +  struct C { };
> +private:
> +  template<int N> struct D { };
> +};
> +
> +struct B<&A::j> b;       // { dg-error "private" }
> +struct B<&A::j>::C c;    // { dg-error "private" }
> +struct B<&A::i>::D<0> d; // { dg-error "private" }
> -- 
> 2.39.0.189.g4dbebc36b0
> 
> 

Re: [PATCH] c++: class-head parsing and CPP_TEMPLATE_ID access [PR108275]

[Jason Merrill]

On 1/5/23 12:20, Patrick Palka wrote:
> When tentatively parsing what is really an elaborated-type-specifier
> first as a class-specifier, we may form a CPP_TEMPLATE_ID token that
> later gets reused in the fallback parse if the tentative parse fails.
> These special tokens also capture the access checks that have been
> deferred while parsing the template-id.  But here, we form such a token
> when the access check state is dk_no_check, and so the token captures
> no access checks.  This effectively bypasses access checking for the
> template-id during the subsequent parse as an elaborated-type-specifier.
> 
> This patch fixes this by using dk_deferred instead of dk_no_check when
> parsing the class name.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> 	PR c++/108275
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_class_head): Use dk_deferred instead of
> 	dk_no_check when parsing the class name.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/parse/access14.C: New test.
> ---
>   gcc/cp/parser.cc                      | 23 +++++++++++++++++------
>   gcc/testsuite/g++.dg/parse/access14.C | 18 ++++++++++++++++++
>   2 files changed, 35 insertions(+), 6 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/parse/access14.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index bfd8aeae39f..8b1658decba 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -26559,7 +26559,23 @@ cp_parser_class_head (cp_parser* parser,
>     if (cp_parser_global_scope_opt (parser, /*current_scope_valid_p=*/false))
>       qualified_p = true;
>   
> -  push_deferring_access_checks (dk_no_check);
> +  /* It is OK to define an inaccessible class; for example:
> +
> +       class A { class B; };
> +       class A::B {};
> +
> +     So we want to ignore access when parsing the class name.
> +     However, we might be tentatively parsing what is really an
> +     elaborated-type-specifier naming a template-id, e.g.
> +
> +       struct C<&D::m> c;
> +
> +     In this case the tentative parse as a class-head will fail, but not
> +     before cp_parser_template_id splices in a CPP_TEMPLATE_ID token.
> +     Since dk_no_check is sticky, we must instead use dk_deferred so that
> +     any such CPP_TEMPLATE_ID token created during this tentative parse
> +     will correctly capture the access checks imposed by the template-id . */
> +  push_deferring_access_checks (dk_deferred);
>   
>     /* Determine the name of the class.  Begin by looking for an
>        optional nested-name-specifier.  */
> @@ -26586,11 +26602,6 @@ cp_parser_class_head (cp_parser* parser,
>   	 The proposed resolution for Core Issue 180 says that wherever
>   	 you see `class T::X' you should treat `X' as a type-name.
>   
> -	 It is OK to define an inaccessible class; for example:
> -
> -	   class A { class B; };
> -	   class A::B {};
> -
>   	 We do not know if we will see a class-name, or a
>   	 template-name.  We look for a class-name first, in case the
>   	 class-name is a template-id; if we looked for the
> diff --git a/gcc/testsuite/g++.dg/parse/access14.C b/gcc/testsuite/g++.dg/parse/access14.C
> new file mode 100644
> index 00000000000..bdbc7f6ee2b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/access14.C
> @@ -0,0 +1,18 @@
> +// PR c++/108275
> +
> +struct A {
> +  int i;
> +private:
> +  int j;
> +};
> +
> +template<int A::* V>
> +struct B {
> +  struct C { };
> +private:
> +  template<int N> struct D { };
> +};
> +
> +struct B<&A::j> b;       // { dg-error "private" }
> +struct B<&A::j>::C c;    // { dg-error "private" }
> +struct B<&A::i>::D<0> d; // { dg-error "private" }