Patchwork [6/9] Emit the prologue/epilogue using frame offsets.

login
register
mail settings
Submitter Richard Henderson
Date Aug. 5, 2010, 6:08 p.m.
Message ID <4C5AFE39.8090008@twiddle.net>
Download mbox | patch
Permalink /patch/61011/
State New
Headers show

Comments

Richard Henderson - Aug. 5, 2010, 6:08 p.m.
On 08/05/2010 09:18 AM, H.J. Lu wrote:
>> FAIL: g++.dg/torture/stackalign/eh-thiscall-1.C  -O2 -fwhopr  execution test
>>
>> I saw them on Fedora 13.
>>
> 
> Those failing tests are compiled with -fpic. Here is stack trace with
> -O -fpic -static:

Fixed.

The problem was improper computation of the mask used for the unwind info.
The test could still pass at runtime depending on the memory layout that
the program receives.


r~
PR target/45189
	* config/i386/i386.c (ix86_emit_save_reg_using_mov): Make sure
	the alignment constant is properly sign-extended.
H.J. Lu - Aug. 5, 2010, 6:19 p.m.
On Thu, Aug 5, 2010 at 11:08 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 08/05/2010 09:18 AM, H.J. Lu wrote:
>>> FAIL: g++.dg/torture/stackalign/eh-thiscall-1.C  -O2 -fwhopr  execution test
>>>
>>> I saw them on Fedora 13.
>>>
>>
>> Those failing tests are compiled with -fpic. Here is stack trace with
>> -O -fpic -static:
>
> Fixed.
>
> The problem was improper computation of the mask used for the unwind info.
> The test could still pass at runtime depending on the memory layout that
> the program receives.
>

It is interesting. I saw it on 32bit host with 32bit HOST_WIDE_INT. Is it
a signed vs. unsigned issue? stack_alignment_needed is unsigned.
Richard Henderson - Aug. 5, 2010, 6:26 p.m.
On 08/05/2010 11:19 AM, H.J. Lu wrote:
> It is interesting. I saw it on 32bit host with 32bit HOST_WIDE_INT. Is it
> a signed vs. unsigned issue? stack_alignment_needed is unsigned.

Yes, signed is the issue.  Previously we'd have

  -stack_alignment_needed / 8
  -256u / 8
  0xffffff00u / 8
  0x1fffffe0u

If you receive a memory layout such that the stack is <= 0x1fffffff,
the program will happen to work.  With the cast we get -32 and not
a large unsigned number, which is what we really wanted.


r~

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index dd81825..204211a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8639,7 +8639,8 @@  ix86_emit_save_reg_using_mov (enum machine_mode mode, unsigned int regno,
 	     reference to the locations within the frame.  Instead,
 	     simply compute the location of the aligned frame from
 	     the frame pointer.  */
-	  addr = GEN_INT (-crtl->stack_alignment_needed / BITS_PER_UNIT);
+	  addr = GEN_INT (-(HOST_WIDE_INT)crtl->stack_alignment_needed
+			  / BITS_PER_UNIT);
 	  addr = gen_rtx_AND (Pmode, hard_frame_pointer_rtx, addr);
 	  addr = plus_constant (addr, -cfa_offset);
 	  mem = gen_rtx_MEM (mode, addr);