diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1081,7 +1081,9 @@ config->emitBitcodeBundle = args.hasArg(OPT_bitcode_bundle); config->emitDataInCodeInfo = args.hasFlag(OPT_data_in_code_info, OPT_no_data_in_code_info, true); - config->dedupLiterals = args.hasArg(OPT_deduplicate_literals); + config->icfLevel = getICFLevel(args); + config->dedupLiterals = args.hasArg(OPT_deduplicate_literals) || + config->icfLevel != ICFLevel::none; // FIXME: Add a commandline flag for this too. config->zeroModTime = getenv("ZERO_AR_DATE"); @@ -1124,8 +1126,6 @@ config->undefinedSymbolTreatment = getUndefinedSymbolTreatment(args); - config->icfLevel = getICFLevel(args); - if (config->outputType == MH_EXECUTE) config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"), /*file=*/nullptr, diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp --- a/lld/MachO/ICF.cpp +++ b/lld/MachO/ICF.cpp @@ -104,23 +104,22 @@ if (isa(sa)) { const auto *da = dyn_cast(sa); const auto *db = dyn_cast(sb); - if (da->value != db->value) - return false; - if (da->isAbsolute() != db->isAbsolute()) - return false; - if (da->isec) { + if (da->isec && db->isec) { if (da->isec->kind() != db->isec->kind()) return false; if (const auto *isecA = dyn_cast(da->isec)) { const auto *isecB = cast(db->isec); - if (isecA->icfEqClass[icfPass % 2] != - isecB->icfEqClass[icfPass % 2]) - return false; - } else { - // FIXME: implement ICF for other InputSection kinds - return false; + return da->value == db->value && isecA->icfEqClass[icfPass % 2] == + isecB->icfEqClass[icfPass % 2]; } + // Else we have two literal sections. References to them are + // constant-equal if their offsets in the output section are equal. + return da->isec->parent == db->isec->parent && + da->isec->getOffset(da->value) == + db->isec->getOffset(db->value); } + assert(da->isAbsolute() && db->isAbsolute()); + return da->value != db->value; } else if (isa(sa)) { // There is one DylibSymbol per gotIndex and we already checked for // symbol equality, thus we know that these must be different. @@ -138,8 +137,9 @@ if (isecB->icfEqClass[icfPass % 2] != isecB->icfEqClass[icfPass % 2]) return false; } else { - // FIXME: implement ICF for other InputSection kinds - return false; + assert(isa(sa) || + isa(sa)); + return sa->getOffset(ra.addend) == sb->getOffset(rb.addend); } } return true; @@ -207,11 +207,13 @@ if (auto *dylibSym = dyn_cast(sym)) hash += dylibSym->stubsHelperIndex; else if (auto *defined = dyn_cast(sym)) { - hash += defined->value; - if (defined->isec) - if (auto isec = cast(defined->isec)) - hash += isec->icfEqClass[icfPass % 2]; - // FIXME: implement ICF for other InputSection kinds + if (defined->isec) { + if (auto isec = dyn_cast(defined->isec)) + hash += isec->icfEqClass[icfPass % 2] + defined->value; + else + hash += reinterpret_cast(defined->isec) + + defined->isec->getOffset(defined->value); + } } else llvm_unreachable("foldIdenticalSections symbol kind"); } diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp --- a/lld/MachO/InputSection.cpp +++ b/lld/MachO/InputSection.cpp @@ -62,9 +62,7 @@ case S_8BYTE_LITERALS: case S_16BYTE_LITERALS: case S_LITERAL_POINTERS: - // FIXME(jezng): We should have any ConcatInputSections of these types when - // running ICF. - return false; + llvm_unreachable("found unexpected literal type in ConcatInputSection"); case S_ZEROFILL: case S_GB_ZEROFILL: case S_NON_LAZY_SYMBOL_POINTERS: diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td --- a/lld/MachO/Options.td +++ b/lld/MachO/Options.td @@ -55,7 +55,7 @@ HelpText<"Specify time trace output file">, Group; def deduplicate_literals: Flag<["--"], "deduplicate-literals">, - HelpText<"Enable literal deduplication">, + HelpText<"Enable literal deduplication. This is implied by --icf={safe,all}">, Group; def print_dylib_search: Flag<["--"], "print-dylib-search">, HelpText<"Print which paths lld searched when trying to find dylibs">, diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h --- a/lld/MachO/SyntheticSections.h +++ b/lld/MachO/SyntheticSections.h @@ -523,7 +523,7 @@ CStringSection(); void addInput(CStringInputSection *); uint64_t getSize() const override { return builder.getSize(); } - void finalize() override; + void finalizeContents(); bool isNeeded() const override { return !inputs.empty(); } void writeTo(uint8_t *buf) const override { builder.write(buf); } diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp --- a/lld/MachO/SyntheticSections.cpp +++ b/lld/MachO/SyntheticSections.cpp @@ -1187,7 +1187,7 @@ inputs.push_back(isec); } -void CStringSection::finalize() { +void CStringSection::finalizeContents() { // Add all string pieces to the string table builder to create section // contents. for (const CStringInputSection *isec : inputs) diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -52,6 +52,7 @@ void scanSymbols(); template void createOutputSections(); template void createLoadCommands(); + void foldIdenticalLiterals(); void foldIdenticalSections(); void finalizeAddresses(); void finalizeLinkEditSegment(); @@ -942,6 +943,12 @@ linkEditSegment = getOrCreateOutputSegment(segment_names::linkEdit); } +void Writer::foldIdenticalLiterals() { + if (in.cStringSection) + in.cStringSection->finalizeContents(); + // TODO: WordLiteralSection & CFStringSection should be finalized here too +} + void Writer::foldIdenticalSections() { if (config->icfLevel == ICFLevel::none) return; @@ -973,8 +980,8 @@ else concatIsec->icfEqClass[0] = ++icfUniqueID; } - // FIXME: hash literal sections here? } + // FIXME: hash literal sections here too? parallelForEach(hashable, [](ConcatInputSection *isec) { isec->hashForICF(); }); // Now that every input section is either hashed or marked as unique, @@ -1118,6 +1125,9 @@ in.stubHelper->setup(); scanSymbols(); createOutputSections(); + // ICF assumes that all literals have been folded already, so we must run + // foldIdenticalLiterals before foldIdenticalSections. + foldIdenticalLiterals(); foldIdenticalSections(); // After this point, we create no new segments; HOWEVER, we might // yet create branch-range extension thunks for architectures whose diff --git a/lld/test/MachO/icf-literals.s b/lld/test/MachO/icf-literals.s new file mode 100644 --- /dev/null +++ b/lld/test/MachO/icf-literals.s @@ -0,0 +1,72 @@ +# REQUIRES: x86 +# RUN: rm -rf %t; mkdir %t +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o +# RUN: %lld -dylib --icf=all -o %t/test %t/test.o +# RUN: llvm-objdump --macho --syms %t/test | FileCheck %s + +# CHECK: [[#%.16x,FOO:]] l O __TEXT,__cstring _foo1 +# CHECK-NEXT: [[#%.16x,FOO:]] l O __TEXT,__cstring _foo2 +# CHECK-NEXT: [[#%.16x,BAR:]] l O __TEXT,__cstring _bar1 +# CHECK-NEXT: [[#%.16x,BAR:]] l O __TEXT,__cstring _bar2 +# CHECK-NEXT: [[#%.16x,BAZ:]] l O __TEXT,__literals _baz1 +# CHECK-NEXT: [[#%.16x,BAZ:]] l O __TEXT,__literals _baz2 +# CHECK-NEXT: [[#%.16x,QUX:]] l O __TEXT,__literals _qux1 +# CHECK-NEXT: [[#%.16x,QUX:]] l O __TEXT,__literals _qux2 +# CHECK-NEXT: [[#%.16x,FOO_REF:]] l F __TEXT,__text _foo1_ref +# CHECK-NEXT: [[#%.16x,FOO_REF:]] l F __TEXT,__text _foo2_ref +# CHECK-NEXT: [[#%.16x,BAR_REF:]] l F __TEXT,__text _bar1_ref +# CHECK-NEXT: [[#%.16x,BAR_REF:]] l F __TEXT,__text _bar2_ref +# CHECK-NEXT: [[#%.16x,BAZ_REF:]] l F __TEXT,__text _baz1_ref +# CHECK-NEXT: [[#%.16x,BAZ_REF:]] l F __TEXT,__text _baz2_ref +# CHECK-NEXT: [[#%.16x,QUX_REF:]] l F __TEXT,__text _qux1_ref +# CHECK-NEXT: [[#%.16x,QUX_REF:]] l F __TEXT,__text _qux2_ref + +## _foo1 vs _bar1: same section, different offsets +## _foo1 vs _baz1: same offset, different sections + +.cstring +_foo1: + .asciz "foo" +_foo2: + .asciz "foo" +_bar1: + .asciz "foo" +_bar2: + .asciz "foo" + +.literal8 +_baz1: + .quad 0xdead +_baz2: + .quad 0xdead +_qux1: + .quad 0xbeef +_qux2: + .quad 0xbeef + +.text +_foo1_ref: + mov _foo1@GOTPCREL(%rip), %rax + +_foo2_ref: + mov _foo2@GOTPCREL(%rip), %rax + +_bar1_ref: + mov _bar1@GOTPCREL(%rip), %rax + +_bar2_ref: + mov _bar2@GOTPCREL(%rip), %rax + +_baz1_ref: + mov _baz1@GOTPCREL(%rip), %rax + +_baz2_ref: + mov _baz2@GOTPCREL(%rip), %rax + +_qux1_ref: + mov _qux1@GOTPCREL(%rip), %rax + +_qux2_ref: + mov _qux2@GOTPCREL(%rip), %rax + +.subsections_via_symbols