Author: Younan Zhang Date: 2023-09-02T18:53:06+08:00 New Revision: e257c0a9190637e44e292271103a13d70bec4b03
URL: https://github.com/llvm/llvm-project/commit/e257c0a9190637e44e292271103a13d70bec4b03 DIFF: https://github.com/llvm/llvm-project/commit/e257c0a9190637e44e292271103a13d70bec4b03.diff LOG: [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 https://github.com/clangd/clangd/issues/1745. Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D158967 Added: clang-tools-extra/clangd/test/infinite-instantiation.test Modified: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/index/Background.cpp clang-tools-extra/clangd/tool/ClangdMain.cpp clang/lib/Frontend/FrontendAction.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 6610961b2202d1..57a4897a85efb1 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -34,6 +34,7 @@ #include "support/MemoryTree.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" +#include "clang/Basic/Stack.h" #include "clang/Format/Format.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CompilationDatabase.h" @@ -52,8 +53,8 @@ #include <optional> #include <string> #include <type_traits> -#include <vector> #include <utility> +#include <vector> namespace clang { namespace clangd { @@ -112,6 +113,7 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { FIndex(FIndex), // shared_ptr extends lifetime Stdlib(Stdlib)]() mutable { + clang::noteBottomOfStack(); IndexFileIn IF; IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS); if (Stdlib->isBest(LO)) diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index dd2ce16147a5dd..324ba1fc8cb895 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -63,6 +63,7 @@ #include "support/ThreadCrashReporter.h" #include "support/Threading.h" #include "support/Trace.h" +#include "clang/Basic/Stack.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/FunctionExtras.h" @@ -464,6 +465,10 @@ class PreambleThread { } void run() { + // We mark the current as the stack bottom so that clang running on this + // thread can notice the stack usage and prevent stack overflow with best + // efforts. Same applies to other calls thoughout clangd. + clang::noteBottomOfStack(); while (true) { std::optional<PreambleThrottlerRequest> Throttle; { @@ -1383,6 +1388,7 @@ void ASTWorker::startTask(llvm::StringRef Name, } void ASTWorker::run() { + clang::noteBottomOfStack(); while (true) { { std::unique_lock<std::mutex> Lock(Mutex); @@ -1777,6 +1783,7 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File, Ctx = Context::current().derive(FileBeingProcessed, std::string(File)), Action = std::move(Action), this]() mutable { + clang::noteBottomOfStack(); ThreadCrashReporter ScopedReporter([&Name, &Contents, &Command]() { llvm::errs() << "Signalled during preamble action: " << Name << "\n"; crashDumpCompileCommand(llvm::errs(), Command); diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp index cbeb74d145401c..5cde4937fee785 100644 --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -30,6 +30,7 @@ #include "support/Trace.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/Stack.h" #include "clang/Frontend/FrontendAction.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseSet.h" @@ -108,6 +109,7 @@ BackgroundIndex::BackgroundIndex( for (unsigned I = 0; I < Opts.ThreadPoolSize; ++I) { ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this, Ctx(Context::current().clone())]() mutable { + clang::noteBottomOfStack(); WithContext BGContext(std::move(Ctx)); Queue.work([&] { Rebuilder.idle(); }); }); diff --git a/clang-tools-extra/clangd/test/infinite-instantiation.test b/clang-tools-extra/clangd/test/infinite-instantiation.test new file mode 100644 index 00000000000000..85a1b656f49086 --- /dev/null +++ b/clang-tools-extra/clangd/test/infinite-instantiation.test @@ -0,0 +1,13 @@ +// RUN: cp %s %t.cpp +// RUN: not clangd -check=%t.cpp 2>&1 | FileCheck -strict-whitespace %s + +// CHECK: [template_recursion_depth_exceeded] + +template <typename... T> +constexpr int f(T... args) { + return f(0, args...); +} + +int main() { + auto i = f(); +} diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index ca5cced197cd24..f656a8c587c653 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -29,6 +29,7 @@ #include "support/ThreadCrashReporter.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" +#include "clang/Basic/Stack.h" #include "clang/Format/Format.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" @@ -710,6 +711,9 @@ enum class ErrorResultCode : int { }; int clangdMain(int argc, char *argv[]) { + // Clang could run on the main thread. e.g., when the flag '-check' or '-sync' + // is enabled. + clang::noteBottomOfStack(); llvm::InitializeAllTargetInfos(); llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); llvm::sys::AddSignalHandler( diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index f14557c316ec7a..2da25be1b9c7a2 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -15,6 +15,7 @@ #include "clang/Basic/FileEntry.h" #include "clang/Basic/LangStandard.h" #include "clang/Basic/Sarif.h" +#include "clang/Basic/Stack.h" #include "clang/Frontend/ASTUnit.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendDiagnostic.h" @@ -1155,6 +1156,10 @@ void ASTFrontendAction::ExecuteAction() { CompilerInstance &CI = getCompilerInstance(); if (!CI.hasPreprocessor()) return; + // This is a fallback: If the client forgets to invoke this, we mark the + // current stack as the bottom. Though not optimal, this could help prevent + // stack overflow during deep recursion. + clang::noteBottomOfStack(); // FIXME: Move the truncation aspect of this into Sema, we delayed this till // here so the source manager would be initialized. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits