[lld] r230791 - PECOFF: Move a call of WinLinkDriver::parse from FileCOFF::doParse to FileCOFF::beforeLink

Rui Ueyama ruiu at google.com
Fri Feb 27 12:39:20 PST 2015


Author: ruiu
Date: Fri Feb 27 14:39:20 2015
New Revision: 230791

URL: http://llvm.org/viewvc/llvm-project?rev=230791&view=rev
Log:
PECOFF: Move a call of WinLinkDriver::parse from FileCOFF::doParse to FileCOFF::beforeLink

In doParse, we shouldn't do anything that has side effects. That function may be
called speculatively and possibly in parallel.

We called WinLinkDriver::parse from doParse to parse a command line in a .drectve
section. The parse function updates a linking context object, so it has many side
effects. It was not safe to call that function from doParse. beforeLink is a
function for a File object to do something that has side effects. Moving a call
of WinLinkDriver::parse to there.

Modified:
    lld/trunk/include/lld/Driver/Driver.h
    lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
    lld/trunk/lib/Driver/WinLinkDriver.cpp
    lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp

Modified: lld/trunk/include/lld/Driver/Driver.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Driver/Driver.h?rev=230791&r1=230790&r2=230791&view=diff
==============================================================================
--- lld/trunk/include/lld/Driver/Driver.h (original)
+++ lld/trunk/include/lld/Driver/Driver.h Fri Feb 27 14:39:20 2015
@@ -127,15 +127,13 @@ public:
   /// Returns true iff there was an error.
   static bool parse(int argc, const char *argv[], PECOFFLinkingContext &info,
                     raw_ostream &diag = llvm::errs(),
-                    bool isDirective = false,
-                    std::set<StringRef> *undefinedSymbols = nullptr);
+                    bool isDirective = false);
 
   // Same as parse(), but restricted to the context of directives.
   static bool parseDirectives(int argc, const char *argv[],
                     PECOFFLinkingContext &info,
-                    raw_ostream &diag = llvm::errs(),
-                    std::set<StringRef> *undefinedSymbols = nullptr) {
-    return parse(argc, argv, info, diag, true, undefinedSymbols);
+                    raw_ostream &diag = llvm::errs()) {
+    return parse(argc, argv, info, diag, true);
   }
 
 private:

