[lld] d07f436 - [LLD] [COFF] Fix handling of LTO comdats with nontrivial selection types after 728cc0075e5dfdb454eb

[Martin Storsjö via llvm-commits]

Author: Martin Storsjö
Date: 2021-06-25T09:39:56+03:00
New Revision: d07f43641f98a8e0024cf8e94ef98c7c912221d9

URL: https://github.com/llvm/llvm-project/commit/d07f43641f98a8e0024cf8e94ef98c7c912221d9
DIFF: https://github.com/llvm/llvm-project/commit/d07f43641f98a8e0024cf8e94ef98c7c912221d9.diff

LOG: [LLD] [COFF] Fix handling of LTO comdats with nontrivial selection types after 728cc0075e5dfdb454eb

Commit 728cc0075e5dfdb454ebe6418b63bdffae71ec14 made comdat symbols
from LTO objects be treated as any regular comdat symbol. This works
great for symbols that actually are IMAGE_COMDAT_SELECT_ANY, but
if the symbols have a less trivial selection type that require comparing
either the section chunk size or contents, we can't check that before
actually doing the LTO compilation.

Therefore bring back one aspect of handling from before; that comdat
resolution with a leader from an LTO symbol is essentially skipped,
like it was before 728cc0075e5dfdb454ebe6418b63bdffae71ec14.

Differential Revision: https://reviews.llvm.org/D104605

Added: 
    lld/test/COFF/lto-comdat-samesize.ll

Modified: 
    lld/COFF/InputFiles.cpp

Removed: 
    


################################################################################
diff  --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index f4abeb4acfb4..9d36b5e25cfa 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -500,12 +500,15 @@ void ObjFile::handleComdatSelection(
   // symbol in `Sym` should be discarded, produce a duplicate symbol
   // error, etc.
 
-  SectionChunk *leaderChunk = nullptr;
-  COMDATType leaderSelection = IMAGE_COMDAT_SELECT_ANY;
+  SectionChunk *leaderChunk = leader->getChunk();
+  COMDATType leaderSelection = leaderChunk->selection;
 
   assert(leader->data && "Comdat leader without SectionChunk?");
-  leaderChunk = leader->getChunk();
-  leaderSelection = leaderChunk->selection;
+  if (isa<BitcodeFile>(leader->file)) {
+    // If the leader is only a LTO symbol, we don't know e.g. its final size
+    // yet, so we can't do the full strict comdat selection checking yet.
+    selection = leaderSelection = IMAGE_COMDAT_SELECT_ANY;
+  }
 
   if ((selection == IMAGE_COMDAT_SELECT_ANY &&
        leaderSelection == IMAGE_COMDAT_SELECT_LARGEST) ||
@@ -558,8 +561,10 @@ void ObjFile::handleComdatSelection(
       if (!config->mingw) {
         symtab->reportDuplicate(leader, this);
       } else {
-        const coff_aux_section_definition *leaderDef = findSectionDef(
-            leaderChunk->file->getCOFFObj(), leaderChunk->getSectionNumber());
+        const coff_aux_section_definition *leaderDef = nullptr;
+        if (leaderChunk->file)
+          leaderDef = findSectionDef(leaderChunk->file->getCOFFObj(),
+                                     leaderChunk->getSectionNumber());
         if (!leaderDef || leaderDef->Length != def->Length)
           symtab->reportDuplicate(leader, this);
       }
@@ -1050,8 +1055,9 @@ class FakeSection {
 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".
+    // Comdats from LTO files can't be fully treated as regular comdats
+    // at this point; we don't know what size or contents they are going to
+    // have, so we can't do proper checking of such aspects of them.
     chunk.selection = IMAGE_COMDAT_SELECT_ANY;
   }
 

diff  --git a/lld/test/COFF/lto-comdat-samesize.ll b/lld/test/COFF/lto-comdat-samesize.ll
new file mode 100644
index 000000000000..bdc68704fb4d
--- /dev/null
+++ b/lld/test/COFF/lto-comdat-samesize.ll
@@ -0,0 +1,36 @@
+; REQUIRES: x86
+
+; RUN: split-file %s %t.dir
+; RUN: llvm-as %t.dir/other.ll -o %t.other.bc
+; RUN: llc -filetype=obj -o %t.main.obj %t.dir/main.ll
+
+; RUN: lld-link -out:%t.exe -subsystem:console %t.other.bc %t.main.obj
+
+#--- main.ll
+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-unknown-windows-msvc19.14.0"
+
+$comdatData = comdat samesize
+
+ at comdatData = weak_odr dso_local global i32 42, comdat
+
+define dso_local void @mainCRTStartup() {
+entry:
+  tail call void @other()
+  ret void
+}
+
+declare dso_local void @other()
+
+#--- other.ll
+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-unknown-windows-msvc19.14.0"
+
+$comdatData = comdat samesize
+
+ at comdatData = weak_odr dso_local global i32 42, comdat
+
+define dso_local void @other() {
+entry:
+  ret void
+}