[lld][MachO] Follow-up to use madvise() for threaded file page-in. (llvm#157917)

Further to
llvm#147134 (comment),
switch to use the madvise() api to page in mmap'd files and

1) All new code compiled in #if LLVM_ENABLE_THREADS is set so it can be
seen where the changes were from this PR.
2) The new PR moves to use madvise() instead of the ad-hoc page
referencing code I wrote which should avoid SIGSEGVs if the buffer is
deallocated.
3) A new property SerialBackgroundQueue().stopAllWork to be used to stop
background workers when there is no further call for them. Usually the
background "page-in" threads have completed first but it seems with this
troublesome test this is not always the case and buffers stored in the
static input file cache are being deallocated while being referenced.

---------

Co-authored-by: James Henderson <[email protected]>

Author: 2 people

lld/MachO/Driver.cpp

@@ -41,6 +41,7 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Parallel.h"
 #include "llvm/Support/Path.h"
@@ -53,6 +54,10 @@
 #include "llvm/TextAPI/Architecture.h"
 #include "llvm/TextAPI/PackedVersion.h"
 
+#if !_WIN32
+#include <sys/mman.h>
+#endif
+
 using namespace llvm;
 using namespace llvm::MachO;
 using namespace llvm::object;
@@ -292,12 +297,13 @@ struct DeferredFile {
 using DeferredFiles = std::vector<DeferredFile>;
 
 #if LLVM_ENABLE_THREADS
-class SerialBackgroundQueue {
+class SerialBackgroundWorkQueue {
   std::deque<std::function<void()>> queue;
   std::thread *running;
   std::mutex mutex;
 
 public:
+  std::atomic_bool stopAllWork = false;
   void queueWork(std::function<void()> work) {
     mutex.lock();
     if (running && queue.empty()) {
@@ -312,7 +318,7 @@ class SerialBackgroundQueue {
       queue.emplace_back(std::move(work));
       if (!running)
         running = new std::thread([&]() {
-          while (true) {
+          while (!stopAllWork) {
             mutex.lock();
             if (queue.empty()) {
               mutex.unlock();
@@ -331,6 +337,8 @@ class SerialBackgroundQueue {
   }
 };
 
+static SerialBackgroundWorkQueue pageInQueue;
+
 // Most input files have been mapped but not yet paged in.
 // This code forces the page-ins on multiple threads so
 // the process is not stalled waiting on disk buffer i/o.
@@ -339,51 +347,61 @@ void multiThreadedPageInBackground(DeferredFiles &deferred) {
   static const size_t largeArchive = 10 * 1024 * 1024;
 #ifndef NDEBUG
   using namespace std::chrono;
-  std::atomic_int numDeferedFilesTouched = 0;
   static std::atomic_uint64_t totalBytes = 0;
+  std::atomic_int numDeferedFilesAdvised = 0;
   auto t0 = high_resolution_clock::now();
 #endif
 
   auto preloadDeferredFile = [&](const DeferredFile &deferredFile) {
     const StringRef &buff = deferredFile.buffer.getBuffer();
     if (buff.size() > largeArchive)
       return;
+
 #ifndef NDEBUG
     totalBytes += buff.size();
-    numDeferedFilesTouched += 1;
+    numDeferedFilesAdvised += 1;
 #endif
-
+#if _WIN32
     // Reference all file's mmap'd pages to load them into memory.
-    for (const char *page = buff.data(), *end = page + buff.size(); page < end;
-         page += pageSize) {
+    for (const char *page = buff.data(), *end = page + buff.size();
+         page < end && !pageInQueue.stopAllWork; page += pageSize) {
       [[maybe_unused]] volatile char t = *page;
       (void)t;
     }
+#else
+#define DEBUG_TYPE "lld-madvise"
+    auto aligned =
+        llvm::alignDown(reinterpret_cast<uintptr_t>(buff.data()), pageSize);
+    if (madvise((void *)aligned, buff.size(), MADV_WILLNEED) < 0)
+      LLVM_DEBUG(llvm::dbgs() << "madvise error: " << strerror(errno) << "\n");
+#undef DEBUG_TYPE
+#endif
   };
+
   { // Create scope for waiting for the taskGroup
     std::atomic_size_t index = 0;
     llvm::parallel::TaskGroup taskGroup;
     for (int w = 0; w < config->readWorkers; w++)
       taskGroup.spawn([&index, &preloadDeferredFile, &deferred]() {
-        while (true) {
+        while (!pageInQueue.stopAllWork) {
           size_t localIndex = index.fetch_add(1);
           if (localIndex >= deferred.size())
             break;
           preloadDeferredFile(deferred[localIndex]);
         }
       });
   }
+
 #ifndef NDEBUG
   auto dt = high_resolution_clock::now() - t0;
   if (Process::GetEnv("LLD_MULTI_THREAD_PAGE"))
     llvm::dbgs() << "multiThreadedPageIn " << totalBytes << "/"
-                 << numDeferedFilesTouched << "/" << deferred.size() << "/"
+                 << numDeferedFilesAdvised << "/" << deferred.size() << "/"
                  << duration_cast<milliseconds>(dt).count() / 1000. << "\n";
 #endif
 }
 
 static void multiThreadedPageIn(const DeferredFiles &deferred) {
-  static SerialBackgroundQueue pageInQueue;
   pageInQueue.queueWork([=]() {
     DeferredFiles files = deferred;
     multiThreadedPageInBackground(files);
@@ -489,7 +507,7 @@ static InputFile *processFile(std::optional<MemoryBufferRef> buffer,
             continue;
           }
 
-          if (archiveContents)
+          if (config->readWorkers && archiveContents)
             archiveContents->push_back({path, isLazy, *mb});
           if (!hasObjCSection(*mb))
             continue;
@@ -1447,6 +1465,8 @@ static void createFiles(const InputArgList &args) {
       multiThreadedPageIn(archiveContents);
     for (auto *archive : archives)
       archive->addLazySymbols();
+
+    pageInQueue.stopAllWork = true;
   }
 #endif
 }
@@ -1845,8 +1865,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
             "'");
     config->readWorkers = workers;
 #else
-    error(arg->getSpelling() +
-          ": option unavailable because lld was not built with thread support");
+    warn(arg->getSpelling() +
+         ": option unavailable because lld was not built with thread support");
 #endif
   }
   if (auto *arg = args.getLastArg(OPT_threads_eq)) {

lld/MachO/InputFiles.cpp

@@ -217,7 +217,8 @@ std::optional<MemoryBufferRef> macho::readFile(StringRef path) {
   if (entry != cachedReads.end())
     return entry->second;
 
-  ErrorOr<std::unique_ptr<MemoryBuffer>> mbOrErr = MemoryBuffer::getFile(path);
+  ErrorOr<std::unique_ptr<MemoryBuffer>> mbOrErr =
+      MemoryBuffer::getFile(path, false, /*RequiresNullTerminator=*/false);
   if (std::error_code ec = mbOrErr.getError()) {
     error("cannot open " + path + ": " + ec.message());
     return std::nullopt;

lld/test/MachO/read-workers.s

@@ -1,7 +1,4 @@
 # REQUIRES: x86 && thread_support
-## Sometimes fails, particularly in an ASAN build, do not run until
-## https://github.com/llvm/llvm-project/pull/157917 addresses the cause.
-# UNSUPPORTED: target={{.*}}
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 
 ## A non-negative integer is allowed.

llvm/lib/Object/Archive.cpp

@@ -582,7 +582,8 @@ Expected<StringRef> Archive::Child::getBuffer() const {
   if (!FullNameOrErr)
     return FullNameOrErr.takeError();
   const std::string &FullName = *FullNameOrErr;
-  ErrorOr<std::unique_ptr<MemoryBuffer>> Buf = MemoryBuffer::getFile(FullName);
+  ErrorOr<std::unique_ptr<MemoryBuffer>> Buf =
+      MemoryBuffer::getFile(FullName, false, /*RequiresNullTerminator=*/false);
   if (std::error_code EC = Buf.getError())
     return errorCodeToError(EC);
   Parent->ThinBuffers.push_back(std::move(*Buf));