From: Mandeep Singh Grang Date: Wed, 15 May 2019 21:23:41 +0000 (+0000) Subject: [AArch64] only indicate CFI on Windows if we emitted CFI X-Git-Tag: llvmorg-10-init~5456 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=814435fe87412dc3ac1cb0b00d6a3786accb5e0d;p=platform%2Fupstream%2Fllvm.git [AArch64] only indicate CFI on Windows if we emitted CFI Summary: Otherwise, we emit directives for CFI without any actual CFI opcodes to go with them, which causes tools to malfunction. The technique is similar to what the x86 backend already does. Fixes https://bugs.llvm.org/show_bug.cgi?id=40876 Patch by: froydnj (Nathan Froyd) Reviewers: mstorsjo, eli.friedman, rnk, mgrang, ssijaric Reviewed By: rnk Subscribers: javed.absar, kristof.beyls, llvm-commits, dmajor Tags: #llvm Differential Revision: https://reviews.llvm.org/D61960 llvm-svn: 360816 --- diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp index 597b25c..98cfbf8 100644 --- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -587,7 +587,7 @@ static void fixupSEHOpcode(MachineBasicBlock::iterator MBBI, static MachineBasicBlock::iterator convertCalleeSaveRestoreToSPPrePostIncDec( MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI, const DebugLoc &DL, const TargetInstrInfo *TII, int CSStackSizeInc, - bool NeedsWinCFI, bool InProlog = true) { + bool NeedsWinCFI, bool *HasWinCFI, bool InProlog = true) { // Ignore instructions that do not operate on SP, i.e. shadow call stack // instructions and associated CFI instruction. while (MBBI->getOpcode() == AArch64::STRXpost || @@ -673,9 +673,11 @@ static MachineBasicBlock::iterator convertCalleeSaveRestoreToSPPrePostIncDec( MIB.setMemRefs(MBBI->memoperands()); // Generate a new SEH code that corresponds to the new instruction. - if (NeedsWinCFI) + if (NeedsWinCFI) { + *HasWinCFI = true; InsertSEH(*MIB, *TII, InProlog ? MachineInstr::FrameSetup : MachineInstr::FrameDestroy); + } return std::prev(MBB.erase(MBBI)); } @@ -684,7 +686,8 @@ static MachineBasicBlock::iterator convertCalleeSaveRestoreToSPPrePostIncDec( // combined SP bump by adding the local stack size to the stack offsets. static void fixupCalleeSaveRestoreStackOffset(MachineInstr &MI, unsigned LocalStackSize, - bool NeedsWinCFI) { + bool NeedsWinCFI, + bool *HasWinCFI) { if (AArch64InstrInfo::isSEHInstruction(MI)) return; @@ -731,6 +734,7 @@ static void fixupCalleeSaveRestoreStackOffset(MachineInstr &MI, OffsetOpnd.setImm(OffsetOpnd.getImm() + LocalStackSize / Scale); if (NeedsWinCFI) { + *HasWinCFI = true; auto MBBI = std::next(MachineBasicBlock::iterator(MI)); assert(MBBI != MI.getParent()->end() && "Expecting a valid instruction"); assert(AArch64InstrInfo::isSEHInstruction(*MBBI) && @@ -802,7 +806,9 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, !MF.getTarget().getMCAsmInfo()->usesWindowsCFI(); bool HasFP = hasFP(MF); bool NeedsWinCFI = needsWinCFI(MF); - MF.setHasWinCFI(NeedsWinCFI); + bool HasWinCFI = false; + auto Cleanup = make_scope_exit([&]() { MF.setHasWinCFI(HasWinCFI); }); + bool IsFunclet = MBB.isEHFuncletEntry(); // At this point, we're going to decide whether or not the function uses a @@ -858,7 +864,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, ++NumRedZoneFunctions; } else { emitFrameOffset(MBB, MBBI, DL, AArch64::SP, AArch64::SP, -NumBytes, TII, - MachineInstr::FrameSetup, false, NeedsWinCFI); + MachineInstr::FrameSetup, false, NeedsWinCFI, &HasWinCFI); if (!NeedsWinCFI) { // Label used to tie together the PROLOG_LABEL and the MachineMoves. MCSymbol *FrameLabel = MMI.getContext().createTempSymbol(); @@ -871,9 +877,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, } } - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_PrologEnd)) .setMIFlag(MachineInstr::FrameSetup); + } return; } @@ -891,11 +899,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, bool CombineSPBump = shouldCombineCSRLocalStackBump(MF, NumBytes); if (CombineSPBump) { emitFrameOffset(MBB, MBBI, DL, AArch64::SP, AArch64::SP, -NumBytes, TII, - MachineInstr::FrameSetup, false, NeedsWinCFI); + MachineInstr::FrameSetup, false, NeedsWinCFI, &HasWinCFI); NumBytes = 0; } else if (PrologueSaveSize != 0) { MBBI = convertCalleeSaveRestoreToSPPrePostIncDec( - MBB, MBBI, DL, TII, -PrologueSaveSize, NeedsWinCFI); + MBB, MBBI, DL, TII, -PrologueSaveSize, NeedsWinCFI, &HasWinCFI); NumBytes -= PrologueSaveSize; } assert(NumBytes >= 0 && "Negative stack allocation size!?"); @@ -907,7 +915,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, while (MBBI != End && MBBI->getFlag(MachineInstr::FrameSetup)) { if (CombineSPBump) fixupCalleeSaveRestoreStackOffset(*MBBI, AFI->getLocalStackSize(), - NeedsWinCFI); + NeedsWinCFI, &HasWinCFI); ++MBBI; } @@ -915,9 +923,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, // opcodes that we needed to emit. The FP and BP belong to the containing // function. if (IsFunclet) { - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_PrologEnd)) .setMIFlag(MachineInstr::FrameSetup); + } // SEH funclets are passed the frame pointer in X1. If the parent // function uses the base register, then the base register is used @@ -946,12 +956,13 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, // Note: All stores of callee-saved registers are marked as "FrameSetup". // This code marks the instruction(s) that set the FP also. emitFrameOffset(MBB, MBBI, DL, AArch64::FP, AArch64::SP, FPOffset, TII, - MachineInstr::FrameSetup, false, NeedsWinCFI); + MachineInstr::FrameSetup, false, NeedsWinCFI, &HasWinCFI); } if (windowsRequiresStackProbe(MF, NumBytes)) { uint32_t NumWords = NumBytes >> 4; if (NeedsWinCFI) { + HasWinCFI = true; // alloc_l can hold at most 256MB, so assume that NumBytes doesn't // exceed this amount. We need to move at most 2^24 - 1 into x15. // This is at most two instructions, MOVZ follwed by MOVK. @@ -995,9 +1006,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, .addReg(AArch64::X17, RegState::Implicit | RegState::Define | RegState::Dead) .addReg(AArch64::NZCV, RegState::Implicit | RegState::Define | RegState::Dead) .setMIFlags(MachineInstr::FrameSetup); - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_Nop)) .setMIFlag(MachineInstr::FrameSetup); + } break; case CodeModel::Large: BuildMI(MBB, MBBI, DL, TII->get(AArch64::MOVaddrEXT)) @@ -1005,9 +1018,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, .addExternalSymbol("__chkstk") .addExternalSymbol("__chkstk") .setMIFlags(MachineInstr::FrameSetup); - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_Nop)) .setMIFlag(MachineInstr::FrameSetup); + } BuildMI(MBB, MBBI, DL, TII->get(AArch64::BLR)) .addReg(AArch64::X16, RegState::Kill) @@ -1016,9 +1031,11 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, .addReg(AArch64::X17, RegState::Implicit | RegState::Define | RegState::Dead) .addReg(AArch64::NZCV, RegState::Implicit | RegState::Define | RegState::Dead) .setMIFlags(MachineInstr::FrameSetup); - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_Nop)) .setMIFlag(MachineInstr::FrameSetup); + } break; } @@ -1027,10 +1044,12 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, .addReg(AArch64::X15, RegState::Kill) .addImm(AArch64_AM::getArithExtendImm(AArch64_AM::UXTX, 4)) .setMIFlags(MachineInstr::FrameSetup); - if (NeedsWinCFI) - BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)) - .addImm(NumBytes) - .setMIFlag(MachineInstr::FrameSetup); + if (NeedsWinCFI) { + HasWinCFI = true; + BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)) + .addImm(NumBytes) + .setMIFlag(MachineInstr::FrameSetup); + } NumBytes = 0; } @@ -1050,7 +1069,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, // the correct value here, as NumBytes also includes padding bytes, // which shouldn't be counted here. emitFrameOffset(MBB, MBBI, DL, scratchSPReg, AArch64::SP, -NumBytes, TII, - MachineInstr::FrameSetup, false, NeedsWinCFI); + MachineInstr::FrameSetup, false, NeedsWinCFI, &HasWinCFI); if (NeedsRealignment) { const unsigned Alignment = MFI.getMaxAlignment(); @@ -1073,10 +1092,12 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, .addReg(scratchSPReg, RegState::Kill) .addImm(andMaskEncoded); AFI->setStackRealigned(true); - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)) .addImm(NumBytes & andMaskEncoded) .setMIFlag(MachineInstr::FrameSetup); + } } } @@ -1090,16 +1111,19 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, if (RegInfo->hasBasePointer(MF)) { TII->copyPhysReg(MBB, MBBI, DL, RegInfo->getBaseRegister(), AArch64::SP, false); - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_Nop)) .setMIFlag(MachineInstr::FrameSetup); + } } // The very last FrameSetup instruction indicates the end of prologue. Emit a // SEH opcode indicating the prologue end. - if (NeedsWinCFI) + if (NeedsWinCFI && HasWinCFI) { BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_PrologEnd)) .setMIFlag(MachineInstr::FrameSetup); + } if (needsFrameMoves) { const DataLayout &TD = MF.getDataLayout(); @@ -1243,7 +1267,12 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF, DebugLoc DL; bool IsTailCallReturn = false; bool NeedsWinCFI = needsWinCFI(MF); + bool HasWinCFI = false; bool IsFunclet = false; + auto WinCFI = make_scope_exit([&]() { + if (!MF.hasWinCFI()) + MF.setHasWinCFI(HasWinCFI); + }); if (MBB.end() != MBBI) { DL = MBBI->getDebugLoc(); @@ -1338,7 +1367,7 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF, // If the offset is 0, convert it to a post-index ldp. if (OffsetOp.getImm() == 0) convertCalleeSaveRestoreToSPPrePostIncDec( - MBB, Pop, DL, TII, PrologueSaveSize, NeedsWinCFI, false); + MBB, Pop, DL, TII, PrologueSaveSize, NeedsWinCFI, &HasWinCFI, false); else { // If not, make sure to emit an add after the last ldp. // We're doing this by transfering the size to be restored from the @@ -1360,19 +1389,21 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF, break; } else if (CombineSPBump) fixupCalleeSaveRestoreStackOffset(*LastPopI, AFI->getLocalStackSize(), - NeedsWinCFI); + NeedsWinCFI, &HasWinCFI); } - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, LastPopI, DL, TII->get(AArch64::SEH_EpilogStart)) .setMIFlag(MachineInstr::FrameDestroy); + } // If there is a single SP update, insert it before the ret and we're done. if (CombineSPBump) { emitFrameOffset(MBB, MBB.getFirstTerminator(), DL, AArch64::SP, AArch64::SP, NumBytes + AfterCSRPopSize, TII, MachineInstr::FrameDestroy, - false, NeedsWinCFI); - if (NeedsWinCFI) + false, NeedsWinCFI, &HasWinCFI); + if (NeedsWinCFI && HasWinCFI) BuildMI(MBB, MBB.getFirstTerminator(), DL, TII->get(AArch64::SEH_EpilogEnd)) .setMIFlag(MachineInstr::FrameDestroy); @@ -1404,12 +1435,14 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF, emitFrameOffset(MBB, LastPopI, DL, AArch64::SP, AArch64::SP, StackRestoreBytes, TII, MachineInstr::FrameDestroy, false, - NeedsWinCFI); + NeedsWinCFI, &HasWinCFI); if (Done) { - if (NeedsWinCFI) + if (NeedsWinCFI) { + HasWinCFI = true; BuildMI(MBB, MBB.getFirstTerminator(), DL, TII->get(AArch64::SEH_EpilogEnd)) .setMIFlag(MachineInstr::FrameDestroy); + } return; } @@ -1448,11 +1481,13 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF, emitFrameOffset(MBB, FirstSPPopI, DL, AArch64::SP, AArch64::SP, AfterCSRPopSize, TII, MachineInstr::FrameDestroy, false, - NeedsWinCFI); + NeedsWinCFI, &HasWinCFI); } - if (NeedsWinCFI) + if (NeedsWinCFI && HasWinCFI) BuildMI(MBB, MBB.getFirstTerminator(), DL, TII->get(AArch64::SEH_EpilogEnd)) .setMIFlag(MachineInstr::FrameDestroy); + + MF.setHasWinCFI(HasWinCFI); } /// getFrameIndexReference - Provide a base+offset reference to an FI slot for diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 0b5c7bb..11b8328 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -2959,7 +2959,7 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB, unsigned DestReg, unsigned SrcReg, int Offset, const TargetInstrInfo *TII, MachineInstr::MIFlag Flag, bool SetNZCV, - bool NeedsWinCFI) { + bool NeedsWinCFI, bool *HasWinCFI) { if (DestReg == SrcReg && Offset == 0) return; @@ -3004,10 +3004,13 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB, .addImm(AArch64_AM::getShifterImm(AArch64_AM::LSL, ShiftSize)) .setMIFlag(Flag); - if (NeedsWinCFI && SrcReg == AArch64::SP && DestReg == AArch64::SP) - BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)) - .addImm(ThisVal) - .setMIFlag(Flag); + if (NeedsWinCFI && SrcReg == AArch64::SP && DestReg == AArch64::SP) { + if (HasWinCFI) + *HasWinCFI = true; + BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)) + .addImm(ThisVal) + .setMIFlag(Flag); + } SrcReg = DestReg; Offset -= ThisVal; @@ -3023,6 +3026,8 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB, if (NeedsWinCFI) { if ((DestReg == AArch64::FP && SrcReg == AArch64::SP) || (SrcReg == AArch64::FP && DestReg == AArch64::SP)) { + if (HasWinCFI) + *HasWinCFI = true; if (Offset == 0) BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_SetFP)). setMIFlag(Flag); @@ -3030,6 +3035,8 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB, BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_AddFP)). addImm(Offset).setMIFlag(Flag); } else if (DestReg == AArch64::SP) { + if (HasWinCFI) + *HasWinCFI = true; BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_StackAlloc)). addImm(Offset).setMIFlag(Flag); } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index b33edc5..4823087 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -293,7 +293,8 @@ void emitFrameOffset(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI, const DebugLoc &DL, unsigned DestReg, unsigned SrcReg, int Offset, const TargetInstrInfo *TII, MachineInstr::MIFlag = MachineInstr::NoFlags, - bool SetNZCV = false, bool NeedsWinCFI = false); + bool SetNZCV = false, bool NeedsWinCFI = false, + bool *HasWinCFI = nullptr); /// rewriteAArch64FrameIndex - Rewrite MI to access 'Offset' bytes from the /// FP. Return false if the offset could not be handled directly in MI, and diff --git a/llvm/test/CodeGen/AArch64/win64-nocfi.ll b/llvm/test/CodeGen/AArch64/win64-nocfi.ll new file mode 100644 index 0000000..a1ca561 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/win64-nocfi.ll @@ -0,0 +1,13 @@ +; RUN: llc < %s -mtriple=aarch64-pc-windows-msvc | FileCheck %s + +define dso_local void @"?f@@YAXXZ"() nounwind sspstrong uwtable { +; CHECK-LABEL: f@@YAXXZ +; CHECK-NOT: .seh_proc +; CHECK-NOT: .seh_handlerdata +; CHECK-NOT: .seh_endproc +entry: + call void @llvm.trap() + ret void +} + +declare void @llvm.trap() noreturn nounwind