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

[Dmitry via Phabricator via cfe-commits]

Dmitry.Kozhevnikov updated this revision to Diff 161954.
Dmitry.Kozhevnikov added a comment.

I've moved the platform-specific implementation to LLVM/Support/Threading and created https://reviews.llvm.org/D51103.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50993

Files:
  clangd/Threading.cpp
  unittests/clangd/ClangdTests.cpp


Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -963,6 +963,27 @@
                                        Field(&CodeCompletion::Name, "baz")));
 }
 
+TEST_F(ClangdVFSTest, TestStackOverflow) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  const auto 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
Index: clangd/Threading.cpp
===================================================================
--- clangd/Threading.cpp
+++ 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"
@@ -67,16 +68,25 @@
     }
   });
 
-  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();
+  // Manually capture Action and CleanupTask for the lack of C++14 generalized
+  // lambda captures
+  struct Callable {
+    std::string ThreadName;
+    decltype(Action) ThreadFunc;
+    decltype(CleanupTask) Cleanup;
+
+    void operator()() {
+      llvm::set_thread_name(ThreadName);
+      ThreadFunc();
+      // Make sure function stored by ThreadFunc is destroyed before Cleanup is
+      // run.
+      ThreadFunc = nullptr;
+    }
+  };
+
+  llvm::llvm_execute_on_thread_async(
+      Callable{Name.str(), std::move(Action), std::move(CleanupTask)},
+      clang::DesiredStackSize);
 }
 
 Deadline timeoutSeconds(llvm::Optional<double> Seconds) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50993.161954.patch
Type: text/x-patch
Size: 2449 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180822/3f3b5227/attachment-0001.bin>