[PATCH] D150033: fix stack probe lowering for x86_intrcc

[Tom Dohrmann via Phabricator via llvm-commits]

Freax13 created this revision.
Freax13 added a reviewer: llvm-commits.
Herald added subscribers: pengfei, JDevlieghere, hiraditya.
Herald added a project: All.
Freax13 requested review of this revision.
Herald added a project: LLVM.

The x86_intrcc calling convention will build two STACKALLOC_W_PROBING machine instructions if the function takes an error code. This is caused by an additional call to emitSPUpdate in llvm/lib/Target/X86/X86FrameLowering.cpp:1650. Previously only the first STACKALLOC_W_PROBING machine instruction was properly handled, the second one was simply ignored. This lead to miscompilations where the stack pointer wasn't properly updated (see https://github.com/rust-lang/rust/issues/109918). This patch fixes this by handling all STACKALLOC_W_PROBING machine instructions.

      

To be honest I don't quite understand why this didn't lead to more noticeable miscompilations previously.

This is my first time contributing to LLVM.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150033

Files:
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/test/CodeGen/X86/x86-64-intrcc.ll


Index: llvm/test/CodeGen/X86/x86-64-intrcc.ll
===================================================================
--- llvm/test/CodeGen/X86/x86-64-intrcc.ll
+++ llvm/test/CodeGen/X86/x86-64-intrcc.ll
@@ -174,5 +174,23 @@
   ret void
 }
 
+define x86_intrcc void @test_stack_allocation(ptr byval(%struct.interrupt_frame) %frame, i64 %err) #1 {
+  ; CHECK-LABEL: test_stack_allocation:
+  ; CHECK: # %bb.0: # %entry
+  
+  ; Ensure that STACKALLOC_W_PROBING isn't emitted.
+  ; CHECK-NOT: # fixed size alloca with probing
+  ; Ensure that stack space is allocated. 
+  ; CHECK: subq    $280, %rsp
+entry:
+  %some_allocation = alloca i64
+  ; Call a un-inlineable function to ensure the allocation isn't put in the red
+  ; zone.
+  call void @external_function(ptr %some_allocation)
+  ret void
+}
+
+declare void @external_function(ptr)
 
 attributes #0 = { nounwind "frame-pointer"="all" }
+attributes #1 = { nounwind "probe-stack"="inline-asm" }
Index: llvm/lib/Target/X86/X86FrameLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86FrameLowering.cpp
+++ llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -651,10 +651,13 @@
 
 void X86FrameLowering::inlineStackProbe(MachineFunction &MF,
                                         MachineBasicBlock &PrologMBB) const {
-  auto Where = llvm::find_if(PrologMBB, [](MachineInstr &MI) {
-    return MI.getOpcode() == X86::STACKALLOC_W_PROBING;
-  });
-  if (Where != PrologMBB.end()) {
+  while (1) {
+    auto Where = llvm::find_if(PrologMBB, [](MachineInstr &MI) {
+      return MI.getOpcode() == X86::STACKALLOC_W_PROBING;
+    });
+    if (Where == PrologMBB.end())
+      break;
+
     DebugLoc DL = PrologMBB.findDebugLoc(Where);
     emitStackProbeInline(MF, PrologMBB, Where, DL, true);
     Where->eraseFromParent();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150033.520066.patch
Type: text/x-patch
Size: 1822 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230506/e22b0906/attachment.bin>