[PR89341] Fix ICE on function definition with weakref/alias attributes attached

Message ID 8111902f-1d84-3dab-e8cf-c73fc417417a@linux.alibaba.com
State New
Headers show
Series
  • [PR89341] Fix ICE on function definition with weakref/alias attributes attached
Related show

Commit Message

JunMa March 26, 2019, 11:40 a.m.
Hi

According to gnu document of function attributes, neither weakref nor alias
could be attached to a function defined in current translation unit.
Although GCC checks the attributes under some circumstances, it still fails
on some cases and even causes ICE.

This patch checks whether alias/weakref attribute attaches on a function
declaration which has body, and removes the attribute later.
This also avoid the ICE.

Bootstrapped/regtested on x86_64-linux, ok for GCC10?

Regards
JunMa


gcc/ChangeLog

2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

     PR89341
     * cgraphunit.c (handle_alias_pairs): Check whether alias attribute 
attaches
     on a function declaration which has body.

gcc/testsuite/ChangeLog

2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

     PR89341
     * gcc.dg/attr-alias-6.c: New test.
     * gcc.dg/attr-weakref-5.c: Likewise.
---
 gcc/cgraphunit.c                      | 11 ++++++++---
 gcc/testsuite/gcc.dg/attr-alias-6.c   |  4 ++++
 gcc/testsuite/gcc.dg/attr-weakref-5.c |  4 ++++
 3 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/attr-alias-6.c
 create mode 100644 gcc/testsuite/gcc.dg/attr-weakref-5.c

Comments

JunMa May 5, 2019, 1:23 a.m. | #1
在 2019/3/26 下午7:40, JunMa 写道:
> Hi

>

> According to gnu document of function attributes, neither weakref nor 

> alias

> could be attached to a function defined in current translation unit.

> Although GCC checks the attributes under some circumstances, it still 

> fails

> on some cases and even causes ICE.

>

> This patch checks whether alias/weakref attribute attaches on a function

> declaration which has body, and removes the attribute later.

> This also avoid the ICE.

>

> Bootstrapped/regtested on x86_64-linux, ok for GCC10?

>

> Regards

> JunMa

>

>

> gcc/ChangeLog

>

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>

>     PR89341

>     * cgraphunit.c (handle_alias_pairs): Check whether alias attribute 

> attaches

>     on a function declaration which has body.

>

> gcc/testsuite/ChangeLog

>

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>

>     PR89341

>     * gcc.dg/attr-alias-6.c: New test.

>     * gcc.dg/attr-weakref-5.c: Likewise.


Ping.

Regards
JunMa
JunMa May 13, 2019, 8:53 a.m. | #2
在 2019/3/26 下午7:40, JunMa 写道:
> Hi

>

> According to gnu document of function attributes, neither weakref nor 

> alias

> could be attached to a function defined in current translation unit.

> Although GCC checks the attributes under some circumstances, it still 

> fails

> on some cases and even causes ICE.

>

> This patch checks whether alias/weakref attribute attaches on a function

> declaration which has body, and removes the attribute later.

> This also avoid the ICE.

>

> Bootstrapped/regtested on x86_64-linux, ok for GCC10?

>

> Regards

> JunMa

>

>

> gcc/ChangeLog

>

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>

>     PR89341

>     * cgraphunit.c (handle_alias_pairs): Check whether alias attribute 

> attaches

>     on a function declaration which has body.

>

> gcc/testsuite/ChangeLog

>

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>

>     PR89341

>     * gcc.dg/attr-alias-6.c: New test.

>     * gcc.dg/attr-weakref-5.c: Likewise.

Hi Jakub

   Since you are owner of this part, so I add you to cc list.
   Ping?

Regards
JunMa
Jakub Jelinek May 13, 2019, 9:02 a.m. | #3
On Mon, May 13, 2019 at 04:53:52PM +0800, JunMa wrote:
> > According to gnu document of function attributes, neither weakref nor

> > alias

> > could be attached to a function defined in current translation unit.

> > Although GCC checks the attributes under some circumstances, it still

> > fails

> > on some cases and even causes ICE.

> > 

> > This patch checks whether alias/weakref attribute attaches on a function

> > declaration which has body, and removes the attribute later.

> > This also avoid the ICE.

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for GCC10?

> > 

> > 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

> > 

> >     PR89341

> >     * cgraphunit.c (handle_alias_pairs): Check whether alias attribute

> > attaches

> >     on a function declaration which has body.

> > 

> > gcc/testsuite/ChangeLog

> > 

> > 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

> > 

> >     PR89341

> >     * gcc.dg/attr-alias-6.c: New test.

> >     * gcc.dg/attr-weakref-5.c: Likewise.

> Hi Jakub

> 

>   Since you are owner of this part, so I add you to cc list.

>   Ping?


I'm not the maintainer of the callgraph, Honza is:
callgraph               Jan Hubicka             <hubicka@ucw.cz>

	Jakub
JunMa May 13, 2019, 9:11 a.m. | #4
在 2019/5/13 下午5:02, Jakub Jelinek 写道:
> On Mon, May 13, 2019 at 04:53:52PM +0800, JunMa wrote:

>>> According to gnu document of function attributes, neither weakref nor

>>> alias

>>> could be attached to a function defined in current translation unit.

>>> Although GCC checks the attributes under some circumstances, it still

>>> fails

>>> on some cases and even causes ICE.

>>>

>>> This patch checks whether alias/weakref attribute attaches on a function

>>> declaration which has body, and removes the attribute later.

>>> This also avoid the ICE.

>>>

>>> Bootstrapped/regtested on x86_64-linux, ok for GCC10?

>>>

>>> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>>>

>>>      PR89341

>>>      * cgraphunit.c (handle_alias_pairs): Check whether alias attribute

