[PECOFF] Make WinLinkDriver::parse() and allocate*() functions thread-safe.

Looks like a major cause of instability on Windows is this thread-safety bug.

llvm-svn: 203889

3 files changed, 38 insertions, 8 deletions

diff --git a/lld/include/lld/Driver/Driver.h b/lld/include/lld/Driver/Driver.h
index 7ea6029efe7..4d283c10381 100644
--- a/lld/include/lld/Driver/Driver.h
+++ b/lld/include/lld/Driver/Driver.h
@@ -115,6 +115,9 @@ public:
                     bool isDirective = false);
 
 private:
+  static bool doParse(int argc, const char *argv[], PECOFFLinkingContext &info,
+                      raw_ostream &diagnostics, bool isDirective);
+
   WinLinkDriver() LLVM_DELETED_FUNCTION;
 };
 
diff --git a/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h b/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h
index 73ebb710392..15c48257e7d 100644
--- a/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h
+++ b/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h
@@ -19,6 +19,7 @@
 #include "llvm/Support/COFF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Mutex.h"
 
 #include <map>
 #include <set>
@@ -35,9 +36,10 @@ class Group;
 class PECOFFLinkingContext : public LinkingContext {
 public:
   PECOFFLinkingContext()
-      : _baseAddress(invalidBaseAddress), _stackReserve(1024 * 1024),
-        _stackCommit(4096), _heapReserve(1024 * 1024), _heapCommit(4096),
-        _noDefaultLibAll(false), _sectionDefaultAlignment(4096),
+      : _mutex(true), _allocMutex(false), _baseAddress(invalidBaseAddress),
+        _stackReserve(1024 * 1024), _stackCommit(4096),
+        _heapReserve(1024 * 1024), _heapCommit(4096), _noDefaultLibAll(false),
+        _sectionDefaultAlignment(4096),
         _subsystem(llvm::COFF::IMAGE_SUBSYSTEM_UNKNOWN),
         _machineType(llvm::COFF::IMAGE_FILE_MACHINE_I386), _imageVersion(0, 0),
         _minOSVersion(6, 0), _nxCompat(true), _largeAddressAware(false),
@@ -234,7 +236,9 @@ public:
   const std::set<ExportDesc> &getDllExports() const { return _dllExports; }
 
   StringRef allocate(StringRef ref) const {
+    _allocMutex.acquire();
     char *x = _allocator.Allocate<char>(ref.size() + 1);
+    _allocMutex.release();
     memcpy(x, ref.data(), ref.size());
     x[ref.size()] = '\0';
     return x;
@@ -242,13 +246,18 @@ public:
 
   ArrayRef<uint8_t> allocate(ArrayRef<uint8_t> array) const {
     size_t size = array.size();
+    _allocMutex.acquire();
     uint8_t *p = _allocator.Allocate<uint8_t>(size);
+    _allocMutex.release();
     memcpy(p, array.data(), size);
     return ArrayRef<uint8_t>(p, p + array.size());
   }
 
   template <typename T> T &allocateCopy(const T &x) const {
-    return *new (_allocator) T(x);
+    _allocMutex.acquire();
+    T *r = new (_allocator) T(x);
+    _allocMutex.release();
+    return *r;
   }
 
   virtual bool hasInputGraph() { return !!_inputGraph; }
@@ -256,6 +265,9 @@ public:
   void setLibraryGroup(Group *group) { _libraryGroup = group; }
   Group *getLibraryGroup() const { return _libraryGroup; }
 
+  void lock() { _mutex.acquire(); }
+  void unlock() { _mutex.release(); }
+
 protected:
   /// Method to create a internal file for the entry symbol
   std::unique_ptr<File> createEntrySymbolFile() const override;
@@ -268,7 +280,10 @@ private:
     invalidBaseAddress = UINT64_MAX,
     pe32DefaultBaseAddress = 0x400000U,
     pe32PlusDefaultBaseAddress = 0x140000000U
- };
+  };
+
+  llvm::sys::SmartMutex<false> _mutex;
+  mutable llvm::sys::SmartMutex<false> _allocMutex;
 
   // The start address for the program. The default value for the executable is
   // 0x400000, but can be altered using /base command line option.
diff --git a/lld/lib/Driver/WinLinkDriver.cpp b/lld/lib/Driver/WinLinkDriver.cpp
index 9d8f0890893..393bc99e4d1 100644
--- a/lld/lib/Driver/WinLinkDriver.cpp
+++ b/lld/lib/Driver/WinLinkDriver.cpp
@@ -767,9 +767,9 @@ bool WinLinkDriver::linkPECOFF(int argc, const char **argv, raw_ostream &diag) {
   return link(context, diag);
 }
 
-bool WinLinkDriver::parse(int argc, const char *argv[],
-                          PECOFFLinkingContext &ctx, raw_ostream &diag,
-                          bool isReadingDirectiveSection) {
+bool WinLinkDriver::doParse(int argc, const char *argv[],
+                            PECOFFLinkingContext &ctx, raw_ostream &diag,
+                            bool isReadingDirectiveSection) {
   std::map<StringRef, StringRef> failIfMismatchMap;
   // Parse the options.
   std::unique_ptr<llvm::opt::InputArgList> parsedArgs =
@@ -1251,4 +1251,16 @@ bool WinLinkDriver::parse(int argc, const char *argv[],
   return ctx.validate(diag);
 }
 
+// Parse may be called from multiple threads simultaneously to parse .drectve
+// sections. doParse() is not thread-safe because it mutates the context
+// object. This function wraps doParse() with a mutex.
+bool WinLinkDriver::parse(int argc, const char *argv[],
+                          PECOFFLinkingContext &ctx, raw_ostream &diag,
+                          bool isReadingDirectiveSection) {
+  ctx.lock();
+  bool r = doParse(argc, argv, ctx, diag, isReadingDirectiveSection);
+  ctx.unlock();
+  return r;
+}
+
 } // namespace lld