Modified: lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h?rev=230791&r1=230790&r2=230791&view=diff
==============================================================================
--- lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h (original)
+++ lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h Fri Feb 27 14:39:20 2015
@@ -84,7 +84,7 @@ public:
   };
 
   typedef bool (*ParseDirectives)(int, const char **, PECOFFLinkingContext &,
-                                  raw_ostream &, std::set<StringRef> *);
+                                  raw_ostream &);
 
   /// \brief Casting support
   static inline bool classof(const LinkingContext *info) { return true; }

Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=230791&r1=230790&r2=230791&view=diff
==============================================================================
--- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)
+++ lld/trunk/lib/Driver/WinLinkDriver.cpp Fri Feb 27 14:39:20 2015
@@ -881,8 +881,7 @@ bool WinLinkDriver::linkPECOFF(int argc,
 
 bool WinLinkDriver::parse(int argc, const char *argv[],
                           PECOFFLinkingContext &ctx, raw_ostream &diag,
-                          bool isReadingDirectiveSection,
-                          std::set<StringRef> *undefinedSymbols) {
+                          bool isReadingDirectiveSection) {
   // Parse may be called from multiple threads simultaneously to parse .drectve
   // sections. This function is not thread-safe because it mutates the context
   // object. So acquire the lock.
@@ -1231,14 +1230,8 @@ bool WinLinkDriver::parse(int argc, cons
     ctx.setDosStub(contents);
   }
 
-  for (auto *arg : parsedArgs->filtered(OPT_incl)) {
-    StringRef sym = ctx.allocate(arg->getValue());
-    if (isReadingDirectiveSection) {
-      undefinedSymbols->insert(sym);
-    } else {
-      ctx.addInitialUndefinedSymbol(sym);
-    }
-  }
+  for (auto *arg : parsedArgs->filtered(OPT_incl))
+    ctx.addInitialUndefinedSymbol(ctx.allocate(arg->getValue()));
 
   if (parsedArgs->hasArg(OPT_noentry))
     ctx.setHasEntry(false);

Modified: lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp?rev=230791&r1=230790&r2=230791&view=diff
==============================================================================
--- lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp (original)
+++ lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp Fri Feb 27 14:39:20 2015
@@ -114,8 +114,7 @@ public:
 
   AliasAtom *createAlias(StringRef name, const DefinedAtom *target, int cnt);
   void createAlternateNameAtoms();
-  std::error_code parseDirectiveSection(
-    StringRef directives, std::set<StringRef> *undefinedSymbols);
+  std::error_code parseDirectiveSection(StringRef directives);
 
   mutable llvm::BumpPtrAllocator _alloc;
 
@@ -332,20 +331,6 @@ std::error_code FileCOFF::doParse() {
     return NativeReaderError::conflicting_target_machine;
   }
 
-  // The set to contain the symbols specified as arguments of
-  // /INCLUDE option.
-  std::set<StringRef> undefinedSymbols;
-
-  // Interpret .drectve section if the section has contents.
-  // Read .drectve section if exists.
-  ArrayRef<uint8_t> directives;
-  if (std::error_code ec = getSectionContents(".drectve", directives))
-    return ec;
-  if (!directives.empty())
-    if (std::error_code ec = parseDirectiveSection(
-          ArrayRefToString(directives), &undefinedSymbols))
-      return ec;
-
   if (std::error_code ec = getReferenceArch(_referenceArch))
     return ec;
 
@@ -374,22 +359,38 @@ std::error_code FileCOFF::doParse() {
                  << " is not compatible with SEH.\n";
     return llvm::object::object_error::parse_failed;
   }
+  return std::error_code();
+}
+
+void FileCOFF::beforeLink() {
+  // Acquire the mutex to mutate _ctx.
+  std::lock_guard<std::recursive_mutex> lock(_ctx.getMutex());
+  std::set<StringRef> undefSyms;
+
+  // Interpret .drectve section if the section has contents.
+  ArrayRef<uint8_t> directives;
+  if (getSectionContents(".drectve", directives))
+    return;
+  if (!directives.empty()) {
+    std::set<StringRef> orig;
+    for (StringRef sym : _ctx.initialUndefinedSymbols())
+      orig.insert(sym);
+    if (parseDirectiveSection(ArrayRefToString(directives)))
+      return;
+    for (StringRef sym : _ctx.initialUndefinedSymbols())
+      if (orig.count(sym) == 0)
+        undefSyms.insert(sym);
+  }
 
   // Add /INCLUDE'ed symbols to the file as if they existed in the
   // file as undefined symbols.
-  for (StringRef sym : undefinedSymbols)
+  for (StringRef sym : undefSyms)
     addUndefinedSymbol(sym);
 
   // One can define alias symbols using /alternatename:<sym>=<sym> option.
   // The mapping for /alternatename is in the context object. This helper
   // function iterate over defined atoms and create alias atoms if needed.
   createAlternateNameAtoms();
-  return std::error_code();
-}
-
-void FileCOFF::beforeLink() {
-  // Acquire the mutex to mutate _ctx.
-  std::lock_guard<std::recursive_mutex> lock(_ctx.getMutex());
 
   // In order to emit SEH table, all input files need to be compatible with
   // SEH. Disable SEH if the file being read is not compatible.
@@ -890,8 +891,7 @@ void FileCOFF::createAlternateNameAtoms(
 // The section mainly contains /defaultlib (-l in Unix), but can contain any
 // options as long as they are valid.
 std::error_code
-FileCOFF::parseDirectiveSection(StringRef directives,
-                                std::set<StringRef> *undefinedSymbols) {
+FileCOFF::parseDirectiveSection(StringRef directives) {
   DEBUG(llvm::dbgs() << ".drectve: " << directives << "\n");
 
   // Split the string into tokens, as the shell would do for argv.
@@ -908,8 +908,7 @@ FileCOFF::parseDirectiveSection(StringRe
   llvm::raw_string_ostream stream(errorMessage);
   PECOFFLinkingContext::ParseDirectives parseDirectives =
     _ctx.getParseDirectives();
-  bool parseFailed = !parseDirectives(argc, argv, _ctx, stream,
-                                      undefinedSymbols);
+  bool parseFailed = !parseDirectives(argc, argv, _ctx, stream);
   stream.flush();
   // Print error message if error.
   if (parseFailed) {





More information about the llvm-commits mailing list