diff mbox series

c++: ICE with delayed noexcept and attribute used [PR97966]

Message ID 20210113021346.98238-1-polacek@redhat.com
State New
Headers show
Series c++: ICE with delayed noexcept and attribute used [PR97966] | expand

Commit Message

Marek Polacek Jan. 13, 2021, 2:13 a.m. UTC
Another ICE with delayed noexcept parsing, but a bit gnarlier.

A function definition marked with __attribute__((used)) ought to be
emitted even when it is not referenced in a TU.  For a member function
template marked with __attribute__((used)) this means that it will
be instantiated: in instantiate_class_template_1 we have

11971               /* Instantiate members marked with attribute used.  */
11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
11973                 mark_used (r);

It is not so surprising that this doesn't work well with delayed
noexcept parsing: when we're processing the function template we delay
the parsing, so the member "foo" is found, but then when we're
instantiating it, "foo" hasn't yet been seen, which creates a
discrepancy and a crash ensues.  "foo" hasn't yet been seen because
instantiate_class_template_1 just loops over the class members and
instantiates right away.

It stands to reason to disable delayed noexcept parsing when the
function is marked with used (clang++ also rejects).  Unfortunately
we can't just use lookup_attribute when parsing and check if "used"
is there and if so, clear CP_PARSER_FLAGS_DELAY_NOEXCEPT, because
we accept attributes following the noexcept-specifier, like this:

  void g() noexcept(...) __attribute__((used));

Oh well.  This patch should handle various forms of attributes in
one place.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

gcc/cp/ChangeLog:

	PR c++/97966
	* parser.c (cp_parser_save_default_args): Don't delay parsing of
	the noexcept-specifier of a function marked with attribute used.

gcc/testsuite/ChangeLog:

	PR c++/97966
	* g++.dg/cpp0x/noexcept63.C: New test.
---
 gcc/cp/parser.c                         | 20 +++++++++-
 gcc/testsuite/g++.dg/cpp0x/noexcept63.C | 49 +++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept63.C


base-commit: cfaaa6a1ca744c1a93fa08a3e7ab2a821383cac1

Comments

Jason Merrill Jan. 19, 2021, 8:37 p.m. UTC | #1
On 1/12/21 9:13 PM, Marek Polacek wrote:
> Another ICE with delayed noexcept parsing, but a bit gnarlier.
> 
> A function definition marked with __attribute__((used)) ought to be
> emitted even when it is not referenced in a TU.  For a member function
> template marked with __attribute__((used)) this means that it will
> be instantiated: in instantiate_class_template_1 we have
> 
> 11971               /* Instantiate members marked with attribute used.  */
> 11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
> 11973                 mark_used (r);
> 
> It is not so surprising that this doesn't work well with delayed
> noexcept parsing: when we're processing the function template we delay
> the parsing, so the member "foo" is found, but then when we're
> instantiating it, "foo" hasn't yet been seen, which creates a
> discrepancy and a crash ensues.  "foo" hasn't yet been seen because
> instantiate_class_template_1 just loops over the class members and
> instantiates right away.

That seems like the bug; we shouldn't instantiate any members until 
we're done instantiating the class.

Jason
Marek Polacek Jan. 21, 2021, 12:49 a.m. UTC | #2
On Tue, Jan 19, 2021 at 03:37:25PM -0500, Jason Merrill wrote:
> On 1/12/21 9:13 PM, Marek Polacek wrote:
> > Another ICE with delayed noexcept parsing, but a bit gnarlier.
> > 
> > A function definition marked with __attribute__((used)) ought to be
> > emitted even when it is not referenced in a TU.  For a member function
> > template marked with __attribute__((used)) this means that it will
> > be instantiated: in instantiate_class_template_1 we have
> > 
> > 11971               /* Instantiate members marked with attribute used.  */
> > 11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
> > 11973                 mark_used (r);
> > 
> > It is not so surprising that this doesn't work well with delayed
> > noexcept parsing: when we're processing the function template we delay
> > the parsing, so the member "foo" is found, but then when we're
> > instantiating it, "foo" hasn't yet been seen, which creates a
> > discrepancy and a crash ensues.  "foo" hasn't yet been seen because
> > instantiate_class_template_1 just loops over the class members and
> > instantiates right away.
> 
> That seems like the bug; we shouldn't instantiate any members until we're
> done instantiating the class.

