Re: [PATCH] Fix cgraph_node::rtl_info (PR target/77587)

[Jan Hubicka]

> Hi!
> 
> On the following testcase the weak alias can be overridden, so we shouldn't
> assume because we see bar to be alias to foo and have foo definition that
> call to bar will bind to the current TU's foo.
> cgraph_node::rtl_info calls ultimate_alias_target without checking if it can
> be overridden or not.
> The following patch fixes that, the decl != current_function_decl in there
> is there so that for the current function it always returns non-NULL,
> otherwise we ICE during bootstrap, and the node->decl != current_function_decl
> is there e.g. for the case where we'd call an alias ultimately pointing to
> the current function (TREE_ASM_WRITTEN would still not be set in that case).
> 
> The patch also removes extra calls to ultimate_alias_target, the function
> called it up to 4 times, while only one is enough.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK, thanks.
It may make sense to backrport this eventually to release branches.

Honza
> 
> 2016-09-16  Jakub Jelinek  
>   Jan Hubicka  
> 
>   PR target/77587
>   * cgraph.c (cgraph_node::rtl_info): Pass  to
>   ultimate_alias_target call, return NULL if avail < AVAIL_AVAILABLE.
>   Call ultimate_alias_target just once, not up to 4 times.
> 
>   * gcc.dg/pr77587.c: New test.
>   * gcc.dg/pr77587a.c: New file.
> 
> --- gcc/cgraph.c.jj   2016-08-06 12:11:49.0 +0200
> +++ gcc/cgraph.c  2016-09-16 14:19:31.553999007 +0200
> @@ -1955,14 +1955,17 @@ cgraph_node::rtl_info (tree decl)
>cgraph_node *node = get (decl);
>if (!node)
>  return NULL;
> -  node = node->ultimate_alias_target ();
> -  if (node->decl != current_function_decl
> -  && !TREE_ASM_WRITTEN (node->decl))
> +  enum availability avail;
> +  node = node->ultimate_alias_target ();
> +  if (decl != current_function_decl
> +  && (avail < AVAIL_AVAILABLE
> +   || (node->decl != current_function_decl
> +   && !TREE_ASM_WRITTEN (node->decl
>  return NULL;
> -  /* Allocate if it doesnt exist.  */
> -  if (node->ultimate_alias_target ()->rtl == NULL)
> -node->ultimate_alias_target ()->rtl = ggc_cleared_alloc 
> ();
> -  return node->ultimate_alias_target ()->rtl;
> +  /* Allocate if it doesn't exist.  */
> +  if (node->rtl == NULL)
> +node->rtl = ggc_cleared_alloc ();
> +  return node->rtl;
>  }
>  
>  /* Return a string describing the failure REASON.  */
> --- gcc/testsuite/gcc.dg/pr77587.c.jj 2016-09-15 14:06:09.449901026 +0200
> +++ gcc/testsuite/gcc.dg/pr77587.c2016-09-15 14:18:53.615467361 +0200
> @@ -0,0 +1,14 @@
> +/* PR target/77587 */
> +/* { dg-do run } */
> +/* { dg-require-weak-override "" } */
> +/* { dg-additional-sources "pr77587a.c" } */
> +
> +void
> +bar (long x, long y, long z)
> +{
> +  struct __attribute__((aligned (16))) S { long a, b, c, d; } s;
> +  char *p = (char *) 
> +  __asm ("" : "+r" (p));
> +  if (((__UINTPTR_TYPE__) p) & 15)
> +__builtin_abort ();
> +}
> --- gcc/testsuite/gcc.dg/pr77587a.c.jj2016-09-15 14:05:58.873031952 
> +0200
> +++ gcc/testsuite/gcc.dg/pr77587a.c   2016-09-15 14:16:57.242903986 +0200
> @@ -0,0 +1,23 @@
> +/* PR target/77587 */
> +/* { dg-do compile } */
> +/* { dg-require-weak-override "" } */
> +
> +void
> +foo (long x, long y, long z)
> +{
> +}
> +
> +void bar (long x, long y, long z) __attribute__ ((weak, alias ("foo")));
> +
> +void
> +baz (long x, long y, long z)
> +{
> +  bar (0, 0, 0);
> +}
> +
> +int
> +main ()
> +{
> +  baz (0, 0, 0);
> +  return 0;
> +}
> 
>   Jakub

[PATCH] Fix cgraph_node::rtl_info (PR target/77587)

[Jakub Jelinek]

Hi!

On the following testcase the weak alias can be overridden, so we shouldn't
assume because we see bar to be alias to foo and have foo definition that
call to bar will bind to the current TU's foo.
cgraph_node::rtl_info calls ultimate_alias_target without checking if it can
be overridden or not.
The following patch fixes that, the decl != current_function_decl in there
is there so that for the current function it always returns non-NULL,
otherwise we ICE during bootstrap, and the node->decl != current_function_decl
is there e.g. for the case where we'd call an alias ultimately pointing to
the current function (TREE_ASM_WRITTEN would still not be set in that case).

The patch also removes extra calls to ultimate_alias_target, the function
called it up to 4 times, while only one is enough.

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

2016-09-16  Jakub Jelinek  
Jan Hubicka  

PR target/77587
* cgraph.c (cgraph_node::rtl_info): Pass  to
ultimate_alias_target call, return NULL if avail < AVAIL_AVAILABLE.
Call ultimate_alias_target just once, not up to 4 times.

* gcc.dg/pr77587.c: New test.
* gcc.dg/pr77587a.c: New file.

--- gcc/cgraph.c.jj 2016-08-06 12:11:49.0 +0200
+++ gcc/cgraph.c2016-09-16 14:19:31.553999007 +0200
@@ -1955,14 +1955,17 @@ cgraph_node::rtl_info (tree decl)
   cgraph_node *node = get (decl);
   if (!node)
 return NULL;
-  node = node->ultimate_alias_target ();
-  if (node->decl != current_function_decl
-  && !TREE_ASM_WRITTEN (node->decl))
+  enum availability avail;
+  node = node->ultimate_alias_target ();
+  if (decl != current_function_decl
+  && (avail < AVAIL_AVAILABLE
+ || (node->decl != current_function_decl
+ && !TREE_ASM_WRITTEN (node->decl
 return NULL;
-  /* Allocate if it doesnt exist.  */
-  if (node->ultimate_alias_target ()->rtl == NULL)
-node->ultimate_alias_target ()->rtl = ggc_cleared_alloc 
();
-  return node->ultimate_alias_target ()->rtl;
+  /* Allocate if it doesn't exist.  */
+  if (node->rtl == NULL)
+node->rtl = ggc_cleared_alloc ();
+  return node->rtl;
 }
 
 /* Return a string describing the failure REASON.  */
--- gcc/testsuite/gcc.dg/pr77587.c.jj   2016-09-15 14:06:09.449901026 +0200
+++ gcc/testsuite/gcc.dg/pr77587.c  2016-09-15 14:18:53.615467361 +0200
@@ -0,0 +1,14 @@
+/* PR target/77587 */
+/* { dg-do run } */
+/* { dg-require-weak-override "" } */
+/* { dg-additional-sources "pr77587a.c" } */
+
+void
+bar (long x, long y, long z)
+{
+  struct __attribute__((aligned (16))) S { long a, b, c, d; } s;
+  char *p = (char *) 
+  __asm ("" : "+r" (p));
+  if (((__UINTPTR_TYPE__) p) & 15)
+__builtin_abort ();
+}
--- gcc/testsuite/gcc.dg/pr77587a.c.jj  2016-09-15 14:05:58.873031952 +0200
+++ gcc/testsuite/gcc.dg/pr77587a.c 2016-09-15 14:16:57.242903986 +0200
@@ -0,0 +1,23 @@
+/* PR target/77587 */
+/* { dg-do compile } */
+/* { dg-require-weak-override "" } */
+
+void
+foo (long x, long y, long z)
+{
+}
+
+void bar (long x, long y, long z) __attribute__ ((weak, alias ("foo")));
+
+void
+baz (long x, long y, long z)
+{
+  bar (0, 0, 0);
+}
+
+int
+main ()
+{
+  baz (0, 0, 0);
+  return 0;
+}

Jakub