c++: Fix apply_identity_attributes [PR102548]

Message ID 20211005075012.GA920498@tucnak
State Committed
Headers
Series c++: Fix apply_identity_attributes [PR102548] |

Commit Message

Jakub Jelinek Oct. 5, 2021, 7:50 a.m. UTC
  Hi!

The following testcase ICEs on x86_64-linux with -m32 due to a bug in
apply_identity_attributes.  The function is being smart and attempts not
to duplicate the chain unnecessarily, if either there are no attributes
that affect type identity or there is possibly empty set of attributes
that do not affect type identity in the chain followed by attributes
that do affect type identity, it reuses that attribute chain.

The function mishandles the cases where in the chain an attribute affects
type identity and is followed by one or more attributes that don't
affect type identity (and then perhaps some further ones that do).

There are two bugs.  One is that when we notice first attribute that
doesn't affect type identity after first attribute that does affect type
identity (with perhaps some further such attributes in the chain after it),
we want to put into the new chain just attributes starting from
(inclusive) first_ident and up to (exclusive) the current attribute a,
but the code puts into the chain all attributes starting with first_ident,
including the ones that do not affect type identity and if e.g. we have
doesn't0 affects1 doesn't2 affects3 affects4 sequence of attributes, the
resulting sequence would have
affects1 doesn't2 affects3 affects4 affects3 affects4
attributes, i.e. one attribute that shouldn't be there and two attributes
duplicated.  That is fixed by the a2 -> a2 != a change.

The second one is that we ICE once we see second attribute that doesn't
affect type identity after an attribute that affects it.  That is because
first_ident is set to error_mark_node after handling the first attribute
that doesn't affect type identity (i.e. after we've copied the
[first_ident, a) set of attributes to the new chain) to denote that from
that time on, each attribute that affects type identity should be copied
whenever it is seen (the if (as && as->affects_type_identity) code does
that correctly).  But that condition is false and first_ident is
error_mark_node, we enter else if (first_ident) and use TREE_PURPOSE
/TREE_VALUE/TREE_CHAIN on error_mark_node, which ICEs.  When
first_ident is error_mark_node and a doesn't affect type identity,
we want to do nothing.  So that is the && first_ident != error_mark_node
chunk.

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

2021-10-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102548
	* tree.c (apply_identity_attributes): Fix handling of the
	case where an attribute in the list doesn't affect type
	identity but some attribute before it does.

	* g++.target/i386/pr102548.C: New test.


	Jakub
  

Comments

Jason Merrill Oct. 5, 2021, 6:47 p.m. UTC | #1
On 10/5/21 03:50, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase ICEs on x86_64-linux with -m32 due to a bug in
> apply_identity_attributes.  The function is being smart and attempts not
> to duplicate the chain unnecessarily, if either there are no attributes
> that affect type identity or there is possibly empty set of attributes
> that do not affect type identity in the chain followed by attributes
> that do affect type identity, it reuses that attribute chain.
> 
> The function mishandles the cases where in the chain an attribute affects
> type identity and is followed by one or more attributes that don't
> affect type identity (and then perhaps some further ones that do).
> 
> There are two bugs.  One is that when we notice first attribute that
> doesn't affect type identity after first attribute that does affect type
> identity (with perhaps some further such attributes in the chain after it),
> we want to put into the new chain just attributes starting from
> (inclusive) first_ident and up to (exclusive) the current attribute a,
> but the code puts into the chain all attributes starting with first_ident,
> including the ones that do not affect type identity and if e.g. we have
> doesn't0 affects1 doesn't2 affects3 affects4 sequence of attributes, the
> resulting sequence would have
> affects1 doesn't2 affects3 affects4 affects3 affects4
> attributes, i.e. one attribute that shouldn't be there and two attributes
> duplicated.  That is fixed by the a2 -> a2 != a change.
> 
> The second one is that we ICE once we see second attribute that doesn't
> affect type identity after an attribute that affects it.  That is because
> first_ident is set to error_mark_node after handling the first attribute
> that doesn't affect type identity (i.e. after we've copied the
> [first_ident, a) set of attributes to the new chain) to denote that from
> that time on, each attribute that affects type identity should be copied
> whenever it is seen (the if (as && as->affects_type_identity) code does
> that correctly).  But that condition is false and first_ident is
> error_mark_node, we enter else if (first_ident) and use TREE_PURPOSE
> /TREE_VALUE/TREE_CHAIN on error_mark_node, which ICEs.  When
> first_ident is error_mark_node and a doesn't affect type identity,
> we want to do nothing.  So that is the && first_ident != error_mark_node
> chunk.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> and release branches?