I reckon we can make it work, then:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Another ICE with delayed noexcept parsing, but a bit gnarlier.

A function definition marked with __attribute__((used)) ought to be
emitted even when it is not referenced in the TU.  For a member function
template marked with __attribute__((used)) this means that it will
be instantiated: in instantiate_class_template_1 we have

11971               /* Instantiate members marked with attribute used.  */
11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
11973                 mark_used (r);

It is not so surprising that this doesn't work well with delayed
noexcept parsing: when we're processing the function template we delay
the parsing, so the member "foo" is found, but then when we're
instantiating it, "foo" hasn't yet been seen, which creates a
discrepancy and a crash ensues.  "foo" hasn't yet been seen because
instantiate_class_template_1 just loops over the class members and
instantiates right away.

To make it work, this patch uses a vector to keep track of members
marked with attribute used and uses it to instantiate such members
only after we're done with the class; in particular, after we have
called finish_member_declaration for each member.  And we ought to
be verifying that we did emit such members, so I've added a bunch
of dg-finals.

gcc/cp/ChangeLog:

	PR c++/97966
	* pt.c (instantiate_class_template_1): Instantiate members
	marked with attribute used only after we're done instantiating
	the class.

gcc/testsuite/ChangeLog:

	PR c++/97966
	* g++.dg/cpp0x/noexcept63.C: New test.
---
 gcc/cp/pt.c                             | 12 ++++-
 gcc/testsuite/g++.dg/cpp0x/noexcept63.C | 63 +++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept63.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 373f8279604..81c78322a46 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11895,6 +11895,9 @@ instantiate_class_template_1 (tree type)
      relative to the scope of the class.  */
   pop_to_parent_deferring_access_checks ();
 
+  /* A vector to hold members marked with attribute used. */
+  auto_vec<tree> used;
+
   /* Now members are processed in the order of declaration.  */
   for (member = CLASSTYPE_DECL_LIST (pattern);
        member; member = TREE_CHAIN (member))
@@ -11968,7 +11971,7 @@ instantiate_class_template_1 (tree type)
 	      finish_member_declaration (r);
 	      /* Instantiate members marked with attribute used.  */
 	      if (r != error_mark_node && DECL_PRESERVE_P (r))
-		mark_used (r);
+		used.safe_push (r);
 	      if (TREE_CODE (r) == FUNCTION_DECL
 		  && DECL_OMP_DECLARE_REDUCTION_P (r))
 		cp_check_omp_declare_reduction (r);
@@ -12034,7 +12037,7 @@ instantiate_class_template_1 (tree type)
 			     /*flags=*/0);
 			  /* Instantiate members marked with attribute used. */
 			  if (r != error_mark_node && DECL_PRESERVE_P (r))
-			    mark_used (r);
+			    used.safe_push (r);
 			}
 		      else if (TREE_CODE (r) == FIELD_DECL)
 			{
@@ -12185,6 +12188,11 @@ instantiate_class_template_1 (tree type)
 	}
     }
 
