[C++] Avoid ICE due to the attribute exclusion additions (PR c++/83322)

Message ID 20171212190124.GY2353@tucnak
State New
Headers show
Series
  • [C++] Avoid ICE due to the attribute exclusion additions (PR c++/83322)
Related show

Commit Message

Jakub Jelinek Dec. 12, 2017, 7:01 p.m.
Hi!

This patch avoids ICEs when last_decl isn't a decl.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Though, I believe it would be better to do the attribute exclusions check
in duplicate_decls instead if the attributes don't appear together already
on a single decl, instead of trying to do another lookup.  I've filed a PR
where it makes a difference.

2017-12-12  Jakub Jelinek  <jakub@redhat.com>

	PR c++/83322
	* decl2.c (cplus_decl_attributes): If last_decl is BASELINK, use
	BASELINK_FUNCTIONS.  If after OVERLOAD checking last_decl isn't
	a decl, clear it.

	* g++.dg/warn/pr83322.C: New test.


	Jakub

Comments

Martin Sebor Dec. 12, 2017, 7:55 p.m. | #1
On 12/12/2017 12:01 PM, Jakub Jelinek wrote:
> Hi!

>

> This patch avoids ICEs when last_decl isn't a decl.

>

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

>

> Though, I believe it would be better to do the attribute exclusions check

> in duplicate_decls instead if the attributes don't appear together already

> on a single decl, instead of trying to do another lookup.  I've filed a PR

> where it makes a difference.


The reason for implementing attribute checking in the middle-end
is so that it can be done consistently across all front-ends and
back-ends, without back-end maintainers having to change the front
end code, and so that conflicting attributes can be dropped before
they are applied.  I went into more detail in my reply to Jason
here:

   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01371.html

I'm just starting to look into the bug you filed (83394) but from
what I've seen so far it seems to be a problem with calling
lookup_name() to find the already declared class member conversion
operator.  I'm sure there's a way to look it up and fix the bug
without changing the fundamental design of the improvement.

Martin
Jakub Jelinek Dec. 12, 2017, 8:03 p.m. | #2
On Tue, Dec 12, 2017 at 12:55:29PM -0700, Martin Sebor wrote:
> On 12/12/2017 12:01 PM, Jakub Jelinek wrote:

> > Hi!

> > 

> > This patch avoids ICEs when last_decl isn't a decl.

> > 

> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

> > 

> > Though, I believe it would be better to do the attribute exclusions check

> > in duplicate_decls instead if the attributes don't appear together already

> > on a single decl, instead of trying to do another lookup.  I've filed a PR

> > where it makes a difference.

> 

> The reason for implementing attribute checking in the middle-end

> is so that it can be done consistently across all front-ends and

> back-ends, without back-end maintainers having to change the front

> end code, and so that conflicting attributes can be dropped before

> they are applied.  I went into more detail in my reply to Jason

> here:

> 

>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01371.html

> 

> I'm just starting to look into the bug you filed (83394) but from

> what I've seen so far it seems to be a problem with calling

> lookup_name() to find the already declared class member conversion

> operator.  I'm sure there's a way to look it up and fix the bug

> without changing the fundamental design of the improvement.


Then you need to duplicate the name lookup, which is quite complicated
and compile time expensive.  Especially in C++, there are too many ways
how attributes can be introduced, e.g. on templates (including late vs.
early attributes), their specializations etc., so I doubt a simple
lookup_name can do what the callers of duplicate_decls do.

	Jakub

Patch

--- gcc/cp/decl2.c.jj	2017-12-07 18:04:58.000000000 +0100
+++ gcc/cp/decl2.c	2017-12-12 15:40:33.634654947 +0100
@@ -1486,6 +1486,9 @@  cplus_decl_attributes (tree *decl, tree
       tree last_decl = (DECL_P (*decl) && DECL_NAME (*decl)
 			? lookup_name (DECL_NAME (*decl)) : NULL_TREE);
 
+      if (last_decl && BASELINK_P (last_decl))
+	last_decl = BASELINK_FUNCTIONS (last_decl);
+
       if (last_decl && TREE_CODE (last_decl) == OVERLOAD)
 	for (ovl_iterator iter (last_decl, true); ; ++iter)
 	  {
@@ -1504,6 +1507,8 @@  cplus_decl_attributes (tree *decl, tree
 		break;
 	      }
 	  }
+      if (last_decl && !DECL_P (last_decl))
+	last_decl = NULL_TREE;
 
       decl_attributes (decl, attributes, flags, last_decl);
     }
--- gcc/testsuite/g++.dg/warn/pr83322.C.jj	2017-12-12 15:40:01.054047913 +0100
+++ gcc/testsuite/g++.dg/warn/pr83322.C	2017-12-12 15:39:13.000000000 +0100
@@ -0,0 +1,8 @@ 
+// PR c++/83322
+// { dg-do compile }
+// { dg-options "" }
+
+struct A {
+  template <class T> operator T ();
+  __attribute__((__always_inline__)) operator int ();
+};