Index: ELF/Driver.cpp =================================================================== --- ELF/Driver.cpp +++ ELF/Driver.cpp @@ -1235,14 +1235,15 @@ } } -static bool keepUnique(Symbol *S) { - if (auto *D = dyn_cast_or_null(S)) { - if (D->Section) { - D->Section->KeepUnique = true; - return true; - } - } - return false; +// The section referred to by S is considered address-significant. Set the +// KeepUnique flag on the section if appropriate. +static void markAddrsig(Symbol *S) { + if (auto *D = dyn_cast_or_null(S)) + if (D->Section) + // We don't need to keep text sections unique under --icf=all even if they + // are address-significant. + if (Config->ICF == ICFLevel::Safe || !(D->Section->Flags & SHF_EXECINSTR)) + D->Section->KeepUnique = true; } // Record sections that define symbols mentioned in --keep-unique @@ -1252,41 +1253,48 @@ static void findKeepUniqueSections(opt::InputArgList &Args) { for (auto *Arg : Args.filtered(OPT_keep_unique)) { StringRef Name = Arg->getValue(); - if (!keepUnique(Symtab->find(Name))) + auto *D = dyn_cast_or_null(Symtab->find(Name)); + if (!D || !D->Section) { warn("could not find symbol " + Name + " to keep unique"); + continue; + } + D->Section->KeepUnique = true; } - if (Config->ICF == ICFLevel::Safe) { - // Symbols in the dynsym could be address-significant in other executables - // or DSOs, so we conservatively mark them as address-significant. - for (Symbol *S : Symtab->getSymbols()) - if (S->includeInDynsym()) - keepUnique(S); - - // Visit the address-significance table in each object file and mark each - // referenced symbol as address-significant. - for (InputFile *F : ObjectFiles) { - auto *Obj = cast>(F); - ArrayRef Syms = Obj->getSymbols(); - if (Obj->AddrsigSec) { - ArrayRef Contents = - check(Obj->getObj().getSectionContents(Obj->AddrsigSec)); - const uint8_t *Cur = Contents.begin(); - while (Cur != Contents.end()) { - unsigned Size; - const char *Err; - uint64_t SymIndex = decodeULEB128(Cur, &Size, Contents.end(), &Err); - if (Err) - fatal(toString(F) + ": could not decode addrsig section: " + Err); - keepUnique(Syms[SymIndex]); - Cur += Size; - } - } else { - // If an object file does not have an address-significance table, - // conservatively mark all of its symbols as address-significant. - for (Symbol *S : Syms) - keepUnique(S); + // --icf=all --ignore-data-address-equality means that we can ignore + // the dynsym and address-significance tables entirely. + if (Config->ICF == ICFLevel::All && Config->IgnoreDataAddressEquality) + return; + + // Symbols in the dynsym could be address-significant in other executables + // or DSOs, so we conservatively mark them as address-significant. + for (Symbol *S : Symtab->getSymbols()) + if (S->includeInDynsym()) + markAddrsig(S); + + // Visit the address-significance table in each object file and mark each + // referenced symbol as address-significant. + for (InputFile *F : ObjectFiles) { + auto *Obj = cast>(F); + ArrayRef Syms = Obj->getSymbols(); + if (Obj->AddrsigSec) { + ArrayRef Contents = + check(Obj->getObj().getSectionContents(Obj->AddrsigSec)); + const uint8_t *Cur = Contents.begin(); + while (Cur != Contents.end()) { + unsigned Size; + const char *Err; + uint64_t SymIndex = decodeULEB128(Cur, &Size, Contents.end(), &Err); + if (Err) + fatal(toString(F) + ": could not decode addrsig section: " + Err); + markAddrsig(Syms[SymIndex]); + Cur += Size; } + } else { + // If an object file does not have an address-significance table, + // conservatively mark all of its symbols as address-significant. + for (Symbol *S : Syms) + markAddrsig(S); } } } Index: ELF/ICF.cpp =================================================================== --- ELF/ICF.cpp +++ ELF/ICF.cpp @@ -172,12 +172,6 @@ !S->Name.startswith(".data.rel.ro.")) return false; - // Don't merge read only data sections unless - // --ignore-data-address-equality or --icf=safe was passed. - if (!(S->Flags & SHF_EXECINSTR) && - !(Config->IgnoreDataAddressEquality || Config->ICF == ICFLevel::Safe)) - return false; - // Don't merge synthetic sections as their Data member is not valid and empty. // The Data member needs to be valid for ICF as it is used by ICF to determine // the equality of section contents. Index: test/ELF/icf-safe.s =================================================================== --- test/ELF/icf-safe.s +++ test/ELF/icf-safe.s @@ -6,6 +6,8 @@ # RUN: ld.lld %t1.o %t2.o -o %t2 --icf=safe --print-icf-sections | FileCheck %s # RUN: ld.lld %t1.o %t2.o -o %t3 --icf=safe --print-icf-sections -shared | FileCheck --check-prefix=EXPORT %s # RUN: ld.lld %t1.o %t2.o -o %t3 --icf=safe --print-icf-sections --export-dynamic | FileCheck --check-prefix=EXPORT %s +# RUN: ld.lld %t1.o %t2.o -o %t2 --icf=all --print-icf-sections | FileCheck --check-prefix=ALL %s +# RUN: ld.lld %t1.o %t2.o -o %t2 --icf=all --print-icf-sections --export-dynamic | FileCheck --check-prefix=ALL-EXPORT %s # RUN: ld.lld %t1copy.o -o %t4 --icf=safe 2>&1 | FileCheck --check-prefix=OBJCOPY %s # CHECK-NOT: selected section {{.*}}:(.rodata.l1) @@ -26,6 +28,28 @@ # CHECK-NOT: selected section {{.*}}:(.text.non_addrsig{{.}}) +# With --icf=all address-significance implies keep-unique only for rodata, not +# text. +# ALL-NOT: selected section {{.*}}:(.rodata.l1) +# ALL: selected section {{.*}}:(.rodata.l3) +# ALL: removing identical section {{.*}}:(.rodata.l4) + +# ALL: selected section {{.*}}:(.text.f3) +# ALL: removing identical section {{.*}}:(.text.f4) + +# ALL: selected section {{.*}}:(.text.f1) +# ALL: removing identical section {{.*}}:(.text.f2) +# ALL: removing identical section {{.*}}:(.text.non_addrsig1) +# ALL: removing identical section {{.*}}:(.text.non_addrsig2) + +# ALL-NOT: selected section {{.*}}:(.rodata.h1) +# ALL: selected section {{.*}}:(.rodata.h3) +# ALL: removing identical section {{.*}}:(.rodata.h4) + +# ALL-NOT: selected section {{.*}}:(.rodata.g1) +# ALL: selected section {{.*}}:(.rodata.g3) +# ALL: removing identical section {{.*}}:(.rodata.g4) + # llvm-mc normally emits an empty .text section into every object file. Since # nothing actually refers to it via a relocation, it doesn't have any associated # symbols (thus nor can anything refer to it via a relocation, making it safe to @@ -44,6 +68,27 @@ # EXPORT: removing identical section {{.*}}:(.text) # EXPORT-NOT: selected section +# If --icf=all is specified when exporting we can also merge the exported text +# sections, but not the exported rodata. +# ALL-EXPORT-NOT: selected section +# ALL-EXPORT: selected section {{.*}}:(.rodata.l3) +# ALL-EXPORT: removing identical section {{.*}}:(.rodata.l4) +# ALL-EXPORT-NOT: selected section +# ALL-EXPORT: selected section {{.*}}:(.text.f3) +# ALL-EXPORT: removing identical section {{.*}}:(.text.f4) +# ALL-EXPORT-NOT: selected section +# ALL-EXPORT: selected section {{.*}}:(.text.f1) +# ALL-EXPORT: removing identical section {{.*}}:(.text.f2) +# ALL-EXPORT: removing identical section {{.*}}:(.text.non_addrsig1) +# ALL-EXPORT: removing identical section {{.*}}:(.text.non_addrsig2) +# ALL-EXPORT-NOT: selected section +# ALL-EXPORT: selected section {{.*}}:(.rodata.h3) +# ALL-EXPORT: removing identical section {{.*}}:(.rodata.h4) +# ALL-EXPORT-NOT: selected section +# ALL-EXPORT: selected section {{.*}}:(.text) +# ALL-EXPORT: removing identical section {{.*}}:(.text) +# ALL-EXPORT-NOT: selected section + # OBJCOPY: --icf=safe is incompatible with object files created using objcopy or ld -r .section .text.f1,"ax",@progbits