OK.

> 2021-10-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102548
> 	* tree.c (apply_identity_attributes): Fix handling of the
> 	case where an attribute in the list doesn't affect type
> 	identity but some attribute before it does.
> 
> 	* g++.target/i386/pr102548.C: New test.
> 
> --- gcc/cp/tree.c.jj	2021-10-01 18:06:54.603452541 +0200
> +++ gcc/cp/tree.c	2021-10-04 19:52:28.767457791 +0200
> @@ -1499,9 +1499,9 @@ apply_identity_attributes (tree result,
>   	      p = &TREE_CHAIN (*p);
>   	    }
>   	}
> -      else if (first_ident)
> +      else if (first_ident && first_ident != error_mark_node)
>   	{
> -	  for (tree a2 = first_ident; a2; a2 = TREE_CHAIN (a2))
> +	  for (tree a2 = first_ident; a2 != a; a2 = TREE_CHAIN (a2))
>   	    {
>   	      *p = tree_cons (TREE_PURPOSE (a2), TREE_VALUE (a2), NULL_TREE);
>   	      p = &TREE_CHAIN (*p);
> --- gcc/testsuite/g++.target/i386/pr102548.C.jj	2021-10-04 20:06:19.314810708 +0200
> +++ gcc/testsuite/g++.target/i386/pr102548.C	2021-10-04 20:05:14.808717194 +0200
> @@ -0,0 +1,12 @@
> +// PR c++/102548
> +// { dg-do compile { target { c++14 && ia32 } } }
> +
> +typedef decltype(sizeof(0)) size_t;
> +struct tm;
> +extern "C" size_t __attribute__((__cdecl__)) strftime (char *, size_t, const char *, const struct tm *);
> +
> +auto
> +foo (void)
> +{
> +  return strftime;
> +}
> 
> 	Jakub
>
  

Patch

--- gcc/cp/tree.c.jj	2021-10-01 18:06:54.603452541 +0200
+++ gcc/cp/tree.c	2021-10-04 19:52:28.767457791 +0200
@@ -1499,9 +1499,9 @@  apply_identity_attributes (tree result,
 	      p = &TREE_CHAIN (*p);
 	    }
 	}
-      else if (first_ident)
+      else if (first_ident && first_ident != error_mark_node)
 	{
-	  for (tree a2 = first_ident; a2; a2 = TREE_CHAIN (a2))
+	  for (tree a2 = first_ident; a2 != a; a2 = TREE_CHAIN (a2))
 	    {
 	      *p = tree_cons (TREE_PURPOSE (a2), TREE_VALUE (a2), NULL_TREE);
 	      p = &TREE_CHAIN (*p);
--- gcc/testsuite/g++.target/i386/pr102548.C.jj	2021-10-04 20:06:19.314810708 +0200
+++ gcc/testsuite/g++.target/i386/pr102548.C	2021-10-04 20:05:14.808717194 +0200
@@ -0,0 +1,12 @@ 
+// PR c++/102548
+// { dg-do compile { target { c++14 && ia32 } } }
+
+typedef decltype(sizeof(0)) size_t;
+struct tm;
+extern "C" size_t __attribute__((__cdecl__)) strftime (char *, size_t, const char *, const struct tm *);
+
+auto
+foo (void)
+{
+  return strftime;
+}