Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64

[belleyb]

Summary

See: http://llvm.org/bugs/show_bug.cgi?id=18582

The llvm X86/Win64 code generator was generating 32-bit relative calls
when calling these special/intrinsic functions. It turns out that, under
Windows 8 (and 10), the DLL containing these functions is loaded more
than 2 GB away from our jitted code in the process virtual address space.
The MCJIT ELF symbol relocation couldn't do anything about it and that
lead to crashes.

The issue can be fixed by using indirect call via 64-bit register.

This patch includes updated regression tests.

[belleyb]

ping

[belleyb]

ping

[asl]

Should we do this only for large code model? JIT should definitely default to it or something similar.

[belleyb]

I believe that this should be performed in all memory models.

In fact, I now believe that it is probably also affecting non-JIT usage. Windows 8 and 10 performs some address space randomization (DLL loading and memory allocations), probably for security reasons. This makes it more likely to that an inter-DLL function call is located more than 2GB away. 32-bit PC relative relocation cannot be used to perform an inter-DLL function call (unless going through the PLT). This is true for all memory models.

The __chkstk function is an inter-DLL/JIT-module function call (unless one is linking statically to the MSVC runtime, which we can't safely assume while compiling...). So, we need to use a 64-bit safe relocation for this call...

[asl]

In D6856#115156, @belleyb wrote:

I believe that this should be performed in all memory models.

In fact, I now believe that it is probably also affecting non-JIT usage. Windows 8 and 10 performs some address space randomization (DLL loading and memory allocations), probably for security reasons. This makes it more likely to that an inter-DLL function call is located more than 2GB away. 32-bit PC relative relocation cannot be used to perform an inter-DLL function call (unless going through the PLT). This is true for all memory models.

Is there any documentation that Windows 8 and 10 indeed use large code model?

[belleyb]

AFAIU, the code model isn't imposed by the OS. One can use DLLs compiled with either code model on any versions of Windows. One chooses the code model for each DLL depending on whether he's expecting particular sections (.data and .text) of the DLL to be larger than 2^^24 bytes.

See: System V ABI, AMD64 Supplement, http://www.x86-64.org/documentation/abi.pdf, Section 3.5.1

[belleyb]

For example, in the x86_64 small code model, one call only use the 32-bit PC relative call to make calls within the same DLL. When calling a function residing in a different DLL, one still has to use a 64-bit safe relocation, which amount to either loading the 64-bit address directly, loading it for a GOT table or jumping to a PLT stub containing the 64-bit call.

[majnemer]

I'm a little confused here.

The symbols __alloca and __chkstk come from a DLL which means that referring to them goes to a thunk which will then dereference __imp__chkstk and __imp__alloca. The thunks are built by the linker which means that they must be accessible to your EXE or DLL regardless how far away you are from the C runtime's DLL.

I guess my first question would be, how does the symbol for __chkstk work out when running on the JIT? Do we literally find out where the CRT's __chkstk function is in memory and use that address? If so, how about we say that these calls should be indirect if we are using CodeModel::Large ?

[belleyb]

majnemer wrote:

I'm a little confused here.

Me also! :-)

The symbols alloca and chkstk come from a DLL which means that referring to them goes to a thunk which will then dereference impchkstk and impalloca. The thunks are built by the linker which means that they must be accessible to your EXE or DLL regardless how far away you are from the C runtime's DLL.

Probably... I'll have to investigate.

I guess my first question would be, how does the symbol for chkstk work out when running on the JIT? Do we literally find out where the CRT's chkstk function is in memory and use that address?

Yes, that's what the problem is...

If so, how about we say that these calls should be indirect if we are using CodeModel::Large ?

I'm ok about the indirection. But, I really don't understand why going through that impchkstk indirection wouldn't be necessary in the CodeModel::Small and CodeModel::Medium also. AFAIU, this is an X86_64 specific issue, not a large model issue...

[rnk]

We shouldn't be making this call unconditionally indirect. We should be using whatever strategy MCJIT is using to reference external symbols. I assume it does so by creating a PLT or something like it.

lib/Target/X86/X86FrameLowering.cpp

767–772

You left behind the old call op, it seems.

[belleyb]

Adding Lang to the review so that he can comment on the MCJIT requirements...

[belleyb]

Thanks guys for the comments. I will experiment a little bit more and come back once I have a better understanding. It might also be a difference between the behaviour of the COFF vs ELF object files under Windows.

