[PATCH] D49764: Simplify ObjFile::createDefined().

[Rui Ueyama via Phabricator via llvm-commits]

ruiu updated this revision to Diff 157166.
ruiu added a comment.

- update comment


https://reviews.llvm.org/D49764

Files:
  lld/COFF/InputFiles.cpp


Index: lld/COFF/InputFiles.cpp
===================================================================
--- lld/COFF/InputFiles.cpp
+++ lld/COFF/InputFiles.cpp
@@ -207,12 +207,6 @@
     COFFSymbolRef Sym, const coff_aux_section_definition *Def) {
   SectionChunk *Parent = SparseChunks[Def->getNumber(Sym.isBigObj())];
 
-  // If the parent is pending, it probably means that its section definition
-  // appears after us in the symbol table. Leave the associated section as
-  // pending; we will handle it during the second pass in initializeSymbols().
-  if (Parent == PendingComdat)
-    return;
-
   // Check whether the parent is prevailing. If it is, so are we, and we read
   // the section; otherwise mark it as discarded.
   int32_t SectionNumber = Sym.getSectionNumber();
@@ -262,15 +256,13 @@
     } else if (Optional<Symbol *> OptSym = createDefined(COFFSym, ComdatDefs)) {
       Symbols[I] = *OptSym;
     } else {
-      // createDefined() returns None if a symbol belongs to a section that
-      // was pending at the point when the symbol was read. This can happen in
-      // two cases:
-      // 1) section definition symbol for a comdat leader;
-      // 2) symbol belongs to a comdat section associated with a section whose
-      //    section definition symbol appears later in the symbol table.
-      // In both of these cases, we can expect the section to be resolved by
-      // the time we finish visiting the remaining symbols in the symbol
-      // table. So we postpone the handling of this symbol until that time.
+      // createDefined() returns None for an associtave comdat section symbol.
+      // Such symbol is processed in the second pass (in the following for-loop),
+      // so we just record their indices for now.
+      //
+      // We need to do this in two passes to handle forward references. An
+      // associative symbol can refer to a comdat symbols that appears later in
+      // the symbol table.
       PendingIndexes.push_back(I);
     }
     I += COFFSym.getNumberOfAuxSymbols();
@@ -366,20 +358,15 @@
     return Leader;
   }
 
-  // Read associative section definitions and prepare to handle the comdat
-  // leader symbol by setting the section's ComdatDefs pointer if we encounter a
-  // non-associative comdat.
+  // Prepare to handle the comdat leader symbol by setting the section's
+  // ComdatDefs pointer if we encounter a non-associative comdat.
   if (SparseChunks[SectionNumber] == PendingComdat) {
-    if (auto *Def = Sym.getSectionDefinition()) {
-      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
-        readAssociativeDefinition(Sym, Def);
-      else
+    if (auto *Def = Sym.getSectionDefinition())
+      if (Def->Selection != IMAGE_COMDAT_SELECT_ASSOCIATIVE)
         ComdatDefs[SectionNumber] = Def;
-    }
+    return None;
   }
 
-  if (SparseChunks[SectionNumber] == PendingComdat)
-    return None;
   return createRegular(Sym);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49764.157166.patch
Type: text/x-patch
Size: 2936 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180724/60a9e4f3/attachment.bin>