[LLD] [COFF] Fix autoexport from LTO objects with comdat symbols

[mstorsjo]

Summary

Make sure that comdat symbols also have a non-null dummy
SectionChunk associated.

This requires moving around an existing FIXME regarding comdats in
LTO.

[mstorsjo]

Ping @rnk

[rnk]

lgtm

lld/COFF/InputFiles.cpp

void ObjFile::handleComdatSelection(
   // There's already an existing comdat for this symbol: `Leader`.
   // Use the comdats's selection field to determine if the new
   // symbol in `Sym` should be discarded, produce a duplicate symbol
   // error, etc.
 
   SectionChunk *leaderChunk = nullptr;
   COMDATType leaderSelection = IMAGE_COMDAT_SELECT_ANY;
 
-  if (leader->data) {
-    leaderChunk = leader->getChunk();
-    leaderSelection = leaderChunk->selection;
-  } else {
-    // FIXME: comdats from LTO files don't know their selection; treat them
-    // as "any".
-    selection = leaderSelection;
-  }
+  assert(leader->data && "Comdat leader without SectionChunk?");
 
   if ((selection == IMAGE_COMDAT_SELECT_ANY &&
        leaderSelection == IMAGE_COMDAT_SELECT_LARGEST) ||
       (selection == IMAGE_COMDAT_SELECT_LARGEST &&
        leaderSelection == IMAGE_COMDAT_SELECT_ANY)) {
     // cl.exe picks "any" for vftables when building with /GR- and
     // "largest" when building with /GR. To be able to link object files
     // compiled with each flag, "any" and "largest" are merged as "largest".

 // Convenience class for initializing a coff_section with specific flags.
 class FakeSection {
 public:
   FakeSection(int c) { section.Characteristics = c; }
 
   coff_section section;
 };
 
+// Convenience class for initializing a SectionChunk with specific flags.
+class FakeSectionChunk {
+public:
+  FakeSectionChunk(const coff_section *section) : chunk(nullptr, section) {
+    // FIXME: comdats from LTO files don't know their selection; treat them
+    // as "any".
+    chunk.selection = IMAGE_COMDAT_SELECT_ANY;
+  }
+
+  SectionChunk chunk;
+};
+
 FakeSection ltoTextSection(IMAGE_SCN_MEM_EXECUTE);
 FakeSection ltoDataSection(IMAGE_SCN_CNT_INITIALIZED_DATA);
-SectionChunk ltoTextSectionChunk(nullptr, &ltoTextSection.section);
-SectionChunk ltoDataSectionChunk(nullptr, &ltoDataSection.section);
+FakeSectionChunk ltoTextSectionChunk(&ltoTextSection.section);
+FakeSectionChunk ltoDataSectionChunk(&ltoDataSection.section);
 } // namespace
 
 void BitcodeFile::parse() {
   std::vector<std::pair<Symbol *, bool>> comdat(obj->getComdatTable().size());
   for (size_t i = 0; i != obj->getComdatTable().size(); ++i)
     // FIXME: lto::InputFile doesn't keep enough data to do correct comdat
     // selection handling.
     comdat[i] = symtab->addComdat(this, saver.save(obj->getComdatTable()[i]));
   for (const lto::InputFile::Symbol &objSym : obj->symbols()) {
     StringRef symName = saver.save(objSym.getName());
     int comdatIndex = objSym.getComdatIndex();
     Symbol *sym;
     SectionChunk *fakeSC = nullptr;
     if (objSym.isExecutable())
-      fakeSC = &ltoTextSectionChunk;
+      fakeSC = &ltoTextSectionChunk.chunk;
     else
-      fakeSC = &ltoDataSectionChunk;
+      fakeSC = &ltoDataSectionChunk.chunk;
     if (objSym.isUndefined()) {
       sym = symtab->addUndefined(symName, this, false);
     } else if (objSym.isCommon()) {
       sym = symtab->addCommon(this, symName, objSym.getCommonSize());
     } else if (objSym.isWeak() && objSym.isIndirect()) {
       // Weak external.
       sym = symtab->addUndefined(symName, this, true);
       std::string fallback = std::string(objSym.getCOFFWeakExternalFallback());
       Symbol *alias = symtab->addUndefined(saver.save(fallback));
       checkAndSetWeakAlias(symtab, this, sym, alias);
     } else if (comdatIndex != -1) {
-      if (symName == obj->getComdatTable()[comdatIndex])
+      if (symName == obj->getComdatTable()[comdatIndex]) {
         sym = comdat[comdatIndex].first;
-      else if (comdat[comdatIndex].second)
+        if (cast<DefinedRegular>(sym)->data == nullptr)
+          cast<DefinedRegular>(sym)->data = &fakeSC->repl;
+      } else if (comdat[comdatIndex].second) {
         sym = symtab->addRegular(this, symName, nullptr, fakeSC);
-      else
+      } else {
         sym = symtab->addUndefined(symName, this, false);
+      }
     } else {
       sym = symtab->addRegular(this, symName, nullptr, fakeSC);
     }
     symbols.push_back(sym);
     if (objSym.isUsed())
       config->gcroot.push_back(sym);
   }
   directives = obj->getCOFFLinkerOpts();

lld/test/COFF/export-all-lto.ll

 ; REQUIRES: x86
 
 ; RUN: llvm-as %s -o %t.bc
 
 ; RUN: lld-link -lldmingw -dll -out:%t.dll %t.bc -noentry -output-def:%t.def
 ; RUN: llvm-readobj --coff-exports %t.dll | grep Name: | FileCheck %s
 ; RUN: cat %t.def | FileCheck --check-prefix=IMPLIB %s
 
+; CHECK: Name: MyComdatFunc
 ; CHECK: Name: MyExtData
 ; CHECK: Name: MyLibFunc
 
-; IMPLIB: MyExtData @1 DATA
-; IMPLIB: MyLibFunc @2{{$}}
+; IMPLIB: MyComdatFunc @1{{$}}
+; IMPLIB: MyExtData @2 DATA
+; IMPLIB: MyLibFunc @3{{$}}
 
 target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-w64-windows-gnu"
 
 @MyExtData = dso_local global i32 42, align 4
 
+$MyComdatFunc = comdat any
+
 define dso_local void @MyLibFunc() {
 entry:
+  call void @MyComdatFunc()
+  ret void
+}
+
+define linkonce_odr void @MyComdatFunc() comdat {
+entry:
   ret void
 }