[lld-link] Ignore undefined symbols in .addrsig table

[zequanwu]

Summary

Undefined symbols in .addrsig table just mark symbols as addrsig to avoid icf, and not required to be resolved. We could just ignore them.

[MaskRay]

This is not correct. A symbol marked by .addrsig_sym can be referenced via normal means. With this patch, the diagnostic may change from error: undefined symbol: bar to error: relocation against symbol in discarded section: bar

[zequanwu]

In D100966#2705542, @MaskRay wrote:

This is not correct. A symbol marked by .addrsig_sym can be referenced via normal means. With this patch, the diagnostic may change from error: undefined symbol: bar to error: relocation against symbol in discarded section: bar

Thanks for pointing it out. I put a new patch here: D100994

[rnk]

I'm guessing ELF doesn't solve the same problem this way. Does LLVM MC make these .addrsig_sym references weak if there are no other references? That would solve the issue.

lld/COFF/Driver.cpp

if (obj->addrsigSec) {
         unsigned size;
         const char *err;
         uint64_t symIndex = decodeULEB128(cur, &size, contents.end(), &err);
         if (err)
           fatal(toString(obj) + ": could not decode addrsig section: " + err);
         if (symIndex >= syms.size())
           fatal(toString(obj) + ": invalid symbol index in addrsig section");
         markAddrsig(syms[symIndex]);
+        syms[symIndex]->isAddrsig = true;
         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);
     }
if (config->autoImport) {
     // but there's nothing stopping the __imp_ symbols from coming from a
     // normal object file as well (although that won't be used for the
     // actual autoimport later on). If this pass adds new undefined references,
     // we won't iterate further to resolve them.
     symtab->loadMinGWAutomaticImports();
     run();
   }
 
+  // Find symbols in .addrsig table before resolving symbols in case of any of
+  // those symbols is undefined.
+  if (config->doICF != ICFLevel::None)
+    findKeepUniqueSections();
+
   // At this point, we should not have any symbols that cannot be resolved.
   // If we are going to do codegen for link-time optimization, check for
   // unresolvable symbols first, so we don't spend time generating code that
   // will fail to link anyway.
   if (!BitcodeFile::instances.empty() && !config->forceUnresolved)
     symtab->reportUnresolvable();
   if (errorCount())
     return;
void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // Identify unreferenced COMDAT sections.
   if (config->doGC)
     markLive(symtab->getChunks());
 
   // Needs to happen after the last call to addFile().
   convertResources();
 
   // Identify identical COMDAT sections to merge them.
-  if (config->doICF != ICFLevel::None) {
-    findKeepUniqueSections();
+  if (config->doICF != ICFLevel::None)
     doICF(symtab->getChunks(), config->doICF);
-  }
 
   // Write the result.
   writeResult();
 
   // Stop early so we can print the results.
   rootTimer.stop();
   if (config->showTiming)
     Timer::root().print();
 }
 
 } // namespace coff
 } // namespace lld

lld/COFF/SymbolTable.cpp

reportProblemSymbols(const SmallPtrSetImpl<Symbol *> &undefs,
   DenseMap<Symbol *, int> firstDiag;
 
   auto processFile = [&](InputFile *file, ArrayRef<Symbol *> symbols) {
     uint32_t symIndex = (uint32_t)-1;
     for (Symbol *sym : symbols) {
       ++symIndex;
       if (!sym)
         continue;
+      if (sym->isAddrsig)
+        continue;
       if (undefs.count(sym)) {
         auto it = firstDiag.find(sym);
         if (it == firstDiag.end()) {
           firstDiag[sym] = undefDiags.size();
           undefDiags.push_back({sym, {{file, symIndex}}});
         } else {
           undefDiags[it->second].files.push_back({file, symIndex});
         }

lld/COFF/Symbols.h

private:
   void computeName();
 
 protected:
   friend SymbolTable;
   explicit Symbol(Kind k, StringRef n = "")
       : symbolKind(k), isExternal(true), isCOMDAT(false),
         writtenToSymtab(false), pendingArchiveLoad(false), isGCRoot(false),
         isRuntimePseudoReloc(false), deferUndefined(false), canInline(true),
-        nameSize(n.size()), nameData(n.empty() ? nullptr : n.data()) {}
+        isAddrsig(false), nameSize(n.size()),
+        nameData(n.empty() ? nullptr : n.data()) {}
 
   const unsigned symbolKind : 8;
   unsigned isExternal : 1;
 
 public:
   // This bit is used by the \c DefinedRegular subclass.
   unsigned isCOMDAT : 1;
 
public:
   // might be fixed up later.
   unsigned deferUndefined : 1;
 
   // False if LTO shouldn't inline whatever this symbol points to. If a symbol
   // is overwritten after LTO, LTO shouldn't inline the symbol because it
   // doesn't know the final contents of the symbol.
   unsigned canInline : 1;
 
+  // True if the symbol is in .addrsig table. So we don't report it as undefined
+  // when resolving symbols.
+  unsigned isAddrsig : 1;
+
 protected:
   // Symbol name length. Assume symbol lengths fit in a 32-bit integer.
   uint32_t nameSize;
 
   const char *nameData;
 };
 
 // The base class for any defined symbols, including absolute symbols,

lld/test/COFF/undefined-symbol-addrsig.s

+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
+# RUN: lld-link /entry:_start /subsystem:console %t.obj
+
+.text
+.globl _start
+_start:
+  ret
+
+.global bar
+
+.addrsig
+.addrsig_sym bar