diff mbox

[PR,48333] avoid -fcompare-debug errors from builtins in MEM attrs

Message ID orwrjdyt4q.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva April 2, 2011, 8:06 a.m. UTC 
I caught this with bootstrap-debug-lean, then realized a bug report was
open about the same problem.  When Jakub introduced MEM attrs for the
MEMs that refer to the function to be called, it exposed a rare
problem.

Say we're compiling a translation unit that, in one function A, calls
foo (available as __builtin_foo), and in another function B, calls bar,
that GCC internally folded to __builtin_foo, a separate but equivalent
FUNCTION_DECL as far as MEM attrs are concerned.

While compiling A, we introduce in the MEM attrs hash table an attribute
for foo.  Then, while compiling B, we find and reuse the same MEM attr.
So far, so good.

Now, while recompiling for -fcompare-debug, the garbage collector
decides to run between the compilations of A and B, and it finds that
the MEM attr created for A's RTL is no longer necessary, throwing it
away.  Then, when it compiles B, it does not find a pre-existing MEM,
and introduces one for __builtin_foo.

It should be obvious now that the final RTL dumps that -fcompare-debug
compares for B will differ in this case, because in one case it will say
foo, and in the other, __builtin_foo.

This patch avoids this particular instance of the problem by recording
the __builtin FUNCTION_DECL in the MEM attrs for calls.  This ought to
be enough to fix this particular instance of the problem, but I'm a bit
concerned that other, more convoluted versions of it might still be
lingering around.  Changing the tree expressions when creating
attributes might be doable, but I find that highly undesirable; another
option would be to always dump functions as their builtins; there are
probably other options too.

This regstrapped on x86_64-linux-gnu.  Ok to install?

Comments

Alexandre Oliva June 3, 2011, 2:17 a.m. UTC | #1 
Ping?  This fixes a case in which -g might change the executable code,
exposed with bootstrap-debug-lean.

On Apr  2, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> 	PR debug/48333
> 	* calls.c (emit_call_1): Prefer the __builtin declaration of
> 	builtin functions.

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00114.html
Richard Biener June 3, 2011, 12:28 p.m. UTC | #2 
On Fri, Jun 3, 2011 at 4:17 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> Ping?  This fixes a case in which -g might change the executable code,
> exposed with bootstrap-debug-lean.
>
> On Apr  2, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>>       PR debug/48333
>>       * calls.c (emit_call_1): Prefer the __builtin declaration of
>>       builtin functions.
>
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00114.html

Ok.

Thanks,
Richard.

> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
diff mbox

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/48333
	* calls.c (emit_call_1): Prefer the __builtin declaration of
	builtin functions.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c.orig	2011-03-31 01:40:42.960373301 -0300
+++ gcc/calls.c	2011-03-31 01:46:27.655319378 -0300
@@ -272,7 +272,20 @@  emit_call_1 (rtx funexp, tree fntree ATT
 
   funmem = gen_rtx_MEM (FUNCTION_MODE, funexp);
   if (fndecl && TREE_CODE (fndecl) == FUNCTION_DECL)
-    set_mem_expr (funmem, fndecl);
+    {
+      tree t = fndecl;
+      /* Although a built-in FUNCTION_DECL and its non-__builtin
+	 counterpart compare equal and get a shared mem_attrs, they
+	 produce different dump output in compare-debug compilations,
+	 if an entry gets garbage collected in one compilation, then
+	 adds a different (but equivalent) entry, while the other
+	 doesn't run the garbage collector at the same spot and then
+	 shares the mem_attr with the equivalent entry. */
+      if (DECL_BUILT_IN_CLASS (t) == BUILT_IN_NORMAL
+	  && built_in_decls[DECL_FUNCTION_CODE (t)])
+	t = built_in_decls[DECL_FUNCTION_CODE (t)];
+      set_mem_expr (funmem, t);
+    }
   else if (fntree)
     set_mem_expr (funmem, build_fold_indirect_ref (CALL_EXPR_FN (fntree)));