There is a guiding principle though that I'd like to discuss first. AFAIK, the MCJIT engine does not perform any special configuration of the target backends. It uses a vanilla pass manager to create an in-memory object file (ELF or Mach-O). That object file is essentially no different than the one that would have been generated by llc -filetype=obj. I believe that this is a desirable design goal. So, the content of the object file is either right or wrong. The answer should not depend on the intended use of the object file (JIT execution or offline compilation).

In addition, and AFAIK, it is currently a limitation of the RuntimeDyLd symbol relocators that we have to use CodeModel::Large. It is also my understanding that it is a limitation that might be lifted in the future. The corollary is that we shouldn't make any assumption that CodeModel::Large implies MCJIT or vice-versa.

Is my understanding of those parts correct ?

[rnk]

I went ahead and put together a patch that I think does the right thing here:
http://reviews.llvm.org/D7267

Note it doesn't touch EmitLoweredWinAlloca, but that code is a mess and IMO should be refactored to use the fixed code separately.

[belleyb]

Wow! Thanks so much... I am downloading your proposed patch and testing it out inside our product on Windows 7, 8 and 10...

[lhames]

I'm not sure about the static case but when JITing MCJIT should synthesize a PLT entry for you, so you should be ok to emit a direct call.

[belleyb]

Thanks Lang, I will investigate some more next week. I'd like to go to the bottom of this issue and understand it thoroughly.

[belleyb]

Closing. This patch is now irrelevant.

lib/Target/X86/X86FrameLowering.cpp

if (isEAXAlive) {
       assert(!Is64Bit && "EAX is livein in x64 case!");
 
       // Save EAX
       BuildMI(MBB, MBBI, DL, TII.get(X86::PUSH32r))
         .addReg(X86::EAX, RegState::Kill)
         .setMIFlag(MachineInstr::FrameSetup);
     }
 
