ELF: Don't examine values of linker script symbols during ICF.

[pcc]

Summary

These symbols are declared early with the same value, so they otherwise
appear identical to ICF.

[grimar]

LGTM. Minor nits/suggestions.

lld/ELF/Symbols.h

119 ↗

(On Diff #162935)

I think this should be added after the NeedsTocRestore below. Because this group of flags is
not initialized in the constructor.

lld/test/ELF/linkerscript/Inputs/icf.script

2 ↗

(On Diff #162935)

I would avoid adding the input and just used echo "foo = 1; bar = 1" > %t.script instead,

[ruiu]

LGTM

lld/ELF/ICF.cpp

255 ↗

(On Diff #162935)

Please add a comment saying that linker-script-generated placeholder symbols look the same at this moment but they would have different values later.

ELF/ICF.cpp

for (size_t I = 0; I < RA.size(); ++I) {
     if (&SA == &SB) {
       if (AddA == AddB)
         continue;
       return false;
     }
 
     auto *DA = dyn_cast<Defined>(&SA);
     auto *DB = dyn_cast<Defined>(&SB);
-    if (!DA || !DB)
+
+    // Placeholder symbols generated by linker scripts look the same now but
+    // may have different values later.
+    if (!DA || !DB || DA->ScriptDefined || DB->ScriptDefined)
       return false;
 
     // Relocations referring to absolute symbols are constant-equal if their
     // values are equal.
     if (!DA->Section && !DB->Section && DA->Value + AddA == DB->Value + AddB)
       continue;
     if (!DA->Section || !DB->Section)
       return false;

ELF/LinkerScript.cpp

static void declareSymbol(SymbolAssignment *Cmd) {
   uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
   std::tie(Sym, std::ignore) = Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
                                               /*CanOmitFromDynSym*/ false,
                                               /*File*/ nullptr);
   replaceSymbol<Defined>(Sym, nullptr, Cmd->Name, STB_GLOBAL, Visibility,
                          STT_NOTYPE, 0, 0, nullptr);
   Cmd->Sym = cast<Defined>(Sym);
   Cmd->Provide = false;
+  Sym->ScriptDefined = true;
 }
 
 // This method is used to handle INSERT AFTER statement. Here we rebuild
 // the list of script commands to mix sections inserted into.
 void LinkerScript::processInsertCommands() {
   std::vector<BaseCommand *> V;
   auto Insert = [&](std::vector<BaseCommand *> &From) {
     V.insert(V.end(), From.begin(), From.end());

ELF/Symbols.h

public:
   OutputSection *getOutputSection() const;
 
 protected:
   Symbol(Kind K, InputFile *File, StringRefZ Name, uint8_t Binding,
          uint8_t StOther, uint8_t Type)
       : File(File), NameData(Name.Data), NameSize(Name.Size), Binding(Binding),
         Type(Type), StOther(StOther), SymbolKind(K), NeedsPltAddr(false),
         IsInIplt(false), IsInIgot(false), IsPreemptible(false),
-        Used(!Config->GcSections), NeedsTocRestore(false) {}
+        Used(!Config->GcSections), NeedsTocRestore(false),
+        ScriptDefined(false) {}
 
 public:
   // True the symbol should point to its PLT entry.
   // For SharedSymbol only.
   unsigned NeedsPltAddr : 1;
 
   // True if this symbol is in the Iplt sub-section of the Plt.
   unsigned IsInIplt : 1;
 
   // True if this symbol is in the Igot sub-section of the .got.plt or .got.
   unsigned IsInIgot : 1;
 
   // True if this symbol is preemptible at load time.
   unsigned IsPreemptible : 1;
 
   // True if an undefined or shared symbol is used from a live section.
   unsigned Used : 1;
 
   // True if a call to this symbol needs to be followed by a restore of the
   // PPC64 toc pointer.
   unsigned NeedsTocRestore : 1;
 
+  // True if this symbol is defined by a linker script.
+  unsigned ScriptDefined : 1;
+
   // The Type field may also have this value. It means that we have not yet seen
   // a non-Lazy symbol with this name, so we don't know what its type is. The
   // Type field is normally set to this value for Lazy symbols unless we saw a
   // weak undefined symbol first, in which case we need to remember the original
   // symbol's type in order to check for TLS mismatches.
   enum { UnknownType = 255 };
 
   bool isSection() const { return Type == llvm::ELF::STT_SECTION; }
void replaceSymbol(Symbol *S, ArgT &&... Arg) {
   new (S) T(std::forward<ArgT>(Arg)...);
 
   S->VersionId = Sym.VersionId;
   S->Visibility = Sym.Visibility;
   S->IsUsedInRegularObj = Sym.IsUsedInRegularObj;
   S->ExportDynamic = Sym.ExportDynamic;
   S->CanInline = Sym.CanInline;
   S->Traced = Sym.Traced;
+  S->ScriptDefined = Sym.ScriptDefined;
 
   // Print out a log message if --trace-symbol was specified.
   // This is for debugging.
   if (S->Traced)
     printTraceSymbol(S);
 }
 
 void warnUnorderableSymbol(const Symbol *Sym);
 } // namespace elf
 
 std::string toString(const elf::Symbol &B);
 } // namespace lld
 
 #endif

test/ELF/linkerscript/icf.s

+# REQUIRES: x86
+
+# RUN: echo "foo = 1; bar = 2;" > %t.script
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld %t.o %t.script -o %t --icf=all --print-icf-sections | count 0
+
+.section .text.foo,"ax",@progbits
+jmp foo
+
+.section .text.bar,"ax",@progbits
+jmp bar