From d27cbf03cf9c43b4b98f031d63f23cfcfe3d587a Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Tue, 2 Jun 2020 21:31:26 -0700 Subject: [PATCH] [JITLink] Skip debug sections in MachO objects. Debug sections will not be linked into the final executable and may contain ambiguous relocations*. Skipping them avoids both some unnecessary processing cost and the hassle of dealing with the problematic relocations. * E.g. __debug_ranges contains non-extern relocations to the end of functions hat begin with named symbols. Under the usual rules for interpreting non-extern relocations these will be incorrectly associated with the following block, or no block at all (if there is a gap between one block and the next). --- .../JITLink/MachOLinkGraphBuilder.cpp | 66 ++++++++++++++++++---- .../JITLink/MachOLinkGraphBuilder.h | 12 +++- llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp | 22 ++++++++ llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp | 16 ++++++ .../JITLink/X86/MachO_skip_debug_sections.s | 21 +++++++ 5 files changed, 122 insertions(+), 15 deletions(-) create mode 100644 llvm/test/ExecutionEngine/JITLink/X86/MachO_skip_debug_sections.s diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp index bb96116..fa3f403 100644 --- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp @@ -79,6 +79,11 @@ bool MachOLinkGraphBuilder::isAltEntry(const NormalizedSymbol &NSym) { return NSym.Desc & MachO::N_ALT_ENTRY; } +bool MachOLinkGraphBuilder::isDebugSection(const NormalizedSection &NSec) { + return (NSec.Flags & MachO::S_ATTR_DEBUG && + strcmp(NSec.SegName, "__DWARF") == 0); +} + unsigned MachOLinkGraphBuilder::getPointerSize(const object::MachOObjectFile &Obj) { return Obj.is64Bit() ? 8 : 4; @@ -118,6 +123,11 @@ Error MachOLinkGraphBuilder::createNormalizedSections() { const MachO::section_64 &Sec64 = Obj.getSection64(SecRef.getRawDataRefImpl()); + memcpy(&NSec.SectName, &Sec64.sectname, 16); + NSec.SectName[16] = '\0'; + memcpy(&NSec.SegName, Sec64.segname, 16); + NSec.SegName[16] = '\0'; + NSec.Address = Sec64.addr; NSec.Size = Sec64.size; NSec.Alignment = 1ULL << Sec64.align; @@ -125,6 +135,12 @@ Error MachOLinkGraphBuilder::createNormalizedSections() { DataOffset = Sec64.offset; } else { const MachO::section &Sec32 = Obj.getSection(SecRef.getRawDataRefImpl()); + + memcpy(&NSec.SectName, &Sec32.sectname, 16); + NSec.SectName[16] = '\0'; + memcpy(&NSec.SegName, Sec32.segname, 16); + NSec.SegName[16] = '\0'; + NSec.Address = Sec32.addr; NSec.Size = Sec32.size; NSec.Alignment = 1ULL << Sec32.align; @@ -164,7 +180,14 @@ Error MachOLinkGraphBuilder::createNormalizedSections() { Prot = static_cast(sys::Memory::MF_READ | sys::Memory::MF_WRITE); - NSec.GraphSection = &G->createSection(*Name, Prot); + if (!isDebugSection(NSec)) + NSec.GraphSection = &G->createSection(*Name, Prot); + else + LLVM_DEBUG({ + dbgs() << " " << *Name + << " is a debug section: No graph section will be created.\n"; + }); + IndexToSection.insert(std::make_pair(SecIndex, std::move(NSec))); } @@ -191,12 +214,12 @@ Error MachOLinkGraphBuilder::createNormalizedSections() { auto &Next = *Sections[I + 1]; if (Next.Address < Cur.Address + Cur.Size) return make_error( - "Address range for section " + Cur.GraphSection->getName() + - formatv(" [ {0:x16} -- {1:x16} ] ", Cur.Address, - Cur.Address + Cur.Size) + - "overlaps " + - formatv(" [ {0:x16} -- {1:x16} ] ", Next.Address, - Next.Address + Next.Size)); + "Address range for section " + + formatv("\"{0}/{1}\" [ {2:x16} -- {3:x16} ] ", Cur.SegName, + Cur.SectName, Cur.Address, Cur.Address + Cur.Size) + + "overlaps section \"" + Next.SegName + "/" + Next.SectName + "\"" + + formatv("\"{0}/{1}\" [ {2:x16} -- {3:x16} ] ", Next.SegName, + Next.SectName, Next.Address, Next.Address + Next.Size)); } return Error::success(); @@ -262,16 +285,23 @@ Error MachOLinkGraphBuilder::createNormalizedSymbols() { }); // If this symbol has a section, sanity check that the addresses line up. - NormalizedSection *NSec = nullptr; if (Sect != 0) { - if (auto NSecOrErr = findSectionByIndex(Sect - 1)) - NSec = &*NSecOrErr; - else - return NSecOrErr.takeError(); + auto NSec = findSectionByIndex(Sect - 1); + if (!NSec) + return NSec.takeError(); if (Value < NSec->Address || Value > NSec->Address + NSec->Size) return make_error("Symbol address does not fall within " "section"); + + if (!NSec->GraphSection) { + LLVM_DEBUG({ + dbgs() << " Skipping: Symbol is in section " << NSec->SegName << "/" + << NSec->SectName + << " which has no associated graph section.\n"; + }); + continue; + } } IndexToSymbol[SymbolIndex] = @@ -364,6 +394,14 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() { auto SecIndex = KV.first; auto &NSec = KV.second; + if (!NSec.GraphSection) { + LLVM_DEBUG({ + dbgs() << " " << NSec.SegName << "/" << NSec.SectName + << " has no graph section. Skipping.\n"; + }); + continue; + } + // Skip sections with custom parsers. if (CustomSectionParserFunctions.count(NSec.GraphSection->getName())) { LLVM_DEBUG({ @@ -526,6 +564,10 @@ Error MachOLinkGraphBuilder::graphifySectionsWithCustomParsers() { for (auto &KV : IndexToSection) { auto &NSec = KV.second; + // Skip non-graph sections. + if (!NSec.GraphSection) + continue; + auto HI = CustomSectionParserFunctions.find(NSec.GraphSection->getName()); if (HI != CustomSectionParserFunctions.end()) { auto &Parse = HI->second; diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.h index ffe0100..dd3bcf2 100644 --- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.h @@ -60,6 +60,8 @@ protected: Symbol *GraphSymbol = nullptr; }; + // Normalized section representation. Section and segment names are guaranteed + // to be null-terminated, hence the extra bytes on SegName and SectName. class NormalizedSection { friend class MachOLinkGraphBuilder; @@ -67,12 +69,14 @@ protected: NormalizedSection() = default; public: - Section *GraphSection = nullptr; + char SectName[17]; + char SegName[17]; uint64_t Address = 0; uint64_t Size = 0; uint64_t Alignment = 0; uint32_t Flags = 0; const char *Data = nullptr; + Section *GraphSection = nullptr; }; using SectionParserFunction = std::function; @@ -112,7 +116,7 @@ protected: auto I = IndexToSection.find(Index); if (I == IndexToSection.end()) return make_error("No section recorded for index " + - formatv("{0:u}", Index)); + formatv("{0:d}", Index)); return I->second; } @@ -125,7 +129,7 @@ protected: auto *Sym = IndexToSymbol[Index]; if (!Sym) return make_error("No symbol at index " + - formatv("{0:u}", Index)); + formatv("{0:d}", Index)); return *Sym; } @@ -153,6 +157,8 @@ protected: static Scope getScope(StringRef Name, uint8_t Type); static bool isAltEntry(const NormalizedSymbol &NSym); + static bool isDebugSection(const NormalizedSection &NSec); + MachO::relocation_info getRelocationInfo(const object::relocation_iterator RelItr) { MachO::any_relocation_info ARI = diff --git a/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp b/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp index 55c7b36..463845a 100644 --- a/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp @@ -185,6 +185,28 @@ private: JITTargetAddress SectionAddress = S.getAddress(); + // Skip relocations virtual sections. + if (S.isVirtual()) { + if (S.relocation_begin() != S.relocation_end()) + return make_error("Virtual section contains " + "relocations"); + continue; + } + + // Skip relocations for debug symbols. + { + auto &NSec = + getSectionByIndex(Obj.getSectionIndex(S.getRawDataRefImpl())); + if (!NSec.GraphSection) { + LLVM_DEBUG({ + dbgs() << "Skipping relocations for MachO section " << NSec.SegName + << "/" << NSec.SectName + << " which has no associated graph section\n"; + }); + continue; + } + } + for (auto RelItr = S.relocation_begin(), RelEnd = S.relocation_end(); RelItr != RelEnd; ++RelItr) { diff --git a/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp b/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp index 9abeddf..a91bc3b 100644 --- a/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp @@ -187,6 +187,7 @@ private: JITTargetAddress SectionAddress = S.getAddress(); + // Skip relocations virtual sections. if (S.isVirtual()) { if (S.relocation_begin() != S.relocation_end()) return make_error("Virtual section contains " @@ -194,6 +195,21 @@ private: continue; } + // Skip relocations for debug symbols. + { + auto &NSec = + getSectionByIndex(Obj.getSectionIndex(S.getRawDataRefImpl())); + if (!NSec.GraphSection) { + LLVM_DEBUG({ + dbgs() << "Skipping relocations for MachO section " << NSec.SegName + << "/" << NSec.SectName + << " which has no associated graph section\n"; + }); + continue; + } + } + + // Add relocations for section. for (auto RelItr = S.relocation_begin(), RelEnd = S.relocation_end(); RelItr != RelEnd; ++RelItr) { diff --git a/llvm/test/ExecutionEngine/JITLink/X86/MachO_skip_debug_sections.s b/llvm/test/ExecutionEngine/JITLink/X86/MachO_skip_debug_sections.s new file mode 100644 index 0000000..4d43ade --- /dev/null +++ b/llvm/test/ExecutionEngine/JITLink/X86/MachO_skip_debug_sections.s @@ -0,0 +1,21 @@ +# REQUIRES: asserts +# RUN: llvm-mc -triple=x86_64-apple-macosx10.9 -filetype=obj -o %t %s +# RUN: llvm-jitlink -debug-only=jitlink -noexec %t 2>&1 | FileCheck %s +# +# Check that debug sections are not emitted, and consequently that we don't +# error out due to buggy past-the-end anonymous relocations in __debug_ranges. +# +# CHECK: __debug_ranges is a debug section: No graph section will be created. + .section __TEXT,__text,regular,pure_instructions + .macosx_version_min 10, 15 + .globl _main + .p2align 4, 0x90 +_main: + retq +Lpast_the_end: + + .section __DWARF,__debug_ranges + .p2align 4 + .quad Lpast_the_end + +.subsections_via_symbols -- 2.7.4