+  /* Now that we've gone through all the members, instantiate those
+     marked with attribute used.  */
+  for (tree &x : used)
+    mark_used (x);
+
   if (fn_context)
     {
       /* Restore these before substituting into the lambda capture
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept63.C b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
new file mode 100644
index 00000000000..cf048f56c2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
@@ -0,0 +1,63 @@
+// PR c++/97966
+// { dg-do compile { target c++11 } }
+
+template <int>
+struct S1 {
+  __attribute__((used)) S1() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S2 {
+  __attribute__((used)) void bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S3 {
+  void __attribute__((used)) bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S4 {
+  [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S5 {
+  void bar() noexcept(noexcept(this->foo())) __attribute__((used)) { }
+  void foo();
+};
+
+template <int>
+struct S6 {
+  template <int>
+  struct N {
+    [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
+    void foo();
+  };
+};
+
+void
+g ()
+{
+  S1<1> s1;
+  S2<1> s2;
+  S3<1> s3;
+  S4<1> s4;
+  S5<1> s5;
+  S6<1>::N<1> n;
+}
+
+// Make sure that we did emit the functions marked with attribute used
+// even though they're not referenced in this TU.  (Well, the S1()
+// constructor is.)
+// { dg-final { scan-assembler "_ZN2S1ILi1EEC1Ev" } }
+// { dg-final { scan-assembler "_ZN2S1ILi1EEC2Ev" } }
+// { dg-final { scan-assembler "_ZN2S2ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S3ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S4ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S5ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S6ILi1EE1NILi1EE3barEv" } }

base-commit: b93d0e36c0a86c3d15310fe7383321ca63aeb04d
Jason Merrill Jan. 21, 2021, 6:55 a.m. UTC | #3
On 1/20/21 7:49 PM, Marek Polacek wrote:
> On Tue, Jan 19, 2021 at 03:37:25PM -0500, Jason Merrill wrote:
>> On 1/12/21 9:13 PM, Marek Polacek wrote:
>>> Another ICE with delayed noexcept parsing, but a bit gnarlier.
>>>
>>> A function definition marked with __attribute__((used)) ought to be
>>> emitted even when it is not referenced in a TU.  For a member function
>>> template marked with __attribute__((used)) this means that it will
>>> be instantiated: in instantiate_class_template_1 we have
>>>
>>> 11971               /* Instantiate members marked with attribute used.  */
>>> 11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
>>> 11973                 mark_used (r);
>>>
>>> It is not so surprising that this doesn't work well with delayed
>>> noexcept parsing: when we're processing the function template we delay
>>> the parsing, so the member "foo" is found, but then when we're
>>> instantiating it, "foo" hasn't yet been seen, which creates a
>>> discrepancy and a crash ensues.  "foo" hasn't yet been seen because
>>> instantiate_class_template_1 just loops over the class members and
>>> instantiates right away.
>>
>> That seems like the bug; we shouldn't instantiate any members until we're
>> done instantiating the class.
> 
> I reckon we can make it work, then:
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> Another ICE with delayed noexcept parsing, but a bit gnarlier.
> 
> A function definition marked with __attribute__((used)) ought to be
> emitted even when it is not referenced in the TU.  For a member function
> template marked with __attribute__((used)) this means that it will
> be instantiated: in instantiate_class_template_1 we have
> 
> 11971               /* Instantiate members marked with attribute used.  */
> 11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
> 11973                 mark_used (r);
> 
> It is not so surprising that this doesn't work well with delayed
> noexcept parsing: when we're processing the function template we delay
> the parsing, so the member "foo" is found, but then when we're
> instantiating it, "foo" hasn't yet been seen, which creates a
> discrepancy and a crash ensues.  "foo" hasn't yet been seen because
> instantiate_class_template_1 just loops over the class members and
> instantiates right away.
> 
> To make it work, this patch uses a vector to keep track of members
> marked with attribute used and uses it to instantiate such members
> only after we're done with the class; in particular, after we have
> called finish_member_declaration for each member.  And we ought to
> be verifying that we did emit such members, so I've added a bunch
> of dg-finals.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97966
> 	* pt.c (instantiate_class_template_1): Instantiate members
> 	marked with attribute used only after we're done instantiating
> 	the class.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/97966
> 	* g++.dg/cpp0x/noexcept63.C: New test.
> ---
>   gcc/cp/pt.c                             | 12 ++++-
>   gcc/testsuite/g++.dg/cpp0x/noexcept63.C | 63 +++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 373f8279604..81c78322a46 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -11895,6 +11895,9 @@ instantiate_class_template_1 (tree type)
>        relative to the scope of the class.  */
>     pop_to_parent_deferring_access_checks ();
>   
> +  /* A vector to hold members marked with attribute used. */
> +  auto_vec<tree> used;
> +
>     /* Now members are processed in the order of declaration.  */
>     for (member = CLASSTYPE_DECL_LIST (pattern);
>          member; member = TREE_CHAIN (member))
> @@ -11968,7 +11971,7 @@ instantiate_class_template_1 (tree type)
>   	      finish_member_declaration (r);
>   	      /* Instantiate members marked with attribute used.  */
>   	      if (r != error_mark_node && DECL_PRESERVE_P (r))
> -		mark_used (r);
> +		used.safe_push (r);
>   	      if (TREE_CODE (r) == FUNCTION_DECL
>   		  && DECL_OMP_DECLARE_REDUCTION_P (r))
>   		cp_check_omp_declare_reduction (r);
> @@ -12034,7 +12037,7 @@ instantiate_class_template_1 (tree type)
>   			     /*flags=*/0);
>   			  /* Instantiate members marked with attribute used. */
>   			  if (r != error_mark_node && DECL_PRESERVE_P (r))
> -			    mark_used (r);
> +			    used.safe_push (r);
>   			}
>   		      else if (TREE_CODE (r) == FIELD_DECL)
>   			{
> @@ -12185,6 +12188,11 @@ instantiate_class_template_1 (tree type)
>   	}
>       }
>   
> +  /* Now that we've gone through all the members, instantiate those
> +     marked with attribute used.  */
> +  for (tree &x : used)

