[llvm] x86: fix musttail sibcall miscompilation (PR #168956)

Folkert de Vries via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 25 04:26:17 PST 2025


================
@@ -2404,16 +2530,27 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
       FIN = DAG.getFrameIndex(FI, getPointerTy(DAG.getDataLayout()));
 
       if (Flags.isByVal()) {
-        // Copy relative to framepointer.
-        SDValue Source = DAG.getIntPtrConstant(VA.getLocMemOffset(), dl);
-        if (!StackPtr.getNode())
-          StackPtr = DAG.getCopyFromReg(Chain, dl, RegInfo->getStackRegister(),
-                                        getPointerTy(DAG.getDataLayout()));
-        Source = DAG.getNode(ISD::ADD, dl, getPointerTy(DAG.getDataLayout()),
-                             StackPtr, Source);
-
-        MemOpChains2.push_back(
-            CreateCopyOfByValArgument(Source, FIN, Chain, Flags, DAG, dl));
+        SDValue ByValSrc;
+        bool NeedsStackCopy;
+        if (auto It = ByValTemporaries.find(OutsIndex);
+            It != ByValTemporaries.end()) {
+          ByValSrc = It->second;
+          NeedsStackCopy = true;
+        } else {
+          ByValSrc = Arg;
+          NeedsStackCopy = !isTailCall;
+        }
+
+        // FIXME: contrary to the arm backend, with the current logic we always
+        // seem to need a stack copy.
+        (void)NeedsStackCopy;
----------------
folkertdev wrote:

This unfortunately is still needed, guarding the copy below with `if (NeedsStackCopy)` leads to this diff

```diff
@@ -76,10 +76,8 @@ define dso_local i32 @swapByValArguments(ptr byval(%struct.1xi32) %0, ptr byval(
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    mov eax, dword ptr [rsp + 8]
 ; CHECK-NEXT:    mov dword ptr [rsp - 16], eax
-; CHECK-NEXT:    mov ecx, dword ptr [rsp + 16]
-; CHECK-NEXT:    mov dword ptr [rsp - 8], ecx
-; CHECK-NEXT:    mov dword ptr [rsp + 8], ecx
-; CHECK-NEXT:    mov dword ptr [rsp + 16], eax
+; CHECK-NEXT:    mov eax, dword ptr [rsp + 16]
+; CHECK-NEXT:    mov dword ptr [rsp - 8], eax
 ; CHECK-NEXT:    jmp swap # TAILCALL
```

i.e. the values are stored on the stack, but then no longer copied back into the correct argument position.

https://github.com/llvm/llvm-project/pull/168956


More information about the llvm-commits mailing list