[RISCV] Do not tail call optimize if arguments are passed indirectly#184972
[RISCV] Do not tail call optimize if arguments are passed indirectly#184972
Conversation
When a function argument is passed indirectly (CCValAssign::Indirect), the caller allocates stack space for the value and passes a pointer to the callee. If the call is tail-called, the caller's stack frame is deallocated before the callee executes, leaving the pointer dangling (use-after-free on the stack). X86 already guards against this case in isEligibleForSiblingCallOpt. The RISC-V backend was missing this check in isEligibleForTailCallOptimization. The existing tail-calls.ll test had a comment stating "Do not tail call optimize if parameters need to be passed indirectly" but the CHECK lines were asserting the incorrect (buggy) behavior, where a tail call was generated despite the dangling pointer. Regenerated via update_llc_test_checks.py. Assisted-by: Claude (Anthropic) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@llvm/pr-subscribers-backend-risc-v Author: Xavier Roche (xroche) ChangesWhen a function argument is passed indirectly ( X86 already guards against this case in The fix adds a check that rejects tail call optimization when any argument location has The existing Note: AI DisclosureThis patch was developed with assistance from Claude (Anthropic). All code has been reviewed and validated by the author. Full diff: https://github.com/llvm/llvm-project/pull/184972.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 891bc22a7463d..2cbe7722726f4 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -24578,6 +24578,15 @@ bool RISCVTargetLowering::isEligibleForTailCallOptimization(
if (CCInfo.getStackSize() > RVFI->getArgumentStackSize())
return false;
+ // Do not tail call optimize if any argument needs to be passed indirectly.
+ // The caller allocates stack space and passes a pointer to the callee. On a
+ // tail call the caller's stack frame is deallocated before the callee
+ // executes, invalidating the pointer (use-after-free).
+ for (const auto &VA : ArgLocs) {
+ if (VA.getLocInfo() == CCValAssign::Indirect)
+ return false;
+ }
+
// Do not tail call opt if either caller or callee uses struct return
// semantics.
auto IsCallerStructRet = Caller.hasStructRetAttr();
diff --git a/llvm/test/CodeGen/RISCV/tail-calls.ll b/llvm/test/CodeGen/RISCV/tail-calls.ll
index 33feba3c6fba1..79855aa03adcf 100644
--- a/llvm/test/CodeGen/RISCV/tail-calls.ll
+++ b/llvm/test/CodeGen/RISCV/tail-calls.ll
@@ -247,20 +247,24 @@ declare i32 @callee_indirect_args(fp128 %a)
define void @caller_indirect_args() nounwind {
; CHECK-LABEL: caller_indirect_args:
; CHECK: # %bb.0: # %entry
-; CHECK-NEXT: addi sp, sp, -16
+; CHECK-NEXT: addi sp, sp, -32
+; CHECK-NEXT: sw ra, 28(sp) # 4-byte Folded Spill
; CHECK-NEXT: lui a1, 262128
; CHECK-NEXT: mv a0, sp
; CHECK-NEXT: sw zero, 0(sp)
; CHECK-NEXT: sw zero, 4(sp)
; CHECK-NEXT: sw zero, 8(sp)
; CHECK-NEXT: sw a1, 12(sp)
-; CHECK-NEXT: addi sp, sp, 16
-; CHECK-NEXT: tail callee_indirect_args
+; CHECK-NEXT: call callee_indirect_args
+; CHECK-NEXT: lw ra, 28(sp) # 4-byte Folded Reload
+; CHECK-NEXT: addi sp, sp, 32
+; CHECK-NEXT: ret
;
; CHECK-LARGE-ZICFILP-LABEL: caller_indirect_args:
; CHECK-LARGE-ZICFILP: # %bb.0: # %entry
; CHECK-LARGE-ZICFILP-NEXT: lpad 0
-; CHECK-LARGE-ZICFILP-NEXT: addi sp, sp, -16
+; CHECK-LARGE-ZICFILP-NEXT: addi sp, sp, -32
+; CHECK-LARGE-ZICFILP-NEXT: sw ra, 28(sp) # 4-byte Folded Spill
; CHECK-LARGE-ZICFILP-NEXT: lui a1, 262128
; CHECK-LARGE-ZICFILP-NEXT: .Lpcrel_hi9:
; CHECK-LARGE-ZICFILP-NEXT: auipc a0, %pcrel_hi(.LCPI7_0)
@@ -270,8 +274,10 @@ define void @caller_indirect_args() nounwind {
; CHECK-LARGE-ZICFILP-NEXT: sw zero, 4(sp)
; CHECK-LARGE-ZICFILP-NEXT: sw zero, 8(sp)
; CHECK-LARGE-ZICFILP-NEXT: sw a1, 12(sp)
-; CHECK-LARGE-ZICFILP-NEXT: addi sp, sp, 16
-; CHECK-LARGE-ZICFILP-NEXT: jr t2
+; CHECK-LARGE-ZICFILP-NEXT: jalr t2
+; CHECK-LARGE-ZICFILP-NEXT: lw ra, 28(sp) # 4-byte Folded Reload
+; CHECK-LARGE-ZICFILP-NEXT: addi sp, sp, 32
+; CHECK-LARGE-ZICFILP-NEXT: ret
entry:
%call = tail call i32 @callee_indirect_args(fp128 0xL00000000000000003FFF000000000000)
ret void
|
efriedma-quic
left a comment
There was a problem hiding this comment.
Please add a testcase for a musttail call. (For musttail, we know the signatures match, so we have a place to store the value: the memory allocated by the caller.)
|
Oh, wait, I see, you added a comment about musttail... please file a bug with your findings. |
Tentative PR with associated bug: |
efriedma-quic
left a comment
There was a problem hiding this comment.
LGTM ... but please wait for @folkertdev to comment, since he modified this test recently in #170547. (CC @topperc )
|
Should this PR be closed ? #185094 includes this fix too, and it might make more sense to include all in one. |
When an argument is passed indirectly (
CCValAssign::Indirect), the caller allocates stack space and passes a pointer. A tail call deallocates the caller's frame before the callee executes, leaving the pointer dangling.X86 already guards against this in
isEligibleForSiblingCallOpt. This adds the same check to the RISC-V backend. Themusttailcase is handled separately in #185094.The existing
tail-calls.llCHECK lines were asserting the buggy behavior; regenerated viaupdate_llc_test_checks.py.AI Disclosure
This patch was developed with assistance from Claude (Anthropic). All code has been reviewed and validated by the author.