This doesn't need to be a reference.  And I think we want this to happen 
even later, after finish_struct_1.

> +    mark_used (x);
> +
>     if (fn_context)
>       {
>         /* Restore these before substituting into the lambda capture
> diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept63.C b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> new file mode 100644
> index 00000000000..cf048f56c2a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> @@ -0,0 +1,63 @@
> +// PR c++/97966
> +// { dg-do compile { target c++11 } }
> +
> +template <int>
> +struct S1 {
> +  __attribute__((used)) S1() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S2 {
> +  __attribute__((used)) void bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S3 {
> +  void __attribute__((used)) bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S4 {
> +  [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S5 {
> +  void bar() noexcept(noexcept(this->foo())) __attribute__((used)) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S6 {
> +  template <int>
> +  struct N {
> +    [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
> +    void foo();
> +  };
> +};
> +
> +void
> +g ()
> +{
> +  S1<1> s1;
> +  S2<1> s2;
> +  S3<1> s3;
> +  S4<1> s4;
> +  S5<1> s5;
> +  S6<1>::N<1> n;
> +}
> +
> +// Make sure that we did emit the functions marked with attribute used
> +// even though they're not referenced in this TU.  (Well, the S1()
> +// constructor is.)
> +// { dg-final { scan-assembler "_ZN2S1ILi1EEC1Ev" } }
> +// { dg-final { scan-assembler "_ZN2S1ILi1EEC2Ev" } }
> +// { dg-final { scan-assembler "_ZN2S2ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S3ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S4ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S5ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S6ILi1EE1NILi1EE3barEv" } }
> 
> base-commit: b93d0e36c0a86c3d15310fe7383321ca63aeb04d
>
Marek Polacek Jan. 21, 2021, 3:39 p.m. UTC | #4
On Thu, Jan 21, 2021 at 01:55:24AM -0500, Jason Merrill wrote:
> > +  /* Now that we've gone through all the members, instantiate those
> > +     marked with attribute used.  */
> > +  for (tree &x : used)
> 
> This doesn't need to be a reference.  And I think we want this to happen
> even later, after finish_struct_1.

Fair enough, here's an updated version.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Another ICE with delayed noexcept parsing, but a bit gnarlier.

A function definition marked with __attribute__((used)) ought to be
emitted even when it is not referenced in the TU.  For a member function
template marked with __attribute__((used)) this means that it will
be instantiated: in instantiate_class_template_1 we have

11971               /* Instantiate members marked with attribute used.  */
11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
11973                 mark_used (r);

It is not so surprising that this doesn't work well with delayed
noexcept parsing: when we're processing the function template we delay
the parsing, so the member "foo" is found, but then when we're
instantiating it, "foo" hasn't yet been seen, which creates a
discrepancy and a crash ensues.  "foo" hasn't yet been seen because
instantiate_class_template_1 just loops over the class members and
instantiates right away.

To make it work, this patch uses a vector to keep track of members
marked with attribute used and uses it to instantiate such members
only after we're done with the class; in particular, after we have
called finish_member_declaration for each member.  And we ought to
be verifying that we did emit such members, so I've added a bunch
of dg-finals.

gcc/cp/ChangeLog:

	PR c++/97966
	* pt.c (instantiate_class_template_1): Instantiate members
	marked with attribute used only after we're done instantiating
	the class.

gcc/testsuite/ChangeLog:

	PR c++/97966
	* g++.dg/cpp0x/noexcept63.C: New test.
---
 gcc/cp/pt.c                             | 12 ++++-
 gcc/testsuite/g++.dg/cpp0x/noexcept63.C | 63 +++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept63.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 373f8279604..1f3850d1048 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11895,6 +11895,9 @@ instantiate_class_template_1 (tree type)
      relative to the scope of the class.  */
   pop_to_parent_deferring_access_checks ();
 
+  /* A vector to hold members marked with attribute used. */
+  auto_vec<tree> used;
+
   /* Now members are processed in the order of declaration.  */
   for (member = CLASSTYPE_DECL_LIST (pattern);
        member; member = TREE_CHAIN (member))
@@ -11968,7 +11971,7 @@ instantiate_class_template_1 (tree type)
 	      finish_member_declaration (r);
 	      /* Instantiate members marked with attribute used.  */
 	      if (r != error_mark_node && DECL_PRESERVE_P (r))
-		mark_used (r);
+		used.safe_push (r);
 	      if (TREE_CODE (r) == FUNCTION_DECL
 		  && DECL_OMP_DECLARE_REDUCTION_P (r))
 		cp_check_omp_declare_reduction (r);
@@ -12034,7 +12037,7 @@ instantiate_class_template_1 (tree type)
 			     /*flags=*/0);
 			  /* Instantiate members marked with attribute used. */
 			  if (r != error_mark_node && DECL_PRESERVE_P (r))
