[PATCH] D51376: ELF: Don't examine values of linker script symbols during ICF.

[Peter Collingbourne via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
pcc marked 3 inline comments as done.
Closed by commit rLLD340998: ELF: Don't examine values of linker script symbols during ICF. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51376?vs=162935&id=163229#toc

Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D51376

Files:
  ELF/ICF.cpp
  ELF/LinkerScript.cpp
  ELF/Symbols.h
  test/ELF/linkerscript/icf.s


Index: ELF/LinkerScript.cpp
===================================================================
--- ELF/LinkerScript.cpp
+++ ELF/LinkerScript.cpp
@@ -209,6 +209,7 @@
                          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
Index: ELF/ICF.cpp
===================================================================
--- ELF/ICF.cpp
+++ ELF/ICF.cpp
@@ -252,7 +252,10 @@
 
     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
Index: ELF/Symbols.h
===================================================================
--- ELF/Symbols.h
+++ ELF/Symbols.h
@@ -164,7 +164,8 @@
       : 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.
@@ -187,6 +188,9 @@
   // 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
@@ -376,6 +380,7 @@
   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.
Index: test/ELF/linkerscript/icf.s
===================================================================
--- test/ELF/linkerscript/icf.s
+++ test/ELF/linkerscript/icf.s
@@ -0,0 +1,11 @@
+# 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", at progbits
+jmp foo
+
+.section .text.bar,"ax", at progbits
+jmp bar


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51376.163229.patch
Type: text/x-patch
Size: 2650 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180829/cc720ef2/attachment.bin>