llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-bolt Author: Alexey Moksyakov (yavtuk) <details> <summary>Changes</summary> @<!-- -->bgergely0 hello, here the patch to disassemble plt and emit new one, I see that you have patched long-jump pass for that, now I see that output binary contains fixed plt entry instructions with bti at the beginning, but need to fix the trampoline address for new plt, I will try to find out where or you can try if you touch this in long-jmp pass. Let's try to start with the minimum to solve your problem, and when it's ready, I'll go over what needs to be kept and what needs to be added. This patch is draft for you only. Please let me know if it not convenient to use. --- Full diff: https://github.com/llvm/llvm-project/pull/173571.diff 9 Files Affected: - (modified) bolt/include/bolt/Core/BinaryFunction.h (+3) - (modified) bolt/include/bolt/Core/MCPlusBuilder.h (+9) - (modified) bolt/lib/Core/BinaryEmitter.cpp (+28-1) - (modified) bolt/lib/Core/BinaryFunction.cpp (+40-5) - (modified) bolt/lib/Passes/FixRelaxationPass.cpp (+5-1) - (modified) bolt/lib/Passes/LongJmp.cpp (+5) - (modified) bolt/lib/Passes/ProfileQualityStats.cpp (+10-1) - (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+57-29) - (modified) bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp (+28-7) ``````````diff diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 12aef415aa050..43bd5c396d88c 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1303,6 +1303,9 @@ class BinaryFunction { return Islands->FunctionColdConstantIslandLabel; } + /// Insert plt instructions + void disassemblePLT(InstructionListType &Instructions); + /// Return true if this is a function representing a PLT entry. bool isPLTFunction() const { return PLTSymbol != nullptr; } diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index fdd86deea4c5d..bd7ff278e2b61 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1788,6 +1788,15 @@ class MCPlusBuilder { llvm_unreachable("not implemented"); } + /// Replace Immediate values in PLT entry to symbol reference + virtual bool handlePLTEntry(InstructionIterator Begin, + InstructionIterator End, + const MCSymbol *PLTSymbol, + MCContext *Ctx) { + llvm_unreachable("not implemented"); + return false; + } + virtual bool analyzeVirtualMethodCall(InstructionIterator Begin, InstructionIterator End, std::vector<MCInst *> &MethodFetchInsns, diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp index 025516af82023..bfe21cb842bcc 100644 --- a/bolt/lib/Core/BinaryEmitter.cpp +++ b/bolt/lib/Core/BinaryEmitter.cpp @@ -275,6 +275,27 @@ void BinaryEmitter::emitFunctions() { } }; + // emit PLT section + if (BC.usesBTI()) { + std::vector<BinaryFunction *> PLTFunctions = BC.getOutputBinaryFunctions(); + llvm::erase_if(PLTFunctions, + [](BinaryFunction *BF) { return !BF->isPLTFunction(); }); + + const bool OriginalAllowAutoPadding = Streamer.getAllowAutoPadding(); + for (BinaryFunction *BF : PLTFunctions) { + MCSection *Section = BC.getCodeSection(BF->getOriginSection()->getName()); + Streamer.switchSection(Section); + Section->setAlignment(Align(BF->getOriginSection()->getAlignment())); + Section->setHasInstructions(true); + for (auto &BB : *BF) { + for (auto Inst : BB) + Streamer.emitInstruction(Inst, *BC.STI); + } + BF->setEmitted(/*KeepCFG=*/false); + Streamer.setAllowAutoPadding(OriginalAllowAutoPadding); + } + } + // Mark the start of hot text. if (opts::HotText) { Streamer.switchSection(BC.getTextSection()); @@ -282,7 +303,13 @@ void BinaryEmitter::emitFunctions() { } // Emit functions in sorted order. - emit(BC.getOutputBinaryFunctions()); + std::vector<BinaryFunction *> SortedFunctions = BC.getOutputBinaryFunctions(); + if (BC.usesBTI()) { + llvm::erase_if(SortedFunctions, + [](BinaryFunction *BF) { return BF->isPLTFunction(); }); + } + + emit(SortedFunctions); // Mark the end of hot text. if (opts::HotText) { diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 255f3d734bdb4..4051f8d9de3d2 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -147,11 +147,6 @@ static cl::opt<bool> TrapOnAVX512( cl::init(false), cl::ZeroOrMore, cl::Hidden, cl::cat(BoltCategory)); bool shouldPrint(const BinaryFunction &Function) { - // PLT stubs are disassembled for BTI binaries, therefore they should be - // printed. - if (Function.getBinaryContext().usesBTI() && Function.isPLTFunction()) - return true; - if (Function.isIgnored()) return false; @@ -4457,7 +4452,47 @@ void BinaryFunction::calculateLoopInfo() { } } +void BinaryFunction::disassemblePLT(InstructionListType &Instructions) { + if (Instructions.empty()) + return; + + BC.SymbolicDisAsm->setSymbolizer(BC.MIB->createTargetSymbolizer(*this)); + // Insert a label at the beginning of the function. This will be our first + // basic block. + Labels[0] = BC.Ctx->createNamedTempSymbol("BB0"); + + if (!BC.MIB->handlePLTEntry(Instructions.begin(), Instructions.end(), + getPLTSymbol(), BC.Ctx.get())) { + if (opts::Verbosity) + outs() << "BOLT-INFO: can't handle plt entry in function " + << getPrintName() << " for entry " + << getPLTSymbol() << "\n"; + // Reset symbolizer for the disassembler. + BC.SymbolicDisAsm->setSymbolizer(nullptr); + return; + } + + uint64_t Offset{0}; + for (MCInst Inst : Instructions) { + addInstruction(Offset, std::move(Inst)); + if (BC.keepOffsetForInstruction(Inst)) + BC.MIB->setOffset(Inst, static_cast<uint32_t>(Offset)); + Offset += BC.computeInstructionSize(Inst); + } + + // Reset symbolizer for the disassembler. + BC.SymbolicDisAsm->setSymbolizer(nullptr); + + updateState(State::Disassembled); +} + void BinaryFunction::updateOutputValues(const BOLTLinker &Linker) { + if (BC.usesBTI() && isPLTFunction()) { + const uint64_t Offset = getAddress() - OriginSection->getAddress(); + setOutputAddress(OriginSection->getOutputAddress() + Offset); + return; + } + if (!isEmitted()) { assert(!isInjected() && "injected function should be emitted"); setOutputAddress(getAddress()); diff --git a/bolt/lib/Passes/FixRelaxationPass.cpp b/bolt/lib/Passes/FixRelaxationPass.cpp index 7c970e464a94e..d4b2fbd7d5eb0 100644 --- a/bolt/lib/Passes/FixRelaxationPass.cpp +++ b/bolt/lib/Passes/FixRelaxationPass.cpp @@ -63,8 +63,12 @@ Error FixRelaxations::runOnFunctions(BinaryContext &BC) { runOnFunction(BF); }; + ParallelUtilities::PredicateTy SkipFunc = [&](const BinaryFunction &BF) { + return BF.isPLTFunction(); + }; + ParallelUtilities::runOnEachFunction( - BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun, nullptr, + BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun, SkipFunc, "FixRelaxations"); return Error::success(); } diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index 798e1ba08918a..8b6b332f771e3 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -977,6 +977,11 @@ Error LongJmpPass::runOnFunctions(BinaryContext &BC) { BC.outs() << "BOLT-INFO: Starting stub-insertion pass\n"; BinaryFunctionListType Sorted = BC.getOutputBinaryFunctions(); + if (BC.usesBTI()) { + llvm::erase_if(Sorted, + [](BinaryFunction *BF) { return BF->isPLTFunction(); }); + } + bool Modified; uint32_t Iterations = 0; do { diff --git a/bolt/lib/Passes/ProfileQualityStats.cpp b/bolt/lib/Passes/ProfileQualityStats.cpp index b2303bdfc8d1d..892e8a9e89a9f 100644 --- a/bolt/lib/Passes/ProfileQualityStats.cpp +++ b/bolt/lib/Passes/ProfileQualityStats.cpp @@ -477,6 +477,9 @@ void computeFlowMappings(const BinaryContext &BC, FlowInfo &TotalFlowMap) { TotalFlowMapTy &TotalOutgoingFlows = TotalFlowMap.TotalOutgoingFlows; for (const auto &BFI : BC.getBinaryFunctions()) { const BinaryFunction *Function = &BFI.second; + if (Function->isPLTFunction()) + continue; + std::vector<uint64_t> &IncomingFlows = TotalIncomingFlows[Function->getFunctionNumber()]; std::vector<uint64_t> &OutgoingFlows = @@ -505,6 +508,9 @@ void computeFlowMappings(const BinaryContext &BC, FlowInfo &TotalFlowMap) { TotalFlowMapTy &TotalMinCountMaps = TotalFlowMap.TotalMinCountMaps; for (const auto &BFI : BC.getBinaryFunctions()) { const BinaryFunction *Function = &BFI.second; + if (Function->isPLTFunction()) + continue; + uint64_t FunctionNum = Function->getFunctionNumber(); std::vector<uint64_t> &IncomingFlows = TotalIncomingFlows[FunctionNum]; std::vector<uint64_t> &OutgoingFlows = TotalOutgoingFlows[FunctionNum]; @@ -528,6 +534,9 @@ void computeFlowMappings(const BinaryContext &BC, FlowInfo &TotalFlowMap) { TotalFlowMap.CallGraphIncomingFlows; for (const auto &BFI : BC.getBinaryFunctions()) { const BinaryFunction *Function = &BFI.second; + if (Function->isPLTFunction()) + continue; + uint64_t FunctionNum = Function->getFunctionNumber(); std::vector<uint64_t> &MaxCountMap = TotalMaxCountMaps[FunctionNum]; std::vector<uint64_t> &MinCountMap = TotalMinCountMaps[FunctionNum]; @@ -667,7 +676,7 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions, } // namespace bool PrintProfileQualityStats::shouldOptimize(const BinaryFunction &BF) const { - if (BF.empty() || !BF.hasValidProfile()) + if (BF.empty() || !BF.hasValidProfile() || BF.isPLTFunction()) return false; return BinaryFunctionPass::shouldOptimize(BF); diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index cc0f11ab63849..a76d5af11bd53 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1835,6 +1835,19 @@ void RewriteInstance::createPLTBinaryFunction(uint64_t TargetAddress, return; } + if (BC->isAArch64()) { + const Relocation *Rel = BC->getDynamicRelocationAt(TargetAddress); + ErrorOr<BinarySection &> Section = BC->getSectionForAddress(EntryAddress); + if (!Rel && EntryAddress == Section->getAddress()) { + BF = BC->createBinaryFunction("__BOLT_PSEUDO_" + Section->getName().str(), + *Section, EntryAddress, 0, EntrySize, + Section->getAlignment()); + + BF->setPLTSymbol(BC->getOrCreateGlobalSymbol(TargetAddress, "FUNCat")); + return; + } + } + const Relocation *Rel = BC->getDynamicRelocationAt(TargetAddress); if (!Rel) return; @@ -1897,37 +1910,35 @@ void RewriteInstance::disassemblePLTSectionAArch64(BinarySection &Section) { while (InstrOffset < SectionSize) { InstructionListType Instructions; MCInst Instruction; - uint64_t EntryOffset = InstrOffset; uint64_t EntrySize = 0; - uint64_t InstrSize; + uint64_t TargetAddress = 0; + const uint64_t EntryAddress = SectionAddress + InstrOffset; // Loop through entry instructions while (InstrOffset < SectionSize) { + uint64_t InstrSize; disassemblePLTInstruction(Section, InstrOffset, Instruction, InstrSize); + if (TargetAddress && !BC->MIB->isNoop(Instruction)) + break; EntrySize += InstrSize; - if (!BC->MIB->isIndirectBranch(Instruction)) { - Instructions.emplace_back(Instruction); - InstrOffset += InstrSize; - continue; + if (BC->MIB->isIndirectBranch(Instruction)) { + TargetAddress = BC->MIB->analyzePLTEntry(Instruction, + Instructions.begin(), + Instructions.end(), + EntryAddress); } - - const uint64_t EntryAddress = SectionAddress + EntryOffset; - const uint64_t TargetAddress = BC->MIB->analyzePLTEntry( - Instruction, Instructions.begin(), Instructions.end(), EntryAddress); - - createPLTBinaryFunction(TargetAddress, EntryAddress, EntrySize); - break; + Instructions.emplace_back(Instruction); + InstrOffset += InstrSize; } - // Branch instruction - InstrOffset += InstrSize; - - // Skip nops if any - while (InstrOffset < SectionSize) { - disassemblePLTInstruction(Section, InstrOffset, Instruction, InstrSize); - if (!BC->MIB->isNoop(Instruction)) - break; + createPLTBinaryFunction(TargetAddress, EntryAddress, EntrySize); - InstrOffset += InstrSize; + if (BC->usesBTI()) { + BinaryFunction *BF = BC->getBinaryFunctionAtAddress(EntryAddress); + if (BF) { + BF->disassemblePLT(Instructions); + // setPLTSymbol sets a fucntion pseudo, need to reset this attriabute + BF->setPseudo(false); + } } } } @@ -2044,8 +2055,8 @@ void RewriteInstance::disassemblePLT() { PltBF = BC->createBinaryFunction( "__BOLT_PSEUDO_" + Section.getName().str(), Section, Section.getAddress(), 0, PLTSI->EntrySize, Section.getAlignment()); + PltBF->setSimple(false); } - PltBF->setPseudo(true); } } @@ -3634,8 +3645,15 @@ void RewriteInstance::processProfileData() { void RewriteInstance::disassembleFunctions() { NamedRegionTimer T("disassembleFunctions", "disassemble functions", TimerGroupName, TimerGroupDesc, opts::TimeRewrite); - for (auto &BFI : BC->getBinaryFunctions()) { - BinaryFunction &Function = BFI.second; + BinaryFunctionListType BFs; + BFs.reserve(BC->getBinaryFunctions().size()); + llvm::transform(llvm::make_second_range(BC->getBinaryFunctions()), + std::back_inserter(BFs), + [](BinaryFunction &BF) { return &BF; }); + llvm::erase_if(BFs, + [](BinaryFunction *BF) { return BF->isPLTFunction(); }); + for (BinaryFunction *BF : BFs) { + BinaryFunction &Function = *BF; ErrorOr<ArrayRef<uint8_t>> FunctionData = Function.getData(); if (!FunctionData) { @@ -3692,11 +3710,21 @@ void RewriteInstance::disassembleFunctions() { Function.print(BC->outs(), "after disassembly"); } + if (opts::PrintAll || opts::PrintDisasm) { + std::for_each(BC->getBinaryFunctions().begin(), + BC->getBinaryFunctions().end(), + [&](auto& BFI) { + BinaryFunction &Function = BFI.second; + if (Function.isPLTFunction()) + Function.print(BC->outs(), "after disassembly"); + }); + } + BC->processInterproceduralReferences(); BC->populateJumpTables(); - for (auto &BFI : BC->getBinaryFunctions()) { - BinaryFunction &Function = BFI.second; + for (BinaryFunction *BF : BFs) { + BinaryFunction &Function = *BF; if (!shouldDisassemble(Function)) continue; @@ -3709,8 +3737,8 @@ void RewriteInstance::disassembleFunctions() { BC->clearJumpTableTempData(); BC->adjustCodePadding(); - for (auto &BFI : BC->getBinaryFunctions()) { - BinaryFunction &Function = BFI.second; + for (BinaryFunction *BF : BFs) { + BinaryFunction &Function = *BF; if (!shouldDisassemble(Function)) continue; diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index a56cf0fc6d9cb..2ca51940dfd6a 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -1699,7 +1699,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { /// If the PLT entry does not contain extra nops, this function will create an /// error. This can happen in binaries linked using BFD. void patchPLTEntryForBTI(BinaryFunction &PLTFunction, MCInst &Call) override { - BinaryContext &BC = PLTFunction.getBinaryContext(); + // BinaryContext &BC = PLTFunction.getBinaryContext(); assert(PLTFunction.isPLTFunction() && "patchPLTEntryForBTI called on a non-PLT function"); // Checking if the PLT entry already starts with the BTI needed for Call. @@ -1718,16 +1718,37 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { << " to have a BTI landing pad. Relink the workload using LLD.\n"; exit(1); } + // If the PLT does not have a BTI, and it has nops, create a new instruction // sequence to patch the entry with. - InstructionListType NewPLTSeq; MCInst BTIInst; createBTI(BTIInst, BTIKind::C); - NewPLTSeq.push_back(BTIInst); - for (auto II = FirstBBI->begin(); II != FirstBBI->end(); ++II) { - NewPLTSeq.push_back(*II); - } - BC.createInstructionPatch(PLTFunction.getAddress(), NewPLTSeq); + + // remove 1 nop + LastBBI->eraseInstruction(LastII); + // insert bti c + FirstBBI->insertInstruction(FirstBBI->begin(), BTIInst); + PLTFunction.dump(); + } + + bool handlePLTEntry(InstructionIterator Begin, + InstructionIterator End, + const MCSymbol *PLTSymbol, + MCContext *Ctx) override { + int64_t Val; + int Count = 0; + for (auto I = Begin; I != End; ++I) { + if (isADRP(*I)) + Count += replaceImmWithSymbolRef(*I, PLTSymbol, 0, Ctx, Val, + ELF::R_AARCH64_ADR_PREL_PG_HI21); + else if (isADD(*I)) + Count += replaceImmWithSymbolRef(*I, PLTSymbol, 0, Ctx, Val, + ELF::R_AARCH64_ADD_ABS_LO12_NC); + else if (mayLoad(*I)) + Count += replaceImmWithSymbolRef(*I, PLTSymbol, 0, Ctx, Val, + ELF::R_AARCH64_LDST64_ABS_LO12_NC); + } + return Count == 3; } unsigned getInvertedBranchOpcode(unsigned Opcode) const { `````````` </details> https://github.com/llvm/llvm-project/pull/173571 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