+    MachineInstrBuilder MIB;
+
     if (Is64Bit) {
       // Handle the 64-bit Windows ABI case where we need to call __chkstk.
       // Function prologue is responsible for adjusting the stack pointer.
       BuildMI(MBB, MBBI, DL, TII.get(X86::MOV64ri), X86::RAX)
         .addImm(NumBytes)
         .setMIFlag(MachineInstr::FrameSetup);
+
+      // R11 will be used to contain the address of __chkstk.
+      // R11 is a volatile register and assumed to be destroyed by the callee, 
+      // so there is no need to save and restore it.
+      BuildMI(MBB, MBBI, DL, TII.get(X86::MOV64ri), X86::R11)
+        .addExternalSymbol(StackProbeSymbol);
+
+      // Create a call to __chkstk function which address contained in R11.
+      MIB = BuildMI(MBB, MBBI, DL, TII.get(X86::CALL64r))
+              .addReg(X86::R11, RegState::Kill);
     } else {
       // Allocate NumBytes-4 bytes on stack in case of isEAXAlive.
       // We'll also use 4 already allocated bytes for EAX.
       BuildMI(MBB, MBBI, DL, TII.get(X86::MOV32ri), X86::EAX)
         .addImm(isEAXAlive ? NumBytes - 4 : NumBytes)
         .setMIFlag(MachineInstr::FrameSetup);
+
+      MIB = BuildMI(MBB, MBBI, DL, TII.get(X86::CALLpcrel32))
+              .addExternalSymbol(StackProbeSymbol);
     }
 
     BuildMI(MBB, MBBI, DL,
             TII.get(CallOp))
       .addExternalSymbol(StackProbeSymbol)
       .addReg(StackPtr,    RegState::Define | RegState::Implicit)
       .addReg(X86::EFLAGS, RegState::Define | RegState::Implicit)
       .setMIFlag(MachineInstr::FrameSetup);
 
     if (Is64Bit) {
       // MSVC x64's __chkstk and cygwin/mingw's ___chkstk_ms do not adjust %rsp
-      // themself. It also does not clobber %rax so we can reuse it when
+      // themselves. It also does not clobber %rax so we can reuse it when
       // adjusting %rsp.
       BuildMI(MBB, MBBI, DL, TII.get(X86::SUB64rr), StackPtr)
         .addReg(StackPtr)
         .addReg(X86::RAX)
         .setMIFlag(MachineInstr::FrameSetup);
     }
     if (isEAXAlive) {
       // Restore EAX

lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::EmitLoweredWinAlloca(MachineInstr *MI,
   DebugLoc DL = MI->getDebugLoc();
 
   assert(!Subtarget->isTargetMachO());
 
   // The lowering is pretty easy: we're just emitting the call to _alloca.  The
   // non-trivial part is impdef of ESP.
 
   if (Subtarget->isTargetWin64()) {
+    const char *StackProbeSymbol = 
+      Subtarget->isTargetCygMing() ? "___chkstk" : "__chkstk";
+    
+    // R11 will be used to contain the address of __chkstk.
+    // R11 is a volotiale register and assumed to be destoyed by the callee, 
+    // so there is no need to save and restore it.
+    BuildMI(*BB, MI, DL, TII->get(X86::MOV64ri), X86::R11)
+      .addExternalSymbol(StackProbeSymbol);
+
+    // Create a call to __chkstk function which address contained in R11.
+    MachineInstrBuilder MIB = BuildMI(*BB, MI, DL, TII->get(X86::CALL64r))
+                  .addReg(X86::R11, RegState::Kill);
+  
     if (Subtarget->isTargetCygMing()) {
       // ___chkstk(Mingw64):
       // Clobbers R10, R11, RAX and EFLAGS.
       // Updates RSP.
-      BuildMI(*BB, MI, DL, TII->get(X86::W64ALLOCA))
-        .addExternalSymbol("___chkstk")
+      MIB
         .addReg(X86::RAX, RegState::Implicit)
         .addReg(X86::RSP, RegState::Implicit)
         .addReg(X86::RAX, RegState::Define | RegState::Implicit)
         .addReg(X86::RSP, RegState::Define | RegState::Implicit)
         .addReg(X86::EFLAGS, RegState::Define | RegState::Implicit);
     } else {
       // __chkstk(MSVCRT): does not update stack pointer.
       // Clobbers R10, R11 and EFLAGS.
-      BuildMI(*BB, MI, DL, TII->get(X86::W64ALLOCA))
-        .addExternalSymbol("__chkstk")
+      MIB
         .addReg(X86::RAX, RegState::Implicit)
         .addReg(X86::EFLAGS, RegState::Define | RegState::Implicit);
       // RAX has the offset to be subtracted from RSP.
       BuildMI(*BB, MI, DL, TII->get(X86::SUB64rr), X86::RSP)
         .addReg(X86::RSP)
         .addReg(X86::RAX);
     }
   } else {

test/CodeGen/X86/win64_alloca_dynalloca.ll

 ; EFI-LABEL: unaligned:
 entry:
 
   %buf0 = alloca i8, i64 4096, align 1
 
 ; ___chkstk_ms does not adjust %rsp.
 ; M64: movq  %rsp, %rbp
 ; M64:       $4096, %rax
-; M64: callq ___chkstk_ms
+; M64: movabsq  $___chkstk_ms, %r11
+; M64: callq  *%r11
 ; M64: subq  %rax, %rsp
 
 ; __chkstk does not adjust %rsp.
 ; W64: movq  %rsp, %rbp
 ; W64:       $4096, %rax
-; W64: callq __chkstk
+; W64: movabsq  $__chkstk, %r11
+; W64: callq  *%r11
 ; W64: subq  %rax, %rsp
 
 ; Freestanding
 ; EFI: movq  %rsp, %rbp
 ; EFI:       $[[B0OFS:4096|4104]], %rsp
 ; EFI-NOT:   call
 
   %buf1 = alloca i8, i64 %n, align 1
 
 ; M64: leaq  15(%{{.*}}), %rax
 ; M64: andq  $-16, %rax
-; M64: callq ___chkstk
+; M64: movabsq  $___chkstk, %r11
+; M64: callq  *%r11
 ; M64-NOT:   %rsp
 ; M64: movq  %rsp, %rax
 
 ; W64: leaq  15(%{{.*}}), %rax
 ; W64: andq  $-16, %rax
-; W64: callq __chkstk
+; W64: movabsq  $__chkstk, %r11
+; W64: callq  *%r11
 ; W64: subq  %rax, %rsp
 ; W64: movq  %rsp, %rax
 
 ; EFI: leaq  15(%{{.*}}), [[R1:%r.*]]
 ; EFI: andq  $-16, [[R1]]
 ; EFI: movq  %rsp, [[R64:%r.*]]
 ; EFI: subq  [[R1]], [[R64]]
 ; EFI: movq  [[R64]], %rsp

 ; W64-LABEL: aligned:
 ; EFI-LABEL: aligned:
 entry:
 
   %buf1 = alloca i8, i64 %n, align 128
 
 ; M64: leaq  15(%{{.*}}), %rax
 ; M64: andq  $-16, %rax
-; M64: callq ___chkstk
+; M64: movabsq  $___chkstk, %r11
+; M64: callq  *%r11
 ; M64: movq  %rsp, [[R2:%r.*]]
 ; M64: andq  $-128, [[R2]]
 ; M64: movq  [[R2]], %rsp
 
 ; W64: leaq  15(%{{.*}}), %rax
 ; W64: andq  $-16, %rax
-; W64: callq __chkstk
+; W64: movabsq  $__chkstk, %r11
+; W64: callq  *%r11
 ; W64: subq  %rax, %rsp
 ; W64: movq  %rsp, [[R2:%r.*]]
 ; W64: andq  $-128, [[R2]]
 ; W64: movq  [[R2]], %rsp
 
 ; EFI: leaq  15(%{{.*}}), [[R1:%r.*]]
 ; EFI: andq  $-16, [[R1]]
 ; EFI: movq  %rsp, [[R64:%r.*]]

test/CodeGen/X86/win64_eh.ll

 define void @foo2() uwtable {
 entry:
   %baz = alloca [4000 x i16], align 2
   ret void
 }
 ; WIN64-LABEL: foo2:
 ; WIN64: .seh_proc foo2
 ; WIN64: movabsq $8000, %rax
-; WIN64: callq {{__chkstk|___chkstk_ms}}
+; WIN64: movabsq ${{__chkstk|___chkstk_ms}}, %r11
+; WIN64: callq   *%r11
 ; WIN64: subq %rax, %rsp
 ; WIN64: .seh_stackalloc 8000
 ; WIN64: .seh_endprologue
 ; WIN64: addq $8000, %rsp
 ; WIN64: ret
 ; WIN64: .seh_endproc
 
 

test/CodeGen/X86/win_chkstk.ll

 ; and the 32-bit version of __chkstk will probe the stack and adjust the stack pointer.
 ; The 64-bit version of __chkstk is only responsible for probing the stack.  The 64-bit
 ; prologue is responsible for adjusting the stack pointer.
 
 ; Stack allocation >= 4096 bytes will require call to __chkstk in the Windows ABI.
 define i32 @main4k() nounwind {
 entry:
 ; WIN_X32:    calll __chkstk
-; WIN_X64:    callq __chkstk
+; WIN_X64:    movabsq $__chkstk, %r11
+; WIN_X64:    callq *%r11
 ; MINGW_X32:  calll __alloca
-; MINGW_X64:  callq ___chkstk_ms
+; MINGW_X64:  movabsq $___chkstk_ms, %r11
+; MINGW_X64:  callq *%r11
 ; LINUX-NOT:  call __chkstk
   %array4096 = alloca [4096 x i8], align 16       ; <[4096 x i8]*> [#uses=0]
   ret i32 0
 }
 
 ; Make sure we don't call __chkstk or __alloca when we have less than a 4096 stack
 ; allocation.
 define i32 @main128() nounwind {
 entry:
 ; WIN_X32:       # BB#0:
 ; WIN_X32-NOT:   calll __chkstk
 ; WIN_X32:       ret
 
 ; WIN_X64:       # BB#0:
-; WIN_X64-NOT:   callq __chkstk
+; WIN_X64-NOT:   movabsq $__chkstk, %r11
+; WIN_X64-NOT:   callq *%r11
 ; WIN_X64:       ret
 
 ; MINGW_X64:     # BB#0:
-; MINGW_X64-NOT: callq ___chkstk_ms
+; MINGW_X64-NOT: movabsq $___chkstk_ms, %r11
+; MINGW_X64-NOT: callq *%r11
 ; MINGW_X64:     ret
 
 ; LINUX:         # BB#0:
 ; LINUX-NOT:     call __chkstk
 ; LINUX:         ret
   %array128 = alloca [128 x i8], align 16         ; <[128 x i8]*> [#uses=0]
   ret i32 0
 }
 
 ; Make sure we don't call __chkstk or __alloca on non-Windows even if the
 ; caller has the Win64 calling convention.
 define x86_64_win64cc i32 @main4k_win64() nounwind {
 entry:
 ; WIN_X32:    calll __chkstk
-; WIN_X64:    callq __chkstk
+; WIN_X64:    movabsq $__chkstk, %r11
+; WIN_X64:    callq *%r11
 ; MINGW_X32:  calll __alloca
-; MINGW_X64:  callq ___chkstk_ms
+; MINGW_X64:  movabsq $___chkstk_ms, %r11
+; MINGW_X64:  callq *%r11
 ; LINUX-NOT:  call __chkstk
   %array4096 = alloca [4096 x i8], align 16       ; <[4096 x i8]*> [#uses=0]
   ret i32 0
 }