[llvm-branch-commits] [clang-tools-extra] 230b872 - [clangd] Increase stack size of the new threads on macOS

[Sam McCall via llvm-branch-commits]

Author: Sam McCall
Date: 2020-06-10T14:08:14+02:00
New Revision: 230b872c290d8c80a60accb06f3267e0703d0c49

URL: https://github.com/llvm/llvm-project/commit/230b872c290d8c80a60accb06f3267e0703d0c49
DIFF: https://github.com/llvm/llvm-project/commit/230b872c290d8c80a60accb06f3267e0703d0c49.diff

LOG: [clangd] Increase stack size of the new threads on macOS

Summary: By default it's 512K, which is way to small for clang parser to run on. There is no way to do it via platform-independent API, so it's implemented via pthreads directly in clangd/Threading.cpp.

Fixes https://github.com/clangd/clangd/issues/273

Patch by Dmitry Kozhevnikov!

Reviewers: ilya-biryukov, sammccall, arphaman

Reviewed By: ilya-biryukov, sammccall, arphaman

Subscribers: dexonsmith, umanwizard, jfb, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D50993

(cherry picked from commit 69a39dc1f0d08ea43bac03a87bb8bff3937ce2e7)

Added: 
    

Modified: 
    clang-tools-extra/clangd/Threading.cpp
    clang-tools-extra/clangd/unittests/ClangdTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Threading.cpp b/clang-tools-extra/clangd/Threading.cpp
index 016a90297c32..0a605719fce2 100644
--- a/clang-tools-extra/clangd/Threading.cpp
+++ b/clang-tools-extra/clangd/Threading.cpp
@@ -1,5 +1,6 @@
 #include "Threading.h"
 #include "Trace.h"
+#include "clang/Basic/Stack.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Threading.h"
@@ -84,16 +85,16 @@ void AsyncTaskRunner::runAsync(const llvm::Twine &Name,
     }
   });
 
-  std::thread(
-      [](std::string Name, decltype(Action) Action, decltype(CleanupTask)) {
-        llvm::set_thread_name(Name);
-        Action();
-        // Make sure function stored by Action is destroyed before CleanupTask
-        // is run.
-        Action = nullptr;
-      },
-      Name.str(), std::move(Action), std::move(CleanupTask))
-      .detach();
+  auto Task = [Name = Name.str(), Action = std::move(Action),
+               Cleanup = std::move(CleanupTask)]() mutable {
+    llvm::set_thread_name(Name);
+    Action();
+    // Make sure function stored by ThreadFunc is destroyed before Cleanup runs.
+    Action = nullptr;
+  };
+
+  // Ensure our worker threads have big enough stacks to run clang.
+  llvm::llvm_execute_on_thread_async(std::move(Task), clang::DesiredStackSize);
 }
 
 Deadline timeoutSeconds(llvm::Optional<double> Seconds) {

diff  --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
index ab88c8925da0..99fd41e29ab7 100644
--- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -1063,6 +1063,27 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
                                 Field(&CodeCompletion::Scope, "ns::"))));
 }
 
+TEST_F(ClangdVFSTest, TestStackOverflow) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  const char *SourceContents = R"cpp(
+    constexpr int foo() { return foo(); }
+    static_assert(foo());
+  )cpp";
+
+  auto FooCpp = testPath("foo.cpp");
+  FS.Files[FooCpp] = SourceContents;
+
+  Server.addDocument(FooCpp, SourceContents);
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  // check that we got a constexpr depth error, and not crashed by stack
+  // overflow
+  EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang