ELF: Read address significance tables with --icf=all.

[pcc]

Summary

Under --icf=all we now only apply KeepUnique to non-executable
address-significant sections. This has the effect of making --icf=all
mean unsafe ICF for executable sections and safe ICF for non-executable
sections.

With this change the meaning of the KeepUnique bit changes to
"does the current ICF mode (together with the --keep-unique and
--ignore-data-address-equality flags) require this section to be
kept unique".

[ruiu]

LGTM

lld/ELF/Driver.cpp

1243 ↗

(On Diff #156653)

if -> even if?

ELF/Driver.cpp

if (shouldDemote<ELFT>(*Sym)) {
       bool Used = Sym->Used;
       replaceSymbol<Undefined>(Sym, nullptr, Sym->getName(), Sym->Binding,
                                Sym->StOther, Sym->Type);
       Sym->Used = Used;
     }
   }
 }
 
-static bool keepUnique(Symbol *S) {
-  if (auto *D = dyn_cast_or_null<Defined>(S)) {
-    if (D->Section) {
+// 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<Defined>(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;
-      return true;
-    }
-  }
-  return false;
 }
 
 // Record sections that define symbols mentioned in --keep-unique <symbol>
 // and symbols referred to by address-significance tables. These sections are
 // ineligible for ICF.
 template <class ELFT>
 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<Defined>(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);
+  // --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;
+
+      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<ObjFile<ELFT>>(F);
-      ArrayRef<Symbol *> Syms = Obj->getSymbols();
-      if (Obj->AddrsigSec) {
-        ArrayRef<uint8_t> 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);
-      }
+        markAddrsig(Syms[SymIndex]);
+        markAddrsig(S);
     }
   }
 }
 
 // Do actual linking. Note that when this function is called,
 // all linker scripts have already been parsed.
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   Target = getTarget();

ELF/ICF.cpp

if (!S->Live || !(S->Flags & SHF_ALLOC))
     return false;
 
   // Don't merge writable sections. .data.rel.ro sections are marked as writable
   // but are semantically read-only.
   if ((S->Flags & SHF_WRITE) && S->Name != ".data.rel.ro" &&
       !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.
   if (isa<SyntheticSection>(S))
     return false;
 
   // .init and .fini contains instructions that must be executed to initialize
   // and finalize the process. They cannot and should not be merged.

test/ELF/icf-safe.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
 # RUN: llvm-objcopy %t1.o %t1copy.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/icf-safe.s -o %t2.o
 # 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)
 # CHECK: selected section {{.*}}:(.rodata.l3)
 # CHECK:   removing identical section {{.*}}:(.rodata.l4)
 
 # CHECK-NOT: selected section {{.*}}:(.text.f1)
 # CHECK: selected section {{.*}}:(.text.f3)
 # CHECK:   removing identical section {{.*}}:(.text.f4)
 
 # CHECK-NOT: selected section {{.*}}:(.rodata.h1)
 # CHECK: selected section {{.*}}:(.rodata.h3)
 # CHECK:   removing identical section {{.*}}:(.rodata.h4)
 
 # CHECK-NOT: selected section {{.*}}:(.rodata.g1)
 # CHECK: selected section {{.*}}:(.rodata.g3)
 # CHECK:   removing identical section {{.*}}:(.rodata.g4)
 
 # 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
 # merge with the empty section in the other input file). Here we check that the
 # only two sections merged are the two empty sections and the sections with only
 # STB_LOCAL or STV_HIDDEN symbols. The dynsym entries should have prevented
 # anything else from being merged.
 # EXPORT-NOT: selected section
 # EXPORT: selected section {{.*}}:(.rodata.l3)
 # EXPORT:   removing identical section {{.*}}:(.rodata.l4)
 # EXPORT-NOT: selected section
 # EXPORT: selected section {{.*}}:(.rodata.h3)
 # EXPORT:   removing identical section {{.*}}:(.rodata.h4)
 # EXPORT-NOT: selected section
 # EXPORT: selected section {{.*}}:(.text)
 # 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
 .globl f1
 f1:
 ret
 
 .section .text.f2,"ax",@progbits