Dmitry.Kozhevnikov created this revision.
Dmitry.Kozhevnikov added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric.

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.


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
@@ -42,6 +42,56 @@
   SlotsChanged.notify_one();
 }
 
+namespace {
+template <typename Data>
+using UserFnPtr = void (*)(Data);
+
+template <typename Data>
+struct ThreadInfo {
+  UserFnPtr<Data> UserFn;
+  Data UserData;
+};
+} // namespace
+
+template <typename Data>
+static void executeOnThreadAndDetach(UserFnPtr<Data> UserFn, Data UserData) {
+#if !defined(__APPLE__)
+  std::thread([UserFn](Data UserData) { UserFn(std::move(UserData)); },
+              std::move(UserData))
+      .detach();
+#else
+  // On macOS, there is no way to change the default 512K stack size for threads
+  // other than main using the std::thread API. 512K, however, is way to small
+  // for the parser to run. Here we're using pthreads API to set the 8 Mb
+  // stack size, as it's the default for the main thread in modern Linux/macOS
+  // distributions, so it's probably the most well-tested value.
+
+  pthread_attr_t Attr;
+  if (::pthread_attr_init(&Attr) != 0)
+    return;
+
+  auto AttrGuard =
+      llvm::make_scope_exit([&] { ::pthread_attr_destroy(&Attr); });
+
+  if (::pthread_attr_setstacksize(&Attr, 8 * 1024 * 1024) != 0)
+    return;
+
+  std::unique_ptr<ThreadInfo<Data>> Info(
+      new ThreadInfo<Data>{UserFn, std::move(UserData)});
+  pthread_t Thread;
+  auto ThreadFun = [](void *Arg) -> void * {
+    std::unique_ptr<ThreadInfo<Data>> TI(static_cast<ThreadInfo<Data> *>(Arg));
+    TI->UserFn(std::move(TI->UserData));
+    return nullptr;
+  };
+  if (::pthread_create(&Thread, &Attr, ThreadFun, Info.get()) != 0)
+    return;
+
+  // now it would be destroyed in the new thread
+  Info.release();
+#endif
+}
+
 AsyncTaskRunner::~AsyncTaskRunner() { wait(); }
 
 bool AsyncTaskRunner::wait(Deadline D) const {
@@ -67,16 +117,22 @@
     }
   });
 
-  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();
+  struct Data {
+    std::string Name;
+    decltype(Action) Action;
+    decltype(CleanupTask) CleanupTask;
+  };
+  Data UserData{Name.str(), std::move(Action), std::move(CleanupTask)};
+
+  auto Func = [](Data UserData) {
+    llvm::set_thread_name(UserData.Name);
+    UserData.Action();
+    // Make sure function stored by Action is destroyed before CleanupTask
+    // is run.
+    UserData.Action = nullptr;
+  };
+
+  executeOnThreadAndDetach<Data>(Func, std::move(UserData));
 }
 
 Deadline timeoutSeconds(llvm::Optional<double> Seconds) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to