[PATCH] c++: outer 'this' leaking into local class [PR106969]

[PATCH] c++: outer 'this' leaking into local class [PR106969]

[Patrick Palka]

Here when resolving the implicit object for '&wrapped' within the
local class Foo, we expect to obtain a dummy object of type Foo& since
there's no 'this' available in this context.  And yet at this point
current_class_ref still corresponds to the outer class Context (and is
const), which confuses maybe_dummy_object into propagating the cv-quals
of current_class_ref and returning an object of type const Foo&.  Thus
decltype(&wrapped) wrongly yields const int* instead of int*.

The problem ultimately seems to be that the 'this' from the enclosing
class appears available for use when parsing the local class, but 'this'
shouldn't leak across classes like that.  This patch fixes this by
clearing current_class_ptr/ref when parsing a class definition.

After this change, for name-clash11.C in C++98 mode we would now
complain about an invalid use of 'this' for e.g.

  ASSERT (sizeof (this->A) == 16);

due to the way the ASSERT macro is defined using a local class.  This
patch redefines it using a local typedef instead.

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

	PR c++/106969

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_class_specifier): Clear current_class_ptr
	and current_class_ref when parsing a class definition.

gcc/testsuite/ChangeLog:

	* g++.dg/lookup/name-clash11.C: New test.
	* g++.dg/lookup/this2.C: New test.
---
 gcc/cp/parser.cc                           | 13 +++++++++----
 gcc/testsuite/g++.dg/lookup/name-clash11.C |  2 +-
 gcc/testsuite/g++.dg/lookup/this2.C        | 22 ++++++++++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lookup/this2.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index a277003ea58..be9c77b415e 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -26151,6 +26151,11 @@ cp_parser_class_specifier (cp_parser* parser)
   saved_in_unbraced_linkage_specification_p
     = parser->in_unbraced_linkage_specification_p;
   parser->in_unbraced_linkage_specification_p = false;
+  /* 'this' from an enclosing non-static member function is unvailable.  */
+  tree saved_ccp = current_class_ptr;
+  tree saved_ccr = current_class_ref;
+  current_class_ptr = NULL_TREE;
+  current_class_ref = NULL_TREE;
 
   /* Start the class.  */
   if (nested_name_specifier_p)
@@ -26369,8 +26374,6 @@ cp_parser_class_specifier (cp_parser* parser)
       /* If there are noexcept-specifiers that have not yet been processed,
 	 take care of them now.  Do this before processing NSDMIs as they
 	 may depend on noexcept-specifiers already having been processed.  */
-      tree save_ccp = current_class_ptr;
-      tree save_ccr = current_class_ref;
       FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
 	{
 	  tree ctx = DECL_CONTEXT (decl);
@@ -26496,8 +26499,8 @@ cp_parser_class_specifier (cp_parser* parser)
 	}
       vec_safe_truncate (unparsed_contracts, 0);
 
-      current_class_ptr = save_ccp;
-      current_class_ref = save_ccr;
+      current_class_ptr = NULL_TREE;
+      current_class_ref = NULL_TREE;
       if (pushed_scope)
 	pop_scope (pushed_scope);
 
@@ -26529,6 +26532,8 @@ cp_parser_class_specifier (cp_parser* parser)
     = saved_num_template_parameter_lists;
   parser->in_unbraced_linkage_specification_p
     = saved_in_unbraced_linkage_specification_p;
+  current_class_ptr = saved_ccp;
+  current_class_ref = saved_ccr;
 
   return type;
 }
diff --git a/gcc/testsuite/g++.dg/lookup/name-clash11.C b/gcc/testsuite/g++.dg/lookup/name-clash11.C
index bc63645e8d3..2ae9a65264d 100644
--- a/gcc/testsuite/g++.dg/lookup/name-clash11.C
+++ b/gcc/testsuite/g++.dg/lookup/name-clash11.C
@@ -7,7 +7,7 @@
 #  define ASSERT(e) static_assert (e, #e)
 #else
 #  define ASSERT(e)                                             \
-  do { struct S { bool: !!(e); } asrt; (void)&asrt; } while (0)
+  do { typedef int asrt[bool(e) ? 1 : -1]; } while (0)
 #endif
 
 
diff --git a/gcc/testsuite/g++.dg/lookup/this2.C b/gcc/testsuite/g++.dg/lookup/this2.C
new file mode 100644
index 00000000000..1450c563d92
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/this2.C
@@ -0,0 +1,22 @@
+// PR c++/106969
+// { dg-do compile { target c++11 } }
+
+struct Context
+{
+    void
+    action() const
+    {
+        struct Foo
+        {
+            int wrapped;
+            decltype( &wrapped ) get() { return &wrapped; }
+        } t;
+
+        *t.get()= 42; // OK, get() returns int* not const int*
+
+        struct Bar
+        {
+            using type = decltype(this); // { dg-error "invalid use of 'this'" }
+        };
+    }
+};
-- 
2.40.0.130.g27d43aaaf5

Re: [PATCH] c++: outer 'this' leaking into local class [PR106969]

[Jason Merrill]

