Skip to content

Commit fcffea0

Browse files
authored
[XRay][X86] Handle conditional calls when lowering patchable tail calls (#89364)
xray instruments tail call function exits by inserting a nop sled before the tail call. When tracing is enabled, the nop sled is replaced with a call to `__xray_FunctionTailExit()`. This currently does not work for conditional tail calls, as the instrumentation assumes that the tail call will be unconditional. This causes two issues: - `__xray_FunctionTailExit()` is inappropately called even when the tail call is not taken. - `__xray_FunctionTailExit()`'s prologue/epilogue adjusts the stack pointer with add/sub instructions. This clobbers condition flags, which can flip the condition used for the tail call, leading to incorrect program behavior. Fix this by rewriting conditional calls when lowering patchable tail calls. With this change, a conditional patchable tail call like: ``` je target ``` Will be lowered to: ``` jne .fallthrough .p2align 1, .. .Lxray_sled_N: SLED_CODE jmp target .fallthrough: ```
1 parent 179fb2e commit fcffea0

File tree

2 files changed

+85
-6
lines changed

2 files changed

+85
-6
lines changed

llvm/lib/Target/X86/X86MCInstLower.cpp

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "X86RegisterInfo.h"
2323
#include "X86ShuffleDecodeConstantPool.h"
2424
#include "X86Subtarget.h"
25+
#include "llvm/ADT/STLExtras.h"
2526
#include "llvm/ADT/SmallString.h"
2627
#include "llvm/ADT/StringExtras.h"
2728
#include "llvm/CodeGen/MachineConstantPool.h"
@@ -1362,6 +1363,35 @@ void X86AsmPrinter::LowerPATCHABLE_RET(const MachineInstr &MI,
13621363

13631364
void X86AsmPrinter::LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI,
13641365
X86MCInstLower &MCIL) {
1366+
MCInst TC;
1367+
TC.setOpcode(convertTailJumpOpcode(MI.getOperand(0).getImm()));
1368+
// Drop the tail jump opcode.
1369+
auto TCOperands = drop_begin(MI.operands());
1370+
bool IsConditional = TC.getOpcode() == X86::JCC_1;
1371+
MCSymbol *FallthroughLabel;
1372+
if (IsConditional) {
1373+
// Rewrite:
1374+
// je target
1375+
//
1376+
// To:
1377+
// jne .fallthrough
1378+
// .p2align 1, ...
1379+
// .Lxray_sled_N:
1380+
// SLED_CODE
1381+
// jmp target
1382+
// .fallthrough:
1383+
FallthroughLabel = OutContext.createTempSymbol();
1384+
EmitToStreamer(
1385+
*OutStreamer,
1386+
MCInstBuilder(X86::JCC_1)
1387+
.addExpr(MCSymbolRefExpr::create(FallthroughLabel, OutContext))
1388+
.addImm(X86::GetOppositeBranchCondition(
1389+
static_cast<X86::CondCode>(MI.getOperand(2).getImm()))));
1390+
TC.setOpcode(X86::JMP_1);
1391+
// Drop the condition code.
1392+
TCOperands = drop_end(TCOperands);
1393+
}
1394+
13651395
NoAutoPaddingScope NoPadScope(*OutStreamer);
13661396

13671397
// Like PATCHABLE_RET, we have the actual instruction in the operands to this
@@ -1383,18 +1413,16 @@ void X86AsmPrinter::LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI,
13831413
OutStreamer->emitLabel(Target);
13841414
recordSled(CurSled, MI, SledKind::TAIL_CALL, 2);
13851415

1386-
unsigned OpCode = MI.getOperand(0).getImm();
1387-
OpCode = convertTailJumpOpcode(OpCode);
1388-
MCInst TC;
1389-
TC.setOpcode(OpCode);
1390-
13911416
// Before emitting the instruction, add a comment to indicate that this is
13921417
// indeed a tail call.
13931418
OutStreamer->AddComment("TAILCALL");
1394-
for (auto &MO : drop_begin(MI.operands()))
1419+
for (auto &MO : TCOperands)
13951420
if (auto MaybeOperand = MCIL.LowerMachineOperand(&MI, MO))
13961421
TC.addOperand(*MaybeOperand);
13971422
OutStreamer->emitInstruction(TC, getSubtargetInfo());
1423+
1424+
if (IsConditional)
1425+
OutStreamer->emitLabel(FallthroughLabel);
13981426
}
13991427

14001428
// Returns instruction preceding MBBI in MachineFunction.

llvm/test/CodeGen/X86/xray-tail-call-sled.ll

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,54 @@ define dso_local i32 @caller() nounwind noinline uwtable "function-instrument"="
6666
; CHECK-MACOS: [[IDX:lxray_fn_idx[0-9]+]]:
6767
; CHECK-MACOS-NEXT: .quad lxray_sleds_start1-[[IDX]]
6868
; CHECK-MACOS-NEXT: .quad 2
69+
70+
define dso_local i32 @conditional_tail_call(i32 %cond) nounwind noinline uwtable "function-instrument"="xray-always" {
71+
; CHECK-LABEL: conditional_tail_call:
72+
; CHECK: .p2align 1, 0x90
73+
; CHECK-LABEL: Lxray_sled_4:
74+
; CHECK: .ascii "\353\t"
75+
; CHECK-NEXT: nopw 512(%rax,%rax)
76+
; CHECK-NEXT: testl %edi, %edi
77+
; CHECK-NEXT: je {{\.?Ltmp5}}
78+
; CHECK: .p2align 1, 0x90
79+
; CHECK-LABEL: Lxray_sled_5:
80+
; CHECK-NEXT: .ascii "\353\t"
81+
; CHECK-NEXT: nopw 512(%rax,%rax)
82+
; CHECK-LABEL: Ltmp6:
83+
; CHECK-NEXT: jmp {{.*}}callee {{.*}}# TAILCALL
84+
; CHECK-LABEL: Ltmp5:
85+
; CHECK: xorl %eax, %eax
86+
; CHECK-NEXT: .p2align 1, 0x90
87+
; CHECK-LABEL: Lxray_sled_6:
88+
; CHECK-NEXT: retq
89+
; CHECK-NEXT: nopw %cs:512(%rax,%rax)
90+
%cmp = icmp ne i32 %cond, 0
91+
br i1 %cmp, label %docall, label %ret
92+
docall:
93+
%retval = tail call i32 @callee()
94+
ret i32 %retval
95+
ret:
96+
ret i32 0
97+
}
98+
99+
; CHECK-LINUX-LABEL: .section xray_instr_map,"ao",@progbits,conditional_tail_call{{$}}
100+
; CHECK-LINUX-LABEL: .Lxray_sleds_start2:
101+
; CHECK-LINUX: .quad .Lxray_sled_4
102+
; CHECK-LINUX: .quad .Lxray_sled_5
103+
; CHECK-LINUX: .quad .Lxray_sled_6
104+
; CHECK-LINUX-LABEL: .Lxray_sleds_end2:
105+
; CHECK-LINUX-LABEL: .section xray_fn_idx,"ao",@progbits,conditional_tail_call{{$}}
106+
; CHECK-LINUX: [[IDX:\.Lxray_fn_idx[0-9]+]]:
107+
; CHECK-LINUX-NEXT: .quad .Lxray_sleds_start2-[[IDX]]
108+
; CHECK-LINUX-NEXT: .quad 3
109+
110+
; CHECK-MACOS-LABEL: .section __DATA,xray_instr_map,regular,live_support{{$}}
111+
; CHECK-MACOS-LABEL: lxray_sleds_start2:
112+
; CHECK-MACOS: .quad Lxray_sled_4
113+
; CHECK-MACOS: .quad Lxray_sled_5
114+
; CHECK-MACOS: .quad Lxray_sled_6
115+
; CHECK-MACOS-LABEL: Lxray_sleds_end2:
116+
; CHECK-MACOS-LABEL: .section __DATA,xray_fn_idx,regular,live_support{{$}}
117+
; CHECK-MACOS: [[IDX:lxray_fn_idx[0-9]+]]:
118+
; CHECK-MACOS-NEXT: .quad lxray_sleds_start2-[[IDX]]
119+
; CHECK-MACOS-NEXT: .quad 3

0 commit comments

Comments
 (0)