Skip to content

Commit 69a39dc

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
1 parent 2a191cf commit 69a39dc

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"
@@ -86,16 +87,16 @@ void AsyncTaskRunner::runAsync(const llvm::Twine &Name,
8687
}
8788
});
8889

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

101102
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
@@ -1065,6 +1065,27 @@ TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
10651065
Field(&CodeCompletion::Scope, "ns::"))));
10661066
}
10671067

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

0 commit comments

Comments
 (0)