Re: JIT test crash

[Max Brister]

On Thu, Aug 2, 2012 at 8:36 AM, Michael Goffioul
<address@hidden> wrote:
> On Thu, Aug 2, 2012 at 1:57 PM, Max Brister <address@hidden> wrote:
>>
>> On Thu, Aug 2, 2012 at 6:05 AM, Michael Goffioul
>> <address@hidden> wrote:
>> > On Thu, Aug 2, 2012 at 7:10 AM, Max Brister <address@hidden> wrote:
>> >>
>> >> On Wed, Aug 1, 2012 at 5:28 PM, Michael Goffioul
>> >> <address@hidden> wrote:
>> >> > On Wed, Aug 1, 2012 at 5:43 PM, Max Brister <address@hidden> wrote:
>> >> >>
>> >> >> On Wed, Aug 1, 2012 at 11:40 AM, marco atzeri
>> >> >> <address@hidden>
>> >> >> wrote:
>> >> >> > On 8/1/2012 6:20 PM, Michael Goffioul wrote:
>> >> >> >>
>> >> >> >> I'm seeing a segfault on my MSVC-compiled version of octave (gui
>> >> >> >> branch,
>> >> >> >> rev 6889217b9d78) when running the test suite in pt-jit.cc:
>> >> >> >>
>> >> >> >> octave.exe:2> test('../src/pt-jit.cc', 'verbose')
>> >> >> >>  >>>>>
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> C:\Software\MinGW\msys\1.0\home\goffioul\src\octave-hg-gui\src/pt-jit.cc
>> >> >> >>    ***** test
>> >> >> >>   inc = 1e-5;
>> >> >> >>   result = 0;
>> >> >> >>   for ii = 0:inc:1
>> >> >> >>     result = result + inc * (1/3 * ii * ii);
>> >> >> >>   endfor
>> >> >> >>   assert (abs (result - 1/9) < 1e-5);
>> >> >> >>    ***** test
>> >> >> >>   inc = 1e-5;
>> >> >> >>   result = 0;
>> >> >> >>   for ii = 0:inc:1
>> >> >> >>     # the ^ operator's result is complex
>> >> >> >>     result = result + inc * (1/3 * ii ^ 2);
>> >> >> >>   endfor
>> >> >> >>   assert (abs (result - 1/9) < 1e-5);
>> >> >> >> panic: Segmentation violation -- stopping myself...
>> >> >> >>
>> >> >> >> Max, any idea what could cause this?
>> >> >> >>
>> >> >> >> Michael.
>> >> >> >>
>> >> >> >
>> >> >> > on cygwin
>> >> >> > PASSES 10 out of 10 tests
>> >> >> > with  default branch  c53c28c7c811
>> >> >> >
>> >> >> > In theory the gui branch should have no influence on the jit
>> >> >> > but there are at least 3 additional changeset regarding jit in the
>> >> >> > default branch that you are missing
>> >> >>
>> >> >> These changes should have any impact on this crash. I think this
>> >> >> might
>> >> >> be an msvc vs gcc issue (or a 64 vs 32 bit issue).
>> >> >
>> >> >
>> >> > Ok, this time it looks like an alignment problem. Maybe you're
>> >> > already
>> >> > aware
>> >> > of it, but MSVC aligns the stack on 4 bytes, while GCC aligns the
>> >> > stack
>> >> > on
>> >> > 16 bytes (and code generated by GCC assumes the incoming stack is
>> >> > also
>> >> > aligned on 16 bytes). I've attached the disassembled code for the
>> >> > following
>> >> > for-loop:
>> >> >
>> >> > inc = 1e-5, result = 0
>> >> > for ii = 0:inc:1, result = result + inc * (1/3 * ii ^ 2);, endfor
>> >> >
>> >> > The crash occurs at the pshufd call (address 02D3010F), because at
>> >> > that
>> >> > point EBP is not aligned on 8 bytes (hence EBP-88h is not aligned on
>> >> > 16
>> >> > bytes), its value is 0x0012F9EC. According to PSHUFD documentation,
>> >> > this
>> >> > generates a general protection fault. I could verify that by altering
>> >> > EBP
>> >> > value, I can avoid the crash. By carefully modifying ESP before
>> >> > calling
>> >> > the
>> >> > LLVM-compiled function, I could even make the for-loop succeed and
>> >> > get
>> >> > the
>> >> > expected result.
>> >> >
>> >> > The bottom-line is: it looks like LLVM-JIT makes assumptions on the
>> >> > stack-alignment that are not always true (this is weird, as I
>> >> > compiled
>> >> > LLVM
>> >> > with MSVC, so it should somehow know how to generate compatible
>> >> > assembly
>> >> > code). Max, does that ring a bell to you? Is there any way to control
>> >> > the
>> >> > assumptions on the stack alignment (GCC has -mstackrealign and
>> >> > -mincoming-stack-boundary flags)?
>> >> >
>> >> > Michael.
>> >> >
>> >>
>> >> That makes sense. It looks like stack realignment is on by default, so
>> >> I'm not sure why that doesn't fix the issue. I have attached a patch
>> >> which changes the alignment to 4 bytes, hopefully it helps. If this is
>> >> indeed the issue, it seems like a bug in llvm to me.
>> >>
>> >> This is a place were llvm 3.0 and 3.1 differ. If you are using llvm
>> >> 3.1, I can prepare a patch for that as well.
>> >>
>> >
>> > Not sure this will be helpful to you, but I recompiled jit code with
>> > OCTAVE_JIT_DEBUG enabled. See result in attachment. I also recompiled
>> > LLVM,
>> > just in case, but it didnt change anything. I'll try to test your patch
>> > today.
>> >
>> > Michael.
>> >
>>
>> The output with OCTAVE_JIT_DEBUG looks correct to me.
>>
>> I have attached the patch for llvm 3.1.
>
>
> I applied it, but it didn't change anything (the generated assembly looks
> exactly the same). If I'm reading this [1] correctly (XF86SubTarget
> constructor), the stack alignment was already set to 4 anyway. And in [2],
> in X86_32TargetMachine constructor, the native stack alignment is also
> specified on 4 bytes (trailing "-S32" at line 45).
>
> Michael.
>
> [1]
> https://github.com/earl/llvm-mirror/blob/master/lib/Target/X86/X86Subtarget.cpp
> [2]
> https://github.com/earl/llvm-mirror/blob/master/lib/Target/X86/X86TargetMachine.cpp

Actually, that makes sense. In order to use the sse instruction, we
really want the stack to 16 byte aligned I think. Can you try changing
the stack alignment to 16 bytes instead of 4?

Max Brister