Skip to content

Commit 230b872

Browse files
committed
[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 clangd/clangd#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 69a39dc)
1 parent d623a06 commit 230b872

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

clang-tools-extra/clangd/Threading.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Threading.h"
22
#include "Trace.h"
3+
#include "clang/Basic/Stack.h"
34
#include "llvm/ADT/ScopeExit.h"
45
#include "llvm/Support/FormatVariadic.h"
56
#include "llvm/Support/Threading.h"
@@ -84,16 +85,16 @@ void AsyncTaskRunner::runAsync(const llvm::Twine &Name,
8485
}
8586
});
8687

87-
std::thread(
88-
[](std::string Name, decltype(Action) Action, decltype(CleanupTask)) {
89-
llvm::set_thread_name(Name);
90-
Action();
91-
// Make sure function stored by Action is destroyed before CleanupTask
92-
// is run.
93-
Action = nullptr;
94-
},
95-
Name.str(), std::move(Action), std::move(CleanupTask))
96-
.detach();
88+
auto Task = [Name = Name.str(), Action = std::move(Action),
89+
Cleanup = std::move(CleanupTask)]() mutable {
90+
llvm::set_thread_name(Name);
91+
Action();
92+
// Make sure function stored by ThreadFunc is destroyed before Cleanup runs.
93+
Action = nullptr;
94+
};
95+
96+
// Ensure our worker threads have big enough stacks to run clang.
97+
llvm::llvm_execute_on_thread_async(std::move(Task), clang::DesiredStackSize);
9798
}
9899

99100
Deadline timeoutSeconds(llvm::Optional<double> Seconds) {

clang-tools-extra/clangd/unittests/ClangdTests.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,27 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
10631063
Field(&CodeCompletion::Scope, "ns::"))));
10641064
}
10651065

1066+
TEST_F(ClangdVFSTest, TestStackOverflow) {
1067+
MockFSProvider FS;
1068+
ErrorCheckingDiagConsumer DiagConsumer;
1069+
MockCompilationDatabase CDB;
1070+
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
1071+
1072+
const char *SourceContents = R"cpp(
1073+
constexpr int foo() { return foo(); }
1074+
static_assert(foo());
1075+
)cpp";
1076+
1077+
auto FooCpp = testPath("foo.cpp");
1078+
FS.Files[FooCpp] = SourceContents;
1079+
1080+
Server.addDocument(FooCpp, SourceContents);
1081+
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
1082+
// check that we got a constexpr depth error, and not crashed by stack
1083+
// overflow
1084+
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
1085+
}
1086+
10661087
} // namespace
10671088
} // namespace clangd
10681089
} // namespace clang

0 commit comments

Comments
 (0)