release/22.x: [X86] Fix missing ByValTemporaries update in CopyViaTemp path for musttail calls (#190540) - #192507
Merged
Merged
Conversation
Member
Author
|
@folkertdev @efriedma-quic @RKSimon What do you think about merging this PR to the release branch? |
Member
Author
|
@llvm/pr-subscribers-backend-x86 Author: llvmbot ChangesBackport abd502a Requested by: @RKSimon Full diff: https://github.com/llvm/llvm-project/pull/192507.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
index 7e1c894655f3f..a5300d3e38ecc 100644
--- a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
+++ b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
@@ -2251,6 +2251,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
SDValue CopyChain =
CreateCopyOfByValArgument(Src, Temp, Chain, Flags, DAG, dl);
ByValCopyChains.push_back(CopyChain);
+ ByValTemporaries[ArgIdx] = Temp;
}
}
if (!ByValCopyChains.empty())
diff --git a/llvm/test/CodeGen/X86/musttail-struct.ll b/llvm/test/CodeGen/X86/musttail-struct.ll
index 735fd674a2ff1..22a2aedb6e213 100644
--- a/llvm/test/CodeGen/X86/musttail-struct.ll
+++ b/llvm/test/CodeGen/X86/musttail-struct.ll
@@ -104,8 +104,10 @@ define dso_local i32 @swapByValArguments(ptr byval(%struct.1xi32) %0, ptr byval(
; X32-NEXT: .cfi_def_cfa_offset 12
; X32-NEXT: mov eax, dword ptr [esp + 12]
; X32-NEXT: mov dword ptr [esp], eax
-; X32-NEXT: mov eax, dword ptr [esp + 16]
-; X32-NEXT: mov dword ptr [esp + 4], eax
+; X32-NEXT: mov ecx, dword ptr [esp + 16]
+; X32-NEXT: mov dword ptr [esp + 4], ecx
+; X32-NEXT: mov dword ptr [esp + 12], ecx
+; X32-NEXT: mov dword ptr [esp + 16], eax
; X32-NEXT: add esp, 8
; X32-NEXT: .cfi_def_cfa_offset 4
; X32-NEXT: jmp swap # TAILCALL
@@ -114,8 +116,10 @@ define dso_local i32 @swapByValArguments(ptr byval(%struct.1xi32) %0, ptr byval(
; X64: # %bb.0:
; X64-NEXT: mov eax, dword ptr [rsp + 8]
; X64-NEXT: mov dword ptr [rsp - 16], eax
-; X64-NEXT: mov eax, dword ptr [rsp + 16]
-; X64-NEXT: mov dword ptr [rsp - 8], eax
+; X64-NEXT: mov ecx, dword ptr [rsp + 16]
+; X64-NEXT: mov dword ptr [rsp - 8], ecx
+; X64-NEXT: mov dword ptr [rsp + 8], ecx
+; X64-NEXT: mov dword ptr [rsp + 16], eax
; X64-NEXT: jmp swap # TAILCALL
%r = musttail call i32 @swap(ptr byval(%struct.1xi32) %1, ptr byval(%struct.1xi32) %0)
ret i32 %r
@@ -218,22 +222,50 @@ define void @swap_byvals(%twenty_bytes* byval(%twenty_bytes) align 4 %a, %twenty
; X32-NEXT: mov ecx, dword ptr [esp + 68]
; X32-NEXT: mov dword ptr [esp + 24], ecx
; X32-NEXT: mov dword ptr [esp + 20], eax
+; X32-NEXT: mov eax, dword ptr [esp + 16]
+; X32-NEXT: mov dword ptr [esp + 80], eax
+; X32-NEXT: mov eax, dword ptr [esp + 12]
+; X32-NEXT: mov dword ptr [esp + 76], eax
+; X32-NEXT: mov eax, dword ptr [esp + 8]
+; X32-NEXT: mov dword ptr [esp + 72], eax
+; X32-NEXT: mov eax, dword ptr [esp]
+; X32-NEXT: mov ecx, dword ptr [esp + 4]
+; X32-NEXT: mov dword ptr [esp + 68], ecx
+; X32-NEXT: mov dword ptr [esp + 64], eax
+; X32-NEXT: mov eax, dword ptr [esp + 36]
+; X32-NEXT: mov dword ptr [esp + 60], eax
+; X32-NEXT: mov eax, dword ptr [esp + 32]
+; X32-NEXT: mov dword ptr [esp + 56], eax
+; X32-NEXT: mov eax, dword ptr [esp + 28]
+; X32-NEXT: mov dword ptr [esp + 52], eax
+; X32-NEXT: mov eax, dword ptr [esp + 20]
+; X32-NEXT: mov ecx, dword ptr [esp + 24]
+; X32-NEXT: mov dword ptr [esp + 48], ecx
+; X32-NEXT: mov dword ptr [esp + 44], eax
; X32-NEXT: add esp, 40
; X32-NEXT: .cfi_def_cfa_offset 4
; X32-NEXT: jmp two_byvals_callee@PLT # TAILCALL
;
; X64-LABEL: swap_byvals:
; X64: # %bb.0: # %entry
-; X64-NEXT: mov eax, dword ptr [rsp + 24]
-; X64-NEXT: mov dword ptr [rsp - 8], eax
; X64-NEXT: movaps xmm0, xmmword ptr [rsp + 8]
-; X64-NEXT: movaps xmmword ptr [rsp - 24], xmm0
-; X64-NEXT: mov eax, dword ptr [rsp + 48]
-; X64-NEXT: mov dword ptr [rsp - 32], eax
-; X64-NEXT: mov rax, qword ptr [rsp + 32]
-; X64-NEXT: mov rcx, qword ptr [rsp + 40]
-; X64-NEXT: mov qword ptr [rsp - 40], rcx
-; X64-NEXT: mov qword ptr [rsp - 48], rax
+; X64-NEXT: movaps xmmword ptr [rsp - 56], xmm0
+; X64-NEXT: mov eax, dword ptr [rsp + 24]
+; X64-NEXT: mov dword ptr [rsp - 40], eax
+; X64-NEXT: mov ecx, dword ptr [rsp + 48]
+; X64-NEXT: mov dword ptr [rsp - 8], ecx
+; X64-NEXT: mov rdx, qword ptr [rsp + 32]
+; X64-NEXT: mov rsi, qword ptr [rsp + 40]
+; X64-NEXT: mov qword ptr [rsp - 16], rsi
+; X64-NEXT: mov qword ptr [rsp - 24], rdx
+; X64-NEXT: mov qword ptr [rsp + 8], rdx
+; X64-NEXT: mov qword ptr [rsp + 16], rsi
+; X64-NEXT: mov dword ptr [rsp + 24], ecx
+; X64-NEXT: mov dword ptr [rsp + 48], eax
+; X64-NEXT: mov rax, qword ptr [rsp - 56]
+; X64-NEXT: mov rcx, qword ptr [rsp - 48]
+; X64-NEXT: mov qword ptr [rsp + 32], rax
+; X64-NEXT: mov qword ptr [rsp + 40], rcx
; X64-NEXT: jmp two_byvals_callee@PLT # TAILCALL
entry:
musttail call void @two_byvals_callee(%twenty_bytes* byval(%twenty_bytes) align 4 %b, %twenty_bytes* byval(%twenty_bytes) align 4 %a)
|
Contributor
|
FYI it's 22.1.4 tomorrow, this needs reviewing if we want this in by then |
RKSimon
approved these changes
Apr 20, 2026
RKSimon
left a comment
RKSimon
left a comment
Contributor
There was a problem hiding this comment.
LGTM - would be nice if one of the original reviewers approve as well if possible
github-project-automation
Bot
moved this from Needs Review
to Needs Merge
in LLVM Release Status
…ttail calls (llvm#190540) This fixes a miscompilation in musttail calls with byval arguments on X86. In the CopyViaTemp path, a temporary stack object is created and the argument is copied into it. However, the temporary is not recorded in ByValTemporaries, so the final lowering phase does not emit the copy to the real outgoing argument slot. As a result, the callee may read incorrect values from the stack. Fix this by recording the temporary in ByValTemporaries so that the final lowering step correctly copies the argument to the expected stack location. Reproducer: llvm#190429 (cherry picked from commit abd502a)
github-project-automation
Bot
moved this from Needs Merge
to Done
in LLVM Release Status
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport abd502a
Requested by: @RKSimon