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

[Rui Ueyama]

Author: ruiu
Date: Thu Mar 13 23:28:38 2014
New Revision: 203889

URL: http://llvm.org/viewvc/llvm-project?rev=203889&view=rev
Log:
[PECOFF] Make WinLinkDriver::parse() and allocate*() functions thread-safe.

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

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

Modified: lld/trunk/include/lld/Driver/Driver.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Driver/Driver.h?rev=203889&r1=203888&r2=203889&view=diff
==============================================================================
--- lld/trunk/include/lld/Driver/Driver.h (original)
+++ lld/trunk/include/lld/Driver/Driver.h Thu Mar 13 23:28:38 2014
@@ -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;
 };
 

Modified: lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h?rev=203889&r1=203888&r2=203889&view=diff
==============================================================================
--- lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h (original)
+++ lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h Thu Mar 13 23:28:38 2014
@@ -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.

Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=203889&r1=203888&r2=203889&view=diff
==============================================================================
--- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)
+++ lld/trunk/lib/Driver/WinLinkDriver.cpp Thu Mar 13 23:28:38 2014
@@ -767,9 +767,9 @@ bool WinLinkDriver::linkPECOFF(int argc,
   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, cons
   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