From: Lang Hames Date: Mon, 13 May 2019 04:51:31 +0000 (+0000) Subject: [JITLink] Track section alignment and make sure it is respected during layout. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3a049ed50c562334acf9b41691547b5ac6b18a9f;p=llvm [JITLink] Track section alignment and make sure it is respected during layout. Previously we had only honored alignments on individual atoms, but tools/runtimes may assume that the section alignment is respected too. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360555 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ExecutionEngine/JITLink/JITLink.h b/include/llvm/ExecutionEngine/JITLink/JITLink.h index dd4fe369c76..01e26a90217 100644 --- a/include/llvm/ExecutionEngine/JITLink/JITLink.h +++ b/include/llvm/ExecutionEngine/JITLink/JITLink.h @@ -22,6 +22,7 @@ #include "llvm/Support/Endian.h" #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/MathExtras.h" #include "llvm/Support/Memory.h" #include "llvm/Support/MemoryBuffer.h" @@ -329,9 +330,12 @@ class Section { friend class AtomGraph; private: - Section(StringRef Name, sys::Memory::ProtectionFlags Prot, unsigned Ordinal, - bool IsZeroFill) - : Name(Name), Prot(Prot), Ordinal(Ordinal), IsZeroFill(IsZeroFill) {} + Section(StringRef Name, uint32_t Alignment, sys::Memory::ProtectionFlags Prot, + unsigned Ordinal, bool IsZeroFill) + : Name(Name), Alignment(Alignment), Prot(Prot), Ordinal(Ordinal), + IsZeroFill(IsZeroFill) { + assert(isPowerOf2_32(Alignment) && "Alignments must be a power of 2"); + } using DefinedAtomSet = DenseSet; @@ -341,6 +345,7 @@ public: ~Section(); StringRef getName() const { return Name; } + uint32_t getAlignment() const { return Alignment; } sys::Memory::ProtectionFlags getProtectionFlags() const { return Prot; } unsigned getSectionOrdinal() const { return Ordinal; } size_t getNextAtomOrdinal() { return ++NextAtomOrdinal; } @@ -388,6 +393,7 @@ private: } StringRef Name; + uint32_t Alignment = 0; sys::Memory::ProtectionFlags Prot; unsigned Ordinal = 0; unsigned NextAtomOrdinal = 0; @@ -403,12 +409,16 @@ class DefinedAtom : public Atom { private: DefinedAtom(Section &Parent, JITTargetAddress Address, uint32_t Alignment) : Atom("", Address), Parent(Parent), Ordinal(Parent.getNextAtomOrdinal()), - Alignment(Alignment) {} + Alignment(Alignment) { + assert(isPowerOf2_32(Alignment) && "Alignments must be a power of two"); + } DefinedAtom(Section &Parent, StringRef Name, JITTargetAddress Address, uint32_t Alignment) : Atom(Name, Address), Parent(Parent), - Ordinal(Parent.getNextAtomOrdinal()), Alignment(Alignment) {} + Ordinal(Parent.getNextAtomOrdinal()), Alignment(Alignment) { + assert(isPowerOf2_32(Alignment) && "Alignments must be a power of two"); + } public: using edge_iterator = EdgeVector::iterator; @@ -510,7 +520,7 @@ inline uint64_t SectionRange::getSize() const { return getEnd() - getStart(); } inline SectionRange Section::getRange() const { if (atoms_empty()) return SectionRange(); - DefinedAtom *First = *DefinedAtoms.begin(), *Last = *DefinedAtoms.end(); + DefinedAtom *First = *DefinedAtoms.begin(), *Last = *DefinedAtoms.begin(); for (auto *DA : atoms()) { if (DA->getAddress() < First->getAddress()) First = DA; @@ -602,10 +612,10 @@ public: support::endianness getEndianness() const { return Endianness; } /// Create a section with the given name, protection flags, and alignment. - Section &createSection(StringRef Name, sys::Memory::ProtectionFlags Prot, - bool IsZeroFill) { + Section &createSection(StringRef Name, uint32_t Alignment, + sys::Memory::ProtectionFlags Prot, bool IsZeroFill) { std::unique_ptr
Sec( - new Section(Name, Prot, Sections.size(), IsZeroFill)); + new Section(Name, Alignment, Prot, Sections.size(), IsZeroFill)); Sections.push_back(std::move(Sec)); return *Sections.back(); } diff --git a/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp index b3477f1ee4c..96e074da122 100644 --- a/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp +++ b/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp @@ -241,6 +241,10 @@ Error JITLinkerBase::allocateSegments(const SegmentLayoutMap &Layout) { for (auto &SI : SegLayout.ContentSections) { assert(!SI.S->atoms_empty() && "Sections in layout must not be empty"); assert(!SI.Atoms.empty() && "Section layouts must not be empty"); + + // Bump to section alignment before processing atoms. + SegContentSize = alignTo(SegContentSize, SI.S->getAlignment()); + for (auto *DA : SI.Atoms) { SegContentSize = alignTo(SegContentSize, DA->getAlignment()); SegContentSize += DA->getSize(); @@ -249,15 +253,22 @@ Error JITLinkerBase::allocateSegments(const SegmentLayoutMap &Layout) { // Get segment content alignment. unsigned SegContentAlign = 1; - if (!SegLayout.ContentSections.empty()) + if (!SegLayout.ContentSections.empty()) { + auto &FirstContentSection = SegLayout.ContentSections.front(); SegContentAlign = - SegLayout.ContentSections.front().Atoms.front()->getAlignment(); + std::max(FirstContentSection.S->getAlignment(), + FirstContentSection.Atoms.front()->getAlignment()); + } // Calculate segment zero-fill size. uint64_t SegZeroFillSize = 0; for (auto &SI : SegLayout.ZeroFillSections) { assert(!SI.S->atoms_empty() && "Sections in layout must not be empty"); assert(!SI.Atoms.empty() && "Section layouts must not be empty"); + + // Bump to section alignment before processing atoms. + SegZeroFillSize = alignTo(SegZeroFillSize, SI.S->getAlignment()); + for (auto *DA : SI.Atoms) { SegZeroFillSize = alignTo(SegZeroFillSize, DA->getAlignment()); SegZeroFillSize += DA->getSize(); @@ -266,9 +277,13 @@ Error JITLinkerBase::allocateSegments(const SegmentLayoutMap &Layout) { // Calculate segment zero-fill alignment. uint32_t SegZeroFillAlign = 1; - if (!SegLayout.ZeroFillSections.empty()) + + if (!SegLayout.ZeroFillSections.empty()) { + auto &FirstZeroFillSection = SegLayout.ZeroFillSections.front(); SegZeroFillAlign = - SegLayout.ZeroFillSections.front().Atoms.front()->getAlignment(); + std::max(FirstZeroFillSection.S->getAlignment(), + FirstZeroFillSection.Atoms.front()->getAlignment()); + } if (SegContentSize == 0) SegContentAlign = SegZeroFillAlign; @@ -314,12 +329,14 @@ Error JITLinkerBase::allocateSegments(const SegmentLayoutMap &Layout) { Alloc->getTargetMemory(static_cast(Prot)); for (auto *SIList : {&SL.ContentSections, &SL.ZeroFillSections}) - for (auto &SI : *SIList) + for (auto &SI : *SIList) { + AtomTargetAddr = alignTo(AtomTargetAddr, SI.S->getAlignment()); for (auto *DA : SI.Atoms) { AtomTargetAddr = alignTo(AtomTargetAddr, DA->getAlignment()); DA->setAddress(AtomTargetAddr); AtomTargetAddr += DA->getSize(); } + } } return Error::success(); diff --git a/lib/ExecutionEngine/JITLink/JITLinkGeneric.h b/lib/ExecutionEngine/JITLink/JITLinkGeneric.h index 9d8238cf2e5..e6fd6e38f7a 100644 --- a/lib/ExecutionEngine/JITLink/JITLinkGeneric.h +++ b/lib/ExecutionEngine/JITLink/JITLinkGeneric.h @@ -150,7 +150,7 @@ private: auto SegMem = Alloc.getWorkingMemory( static_cast(Prot)); char *LastAtomEnd = SegMem.data(); - char *AtomDataPtr = nullptr; + char *AtomDataPtr = LastAtomEnd; LLVM_DEBUG({ dbgs() << " Processing segment " @@ -162,8 +162,16 @@ private: for (auto &SI : SegLayout.ContentSections) { LLVM_DEBUG(dbgs() << " " << SI.S->getName() << ":\n"); + + AtomDataPtr += alignmentAdjustment(AtomDataPtr, SI.S->getAlignment()); + + LLVM_DEBUG({ + dbgs() << " Bumped atom pointer to " << (const void *)AtomDataPtr + << " to meet section alignment " + << " of " << SI.S->getAlignment() << "\n"; + }); + for (auto *DA : SI.Atoms) { - AtomDataPtr = LastAtomEnd; // Align. AtomDataPtr += alignmentAdjustment(AtomDataPtr, DA->getAlignment()); @@ -209,6 +217,7 @@ private: // Update atom end pointer. LastAtomEnd = AtomDataPtr + DA->getContent().size(); + AtomDataPtr = LastAtomEnd; } } diff --git a/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp b/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp index 0c19f8fee0d..b24b2256826 100644 --- a/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp +++ b/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp @@ -85,7 +85,7 @@ MachOAtomGraphBuilder::MachOSection &MachOAtomGraphBuilder::getCommonSection() { if (!CommonSymbolsSection) { auto Prot = static_cast( sys::Memory::MF_READ | sys::Memory::MF_WRITE); - auto &GenericSection = G->createSection("", Prot, true); + auto &GenericSection = G->createSection("", 1, Prot, true); CommonSymbolsSection = MachOSection(GenericSection); } return *CommonSymbolsSection; @@ -102,6 +102,12 @@ Error MachOAtomGraphBuilder::parseSections() { unsigned SectionIndex = SecRef.getIndex() + 1; + uint32_t Align = SecRef.getAlignment(); + if (!isPowerOf2_32(Align)) + return make_error("Section " + Name + + " has non-power-of-2 " + "alignment"); + // FIXME: Get real section permissions // How, exactly, on MachO? sys::Memory::ProtectionFlags Prot; @@ -112,7 +118,7 @@ Error MachOAtomGraphBuilder::parseSections() { Prot = static_cast(sys::Memory::MF_READ | sys::Memory::MF_WRITE); - auto &GenericSection = G->createSection(Name, Prot, SecRef.isBSS()); + auto &GenericSection = G->createSection(Name, Align, Prot, SecRef.isBSS()); LLVM_DEBUG({ dbgs() << "Adding section " << Name << ": " diff --git a/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp b/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp index 7b4ddc3019a..4010678c6d3 100644 --- a/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp +++ b/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp @@ -399,7 +399,7 @@ public: private: Section &getGOTSection() { if (!GOTSection) - GOTSection = &G.createSection("$__GOT", sys::Memory::MF_READ, false); + GOTSection = &G.createSection("$__GOT", 8, sys::Memory::MF_READ, false); return *GOTSection; } @@ -407,7 +407,7 @@ private: if (!StubsSection) { auto StubsProt = static_cast( sys::Memory::MF_READ | sys::Memory::MF_EXEC); - StubsSection = &G.createSection("$__STUBS", StubsProt, false); + StubsSection = &G.createSection("$__STUBS", 8, StubsProt, false); } return *StubsSection; } diff --git a/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s b/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s index b306a490ebe..568faebd965 100644 --- a/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s +++ b/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s @@ -277,4 +277,24 @@ subtractor_with_alt_entry_subtrahend_quad_B: .globl zero_fill_test .zerofill __DATA,__zero_fill_test,zero_fill_test,8,3 +# Check that section alignments are respected. +# We test this by introducing two segments with alignment 8, each containing one +# byte of data. We require both symbols to have an aligned address. +# +# jitlink-check: section_alignment_check1[2:0] = 0 +# jitlink-check: section_alignment_check2[2:0] = 0 + .section __DATA,__sec_align_chk1 + .p2align 3 + + .globl section_alignment_check1 +section_alignment_check1: + .byte 0 + + .section __DATA,__sec_align_chk2 + .p2align 3 + + .globl section_alignment_check2 +section_alignment_check2: + .byte 0 + .subsections_via_symbols