From 1fccd57ea4bb75f508c702279f7caad34b9b9b8f Mon Sep 17 00:00:00 2001 From: hgreving Date: Wed, 29 Jul 2020 16:48:03 -0700 Subject: [PATCH] [MC] Fix memory leak when allocating MCInst with bump allocator Adds the function createMCInst() to MCContext that creates a MCInst using a typed bump alloctor. MCInst contains a SmallVector. The SmallVector is POD only for <= 8 operands. The default untyped bump pointer allocator of MCContext does not delete the MCInst, so if the SmallVector grows, it's a leak. This fixes https://bugs.llvm.org/show_bug.cgi?id=46900. --- include/llvm/MC/MCContext.h | 6 ++++++ lib/MC/MCContext.cpp | 9 +++++++++ .../Hexagon/AsmParser/HexagonAsmParser.cpp | 2 +- .../Disassembler/HexagonDisassembler.cpp | 6 +++--- lib/Target/Hexagon/HexagonMCInstLower.cpp | 2 +- .../Hexagon/MCTargetDesc/HexagonAsmBackend.cpp | 4 ++-- .../Hexagon/MCTargetDesc/HexagonMCCompound.cpp | 18 +++++++++--------- 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/llvm/MC/MCContext.h b/include/llvm/MC/MCContext.h index 45be9bb3d22..d041b06c556 100644 --- a/include/llvm/MC/MCContext.h +++ b/include/llvm/MC/MCContext.h @@ -97,6 +97,7 @@ namespace llvm { SpecificBumpPtrAllocator MachOAllocator; SpecificBumpPtrAllocator WasmAllocator; SpecificBumpPtrAllocator XCOFFAllocator; + SpecificBumpPtrAllocator MCInstAllocator; /// Bindings of names to symbols. SymbolTable Symbols; @@ -380,6 +381,11 @@ namespace llvm { /// @} + /// \name McInst Management + + /// Create and return a new MC instruction. + MCInst *createMCInst(); + /// \name Symbol Management /// @{ diff --git a/lib/MC/MCContext.cpp b/lib/MC/MCContext.cpp index a0f9212f3b1..5b007897431 100644 --- a/lib/MC/MCContext.cpp +++ b/lib/MC/MCContext.cpp @@ -90,6 +90,7 @@ void MCContext::reset() { ELFAllocator.DestroyAll(); MachOAllocator.DestroyAll(); XCOFFAllocator.DestroyAll(); + MCInstAllocator.DestroyAll(); MCSubtargetAllocator.DestroyAll(); InlineAsmUsedLabelNames.clear(); @@ -126,6 +127,14 @@ void MCContext::reset() { HadError = false; } +//===----------------------------------------------------------------------===// +// MCInst Management +//===----------------------------------------------------------------------===// + +MCInst *MCContext::createMCInst() { + return new (MCInstAllocator.Allocate()) MCInst; +} + //===----------------------------------------------------------------------===// // Symbol Manipulation //===----------------------------------------------------------------------===// diff --git a/lib/Target/Hexagon/AsmParser/HexagonAsmParser.cpp b/lib/Target/Hexagon/AsmParser/HexagonAsmParser.cpp index 1e7862c36ea..3759962c415 100644 --- a/lib/Target/Hexagon/AsmParser/HexagonAsmParser.cpp +++ b/lib/Target/Hexagon/AsmParser/HexagonAsmParser.cpp @@ -641,7 +641,7 @@ bool HexagonAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode, return true; return finishBundle(IDLoc, Out); } - MCInst *SubInst = new (getParser().getContext()) MCInst; + MCInst *SubInst = getParser().getContext().createMCInst(); if (matchOneInstruction(*SubInst, IDLoc, Operands, ErrorInfo, MatchingInlineAsm)) { if (InBrackets) diff --git a/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp b/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp index f3a87ef20a6..aeaeac65de9 100644 --- a/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp +++ b/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp @@ -175,7 +175,7 @@ DecodeStatus HexagonDisassembler::getInstruction(MCInst &MI, uint64_t &Size, while (Result == Success && !Complete) { if (Bytes.size() < HEXAGON_INSTR_SIZE) return MCDisassembler::Fail; - MCInst *Inst = new (getContext()) MCInst; + MCInst *Inst = getContext().createMCInst(); Result = getSingleInstruction(*Inst, MI, Bytes, Address, cs, Complete); MI.addOperand(MCOperand::createInst(Inst)); Size += HEXAGON_INSTR_SIZE; @@ -384,8 +384,8 @@ DecodeStatus HexagonDisassembler::getSingleInstruction(MCInst &MI, MCInst &MCB, break; } MI.setOpcode(Hexagon::DuplexIClass0 + duplexIClass); - MCInst *MILow = new (getContext()) MCInst; - MCInst *MIHigh = new (getContext()) MCInst; + MCInst *MILow = getContext().createMCInst(); + MCInst *MIHigh = getContext().createMCInst(); auto TmpExtender = CurrentExtender; CurrentExtender = nullptr; // constant extenders in duplex must always be in slot 1 diff --git a/lib/Target/Hexagon/HexagonMCInstLower.cpp b/lib/Target/Hexagon/HexagonMCInstLower.cpp index 188d91355a3..9507de95231 100644 --- a/lib/Target/Hexagon/HexagonMCInstLower.cpp +++ b/lib/Target/Hexagon/HexagonMCInstLower.cpp @@ -104,7 +104,7 @@ void llvm::HexagonLowerToMC(const MCInstrInfo &MCII, const MachineInstr *MI, HexagonMCInstrInfo::setOuterLoop(MCB); return; } - MCInst *MCI = new (AP.OutContext) MCInst; + MCInst *MCI = AP.OutContext.createMCInst(); MCI->setOpcode(MI->getOpcode()); assert(MCI->getOpcode() == static_cast(MI->getOpcode()) && "MCI opcode should have been set on construction"); diff --git a/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp index e7069819fa5..627c53cadd8 100644 --- a/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp +++ b/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp @@ -74,7 +74,7 @@ public: void setExtender(MCContext &Context) const { if (Extender == nullptr) - const_cast(this)->Extender = new (Context) MCInst; + const_cast(this)->Extender = Context.createMCInst(); } MCInst *takeExtender() const { @@ -736,7 +736,7 @@ public: auto &Inst = const_cast(RF.getInst()); while (Size > 0 && HexagonMCInstrInfo::bundleSize(Inst) < MaxPacketSize) { - MCInst *Nop = new (Context) MCInst; + MCInst *Nop = Context.createMCInst(); Nop->setOpcode(Hexagon::A2_nop); Inst.addOperand(MCOperand::createInst(Nop)); Size -= 4; diff --git a/lib/Target/Hexagon/MCTargetDesc/HexagonMCCompound.cpp b/lib/Target/Hexagon/MCTargetDesc/HexagonMCCompound.cpp index 82b2074c5cd..e7ade7834a9 100644 --- a/lib/Target/Hexagon/MCTargetDesc/HexagonMCCompound.cpp +++ b/lib/Target/Hexagon/MCTargetDesc/HexagonMCCompound.cpp @@ -210,7 +210,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, case Hexagon::A2_tfrsi: Rt = L.getOperand(0); compoundOpcode = J4_jumpseti; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rt); @@ -223,7 +223,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, Rs = L.getOperand(1); compoundOpcode = J4_jumpsetr; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rt); CompoundInsn->addOperand(Rs); @@ -237,7 +237,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, Rt = L.getOperand(2); compoundOpcode = cmpeqBitOpcode[getCompoundOp(R)]; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(Rt); @@ -250,7 +250,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, Rt = L.getOperand(2); compoundOpcode = cmpgtBitOpcode[getCompoundOp(R)]; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(Rt); @@ -263,7 +263,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, Rt = L.getOperand(2); compoundOpcode = cmpgtuBitOpcode[getCompoundOp(R)]; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(Rt); @@ -281,7 +281,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, compoundOpcode = cmpeqiBitOpcode[getCompoundOp(R)]; Rs = L.getOperand(1); - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(L.getOperand(2)); @@ -299,7 +299,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, compoundOpcode = cmpgtiBitOpcode[getCompoundOp(R)]; Rs = L.getOperand(1); - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(L.getOperand(2)); @@ -310,7 +310,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, LLVM_DEBUG(dbgs() << "CX: C2_cmpgtui\n"); Rs = L.getOperand(1); compoundOpcode = cmpgtuiBitOpcode[getCompoundOp(R)]; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(L.getOperand(2)); @@ -321,7 +321,7 @@ static MCInst *getCompoundInsn(MCContext &Context, MCInst const &L, LLVM_DEBUG(dbgs() << "CX: S2_tstbit_i\n"); Rs = L.getOperand(1); compoundOpcode = tstBitOpcode[getCompoundOp(R)]; - CompoundInsn = new (Context) MCInst; + CompoundInsn = Context.createMCInst(); CompoundInsn->setOpcode(compoundOpcode); CompoundInsn->addOperand(Rs); CompoundInsn->addOperand(R.getOperand(1));