X86: Don't emit zero-byte functions on Windows

Empty functions can lead to duplicate entries in the Guard CF Function
Table of a binary due to multiple functions sharing the same RVA,
causing the kernel to refuse to load that binary.

We had a terrific bug due to this in Chromium.

It turns out we were already doing this for Mach-O in certain
situations. This patch expands the code for that in
AsmPrinter::EmitFunctionBody() and renames
TargetInstrInfo::getNoopForMachoTarget() to simply getNoop() since it
seems it was used for not just Mach-O anyway.

Differential Revision: https://reviews.llvm.org/D32330

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301040 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Hans Wennborg 2017-04-21 20:58:12 +00:00
parent 9044c52d56
commit fe76aaa6ee
18 changed files with 54 additions and 36 deletions

View File

@ -1108,7 +1108,7 @@ public:
/// Return the noop instruction to use for a noop.
virtual void getNoopForMachoTarget(MCInst &NopInst) const;
virtual void getNoop(MCInst &NopInst) const;
/// Return true for post-incremented instructions.
virtual bool isPostIncrement(const MachineInstr &MI) const {

View File

@ -1046,15 +1046,17 @@ void AsmPrinter::EmitFunctionBody() {
// If the function is empty and the object file uses .subsections_via_symbols,
// then we need to emit *something* to the function body to prevent the
// labels from collapsing together. Just emit a noop.
if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode)) {
// Similarly, don't emit empty functions on Windows either. It can lead to
// duplicate entries (two functions with the same RVA) in the Guard CF Table
// after linking, causing the kernel not to load the binary:
// https://developercommunity.visualstudio.com/content/problem/45366/vc-linker-creates-invalid-dll-with-clang-cl.html
// FIXME: Hide this behind some API in e.g. MCAsmInfo or MCTargetStreamer.
if (!HasAnyRealCode &&
(MAI->hasSubsectionsViaSymbols() || TM.getTargetTriple().isOSWindows())) {
MCInst Noop;
MF->getSubtarget().getInstrInfo()->getNoopForMachoTarget(Noop);
MF->getSubtarget().getInstrInfo()->getNoop(Noop);
OutStreamer->AddComment("avoids zero-length function");
// Targets can opt-out of emitting the noop here by leaving the opcode
// unspecified.
if (Noop.getOpcode())
OutStreamer->EmitInstruction(Noop, getSubtargetInfo());
OutStreamer->EmitInstruction(Noop, getSubtargetInfo());
}
const Function *F = MF->getFunction();

View File

@ -428,8 +428,8 @@ static const TargetRegisterClass *canFoldCopy(const MachineInstr &MI,
return nullptr;
}
void TargetInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
llvm_unreachable("Not a MachO target");
void TargetInstrInfo::getNoop(MCInst &NopInst) const {
llvm_unreachable("Not implemented");
}
static MachineInstr *foldPatchpoint(MachineFunction &MF, MachineInstr &MI,

View File

@ -3025,7 +3025,7 @@ bool llvm::rewriteAArch64FrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
return false;
}
void AArch64InstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
void AArch64InstrInfo::getNoop(MCInst &NopInst) const {
NopInst.setOpcode(AArch64::HINT);
NopInst.addOperand(MCOperand::createImm(0));
}

View File

@ -205,7 +205,7 @@ public:
const DebugLoc &DL, unsigned DstReg,
ArrayRef<MachineOperand> Cond, unsigned TrueReg,
unsigned FalseReg) const override;
void getNoopForMachoTarget(MCInst &NopInst) const override;
void getNoop(MCInst &NopInst) const override;
/// analyzeCompare - For a comparison instruction, return the source registers
/// in SrcReg and SrcReg2, and the value it compares against in CmpValue.

View File

@ -105,10 +105,6 @@ public:
// Return whether the target has an explicit NOP encoding.
bool hasNOP() const;
virtual void getNoopForElfTarget(MCInst &NopInst) const {
getNoopForMachoTarget(NopInst);
}
// Return the non-pre/post incrementing version of 'Opc'. Return 0
// if there is not such an opcode.
virtual unsigned getUnindexedOpcode(unsigned Opc) const = 0;

View File