>>> attaches

>>>      on a function declaration which has body.

>>>

>>> gcc/testsuite/ChangeLog

>>>

>>> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>>>

>>>      PR89341

>>>      * gcc.dg/attr-alias-6.c: New test.

>>>      * gcc.dg/attr-weakref-5.c: Likewise.

>> Hi Jakub

>>

>>    Since you are owner of this part, so I add you to cc list.

>>    Ping?

> I'm not the maintainer of the callgraph, Honza is:

> callgraph               Jan Hubicka             <hubicka@ucw.cz>

Hi Jakub

     Thanks for your reminder.
     Honza, Ping?

Regards
JunMa
> 	Jakub
Jeff Law June 18, 2019, 8:38 p.m. | #5
On 3/26/19 5:40 AM, JunMa wrote:
> Hi

> 

> According to gnu document of function attributes, neither weakref nor alias

> could be attached to a function defined in current translation unit.

> Although GCC checks the attributes under some circumstances, it still fails

> on some cases and even causes ICE.

> 

> This patch checks whether alias/weakref attribute attaches on a function

> declaration which has body, and removes the attribute later.

> This also avoid the ICE.

> 

> Bootstrapped/regtested on x86_64-linux, ok for GCC10?

> 

> Regards

> JunMa

> 

> 

> gcc/ChangeLog

> 

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

> 

>     PR89341

>     * cgraphunit.c (handle_alias_pairs): Check whether alias attribute

> attaches

>     on a function declaration which has body.

> 

> gcc/testsuite/ChangeLog

> 

> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

> 

>     PR89341

>     * gcc.dg/attr-alias-6.c: New test.

>     * gcc.dg/attr-weakref-5.c: Likewise.

Based on my reading of the BZ, this should result in a hard error,
rather than an "attribute ignored" warning.

Jeff
JunMa June 27, 2019, 7:15 a.m. | #6
在 2019/6/19 上午4:38, Jeff Law 写道:
> On 3/26/19 5:40 AM, JunMa wrote:

>> Hi

>>

>> According to gnu document of function attributes, neither weakref nor alias

>> could be attached to a function defined in current translation unit.

>> Although GCC checks the attributes under some circumstances, it still fails

>> on some cases and even causes ICE.

>>

>> This patch checks whether alias/weakref attribute attaches on a function

>> declaration which has body, and removes the attribute later.

>> This also avoid the ICE.

>>

>> Bootstrapped/regtested on x86_64-linux, ok for GCC10?

>>

>> Regards

>> JunMa

>>

>>

>> gcc/ChangeLog

>>

>> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>>

>>      PR89341

>>      * cgraphunit.c (handle_alias_pairs): Check whether alias attribute

>> attaches

>>      on a function declaration which has body.

>>

>> gcc/testsuite/ChangeLog

>>

>> 2019-03-26  Jun Ma <JunMa@linux.alibaba.com>

>>

>>      PR89341

>>      * gcc.dg/attr-alias-6.c: New test.

>>      * gcc.dg/attr-weakref-5.c: Likewise.

> Based on my reading of the BZ, this should result in a hard error,

> rather than an "attribute ignored" warning.

Sorry for the late reply.

Yes, It should error out with message like "'foo' defined both normally
and as 'alias' attribute". However, this patch just tries to fix ICE, and
keeps remain unchanged.

I do have a patch to fix the message and the testcases, I'll send it later.

Regards
JunMa
> Jeff

Patch

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8bfbd0b..733e184 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1405,14 +1405,20 @@  handle_alias_pairs (void)
   for (i = 0; alias_pairs && alias_pairs->iterate (i, &p);)
     {
       symtab_node *target_node = symtab_node::get_for_asmname (p->target);
-
+      symtab_node *node = symtab_node::get (p->decl);
+      if (node && TREE_CODE (p->decl) == FUNCTION_DECL
+	  && DECL_SAVED_TREE (p->decl))
+	{
+	  node->alias = false;
+	  alias_pairs->unordered_remove (i);
+	  continue;
+	}
       /* Weakrefs with target not defined in current unit are easy to handle:
 	 they behave just as external variables except we need to note the
 	 alias flag to later output the weakref pseudo op into asm file.  */
       if (!target_node
 	  && lookup_attribute ("weakref", DECL_ATTRIBUTES (p->decl)) != NULL)
 	{
-	  symtab_node *node = symtab_node::get (p->decl);
 	  if (node)
 	    {
 	      node->alias_target = p->target;
@@ -1426,7 +1432,6 @@  handle_alias_pairs (void)
       else if (!target_node)
 	{
 	  error ("%q+D aliased to undefined symbol %qE", p->decl, p->target);
-	  symtab_node *node = symtab_node::get (p->decl);
 	  if (node)
 	    node->alias = false;
 	  alias_pairs->unordered_remove (i);
diff --git a/gcc/testsuite/gcc.dg/attr-alias-6.c b/gcc/testsuite/gcc.dg/attr-alias-6.c
new file mode 100644
index 0000000..a3ec311
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-alias-6.c
@@ -0,0 +1,4 @@ 
+/* { dg-do compile } */
+/* { dg-require-alias "" } */
+static void __attribute__((alias("bar"))) foo(void) { } /* { dg-warning "attribute ignored because function is defined" } */
+void bar(void);
diff --git a/gcc/testsuite/gcc.dg/attr-weakref-5.c b/gcc/testsuite/gcc.dg/attr-weakref-5.c
new file mode 100644
index 0000000..bb21126
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-weakref-5.c
@@ -0,0 +1,4 @@ 
+/* { dg-do compile } */
+/* { dg-require-weak "" } */
+static void __attribute__((weakref("bar"))) foo(void) { } /* { dg-warning "attribute ignored because function is defined" } */
+void foo(void);