[PATCH] Fix ELFFileNode::resetNextIndex().

[Rui Ueyama]

Hi Bigcheese, shankarke, kledzik,

ELFLinkingContext has a method addUndefinedAtomsFromSharedLibrary().
The method is being used to skip a shared library within --start-group
and --end-group if it's not the first iteration of the group.

We have the same, incomplete mechanism to skip a shared library within
a group too. That's implemented in ELFFileNode. It's intended to not
return a shared library on the second or further iterations in the
first place. This mechanism is preferred over
addUndefinedAtomsFromSharedLibrary because the policy is implemented
in Input Graph -- that's what Input Graph is for.

This patch removes the dupluicate feature and fixes ELFFileNode.

http://llvm-reviews.chandlerc.com/D3280

Files:
  include/lld/Core/LinkingContext.h
  include/lld/Driver/GnuLdInputGraph.h
  include/lld/ReaderWriter/ELFLinkingContext.h
  lib/Core/Resolver.cpp

Index: include/lld/Core/LinkingContext.h
===================================================================
--- include/lld/Core/LinkingContext.h
+++ include/lld/Core/LinkingContext.h
@@ -158,11 +158,6 @@
   /// to be an error.
   bool allowShlibUndefines() const { return _allowShlibUndefines; }
 
-  /// Add undefined symbols from shared libraries ?
-  virtual bool addUndefinedAtomsFromSharedLibrary(const SharedLibraryFile *) {
-    return true;
-  }
-
   /// If true, core linking will write the path to each input file to stdout
   /// (i.e. llvm::outs()) as it is used.  This is used to implement the -t
   /// linker option.
Index: include/lld/Driver/GnuLdInputGraph.h
===================================================================
--- include/lld/Driver/GnuLdInputGraph.h
+++ include/lld/Driver/GnuLdInputGraph.h
@@ -74,13 +74,11 @@
   /// \brief This is used by Group Nodes, when there is a need to reset the
   /// the file to be processed next. When handling a group node that contains
   /// Input elements, if the group node has to be reprocessed, the linker needs
-  /// to start processing files as part of the inputelement from beginning.
-  /// reset the next file index to 0 only if the node is an archive library or
-  /// a shared library
+  /// to start processing files as part of the input element from beginning.
+  /// Reset the next file index to 0 only if the node is an archive library.
   void resetNextIndex() override {
-    if ((!_attributes._isWholeArchive &&
-         (_files[0]->kind() == File::kindArchiveLibrary)) ||
-        (_files[0]->kind() == File::kindSharedLibrary)) {
+    if (_files[0]->kind() == File::kindArchiveLibrary &&
+        !_attributes._isWholeArchive) {
       _nextFileIndex = 0;
     }
   }
Index: include/lld/ReaderWriter/ELFLinkingContext.h
===================================================================
--- include/lld/ReaderWriter/ELFLinkingContext.h
+++ include/lld/ReaderWriter/ELFLinkingContext.h
@@ -224,13 +224,6 @@
     return _rpathLinkList;
   }
 
-  bool addUndefinedAtomsFromSharedLibrary(const SharedLibraryFile *s) override {
-    if (_undefinedAtomsFromFile.find(s) != _undefinedAtomsFromFile.end())
-      return false;
-    _undefinedAtomsFromFile[s] = true;
-    return true;
-  }
-
   const std::map<std::string, uint64_t> &getAbsoluteSymbols() const {
     return _absoluteSymbols;
   }
@@ -281,7 +274,6 @@
   StringRef _soname;
   StringRefVector _rpathList;
   StringRefVector _rpathLinkList;
-  std::map<const SharedLibraryFile *, bool> _undefinedAtomsFromFile;
   std::map<std::string, uint64_t> _absoluteSymbols;
 };
 } // end namespace lld
Index: lib/Core/Resolver.cpp
===================================================================
--- lib/Core/Resolver.cpp
+++ lib/Core/Resolver.cpp
@@ -69,40 +69,31 @@
 } // namespace
 
 void Resolver::handleFile(const File &file) {
-  const SharedLibraryFile *sharedLibraryFile =
-      dyn_cast<SharedLibraryFile>(&file);
-
   for (const DefinedAtom *atom : file.defined())
     doDefinedAtom(*atom);
-  bool progress = false;
-
-  if (!sharedLibraryFile ||
-      _context.addUndefinedAtomsFromSharedLibrary(sharedLibraryFile)) {
-    progress = !file.undefined().empty();
-
-    for (const UndefinedAtom *undefAtom : file.undefined()) {
-      doUndefinedAtom(*undefAtom);
-      // If the undefined symbol has an alternative name, try to resolve the
-      // symbol with the name to give it a second chance. This feature is used
-      // for COFF "weak external" symbol.
-      if (!_symbolTable.isDefined(undefAtom->name())) {
-        if (const UndefinedAtom *fallbackAtom = undefAtom->fallback()) {
-          doUndefinedAtom(*fallbackAtom);
-          _symbolTable.addReplacement(undefAtom, fallbackAtom);
-        }
+
+  for (const UndefinedAtom *undefAtom : file.undefined()) {
+    doUndefinedAtom(*undefAtom);
+    // If the undefined symbol has an alternative name, try to resolve the
+    // symbol with the name to give it a second chance. This feature is used
+    // for COFF "weak external" symbol.
+    if (!_symbolTable.isDefined(undefAtom->name())) {
+      if (const UndefinedAtom *fallbackAtom = undefAtom->fallback()) {
+        doUndefinedAtom(*fallbackAtom);
+        _symbolTable.addReplacement(undefAtom, fallbackAtom);
       }
     }
   }
-  for (const SharedLibraryAtom *shlibAtom : file.sharedLibrary())
-    doSharedLibraryAtom(*shlibAtom);
 
-  for (const AbsoluteAtom *absAtom : file.absolute())
-    doAbsoluteAtom(*absAtom);
+  for (const SharedLibraryAtom *atom : file.sharedLibrary())
+    doSharedLibraryAtom(*atom);
+  for (const AbsoluteAtom *atom : file.absolute())
+    doAbsoluteAtom(*atom);
 
   // If we make some progress on linking, notify that fact to the input file
   // manager, because it may want to know that for --start-group/end-group.
-  progress = progress || !file.sharedLibrary().empty() ||
-             !file.absolute().empty() || !file.defined().empty();
+  bool progress = !file.defined().empty() || !file.sharedLibrary().empty() ||
+      !file.absolute().empty() || !file.undefined().empty();
   if (progress) {
     _context.inputGraph().notifyProgress();
   }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3280.1.patch
Type: text/x-patch
Size: 5175 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140403/91729978/attachment.bin>