On 3/23/23 11:00, Patrick Palka wrote:
> Here when resolving the implicit object for '&wrapped' within the
> local class Foo, we expect to obtain a dummy object of type Foo& since
> there's no 'this' available in this context.  And yet at this point
> current_class_ref still corresponds to the outer class Context (and is
> const), which confuses maybe_dummy_object into propagating the cv-quals
> of current_class_ref and returning an object of type const Foo&.  Thus
> decltype(&wrapped) wrongly yields const int* instead of int*.
> 
> The problem ultimately seems to be that the 'this' from the enclosing
> class appears available for use when parsing the local class, but 'this'
> shouldn't leak across classes like that.  This patch fixes this by
> clearing current_class_ptr/ref when parsing a class definition.
> 
> After this change, for name-clash11.C in C++98 mode we would now
> complain about an invalid use of 'this' for e.g.
> 
>    ASSERT (sizeof (this->A) == 16);
> 
> due to the way the ASSERT macro is defined using a local class.  This
> patch redefines it using a local typedef instead.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?

OK.

> 	PR c++/106969
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_class_specifier): Clear current_class_ptr
> 	and current_class_ref when parsing a class definition.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/lookup/name-clash11.C: New test.
> 	* g++.dg/lookup/this2.C: New test.
> ---
>   gcc/cp/parser.cc                           | 13 +++++++++----
>   gcc/testsuite/g++.dg/lookup/name-clash11.C |  2 +-
>   gcc/testsuite/g++.dg/lookup/this2.C        | 22 ++++++++++++++++++++++
>   3 files changed, 32 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/lookup/this2.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index a277003ea58..be9c77b415e 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -26151,6 +26151,11 @@ cp_parser_class_specifier (cp_parser* parser)
>     saved_in_unbraced_linkage_specification_p
>       = parser->in_unbraced_linkage_specification_p;
>     parser->in_unbraced_linkage_specification_p = false;
> +  /* 'this' from an enclosing non-static member function is unvailable.  */
> +  tree saved_ccp = current_class_ptr;
> +  tree saved_ccr = current_class_ref;
> +  current_class_ptr = NULL_TREE;
> +  current_class_ref = NULL_TREE;
>   
>     /* Start the class.  */
>     if (nested_name_specifier_p)
> @@ -26369,8 +26374,6 @@ cp_parser_class_specifier (cp_parser* parser)
>         /* If there are noexcept-specifiers that have not yet been processed,
>   	 take care of them now.  Do this before processing NSDMIs as they
>   	 may depend on noexcept-specifiers already having been processed.  */
> -      tree save_ccp = current_class_ptr;
> -      tree save_ccr = current_class_ref;
>         FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
>   	{
>   	  tree ctx = DECL_CONTEXT (decl);
> @@ -26496,8 +26499,8 @@ cp_parser_class_specifier (cp_parser* parser)
>   	}
>         vec_safe_truncate (unparsed_contracts, 0);
>   
> -      current_class_ptr = save_ccp;
> -      current_class_ref = save_ccr;
> +      current_class_ptr = NULL_TREE;
> +      current_class_ref = NULL_TREE;
>         if (pushed_scope)
>   	pop_scope (pushed_scope);
>   
> @@ -26529,6 +26532,8 @@ cp_parser_class_specifier (cp_parser* parser)
>       = saved_num_template_parameter_lists;
>     parser->in_unbraced_linkage_specification_p
>       = saved_in_unbraced_linkage_specification_p;
> +  current_class_ptr = saved_ccp;
> +  current_class_ref = saved_ccr;
>   
>     return type;
>   }
> diff --git a/gcc/testsuite/g++.dg/lookup/name-clash11.C b/gcc/testsuite/g++.dg/lookup/name-clash11.C
> index bc63645e8d3..2ae9a65264d 100644
> --- a/gcc/testsuite/g++.dg/lookup/name-clash11.C
> +++ b/gcc/testsuite/g++.dg/lookup/name-clash11.C
> @@ -7,7 +7,7 @@
>   #  define ASSERT(e) static_assert (e, #e)
>   #else
>   #  define ASSERT(e)                                             \
> -  do { struct S { bool: !!(e); } asrt; (void)&asrt; } while (0)
> +  do { typedef int asrt[bool(e) ? 1 : -1]; } while (0)
>   #endif
>   
>   
> diff --git a/gcc/testsuite/g++.dg/lookup/this2.C b/gcc/testsuite/g++.dg/lookup/this2.C
> new file mode 100644
> index 00000000000..1450c563d92
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/this2.C
> @@ -0,0 +1,22 @@
> +// PR c++/106969
> +// { dg-do compile { target c++11 } }
> +
> +struct Context
> +{
> +    void
> +    action() const
> +    {
> +        struct Foo
> +        {
> +            int wrapped;
> +            decltype( &wrapped ) get() { return &wrapped; }
> +        } t;
> +
> +        *t.get()= 42; // OK, get() returns int* not const int*
> +
> +        struct Bar
> +        {
> +            using type = decltype(this); // { dg-error "invalid use of 'this'" }
> +        };
> +    }
> +};