@ -32,8 +32,8 @@ using namespace llvm;
ARMInstrInfo::ARMInstrInfo(const ARMSubtarget &STI)
: ARMBaseInstrInfo(STI), RI() {}
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void ARMInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
/// Return the noop instruction to use for a noop.
void ARMInstrInfo::getNoop(MCInst &NopInst) const {
if (hasNOP()) {
NopInst.setOpcode(ARM::HINT);
NopInst.addOperand(MCOperand::createImm(0));

View File

@ -25,8 +25,8 @@ class ARMInstrInfo : public ARMBaseInstrInfo {
public:
explicit ARMInstrInfo(const ARMSubtarget &STI);
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void getNoopForMachoTarget(MCInst &NopInst) const override;
/// Return the noop instruction to use for a noop.
void getNoop(MCInst &NopInst) const override;
// Return the non-pre/post incrementing version of 'Opc'. Return 0
// if there is not such an opcode.

View File

@ -211,11 +211,9 @@ void ARMAsmPrinter::EmitSled(const MachineInstr &MI, SledKind Kind)
.addImm(ARMCC::AL).addReg(0));
MCInst Noop;
Subtarget->getInstrInfo()->getNoopForElfTarget(Noop);
Subtarget->getInstrInfo()->getNoop(Noop);
for (int8_t I = 0; I < NoopsInSledCount; I++)
{
OutStreamer->EmitInstruction(Noop, getSubtargetInfo());
}
OutStreamer->EmitLabel(Target);
recordSled(CurSled, MI, Kind);

View File

@ -24,8 +24,8 @@ using namespace llvm;
Thumb1InstrInfo::Thumb1InstrInfo(const ARMSubtarget &STI)
: ARMBaseInstrInfo(STI), RI() {}
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void Thumb1InstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
/// Return the noop instruction to use for a noop.
void Thumb1InstrInfo::getNoop(MCInst &NopInst) const {
NopInst.setOpcode(ARM::tMOVr);
NopInst.addOperand(MCOperand::createReg(ARM::R8));
NopInst.addOperand(MCOperand::createReg(ARM::R8));

View File

@ -25,8 +25,8 @@ class Thumb1InstrInfo : public ARMBaseInstrInfo {
public:
explicit Thumb1InstrInfo(const ARMSubtarget &STI);
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void getNoopForMachoTarget(MCInst &NopInst) const override;
/// Return the noop instruction to use for a noop.
void getNoop(MCInst &NopInst) const override;
// Return the non-pre/post incrementing version of 'Opc'. Return 0
// if there is not such an opcode.

View File

@ -32,8 +32,8 @@ OldT2IfCvt("old-thumb2-ifcvt", cl::Hidden,
Thumb2InstrInfo::Thumb2InstrInfo(const ARMSubtarget &STI)
: ARMBaseInstrInfo(STI), RI() {}
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void Thumb2InstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
/// Return the noop instruction to use for a noop.
void Thumb2InstrInfo::getNoop(MCInst &NopInst) const {
NopInst.setOpcode(ARM::tHINT);
NopInst.addOperand(MCOperand::createImm(0));
NopInst.addOperand(MCOperand::createImm(ARMCC::AL));

View File

@ -26,8 +26,8 @@ class Thumb2InstrInfo : public ARMBaseInstrInfo {
public:
explicit Thumb2InstrInfo(const ARMSubtarget &STI);
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void getNoopForMachoTarget(MCInst &NopInst) const override;
/// Return the noop instruction to use for a noop.
void getNoop(MCInst &NopInst) const override;
// Return the non-pre/post incrementing version of 'Opc'. Return 0
// if there is not such an opcode.

View File

@ -440,8 +440,8 @@ void PPCInstrInfo::insertNoop(MachineBasicBlock &MBB,
BuildMI(MBB, MI, DL, get(Opcode));
}
/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
void PPCInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
/// Return the noop instruction to use for a noop.
void PPCInstrInfo::getNoop(MCInst &NopInst) const {
NopInst.setOpcode(PPC::NOP);
}

View File

@ -269,7 +269,7 @@ public:
///
unsigned getInstSizeInBytes(const MachineInstr &MI) const override;
void getNoopForMachoTarget(MCInst &NopInst) const override;
void getNoop(MCInst &NopInst) const override;
std::pair<unsigned, unsigned>
decomposeMachineOperandsTargetFlags(unsigned TF) const override;

View File

@ -9514,7 +9514,7 @@ void X86InstrInfo::setExecutionDomain(MachineInstr &MI, unsigned Domain) const {
}
/// Return the noop instruction to use for a noop.
void X86InstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
void X86InstrInfo::getNoop(MCInst &NopInst) const {
NopInst.setOpcode(X86::NOOP);
}

View File

@ -457,7 +457,7 @@ public:
int64_t Offset1, int64_t Offset2,
unsigned NumLoads) const override;
void getNoopForMachoTarget(MCInst &NopInst) const override;
void getNoop(MCInst &NopInst) const override;
bool
reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;

View File

@ -0,0 +1,22 @@
; RUN: llc < %s -mtriple=i686-pc-win32 | FileCheck -check-prefix=CHECK -check-prefix=WIN32 %s
; RUN: llc < %s -mtriple=x86_64-pc-win32 | FileCheck -check-prefix=CHECK -check-prefix=WIN64 %s
; RUN: llc < %s -mtriple=i386-linux-gnu | FileCheck -check-prefix=LINUX %s
target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
target triple = "i686-pc-windows-msvc18.0.0"
; Don't emit empty functions on Windows; it can lead to duplicate entries
; (multiple functions sharing the same RVA) in the Guard CF Function Table which
; the kernel refuses to load.
define void @f() {
entry:
unreachable
; CHECK-LABEL: f:
; WIN32: nop
; WIN64: ud2
; LINUX-NOT: nop
; LINUX-NOT: ud2
}