Skip to content

Commit 1bbcedb

Browse files
committed
[xray] Handle conditional calls when lowering patchable tail calls.
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 e2a72fa commit 1bbcedb

File tree

2 files changed

+69
-6
lines changed

2 files changed

+69
-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"
@@ -1361,6 +1362,35 @@ void X86AsmPrinter::LowerPATCHABLE_RET(const MachineInstr &MI,
13611362

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

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

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

13991427
// Returns instruction preceding MBBI in MachineFunction.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
; RUN: llc -mtriple=x86_64 < %s | FileCheck %s
2+
3+
declare void @tail_call_target()
4+
5+
define void @conditional_tail_call(i32 %cond) "function-instrument"="xray-always" nounwind {
6+
; CHECK-LABEL: conditional_tail_call:
7+
; CHECK-NEXT: .Lfunc_begin0:
8+
; CHECK-NEXT: # %bb.0:
9+
; CHECK-NEXT: .p2align 1, 0x90
10+
; CHECK-NEXT: .Lxray_sled_0:
11+
; CHECK-NEXT: .ascii "\353\t"
12+
; CHECK-NEXT: nopw 512(%rax,%rax)
13+
; CHECK-NEXT: testl %edi, %edi
14+
; CHECK-NEXT: je .Ltmp0
15+
; CHECK-NEXT: .p2align 1, 0x90
16+
; CHECK-NEXT: .Lxray_sled_1:
17+
; CHECK-NEXT: .ascii "\353\t"
18+
; CHECK-NEXT: nopw 512(%rax,%rax)
19+
; CHECK-NEXT: .Ltmp1:
20+
; CHECK-NEXT: jmp tail_call_target@PLT # TAILCALL
21+
; CHECK-NEXT: .Ltmp0:
22+
; CHECK-NEXT: # %bb.1:
23+
; CHECK-NEXT: .p2align 1, 0x90
24+
; CHECK-NEXT: .Lxray_sled_2:
25+
; CHECK-NEXT: retq
26+
; CHECK-NEXT: nopw %cs:512(%rax,%rax)
27+
; CHECK-NEXT: .Lfunc_end0:
28+
%cmp = icmp ne i32 %cond, 0
29+
br i1 %cmp, label %docall, label %ret
30+
docall:
31+
tail call void @tail_call_target()
32+
ret void
33+
ret:
34+
ret void
35+
}

0 commit comments

Comments
 (0)