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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo updated this revision to Diff 353490.
mstorsjo added a comment.

Use @rnk's suggsestion of `isa<BitcodFile>()`, simplify the initialization of leaderChunk and leaderSelection.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104605/new/

https://reviews.llvm.org/D104605

Files:
  lld/COFF/InputFiles.cpp
  lld/test/COFF/lto-comdat-samesize.ll


Index: lld/test/COFF/lto-comdat-samesize.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: lld/COFF/InputFiles.cpp
===================================================================
--- lld/COFF/InputFiles.cpp
+++ lld/COFF/InputFiles.cpp
@@ -500,12 +500,15 @@
   // 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 @@
       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,10 @@
 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 e.g. know what size they are going to have, so
+    // we can't do proper checking against a regular comdat regarding
+    // its size or contents.
     chunk.selection = IMAGE_COMDAT_SELECT_ANY;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104605.353490.patch
Type: text/x-patch
Size: 3262 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210621/ae19b0f4/attachment.bin>