[LLVMdev] jit X86 target compilation callback bug

[Török Edwin]

On 02/03/2010 01:08 PM, Kristaps Straupe wrote:
> Hello again.
>
> I still think that you are wrong. Realignement with and esp,-16 not
> always changes stack poiner. If esp is already aligned to 16 byte
> boundary, it will not change! Take a look at following example.
> Assume esp has value 0x000001000 at start of X86CompilationCallback
> function. Then execution of it will yield following esp values:
>
> 0x000000FFC - after push ebp
> 0x000000FFC - after mov ebp, esp
> 0x000000FF8 - after push eax
> 0x000000FF4 - after push edx // edx is pushed to address 0x000000FF4
> 0x000000FF0 - after push ecx // ecx is pushed to address 0x000000FF0
> 0x000000FF0 - after and esp, -16 // not changed!!!!
>
> now next three instructions will corrupt pushed ecx and edx registers:
>
> mov   eax, dword ptr [ebp+4]
> mov   dword ptr [esp+4], eax
> mov   dword ptr [esp], ebp
>
> because [esp+4] - this is where pushed edx resides (0x000000FF0+4 = 0x000000FF4)
> and [esp] is where pushed ecx resides (0x000000FF0)
>
> instead of "and esp,-16" there would be "sub esp,8" then in stack
> there would be 8 bytes free to use (with [esp] and [esp+4]).
>   

The non-MSVC version of the code does a "sub esp, 16" (after "and esp,
-16"):
#  if defined(__APPLE__)
    "andl    $-16, %esp\n"    // Align ESP on 16-byte boundary
#  endif
    "subl    $16, %esp\n"


That would be the correct thing to do on the MSVC version too I think.
"and esp, -16" aligns the stack, and "sub esp, -16" makes room for 8
bytes + padding to keep the 16 byte alignment of
the stack.

This patch should fix the issue:
diff --git a/lib/Target/X86/X86JITInfo.cpp b/lib/Target/X86/X86JITInfo.cpp
index f363903..d297d24 100644
--- a/lib/Target/X86/X86JITInfo.cpp
+++ b/lib/Target/X86/X86JITInfo.cpp
@@ -297,6 +297,7 @@ extern "C" {
       push  edx
       push  ecx
       and   esp, -16
+      sub   esp, 16
       mov   eax, dword ptr [ebp+4]
       mov   dword ptr [esp+4], eax
       mov   dword ptr [esp], ebp


Best regards,
--Edwin