This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix miscompilation in output_constant_def


<Taking this to gcc-patches@ from gcc@; Was: Invalid code generated for Coldfire target>

Meloun Michal wrote:
Hello all,
I tracing bug in GCC for Coldfire target, but I end in dead water and
I need some help from real experts :).
Both gcc 4.4 and 4.3 have same problem

GCC miscompile this small test case.

//-----------------------------------------------------------------
//m68k-elf-gcc -save-temps -da -fdump-tree-all -fdump-ipa-all -c test.c -o test.o
void dummy(char *arg);

static void test1(void)
{
  char tmp[2] = "0";
}

void test2(void)
{
  dummy("0");
}

...


But how to fix this? By my mean, the cache must hold own immutable copy of insn.
But im not gcc expert, so I need help with proper solution. Is this patch OK? Any other solution?

I've too investigated this problem and Meloun's analysis seems correct. Just to sum up:


1. When expanding test1() output_constant_def sees "0" for the first time, generates (mem (symbol)) rtx for it and puts it into a new entry in const_desc_htab. Then output_constant_def returns this exact rtx to the caller.

2. Caller of output_constant_def puts the MEM into an instruction.

3. When reload processes test1(), it reloads the symbol into register thus transforming (mem (symbol)) into (mem (reg)) everywhere including the copy in const_desc_htab. This occurs in reload.c: subst_reloads().

4. When expanding test2() output_constant_def returns tampered rtx reference to "0".

--- varasm.c.orig       2008-11-14 18:04:27.693643900 +0100
+++ varasm.c    2008-11-14 17:58:06.522748300 +0100
@@ -3245,7 +3245,7 @@
     }

   maybe_output_constant_def_contents (desc, defer);
-  return desc->rtl;
+  return copy_rtx (desc->rtl);
 }

This patch seems correct to me. It makes output_constant_def() always return a copy of the value rather than the value.


I will regtest this patch on ColdFire [where the miscompilation occurs], wound anyone volunteer to test it on x86[_64]?

--
Maxim
2008-11-16  Meloun Michal  <meloun@miracle.cz>

	* varasm.c (output_constant_def): Keep original RTX private to
	the hashmap.
Index: varasm.c
===================================================================
--- varasm.c	(revision 141891)
+++ varasm.c	(working copy)
@@ -3245,7 +3245,7 @@ output_constant_def (tree exp, int defer
     }
 
   maybe_output_constant_def_contents (desc, defer);
-  return desc->rtl;
+  return copy_rtx (desc->rtl);
 }
 
 /* Subroutine of output_constant_def: Decide whether or not we need to

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]