diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -599,6 +599,8 @@ // There are 3 cases where we do not need to create a new subsection: // 1. If the input file does not use subsections-via-symbols. // 2. Multiple symbols at the same address only induce one subsection. + // (The symbolOffset == 0 check covers both this case as well as + // the first loop iteration.) // 3. Alternative entry points do not induce new subsections. if (!subsectionsViaSymbols || symbolOffset == 0 || sym.n_desc & N_ALT_ENTRY) { @@ -609,6 +611,8 @@ auto *nextIsec = make(*isec); nextIsec->data = isec->data.slice(symbolOffset); + nextIsec->numRefs = 0; + nextIsec->canOmitFromOutput = false; isec->data = isec->data.slice(0, symbolOffset); // By construction, the symbol will be at offset zero in the new diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h --- a/lld/MachO/InputSection.h +++ b/lld/MachO/InputSection.h @@ -21,8 +21,6 @@ class InputFile; class InputSection; class OutputSection; -class Symbol; -class Defined; class InputSection { public: @@ -45,6 +43,21 @@ uint32_t align = 1; uint32_t flags = 0; + // How many symbols refer to this InputSection. + uint32_t numRefs = 0; + + // True if this InputSection could not be written to the output file. + // With subsections_via_symbols, most symbol have its own InputSection, + // and for weak symbols (e.g. from inline functions), only the + // InputSection from one translation unit will make it to the output, + // while all copies in other translation units are coalesced into the + // first and not copied to the output. + bool canOmitFromOutput = false; + + bool shouldOmitFromOutput() const { + return canOmitFromOutput && numRefs == 0; + } + ArrayRef data; std::vector relocs; }; diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp --- a/lld/MachO/InputSection.cpp +++ b/lld/MachO/InputSection.cpp @@ -51,6 +51,8 @@ } void InputSection::writeTo(uint8_t *buf) { + assert(!shouldOmitFromOutput()); + if (getFileSize() == 0) return; @@ -66,8 +68,11 @@ uint64_t minuendVA; if (const Symbol *toSym = minuend.referent.dyn_cast()) minuendVA = toSym->getVA(); - else - minuendVA = minuend.referent.get()->getVA(); + else { + auto *referentIsec = minuend.referent.get(); + assert(!referentIsec->shouldOmitFromOutput()); + minuendVA = referentIsec->getVA(); + } referentVA = minuendVA - fromSym->getVA() + minuend.addend; } else if (auto *referentSym = r.referent.dyn_cast()) { if (target->hasAttr(r.type, RelocAttrBits::LOAD) && @@ -84,6 +89,7 @@ referentVA -= firstTLVDataSection->addr; } } else if (auto *referentIsec = r.referent.dyn_cast()) { + assert(!referentIsec->shouldOmitFromOutput()); referentVA = referentIsec->getVA(); } target->relocateOne(loc, r, referentVA + r.addend, getVA() + r.offset); diff --git a/lld/MachO/MapFile.cpp b/lld/MachO/MapFile.cpp --- a/lld/MachO/MapFile.cpp +++ b/lld/MachO/MapFile.cpp @@ -67,8 +67,11 @@ if (sym == nullptr) continue; if (auto *d = dyn_cast(sym)) - if (d->isec && d->getFile() == file) + if (d->isec && d->getFile() == file) { + assert(!d->isec->shouldOmitFromOutput() && + "file->symbols should store resolved symbols"); v.push_back(d); + } } return v; } @@ -144,7 +147,11 @@ os << "# Symbols:\n"; os << "# Address\t File Name\n"; for (InputSection *isec : inputSections) { - for (Symbol *sym : sectionSyms[isec]) { + auto symsIt = sectionSyms.find(isec); + assert(!isec->shouldOmitFromOutput() || (symsIt == sectionSyms.end())); + if (symsIt == sectionSyms.end()) + continue; + for (Symbol *sym : symsIt->second) { os << format("0x%08llX\t[%3u] %s\n", sym->getVA(), readerToFileOrdinal[sym->getFile()], symStr[sym].c_str()); } diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -51,13 +51,26 @@ bool overridesWeakDef = false; std::tie(s, wasInserted) = insert(name, file); + assert(!isWeakDef || (isa(file) && !isec) || + (isa(file) && file == isec->file)); + if (!wasInserted) { if (auto *defined = dyn_cast(s)) { if (isWeakDef) { // Both old and new symbol weak (e.g. inline function in two TUs): // If one of them isn't private extern, the merged symbol isn't. - if (defined->isWeakDef()) + if (defined->isWeakDef()) { defined->privateExtern &= isPrivateExtern; + + // FIXME: Handle this for bitcode files. + // FIXME: We currently only do this if both symbols are weak. + // We could do this if either is weak (but getting the + // case where !isWeakDef && defined->isWeakDef() right + // requires some care and testing). + if (isec) + isec->canOmitFromOutput = true; + } + return defined; } if (!defined->isWeakDef()) diff --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h --- a/lld/MachO/Symbols.h +++ b/lld/MachO/Symbols.h @@ -106,7 +106,10 @@ : Symbol(DefinedKind, name, file), isec(isec), value(value), size(size), overridesWeakDef(false), privateExtern(isPrivateExtern), includeInSymtab(true), thumb(isThumb), weakDef(isWeakDef), - external(isExternal) {} + external(isExternal) { + if (isec) + isec->numRefs++; + } bool isWeakDef() const override { return weakDef; } bool isExternalWeakDef() const { diff --git a/lld/MachO/UnwindInfoSection.cpp b/lld/MachO/UnwindInfoSection.cpp --- a/lld/MachO/UnwindInfoSection.cpp +++ b/lld/MachO/UnwindInfoSection.cpp @@ -143,6 +143,8 @@ void UnwindInfoSectionImpl::prepareRelocations(InputSection *isec) { assert(isec->segname == segment_names::ld && isec->name == section_names::compactUnwind); + assert(!isec->shouldOmitFromOutput() && + "__compact_unwind section should not be omitted"); for (Reloc &r : isec->relocs) { assert(target->hasAttr(r.type, RelocAttrBits::UNSIGNED)); @@ -175,6 +177,8 @@ } if (auto *referentIsec = r.referent.dyn_cast()) { + assert(!referentIsec->shouldOmitFromOutput()); + // Personality functions can be referenced via section relocations // if they live in the same object file. Create placeholder synthetic // symbols for them in the GOT. @@ -211,6 +215,8 @@ relocateCompactUnwind(MergedOutputSection *compactUnwindSection, std::vector> &cuVector) { for (const InputSection *isec : compactUnwindSection->inputs) { + assert(isec->parent == compactUnwindSection); + uint8_t *buf = reinterpret_cast(cuVector.data()) + isec->outSecFileOff; memcpy(buf, isec->data.data(), isec->data.size()); @@ -229,7 +235,10 @@ } } else if (auto *referentIsec = r.referent.dyn_cast()) { checkTextSegment(referentIsec); - referentVA = referentIsec->getVA() + r.addend; + if (referentIsec->shouldOmitFromOutput()) + referentVA = UINT64_MAX; // Tombstone value + else + referentVA = referentIsec->getVA() + r.addend; } writeAddress(buf + r.offset, referentVA, r.length); @@ -295,6 +304,18 @@ return a->functionAddress < b->functionAddress; }); + // Dead-stripped functions get a functionAddress of UINT64_MAX in + // relocateCompactUnwind(). Filter them out here. + CompactUnwindEntry tombstone; + tombstone.functionAddress = static_cast(UINT64_MAX); + cuPtrVector.erase( + std::lower_bound(cuPtrVector.begin(), cuPtrVector.end(), &tombstone, + [](const CompactUnwindEntry *a, + const CompactUnwindEntry *b) { + return a->functionAddress < b->functionAddress; + }), + cuPtrVector.end()); + // Fold adjacent entries with matching encoding+personality+lsda // We use three iterators on the same cuPtrVector to fold in-situ: // (1) `foldBegin` is the first of a potential sequence of matching entries diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -570,6 +570,9 @@ void Writer::scanRelocations() { TimeTraceScope timeScope("Scan relocations"); for (InputSection *isec : inputSections) { + if (isec->shouldOmitFromOutput()) + continue; + if (isec->segname == segment_names::ld) { in.unwindInfo->prepareRelocations(isec); continue; @@ -581,7 +584,7 @@ // Skip over the following UNSIGNED relocation -- it's just there as the // minuend, and doesn't have the usual UNSIGNED semantics. We don't want // to emit rebase opcodes for it. - it = std::next(it); + it++; continue; } if (auto *sym = r.referent.dyn_cast()) { @@ -592,6 +595,7 @@ prepareSymbolRelocation(sym, isec, r); } else { assert(r.referent.is()); + assert(!r.referent.get()->shouldOmitFromOutput()); if (!r.pcrel) in.rebase->addEntry(isec, r.offset); } @@ -894,6 +898,8 @@ // Then merge input sections into output sections. MapVector mergedOutputSections; for (InputSection *isec : inputSections) { + if (isec->shouldOmitFromOutput()) + continue; NamePair names = maybeRenameSection({isec->segname, isec->name}); MergedOutputSection *&osec = mergedOutputSections[names]; if (osec == nullptr) diff --git a/lld/test/MachO/weak-definition-gc.s b/lld/test/MachO/weak-definition-gc.s new file mode 100644 --- /dev/null +++ b/lld/test/MachO/weak-definition-gc.s @@ -0,0 +1,131 @@ +# REQUIRES: x86 +# RUN: rm -rf %t; split-file %s %t + +## This tests that if two input files define the same weak symbol, we only +## write it to the output once (...assuming both input files use +## .subsections_via_symbols). + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-sub.s -o %t/weak-sub.o +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-nosub.s -o %t/weak-nosub.o + +## Test that weak symbols are emitted just once with .subsections_via_symbols +# RUN: %lld -dylib -o %t/out.dylib %t/weak-sub.o %t/weak-sub.o +# RUN: llvm-otool -jtV %t/out.dylib | FileCheck --check-prefix=SUB %s +# RUN: %lld -dylib -o %t/out.dylib %t/weak-nosub.o %t/weak-sub.o +# RUN: llvm-otool -jtV %t/out.dylib | FileCheck --check-prefix=SUB %s +# RUN: %lld -dylib -o %t/out.dylib %t/weak-sub.o %t/weak-nosub.o +# RUN: llvm-otool -jtV %t/out.dylib | FileCheck --check-prefix=SUB %s +# SUB: _foo +# SUB-NEXT: retq +# SUB-NOT: retq +# SUB: _bar +# SUB-NEXT: retq +# SUB-NOT: retq + +## We can even strip weak symbols without subsections_via_symbols as long +## as none of the weak symbols in a section are needed. +# RUN: %lld -dylib -o %t/out.dylib %t/weak-nosub.o %t/weak-nosub.o +# RUN: llvm-otool -jtV %t/out.dylib | FileCheck --check-prefix=SUB %s + +## Test that omitted weak symbols don't add entries to the compact unwind table. +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-lsda.s -o %t/weak-sub-lsda.o +# RUN: %lld -dylib -lc++ -o %t/out.dylib %t/weak-sub-lsda.o %t/weak-sub-lsda.o +# RUN: llvm-objdump --macho --unwind-info --syms %t/out.dylib | FileCheck %s --check-prefix=UNWIND -D#%x,BASE=0 + +# UNWIND: SYMBOL TABLE: +# UNWIND-DAG: [[#%x,FOO:]] w F __TEXT,__text _foo +# UNWIND-NOT: __TEXT,__text _foo + +# UNWIND: Contents of __unwind_info section: +# UNWIND: LSDA descriptors: +# UNWIND: [0]: function offset=0x[[#%.8x,FOO-BASE]] +# UNWIND-NOT: [1]: +# UNWIND: Second level indices: +# UNWIND-DAG: [0]: function offset=0x[[#%.8x,FOO-BASE]] +# UNWIND-NOT: [1]: + +## Test interaction with .alt_entry +## FIXME: ld64 manages to strip both one copy of _foo and _bar each. +## We only manage this if we're lucky and the object files are in +## the right order. We're happy to not crash at link time for now. +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-alt.s -o %t/weak-sub-alt.o +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-alt2.s -o %t/weak-sub-alt2.o +# RUN: %lld -dylib -o %t/out.dylib %t/weak-sub-alt.o %t/weak-sub-alt2.o +# RUN: %lld -dylib -o %t/out.dylib %t/weak-sub-alt2.o %t/weak-sub-alt.o + +#--- weak-sub.s +.globl _foo, _bar +.weak_definition _foo, _bar +_foo: + retq +_bar: + retq +.subsections_via_symbols + +#--- weak-nosub.s +.globl _foo, _bar +.weak_definition _foo, _bar +_foo: + retq +_bar: + retq + +#--- weak-sub-lsda.s +.section __TEXT,__text,regular,pure_instructions + +.globl _foo +.weak_definition _foo +_foo: + .cfi_startproc + .cfi_personality 155, ___gxx_personality_v0 + .cfi_lsda 16, Lexception + pushq %rbp + .cfi_def_cfa_offset 128 + .cfi_offset %rbp, 48 + movq %rsp, %rbp + .cfi_def_cfa_register %rbp + popq %rbp + retq + .cfi_endproc + +.section __TEXT,__gcc_except_tab +Lexception: + .space 0x10 + +.subsections_via_symbols + +#--- weak-sub-alt.s +.globl _foo, _bar +.weak_definition _foo +_foo: + retq + +# Alternative entry point to _foo (strong) +.alt_entry _bar +_bar: + retq + +.globl _main, _ref +_main: + callq _ref + callq _bar + +.subsections_via_symbols + +#--- weak-sub-alt2.s +.globl _foo, _bar +.weak_definition _foo +_foo: + retq + +# Alternative entry point to _foo (weak) +.weak_definition _bar +.alt_entry _bar +_bar: + retq + +.globl _ref +_ref: + callq _bar + +.subsections_via_symbols