[lld] Reject --read-workers when lld is built without thread support

[DavidSpickett]

Also expand the #ifdef to remove unused code in this configuration. As suggested in #147134 (comment).

I have also:

• Expanded the error message to explain why it's not allowed.
• Added a test for the error.
• Marked the original test as unsupported when threads are disabled.

Fixes issues we have had on Armv8 with threading disabled where this test would crash every so often.

This change will hopefully be superseded by #157917, but that has been in review a long time and I want to make the bot stable again.

I could just disable the test, but I'd like lld to function properly in general in the meantime too.

Co-authored-by: John Holdsworth github@johnholdsworth.com

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-macho

Author: David Spickett (DavidSpickett)

Changes

Also expand the #ifdef to remove unused code in this configuration. As suggested in #147134 (comment).

I have also:

• Expanded the error message to explain why it's not allowed.
• Added a test for the error.
• Marked the original test as unsupported when threads are disabled.

Fixes issues we have had on Armv8 with threading disabled where this test would crash every so often.

This change will hopefully be superseded by #157917, but that has been in review a long time and I want to make the bot stable again.

I could just disable the test, but I'd like lld to function properly in general in the meantime too.

Co-authored-by: John Holdsworth <github@johnholdsworth.com>

---

Full diff: https://github.com/llvm/llvm-project/pull/163925.diff

6 Files Affected:

• (modified) lld/MachO/Driver.cpp (+9-2)
• (modified) lld/test/CMakeLists.txt (+1)
• (added) lld/test/MachO/read-workers-no-thread-support.s (+10)
• (modified) lld/test/MachO/read-workers.s (+1-1)
• (modified) lld/test/lit.cfg.py (+3)
• (modified) lld/test/lit.site.cfg.py.in (+1)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index bfd35aef72b58..9b67db9fa55cf 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -291,6 +291,7 @@ struct DeferredFile {
 };
 using DeferredFiles = std::vector<DeferredFile>;
 
+#if LLVM_ENABLE_THREADS
 class SerialBackgroundQueue {
   std::deque<std::function<void()>> queue;
   std::thread *running;
@@ -359,7 +360,6 @@ void multiThreadedPageInBackground(DeferredFiles &deferred) {
       (void)t;
     }
   };
-#if LLVM_ENABLE_THREADS
   { // Create scope for waiting for the taskGroup
     std::atomic_size_t index = 0;
     llvm::parallel::TaskGroup taskGroup;
@@ -373,7 +373,6 @@ void multiThreadedPageInBackground(DeferredFiles &deferred) {
         }
       });
   }
-#endif
 #ifndef NDEBUG
   auto dt = high_resolution_clock::now() - t0;
   if (Process::GetEnv("LLD_MULTI_THREAD_PAGE"))
@@ -390,6 +389,7 @@ static void multiThreadedPageIn(const DeferredFiles &deferred) {
     multiThreadedPageInBackground(files);
   });
 }
+#endif
 
 static InputFile *processFile(std::optional<MemoryBufferRef> buffer,
                               DeferredFiles *archiveContents, StringRef path,
@@ -1430,6 +1430,7 @@ static void createFiles(const InputArgList &args) {
     }
   }
 
+#if LLVM_ENABLE_THREADS
   if (config->readWorkers) {
     multiThreadedPageIn(deferredFiles);
 
@@ -1447,6 +1448,7 @@ static void createFiles(const InputArgList &args) {
     for (auto *archive : archives)
       archive->addLazySymbols();
   }
+#endif
 }
 
 static void gatherInputSections() {
@@ -1834,6 +1836,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   }
 
   if (auto *arg = args.getLastArg(OPT_read_workers)) {
+#if LLVM_ENABLE_THREADS
     StringRef v(arg->getValue());
     unsigned workers = 0;
     if (!llvm::to_integer(v, workers, 0))
@@ -1841,6 +1844,10 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
             ": expected a non-negative integer, but got '" + arg->getValue() +
             "'");
     config->readWorkers = workers;
