Skip to content

Commit 179fb2e

Browse files
authored
[XRay][X86] Fix stack alignment for custom event calls (#89360)
Calls to @llvm.xray.{custom,typed}event are lowered to an nop sled that can be patched with a call to an instrumentation function at runtime. Prior to this change, x86 codegen did not guarantee that these patched calls run with the appropriate stack alignment (particularly in leaf functions, where normal stack alignment assumptions may not hold). This leads to crashes on x86, as the custom event hook can end up using SSE instructions that assume a 16-byte aligned stack. Fix this by wrapping custom event hooks in CALLSEQ_START/CALLSEQ_END as done for regular function calls. One downside of this approach is that on functions whose stacks aren't already aligned, we may end up running stack alignment fixup instructions even when instrumentation is disabled. An alternative could be to make the custom event assembly trampolines stack-alignment-agnostic. This was the case in the past, but b46c898 removed this due to complexity in maintaining CFI directives for these stack adjustments. Since we are already willing to pay the call argument setup cost for custom hooks when instrumentation is disabled, I am hoping that an extra push/pop in this hopefully uncommon unaligned stack case is tolerable.
1 parent 12b0ef5 commit 179fb2e

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36377,6 +36377,31 @@ X86TargetLowering::EmitSjLjDispatchBlock(MachineInstr &MI,
3637736377
return BB;
3637836378
}
3637936379

36380+
MachineBasicBlock *
36381+
X86TargetLowering::emitPatchableEventCall(MachineInstr &MI,
36382+
MachineBasicBlock *BB) const {
36383+
// Wrap patchable event calls in CALLSEQ_START/CALLSEQ_END, as tracing
36384+
// calls may require proper stack alignment.
36385+
const TargetInstrInfo &TII = *Subtarget.getInstrInfo();
36386+
const MIMetadata MIMD(MI);
36387+
MachineFunction &MF = *BB->getParent();
36388+
36389+
// Emit CALLSEQ_START right before the instruction.
36390+
MF.getFrameInfo().setAdjustsStack(true);
36391+
unsigned AdjStackDown = TII.getCallFrameSetupOpcode();
36392+
MachineInstrBuilder CallseqStart =
36393+
BuildMI(MF, MIMD, TII.get(AdjStackDown)).addImm(0).addImm(0).addImm(0);
36394+
BB->insert(MachineBasicBlock::iterator(MI), CallseqStart);
36395+
36396+
// Emit CALLSEQ_END right after the instruction.
36397+
unsigned AdjStackUp = TII.getCallFrameDestroyOpcode();
36398+
MachineInstrBuilder CallseqEnd =
36399+
BuildMI(MF, MIMD, TII.get(AdjStackUp)).addImm(0).addImm(0);
36400+
BB->insertAfter(MachineBasicBlock::iterator(MI), CallseqEnd);
36401+
36402+
return BB;
36403+
}
36404+
3638036405
MachineBasicBlock *
3638136406
X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
3638236407
MachineBasicBlock *BB) const {
@@ -36607,7 +36632,7 @@ X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
3660736632

3660836633
case TargetOpcode::PATCHABLE_EVENT_CALL:
3660936634
case TargetOpcode::PATCHABLE_TYPED_EVENT_CALL:
36610-
return BB;
36635+
return emitPatchableEventCall(MI, BB);
3661136636

3661236637
case X86::LCMPXCHG8B: {
3661336638
const X86RegisterInfo *TRI = Subtarget.getRegisterInfo();

llvm/lib/Target/X86/X86ISelLowering.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1798,6 +1798,9 @@ namespace llvm {
17981798
MachineBasicBlock *EmitSjLjDispatchBlock(MachineInstr &MI,
17991799
MachineBasicBlock *MBB) const;
18001800

1801+
MachineBasicBlock *emitPatchableEventCall(MachineInstr &MI,
1802+
MachineBasicBlock *MBB) const;
1803+
18011804
/// Emit flags for the given setcc condition and operands. Also returns the
18021805
/// corresponding X86 condition code constant in X86CC.
18031806
SDValue emitFlagsForSetcc(SDValue Op0, SDValue Op1, ISD::CondCode CC,

llvm/test/CodeGen/X86/xray-custom-log.ll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,21 @@ define i32 @typedevent() nounwind "function-instrument"="xray-always" !dbg !2 {
7575
; CHECK-LABEL: Lxray_sleds_start1:
7676
; CHECK: .quad {{.*}}xray_typed_event_sled_0
7777

78+
; Verify that custom event calls are done with proper stack alignment,
79+
; even in leaf functions.
80+
@leaf_func.event_id = internal constant i32 1, align 4
81+
define void @leaf_func() "function-instrument"="xray-always" "frame-pointer"="none" nounwind {
82+
; CHECK-LABEL: leaf_func:
83+
; CHECK-NEXT: .Lfunc_begin2:
84+
; CHECK: pushq %rax
85+
; CHECK: movl $leaf_func.event_id, %eax
86+
; CHECK-NEXT: movl $4, %ecx
87+
; CHECK-NEXT: .p2align 1, 0x90
88+
; CHECK-NEXT: .Lxray_event_sled_1:
89+
call void @llvm.xray.customevent(ptr @leaf_func.event_id, i64 4)
90+
ret void
91+
}
92+
7893
declare void @llvm.xray.customevent(ptr, i64)
7994
declare void @llvm.xray.typedevent(i64, ptr, i64)
8095

0 commit comments

Comments
 (0)