[i386] Fix wrong argument value on Windows

Message ID 1728770.F5DofJc6GO@polaris
State New
Headers show
Series
  • [i386] Fix wrong argument value on Windows
Related show

Commit Message

Eric Botcazou Feb. 6, 2019, 10:16 a.m.
Hi,

this is a regression present on all active branches: if you compile the 
attached Ada testcase with -O2 -gnatp -fno-omit-frame-pointer for 32-bit 
Windows, you'll see that the compiler swaps a load based on the stack pointer 
with a store based on the frame pointer, thus clobbering a saved argument:

        pushl   %ebp
        movl    %esp, %ebp
        pushl   %esi
        pushl   %ebx
        pushl   %eax              <- %eax save
        movl    $4108, %eax
        call    ___chkstk_ms
        leal    8(%ebp), %esi
        subl    %eax, %esp
        movl    8(%ebp), %ebx
        movl    %edx, -20(%ebp)
        movl    %esi, -12(%ebp)    <- fp-based store
        movl    (%esp,%eax), %eax  <- sp-based load
        ... wrong value in eax...

The load and the store are swapped because there are not based on the same 
register and the offset between them is seen as variable.  The proposed fix is 
to add a memory blockage, like in other frame-related constructs.

Tested on x86/Windows and x86-64/Windows, OK for all active branches?


2019-02-06  Eric Botcazou  <ebotcazou@adacore.com>

	* config/i386/i386.c (ix86_expand_prologue): Generate a memory blockage
	after restoring registers saved to allocate the frame on Windows.


2019-02-06  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/opt76.adb: New test.

-- 
Eric Botcazou

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 268508)
+++ config/i386/i386.c	(working copy)
@@ -13579,8 +13579,9 @@  ix86_expand_prologue (void)
 	}
       m->fs.sp_offset += allocate;
 
-      /* Use stack_pointer_rtx for relative addressing so that code
-	 works for realigned stack, too.  */
+      /* Use stack_pointer_rtx for relative addressing so that code works for
+	 realigned stack.  But this means that we need a blockage to prevent
+	 stores based on the frame pointer from being scheduled before.  */
       if (r10_live && eax_live)
         {
 	  t = gen_rtx_PLUS (Pmode, stack_pointer_rtx, eax);
@@ -13589,6 +13590,7 @@  ix86_expand_prologue (void)
 	  t = plus_constant (Pmode, t, UNITS_PER_WORD);
 	  emit_move_insn (gen_rtx_REG (word_mode, AX_REG),
 			  gen_frame_mem (word_mode, t));
+	  emit_insn (gen_memory_blockage ());
 	}
       else if (eax_live || r10_live)
 	{
@@ -13596,6 +13598,7 @@  ix86_expand_prologue (void)
 	  emit_move_insn (gen_rtx_REG (word_mode,
 				       (eax_live ? AX_REG : R10_REG)),
 			  gen_frame_mem (word_mode, t));
+	  emit_insn (gen_memory_blockage ());
 	}
     }
   gcc_assert (m->fs.sp_offset == frame.stack_pointer_offset);