+#else
+    error(arg->getSpelling() +
+          ": option unavailable because lld was not built with thread support");
+#endif
   }
   if (auto *arg = args.getLastArg(OPT_threads_eq)) {
     StringRef v(arg->getValue());
diff --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index abc8ea75da180..1bd3ad7e1765b 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 llvm_canonicalize_cmake_booleans(
   ENABLE_BACKTRACES
+  LLVM_ENABLE_THREADS
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_ZSTD
   LLVM_ENABLE_LIBXML2
diff --git a/lld/test/MachO/read-workers-no-thread-support.s b/lld/test/MachO/read-workers-no-thread-support.s
new file mode 100644
index 0000000000000..16256be42af73
--- /dev/null
+++ b/lld/test/MachO/read-workers-no-thread-support.s
@@ -0,0 +1,10 @@
+# REQUIRES: x86 && !thread_support
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+
+# RUN: not %lld --read-workers=1 %t.o -o /dev/null
+
+# CHECK: error: --read-workers=: option unavailable because lld was built without thread support
+
+.globl _main
+_main:
+  ret
diff --git a/lld/test/MachO/read-workers.s b/lld/test/MachO/read-workers.s
index 6f0ea4d894408..4d2f88c2a757c 100644
--- a/lld/test/MachO/read-workers.s
+++ b/lld/test/MachO/read-workers.s
@@ -1,4 +1,4 @@
-# REQUIRES: x86
+# REQUIRES: x86 && thread_support
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 
 ## A non-negative integer is allowed.
diff --git a/lld/test/lit.cfg.py b/lld/test/lit.cfg.py
index 336945729954e..906afc52a0d09 100644
--- a/lld/test/lit.cfg.py
+++ b/lld/test/lit.cfg.py
@@ -182,3 +182,6 @@
 # ELF tests expect the default target for ld.lld to be ELF.
 if config.ld_lld_default_mingw:
     config.excludes.append("ELF")
+
+if config.enable_threads:
+    config.available_features.add("thread_support")
\ No newline at end of file
diff --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index bb99976005543..703d3b1fd5337 100644
--- a/lld/test/lit.site.cfg.py.in
+++ b/lld/test/lit.site.cfg.py.in
@@ -26,6 +26,7 @@ config.ld_lld_default_mingw = @LLD_DEFAULT_LD_LLD_IS_MINGW@
 config.build_examples = @LLVM_BUILD_EXAMPLES@
 config.has_plugins = @LLVM_ENABLE_PLUGINS@
 config.linked_bye_extension = @LLVM_BYE_LINK_INTO_TOOLS@
+config.enable_threads = @LLVM_ENABLE_THREADS@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)

[llvm-ci]

LLVM Buildbot has detected a new failure on builder ppc64le-lld-multistage-test running on ppc64le-lld-multistage-test while building lld at step 12 "build-stage2-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/16688

Here is the relevant piece of the build log for the reference

Step 12 (build-stage2-unified-tree) failure: build (failure) (timed out)
...
1139.171 [57/10/6679] Linking CXX executable bin/llvm-isel-fuzzer
1139.190 [57/9/6680] Linking CXX executable bin/dsymutil
1139.425 [57/8/6681] Linking CXX executable bin/clang-linker-wrapper
1139.560 [57/7/6682] Linking CXX executable bin/llvm-split
1139.563 [57/6/6683] Linking CXX executable unittests/MI/MITests
1140.056 [57/5/6684] Linking CXX executable tools/lld/unittests/AsLibAll/LLDAsLibAllTests
1140.072 [57/4/6685] Linking CXX executable bin/opt
1140.331 [57/3/6686] Linking CXX executable bin/lld
1157.122 [57/2/6687] Building CXX object tools/bugpoint-passes/CMakeFiles/BugpointPasses.dir/TestPasses.cpp.o
1157.678 [56/2/6688] Linking CXX shared module lib/BugpointPasses.so
command timed out: 1200 seconds without output running [b'ninja'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=2358.485816