-			    mark_used (r);
+			    used.safe_push (r);
 			}
 		      else if (TREE_CODE (r) == FIELD_DECL)
 			{
@@ -12203,6 +12206,11 @@ instantiate_class_template_1 (tree type)
   finish_struct_1 (type);
   TYPE_BEING_DEFINED (type) = 0;
 
+  /* Now that we've gone through all the members, instantiate those
+     marked with attribute used.  */
+  for (tree x : used)
+    mark_used (x);
+
   /* We don't instantiate default arguments for member functions.  14.7.1:
 
      The implicit instantiation of a class template specialization causes
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept63.C b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
new file mode 100644
index 00000000000..cf048f56c2a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
@@ -0,0 +1,63 @@
+// PR c++/97966
+// { dg-do compile { target c++11 } }
+
+template <int>
+struct S1 {
+  __attribute__((used)) S1() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S2 {
+  __attribute__((used)) void bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S3 {
+  void __attribute__((used)) bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S4 {
+  [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
+  void foo();
+};
+
+template <int>
+struct S5 {
+  void bar() noexcept(noexcept(this->foo())) __attribute__((used)) { }
+  void foo();
+};
+
+template <int>
+struct S6 {
+  template <int>
+  struct N {
+    [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
+    void foo();
+  };
+};
+
+void
+g ()
+{
+  S1<1> s1;
+  S2<1> s2;
+  S3<1> s3;
+  S4<1> s4;
+  S5<1> s5;
+  S6<1>::N<1> n;
+}
+
+// Make sure that we did emit the functions marked with attribute used
+// even though they're not referenced in this TU.  (Well, the S1()
+// constructor is.)
+// { dg-final { scan-assembler "_ZN2S1ILi1EEC1Ev" } }
+// { dg-final { scan-assembler "_ZN2S1ILi1EEC2Ev" } }
+// { dg-final { scan-assembler "_ZN2S2ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S3ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S4ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S5ILi1EE3barEv" } }
+// { dg-final { scan-assembler "_ZN2S6ILi1EE1NILi1EE3barEv" } }

base-commit: 43705f3fa343e08b2fb030460fc5e2a969954398
Jason Merrill Jan. 21, 2021, 10:59 p.m. UTC | #5
On 1/21/21 10:39 AM, Marek Polacek wrote:
> On Thu, Jan 21, 2021 at 01:55:24AM -0500, Jason Merrill wrote:
>>> +  /* Now that we've gone through all the members, instantiate those
>>> +     marked with attribute used.  */
>>> +  for (tree &x : used)
>>
>> This doesn't need to be a reference.  And I think we want this to happen
>> even later, after finish_struct_1.
> 
> Fair enough, here's an updated version.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> Another ICE with delayed noexcept parsing, but a bit gnarlier.
> 
> A function definition marked with __attribute__((used)) ought to be
> emitted even when it is not referenced in the TU.  For a member function
> template marked with __attribute__((used)) this means that it will
> be instantiated: in instantiate_class_template_1 we have
> 
> 11971               /* Instantiate members marked with attribute used.  */
> 11972               if (r != error_mark_node && DECL_PRESERVE_P (r))
> 11973                 mark_used (r);
> 
> It is not so surprising that this doesn't work well with delayed
> noexcept parsing: when we're processing the function template we delay
> the parsing, so the member "foo" is found, but then when we're
> instantiating it, "foo" hasn't yet been seen, which creates a
> discrepancy and a crash ensues.  "foo" hasn't yet been seen because
> instantiate_class_template_1 just loops over the class members and
> instantiates right away.
> 
> To make it work, this patch uses a vector to keep track of members
> marked with attribute used and uses it to instantiate such members
> only after we're done with the class; in particular, after we have
> called finish_member_declaration for each member.  And we ought to
> be verifying that we did emit such members, so I've added a bunch
> of dg-finals.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97966
> 	* pt.c (instantiate_class_template_1): Instantiate members
> 	marked with attribute used only after we're done instantiating
> 	the class.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/97966
> 	* g++.dg/cpp0x/noexcept63.C: New test.
> ---
>   gcc/cp/pt.c                             | 12 ++++-
>   gcc/testsuite/g++.dg/cpp0x/noexcept63.C | 63 +++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 373f8279604..1f3850d1048 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -11895,6 +11895,9 @@ instantiate_class_template_1 (tree type)
>        relative to the scope of the class.  */
>     pop_to_parent_deferring_access_checks ();
>   
> +  /* A vector to hold members marked with attribute used. */
> +  auto_vec<tree> used;
> +
>     /* Now members are processed in the order of declaration.  */
>     for (member = CLASSTYPE_DECL_LIST (pattern);
>          member; member = TREE_CHAIN (member))
> @@ -11968,7 +11971,7 @@ instantiate_class_template_1 (tree type)
>   	      finish_member_declaration (r);
>   	      /* Instantiate members marked with attribute used.  */
>   	      if (r != error_mark_node && DECL_PRESERVE_P (r))
> -		mark_used (r);
> +		used.safe_push (r);
>   	      if (TREE_CODE (r) == FUNCTION_DECL
>   		  && DECL_OMP_DECLARE_REDUCTION_P (r))
>   		cp_check_omp_declare_reduction (r);
> @@ -12034,7 +12037,7 @@ instantiate_class_template_1 (tree type)
>   			     /*flags=*/0);
>   			  /* Instantiate members marked with attribute used. */
>   			  if (r != error_mark_node && DECL_PRESERVE_P (r))
> -			    mark_used (r);
> +			    used.safe_push (r);
>   			}
>   		      else if (TREE_CODE (r) == FIELD_DECL)
>   			{
> @@ -12203,6 +12206,11 @@ instantiate_class_template_1 (tree type)
>     finish_struct_1 (type);
>     TYPE_BEING_DEFINED (type) = 0;
>   
> +  /* Now that we've gone through all the members, instantiate those
> +     marked with attribute used.  */
> +  for (tree x : used)
> +    mark_used (x);

Even farther down, I think, since access checks are part of the class 
instantiation, and I don't think doing it before all the popping gains 
us anything (though I'd be interested in knowing if this is wrong). 
Let's move it just before the return.  OK with that change.

>     /* We don't instantiate default arguments for member functions.  14.7.1:
>   
>        The implicit instantiation of a class template specialization causes
> diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept63.C b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> new file mode 100644
> index 00000000000..cf048f56c2a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
> @@ -0,0 +1,63 @@
> +// PR c++/97966
> +// { dg-do compile { target c++11 } }
> +
> +template <int>
> +struct S1 {
> +  __attribute__((used)) S1() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S2 {
> +  __attribute__((used)) void bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S3 {
> +  void __attribute__((used)) bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S4 {
> +  [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S5 {
> +  void bar() noexcept(noexcept(this->foo())) __attribute__((used)) { }
> +  void foo();
> +};
> +
> +template <int>
> +struct S6 {
> +  template <int>
> +  struct N {
> +    [[gnu::used]] void bar() noexcept(noexcept(this->foo())) { }
> +    void foo();
> +  };
> +};
> +
> +void
> +g ()
> +{
> +  S1<1> s1;
> +  S2<1> s2;
> +  S3<1> s3;
> +  S4<1> s4;
> +  S5<1> s5;
> +  S6<1>::N<1> n;
> +}
> +
> +// Make sure that we did emit the functions marked with attribute used
> +// even though they're not referenced in this TU.  (Well, the S1()
> +// constructor is.)
> +// { dg-final { scan-assembler "_ZN2S1ILi1EEC1Ev" } }
> +// { dg-final { scan-assembler "_ZN2S1ILi1EEC2Ev" } }
> +// { dg-final { scan-assembler "_ZN2S2ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S3ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S4ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S5ILi1EE3barEv" } }
> +// { dg-final { scan-assembler "_ZN2S6ILi1EE1NILi1EE3barEv" } }
> 
> base-commit: 43705f3fa343e08b2fb030460fc5e2a969954398
>
diff mbox series

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c713852fe93..07ab966e099 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -30880,7 +30880,25 @@  cp_parser_save_default_args (cp_parser* parser, tree decl)
   /* Remember if there is a noexcept-specifier to post process.  */
   tree spec = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl));
   if (UNPARSED_NOEXCEPT_SPEC_P (spec))
