Change which file we print when a symbol cannot be ordered

[• espindola]

Summary

Currently there are a few odd things about the warning about symbols that cannot be ordered. This patch fixes:

• When there is an undefined symbol that resolves to a shared file, we print the location of the undefined reference.
• If there are multiple comdats, we report them all.

This makes the warning a bit less noisy and should also help with reusing this in D36351.

[ruiu]

ELF/Writer.cpp

1049

This is perhaps better?

if (File != Sym->File)

[• espindola]

ELF/Writer.cpp

1049

The problem with that is that we would not warn on shared symbols since we don't iterate over SharedSymbols.

Let me try a variation.

[• espindola]

We now iterate over the symbol table for global symbols the object files for the local ones.

This fixes the issues noted before, but still warns if there are multiple unrelated symbols with the same name.

[• espindola]

So, are you OK with the idea of using the symbol table to find the global symbols and iterating over the objects to find the local ones?

[• espindola]

In D45197#1057093, @espindola wrote:

So, are you OK with the idea of using the symbol table to find the global symbols and iterating over the objects to find the local ones?

Ping. Sorry for the supper early ping, but this should help with the section layout patch.

[ruiu]

LGTM

ELF/Writer.cpp

1080

Please add a comment saying that we add all symbols including local symbols so we have two loops.

[• espindola]

329371

ELF/Writer.cpp

static DenseMap<const InputSectionBase *, int> buildSectionOrder() {
   // appear in the symbol ordering file have the lowest priority 0.
   // All explicitly mentioned symbols have negative (higher) priorities.
   DenseMap<StringRef, SymbolOrderEntry> SymbolOrder;
   int Priority = -Config->SymbolOrderingFile.size();
   for (StringRef S : Config->SymbolOrderingFile)
     SymbolOrder.insert({S, {Priority++, false}});
 
   // Build a map from sections to their priorities.
-  for (InputFile *File : ObjectFiles) {
-    for (Symbol *Sym : File->getSymbols()) {
-      auto It = SymbolOrder.find(Sym->getName());
-      if (It == SymbolOrder.end())
-        continue;
-      SymbolOrderEntry &Ent = It->second;
-      Ent.Present = true;
+  auto AddSym = [&](Symbol &Sym) {
+    auto It = SymbolOrder.find(Sym.getName());
+      return;
 
-      auto *D = dyn_cast<Defined>(Sym);
-      if (Config->WarnSymbolOrdering) {
-        if (Sym->isUndefined())
-          warn(File->getName() +
-               ": unable to order undefined symbol: " + Sym->getName());
-        else if (Sym->isShared())
-          warn(File->getName() +
-               ": unable to order shared symbol: " + Sym->getName());
-        else if (D && !D->Section)
-          warn(File->getName() +
-               ": unable to order absolute symbol: " + Sym->getName());
-        else if (D && !D->Section->Live)
-          warn(File->getName() +
-               ": unable to order discarded symbol: " + Sym->getName());
-      }
-      if (!D)
-        continue;
+      auto *D = dyn_cast<Defined>(&Sym);
+      InputFile *File = Sym.File;
+      if (Sym.isUndefined())
+             ": unable to order undefined symbol: " + Sym.getName());
+      else if (Sym.isShared())
+             ": unable to order shared symbol: " + Sym.getName());
+             ": unable to order absolute symbol: " + Sym.getName());
+             ": unable to order discarded symbol: " + Sym.getName());
 
+    if (auto *D = dyn_cast<Defined>(&Sym)) {
       if (auto *Sec = dyn_cast_or_null<InputSectionBase>(D->Section)) {
         int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];
         Priority = std::min(Priority, Ent.Priority);
       }
     }
-  }
+  };
+  for (Symbol *Sym : Symtab->getSymbols())
+    if (!Sym->isLazy())
+      AddSym(*Sym);
+  for (InputFile *File : ObjectFiles)
+    for (Symbol *Sym : File->getSymbols())
+      if (Sym->isLocal())
+        AddSym(*Sym);
 
   if (Config->WarnSymbolOrdering)
     for (auto OrderEntry : SymbolOrder)
       if (!OrderEntry.second.Present)
         warn("symbol ordering file: no such symbol: " + OrderEntry.first);
 
   return SectionOrder;
 }

test/ELF/symbol-ordering-file-warnings.s

 # RUN: echo "discard" >> %t-order-multi.txt
 # RUN: echo "_start" >> %t-order-multi.txt
 # RUN: ld.lld %t1.o %t3.o %t.so -o %t --symbol-ordering-file %t-order-multi.txt --gc-sections -T %t.script \
 # RUN:   --unresolved-symbols=ignore-all 2>&1 | FileCheck %s --check-prefixes=WARN,SAMESYM,ABSOLUTE,SHARED,UNDEFINED,GC,DISCARD,MISSING2
 
 # WARN-NOT:    warning:
 # SAMESYM:     warning: {{.*}}.txt: duplicate ordered symbol: _start
 # WARN-NOT:    warning:
-# ABSOLUTE:    warning: {{.*}}1.o: unable to order absolute symbol: absolute
-# WARN-NOT:    warning:
 # DISCARD:     warning: {{.*}}1.o: unable to order discarded symbol: discard
 # WARN-NOT:    warning:
 # GC:          warning: {{.*}}1.o: unable to order discarded symbol: gc
 # WARN-NOT:    warning:
-# SHARED:      warning: {{.*}}1.o: unable to order shared symbol: shared
+# SHARED:      warning: {{.*}}.so: unable to order shared symbol: shared
 # WARN-NOT:    warning:
 # UNDEFINED:   warning: {{.*}}3.o: unable to order undefined symbol: undefined
 # WARN-NOT:    warning:
+# ABSOLUTE:    warning: {{.*}}1.o: unable to order absolute symbol: absolute
+# WARN-NOT:    warning:
 # MISSING:     warning: symbol ordering file: no such symbol: missing
 # MISSING2:    warning: symbol ordering file: no such symbol: missing_sym
 # ICF:         warning: {{.*}}1.o: unable to order discarded symbol: icf2
 # COMDAT:      warning: {{.*}}1.o: unable to order discarded symbol: comdat
-# COMDAT-NEXT: warning: {{.*}}2.o: unable to order discarded symbol: comdat
-# MULTI:       warning: {{.*}}2.o: unable to order absolute symbol: multi
-# MULTI-NEXT:  warning: {{.*}}3.o: unable to order undefined symbol: multi
+# MULTI:       warning: {{.*}}3.o: unable to order undefined symbol: multi
+# MULTI-NEXT:  warning: {{.*}}2.o: unable to order absolute symbol: multi
 # WARN-NOT:    warning:
 
 absolute = 0x1234
 
 .section .text.gc,"ax",@progbits
 .global gc
 gc:
   nop