0x1eaf updated this revision to Diff 379124. 0x1eaf added a comment. Updated the AST worker crash handler to avoid `std::string` copy: used a char pointer instead.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109506/new/ https://reviews.llvm.org/D109506 Files: clang-tools-extra/clangd/JSONTransport.cpp clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/support/CMakeLists.txt clang-tools-extra/clangd/support/ThreadCrashReporter.cpp clang-tools-extra/clangd/support/ThreadCrashReporter.h clang-tools-extra/clangd/test/crash.test clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/clangd/unittests/CMakeLists.txt clang-tools-extra/clangd/unittests/ThreadCrashReporterTests.cpp
Index: clang-tools-extra/clangd/unittests/ThreadCrashReporterTests.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/unittests/ThreadCrashReporterTests.cpp @@ -0,0 +1,78 @@ +///===- ThreadCrashReporterTests.cpp - Thread local signal handling tests -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "support/ThreadCrashReporter.h" +#include "support/Threading.h" +#include "llvm/Support/Signals.h" +#include "gtest/gtest.h" +#include <csignal> +#include <string> + +namespace clang { +namespace clangd { + +namespace { + +static void infoSignalHandler() { ThreadCrashReporter::runCrashHandlers(); } + +TEST(ThreadCrashReporterTest, All) { +#if defined(_WIN32) + // Simulate signals on Windows for unit testing purposes. + // The `crash.test` lit test checks the end-to-end integration. + auto SignalCurrentThread = []() { infoSignalHandler(); }; +#else + llvm::sys::SetInfoSignalFunction(&infoSignalHandler); + auto SignalCurrentThread = []() { raise(SIGUSR1); }; +#endif + + AsyncTaskRunner Runner; + auto SignalAnotherThread = [&]() { + Runner.runAsync("signal another thread", SignalCurrentThread); + Runner.wait(); + }; + + bool Called; + { + ThreadCrashReporter ScopedReporter([&Called]() { Called = true; }); + // Check handler gets called when a signal gets delivered to the current + // thread. + Called = false; + SignalCurrentThread(); + EXPECT_TRUE(Called); + + // Check handler does not get called when another thread gets signalled. + Called = false; + SignalAnotherThread(); + EXPECT_FALSE(Called); + } + // Check handler does not get called when the reporter object goes out of + // scope. + Called = false; + SignalCurrentThread(); + EXPECT_FALSE(Called); + + std::string Order = ""; + { + ThreadCrashReporter ScopedReporter([&Order] { Order.push_back('a'); }); + { + ThreadCrashReporter ScopedReporter([&Order] { Order.push_back('b'); }); + SignalCurrentThread(); + } + // Check that handlers are called in LIFO order. + EXPECT_EQ(Order, "ba"); + + // Check that current handler is the only one after the nested scope is + // over. + SignalCurrentThread(); + EXPECT_EQ(Order, "baa"); + } +} + +} // namespace +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/unittests/CMakeLists.txt +++ clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -88,6 +88,7 @@ TestIndex.cpp TestTU.cpp TestWorkspace.cpp + ThreadCrashReporterTests.cpp TidyProviderTests.cpp TypeHierarchyTests.cpp URITests.cpp Index: clang-tools-extra/clangd/tool/ClangdMain.cpp =================================================================== --- clang-tools-extra/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -24,6 +24,7 @@ #include "refactor/Rename.h" #include "support/Path.h" #include "support/Shutdown.h" +#include "support/ThreadCrashReporter.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" #include "clang/Format/Format.h" @@ -679,6 +680,8 @@ llvm::InitializeAllTargetInfos(); llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); + llvm::sys::AddSignalHandler( + [](void *) { ThreadCrashReporter::runCrashHandlers(); }, nullptr); llvm::sys::SetInterruptFunction(&requestShutdown); llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { OS << versionString() << "\n" Index: clang-tools-extra/clangd/test/crash.test =================================================================== --- /dev/null +++ clang-tools-extra/clangd/test/crash.test @@ -0,0 +1,5 @@ +# Overflow the recursive json parser, prevent `yes` error due to broken pipe and `clangd` SIGSEGV from being treated as a failure. +# RUN: (yes '[' || :) | head -n 50000 | (clangd --input-style=delimited 2>&1 || :) | FileCheck %s +# CHECK: Signalled while processing message: +# CHECK-NEXT: [ +# CHECK-NEXT: [ Index: clang-tools-extra/clangd/support/ThreadCrashReporter.h =================================================================== --- /dev/null +++ clang-tools-extra/clangd/support/ThreadCrashReporter.h @@ -0,0 +1,58 @@ +//===--- ThreadCrashReporter.h - Thread local signal handling ----*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_THREADCRASHREPORTER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_THREADCRASHREPORTER_H + +#include "llvm/ADT/FunctionExtras.h" + +namespace clang { +namespace clangd { + +/// Allows setting per-thread abort/kill signal callbacks, to print additional +/// information about the crash depending on which thread got signalled. +class ThreadCrashReporter { +public: + using SignalCallback = llvm::unique_function<void(void)>; + + /// Registers the callback as the first one in thread-local callback chain. + /// + /// Asserts if the current thread's callback is already set. The callback is + /// likely to be invoked in a signal handler. Most LLVM signal handling is not + /// strictly async-signal-safe. However reporters should avoid accessing data + /// structures likely to be in a bad state on crash. + ThreadCrashReporter(SignalCallback ThreadLocalCallback); + /// Resets the current thread's callback to nullptr. + ~ThreadCrashReporter(); + + /// Moves are disabled to ease nesting and escaping considerations. + ThreadCrashReporter(ThreadCrashReporter &&RHS) = delete; + ThreadCrashReporter(const ThreadCrashReporter &) = delete; + ThreadCrashReporter &operator=(ThreadCrashReporter &&) = delete; + ThreadCrashReporter &operator=(const ThreadCrashReporter &) = delete; + + /// Calls all currently-active ThreadCrashReporters for the current thread. + /// + /// To be called from sys::AddSignalHandler() callback. Any signal filtering + /// is the responsibility of the caller. While this function is intended to be + /// called from signal handlers, it is not strictly async-signal-safe - see + /// constructor comment. + /// + /// When several reporters are nested, the callbacks are called in LIFO order. + static void runCrashHandlers(); + +private: + SignalCallback Callback; + /// Points to the next reporter up the stack. + ThreadCrashReporter *Next; +}; + +} // namespace clangd +} // namespace clang + +#endif Index: clang-tools-extra/clangd/support/ThreadCrashReporter.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/support/ThreadCrashReporter.cpp @@ -0,0 +1,54 @@ +//===--- ThreadCrashReporter.cpp - Thread local signal handling --*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "support/ThreadCrashReporter.h" +#include "llvm/Support/Signals.h" +#include "llvm/Support/ThreadLocal.h" + +namespace clang { +namespace clangd { + +static thread_local ThreadCrashReporter *CurrentReporter = nullptr; + +void ThreadCrashReporter::runCrashHandlers() { + // No signal handling is done here on top of what AddSignalHandler() does: + // on Windows the signal handling is implmented via + // SetUnhandledExceptionFilter() which is thread-directed, and on Unix + // platforms the handlers are only called for KillSigs out of which only + // SIGQUIT seems to be process-directed and would be delivered to any thread + // that is not blocking it, but if the thread it gets delivered to has a + // ThreadCrashReporter installed during the interrupt â it seems reasonable to + // let it run and print the thread's context information. + + // Call the reporters in LIFO order. + ThreadCrashReporter *Reporter = CurrentReporter; + while (Reporter) { + Reporter->Callback(); + Reporter = Reporter->Next; + } +} + +ThreadCrashReporter::ThreadCrashReporter(SignalCallback ThreadLocalCallback) + : Callback(std::move(ThreadLocalCallback)), Next(nullptr) { + this->Next = CurrentReporter; + CurrentReporter = this; + // Don't reorder subsequent operations: whatever comes after might crash and + // we want the the crash handler to see the reporter values we just set. + std::atomic_signal_fence(std::memory_order_seq_cst); +} + +ThreadCrashReporter::~ThreadCrashReporter() { + assert(CurrentReporter == this); + CurrentReporter = this->Next; + // Don't reorder subsequent operations: whatever comes after might crash and + // we want the the crash handler to see the reporter values we just set. + std::atomic_signal_fence(std::memory_order_seq_cst); +} + +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/support/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/support/CMakeLists.txt +++ clang-tools-extra/clangd/support/CMakeLists.txt @@ -24,6 +24,7 @@ MemoryTree.cpp Path.cpp Shutdown.cpp + ThreadCrashReporter.cpp Threading.cpp ThreadsafeFS.cpp Trace.cpp Index: clang-tools-extra/clangd/TUScheduler.cpp =================================================================== --- clang-tools-extra/clangd/TUScheduler.cpp +++ clang-tools-extra/clangd/TUScheduler.cpp @@ -58,6 +58,7 @@ #include "support/Logger.h" #include "support/MemoryTree.h" #include "support/Path.h" +#include "support/ThreadCrashReporter.h" #include "support/Threading.h" #include "support/Trace.h" #include "clang/Frontend/CompilerInvocation.h" @@ -76,6 +77,7 @@ #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "llvm/Support/Threading.h" +#include "llvm/Support/raw_ostream.h" #include <algorithm> #include <atomic> #include <chrono> @@ -898,6 +900,40 @@ startTask(Name, std::move(Task), /*Update=*/None, Invalidation); } +/// To be called from ThreadCrashReporter's signal handler. +static void crashDumpCompileCommand(llvm::raw_ostream &OS, + const tooling::CompileCommand &Command) { + OS << " Filename: " << Command.Filename << "\n"; + OS << " Directory: " << Command.Directory << "\n"; + OS << " Command Line:"; + for (auto &Arg : Command.CommandLine) { + OS << " " << Arg; + } + OS << "\n"; +} + +/// To be called from ThreadCrashReporter's signal handler. +static void crashDumpFileContents(llvm::raw_ostream &OS, + const std::string &Contents) { + // Avoid flooding the terminal with source code by default, but allow clients + // to opt in. Use an env var to preserve backwards compatibility of the + // command line interface, while allowing it to be set outside the clangd + // launch site for more flexibility. + if (getenv("CLANGD_CRASH_DUMP_SOURCE")) { + OS << " Contents:\n"; + OS << Contents << "\n"; + } +} + +/// To be called from ThreadCrashReporter's signal handler. +static void crashDumpParseInputs(llvm::raw_ostream &OS, + const ParseInputs &FileInputs) { + auto &Command = FileInputs.CompileCommand; + crashDumpCompileCommand(OS, Command); + OS << " Version: " << FileInputs.Version << "\n"; + crashDumpFileContents(OS, FileInputs.Contents); +} + void PreambleThread::build(Request Req) { assert(Req.CI && "Got preamble request with null compiler invocation"); const ParseInputs &Inputs = Req.Inputs; @@ -925,6 +961,11 @@ FileName, Inputs.Version, LatestBuild->Version); } + ThreadCrashReporter ScopedReporter([&Inputs]() { + llvm::errs() << "Signalled while building preamble\n"; + crashDumpParseInputs(llvm::errs(), Inputs); + }); + LatestBuild = clang::clangd::buildPreamble( FileName, *Req.CI, Inputs, StoreInMemory, [this, Version(Inputs.Version)](ASTContext &Ctx, @@ -1281,6 +1322,11 @@ Status.ASTActivity.Name = CurrentRequest->Name; }); WithContext WithProvidedContext(ContextProvider(FileName)); + ThreadCrashReporter ScopedReporter([this]() { + const char *Name = CurrentRequest ? CurrentRequest->Name.c_str() : ""; + llvm::errs() << "Signalled during AST worker action: " << Name << "\n"; + crashDumpParseInputs(llvm::errs(), FileInputs); + }); CurrentRequest->Action(); } @@ -1606,6 +1652,11 @@ Ctx = Context::current().derive(kFileBeingProcessed, std::string(File)), Action = std::move(Action), this]() mutable { + ThreadCrashReporter ScopedReporter([&Name, &Contents, &Command]() { + llvm::errs() << "Signalled during action: " << Name << "\n"; + crashDumpCompileCommand(llvm::errs(), Command); + crashDumpFileContents(llvm::errs(), Contents); + }); std::shared_ptr<const PreambleData> Preamble; if (Consistency == PreambleConsistency::Stale) { // Wait until the preamble is built for the first time, if preamble Index: clang-tools-extra/clangd/JSONTransport.cpp =================================================================== --- clang-tools-extra/clangd/JSONTransport.cpp +++ clang-tools-extra/clangd/JSONTransport.cpp @@ -10,9 +10,11 @@ #include "support/Cancellation.h" #include "support/Logger.h" #include "support/Shutdown.h" +#include "support/ThreadCrashReporter.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/Errno.h" #include "llvm/Support/Error.h" +#include "llvm/Support/Threading.h" #include <system_error> namespace clang { @@ -109,6 +111,11 @@ return llvm::errorCodeToError( std::error_code(errno, std::system_category())); if (readRawMessage(JSON)) { + ThreadCrashReporter ScopedReporter([&JSON]() { + auto &OS = llvm::errs(); + OS << "Signalled while processing message:\n"; + OS << JSON << "\n"; + }); if (auto Doc = llvm::json::parse(JSON)) { vlog(Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc); if (!handleMessage(std::move(*Doc), Handler))
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