-    vec_safe_push (unparsed_noexcepts, decl);
+    {
+      /* Don't actually delay parsing of the noexcept-specifier of
+	 a member function marked with attribute used.  */
+      if (__builtin_expect (DECL_PRESERVE_P (decl), 0))
+	{
+	  auto cleanup = make_temp_override
+	    (parser->local_variables_forbidden_p);
+	  if (DECL_THIS_STATIC (decl))
+	    parser->local_variables_forbidden_p |= THIS_FORBIDDEN;
+	  inject_this_parameter (current_class_type, TYPE_UNQUALIFIED);
+	  spec = cp_parser_late_noexcept_specifier (parser,
+						    TREE_PURPOSE (spec));
+	  if (spec == error_mark_node)
+	    spec = NULL_TREE;
+	  fixup_deferred_exception_variants (TREE_TYPE (decl), spec);
+	}
+      else
+	vec_safe_push (unparsed_noexcepts, decl);
+    }
 }
 
 /* DEFAULT_ARG contains the saved tokens for the initializer of DECL,
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept63.C b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
new file mode 100644
index 00000000000..8efeac93a30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept63.C
@@ -0,0 +1,49 @@ 
+// PR c++/97966
+// { dg-do compile { target c++11 } }
+
+template <int>
+struct S1 {
+  __attribute__((used)) S1() noexcept(noexcept(this->foo())); // { dg-error "has no member" }
+  void foo();
+};
+
+template <int>
+struct S2 {
+  __attribute__((used)) void bar() noexcept(noexcept(this->foo())); // { dg-error "has no member" }
+  void foo();
+};
+
+template <int>
+struct S3 {
+  void __attribute__((used)) bar() noexcept(noexcept(this->foo())); // { dg-error "has no member" }
+  void foo();
+};
+
+template <int>
+struct S4 {
+  [[gnu::used]] void bar() noexcept(noexcept(this->foo())); // { dg-error "has no member" }
+  void foo();
+};
+
+template <int>
+struct S5 {
+  void bar() noexcept(noexcept(this->foo())) __attribute__((used)); // { dg-error "has no member" }
+  void foo();
+};
+
+template <int>
+struct S6 {
+  static void bar() noexcept(noexcept(this->foo())) __attribute__((used)); // { dg-error ".this. may not be used in this context" }
+  void foo();
+};
+
+void
+g ()
+{
+  S1<1> s1;
+  S2<1> s2;
+  S3<1> s3;
+  S4<1> s4;
+  S5<1> s5;
+  S6<1> s6;
+}