[LLD] [RFC] Don't close the memory mapping when exiting

[mstorsjo]

Summary

If exiting due to an error in a multithreaded section of the code, there may be other threads still running, accessing the memory mapped area. Just remove the temporary file (on unix, just remove it, while it has got a remove-on-exit flag set on windows) and close its file descriptor, while keeping the memory mapping intact.

This implements @pcc's suggestion on fixing PR38647.

I'm a little weary about the fact that the existing comment in FileOutputBuffer.cpp says "Close the mapping before deleting the temp file, so that the removal succeeds.", while this approach taken by this patch seems to work fine (at least on windows 10). OTOH we don't actually delete the file on windows, we just close the FD.

[ruiu]

LGTM

I think I like this approach more than https://reviews.llvm.org/D51096 which seems more artificial than this one. This patch should prevent crashing when exiting. If lld worked like that from beginning, we probably wouldn't have been discussing adding D51096.

lld/Common/ErrorHandler.cpp

 
 ErrorHandler &lld::errorHandler() {
   static ErrorHandler Handler;
   return Handler;
 }
 
 void lld::exitLld(int Val) {
   // Delete the output buffer so that any tempory file is deleted.
-  errorHandler().OutputBuffer.reset();
+  if (errorHandler().OutputBuffer)
+    errorHandler().OutputBuffer->discard();
 
   // Dealloc/destroy ManagedStatic variables before calling
   // _exit(). In a non-LTO build, this is a nop. In an LTO
   // build allows us to get the output of -time-passes.
   llvm_shutdown();
 
   outs().flush();
   errs().flush();

llvm/include/llvm/Support/FileOutputBuffer.h

public:
   /// initially requested.
   virtual Error commit() = 0;
 
   /// If this object was previously committed, the destructor just deletes
   /// this object.  If this object was not committed, the destructor
   /// deallocates the buffer and the target file is never written.
   virtual ~FileOutputBuffer() {}
 
+  /// This removes the temporary file (unless it already was committed)
+  /// but keeps the memory mapping alive.
+  virtual void discard() {}
+
 protected:
   FileOutputBuffer(StringRef Path) : FinalPath(Path) {}
 
   std::string FinalPath;
 };
 } // end namespace llvm
 
 #endif

llvm/lib/Support/FileOutputBuffer.cpp

public:
 
   ~OnDiskBuffer() override {
     // Close the mapping before deleting the temp file, so that the removal
     // succeeds.
     Buffer.reset();
     consumeError(Temp.discard());
   }
 
+  void discard() override {
+    // Delete the temp file if it still was open, but keeping the mapping
+    // active. (Closing the FD while keeping the mapping alive seems to work
+    // on windows as well.)
+    consumeError(Temp.discard());
+  }
+
 private:
   std::unique_ptr<fs::mapped_file_region> Buffer;
   fs::TempFile Temp;
 };
 
 // A FileOutputBuffer which keeps data in memory and writes to the final
 // output file on commit(). This is used only when we cannot use OnDiskBuffer.
 class InMemoryBuffer : public FileOutputBuffer {