Skip to content

Commit e257c0a

Browse files
committed
[clang][clangd] Ensure the stack bottom before building AST
`clang::runWithSufficientStackSpace` requires the address of the initial stack bottom to prevent potential stack overflows. In addition, add a fallback to ASTFrontendAction in case any client forgets to call it when not through CompilerInstance::ExecuteAction, which is rare. Fixes clangd/clangd#1745. Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D158967
1 parent 208f9a2 commit e257c0a

File tree

6 files changed

+34
-1
lines changed

6 files changed

+34
-1
lines changed

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "support/MemoryTree.h"
3535
#include "support/ThreadsafeFS.h"
3636
#include "support/Trace.h"
37+
#include "clang/Basic/Stack.h"
3738
#include "clang/Format/Format.h"
3839
#include "clang/Lex/Preprocessor.h"
3940
#include "clang/Tooling/CompilationDatabase.h"
@@ -52,8 +53,8 @@
5253
#include <optional>
5354
#include <string>
5455
#include <type_traits>
55-
#include <vector>
5656
#include <utility>
57+
#include <vector>
5758

5859
namespace clang {
5960
namespace clangd {
@@ -112,6 +113,7 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
112113
FIndex(FIndex),
113114
// shared_ptr extends lifetime
114115
Stdlib(Stdlib)]() mutable {
116+
clang::noteBottomOfStack();
115117
IndexFileIn IF;
116118
IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
117119
if (Stdlib->isBest(LO))

clang-tools-extra/clangd/TUScheduler.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
#include "support/ThreadCrashReporter.h"
6464
#include "support/Threading.h"
6565
#include "support/Trace.h"
66+
#include "clang/Basic/Stack.h"
6667
#include "clang/Frontend/CompilerInvocation.h"
6768
#include "clang/Tooling/CompilationDatabase.h"
6869
#include "llvm/ADT/FunctionExtras.h"
@@ -464,6 +465,10 @@ class PreambleThread {
464465
}
465466

466467
void run() {
468+
// We mark the current as the stack bottom so that clang running on this
469+
// thread can notice the stack usage and prevent stack overflow with best
470+
// efforts. Same applies to other calls thoughout clangd.
471+
clang::noteBottomOfStack();
467472
while (true) {
468473
std::optional<PreambleThrottlerRequest> Throttle;
469474
{
@@ -1383,6 +1388,7 @@ void ASTWorker::startTask(llvm::StringRef Name,
13831388
}
13841389

13851390
void ASTWorker::run() {
1391+
clang::noteBottomOfStack();
13861392
while (true) {
13871393
{
13881394
std::unique_lock<std::mutex> Lock(Mutex);
@@ -1777,6 +1783,7 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
17771783
Ctx = Context::current().derive(FileBeingProcessed,
17781784
std::string(File)),
17791785
Action = std::move(Action), this]() mutable {
1786+
clang::noteBottomOfStack();
17801787
ThreadCrashReporter ScopedReporter([&Name, &Contents, &Command]() {
17811788
llvm::errs() << "Signalled during preamble action: " << Name << "\n";
17821789
crashDumpCompileCommand(llvm::errs(), Command);

clang-tools-extra/clangd/index/Background.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "support/Trace.h"
3131
#include "clang/Basic/SourceLocation.h"
3232
#include "clang/Basic/SourceManager.h"
33+
#include "clang/Basic/Stack.h"
3334
#include "clang/Frontend/FrontendAction.h"
3435
#include "llvm/ADT/ArrayRef.h"
3536
#include "llvm/ADT/DenseSet.h"
@@ -108,6 +109,7 @@ BackgroundIndex::BackgroundIndex(
108109
for (unsigned I = 0; I < Opts.ThreadPoolSize; ++I) {
109110
ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1),
110111
[this, Ctx(Context::current().clone())]() mutable {
112+
clang::noteBottomOfStack();
111113
WithContext BGContext(std::move(Ctx));
112114
Queue.work([&] { Rebuilder.idle(); });
113115
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: cp %s %t.cpp
2+
// RUN: not clangd -check=%t.cpp 2>&1 | FileCheck -strict-whitespace %s
3+
4+
// CHECK: [template_recursion_depth_exceeded]
5+
6+
template <typename... T>
7+
constexpr int f(T... args) {
8+
return f(0, args...);
9+
}
10+
11+
int main() {
12+
auto i = f();
13+
}

clang-tools-extra/clangd/tool/ClangdMain.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "support/ThreadCrashReporter.h"
3030
#include "support/ThreadsafeFS.h"
3131
#include "support/Trace.h"
32+
#include "clang/Basic/Stack.h"
3233
#include "clang/Format/Format.h"
3334
#include "llvm/ADT/SmallString.h"
3435
#include "llvm/ADT/StringRef.h"
@@ -710,6 +711,9 @@ enum class ErrorResultCode : int {
710711
};
711712

712713
int clangdMain(int argc, char *argv[]) {
714+
// Clang could run on the main thread. e.g., when the flag '-check' or '-sync'
715+
// is enabled.
716+
clang::noteBottomOfStack();
713717
llvm::InitializeAllTargetInfos();
714718
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
715719
llvm::sys::AddSignalHandler(

clang/lib/Frontend/FrontendAction.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "clang/Basic/FileEntry.h"
1616
#include "clang/Basic/LangStandard.h"
1717
#include "clang/Basic/Sarif.h"
18+
#include "clang/Basic/Stack.h"
1819
#include "clang/Frontend/ASTUnit.h"
1920
#include "clang/Frontend/CompilerInstance.h"
2021
#include "clang/Frontend/FrontendDiagnostic.h"
@@ -1155,6 +1156,10 @@ void ASTFrontendAction::ExecuteAction() {
11551156
CompilerInstance &CI = getCompilerInstance();
11561157
if (!CI.hasPreprocessor())
11571158
return;
1159+
// This is a fallback: If the client forgets to invoke this, we mark the
1160+
// current stack as the bottom. Though not optimal, this could help prevent
1161+
// stack overflow during deep recursion.
1162+
clang::noteBottomOfStack();
11581163

11591164
// FIXME: Move the truncation aspect of this into Sema, we delayed this till
11601165
// here so the source manager would be initialized.

0 commit comments

Comments
 (0)