njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, JonasToth, LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Create a PrettyStackTraceEvent that will dump the current check as well as the
bound nodes if a `ClangTidyCheck::check` call results in a segfault.
This should help massively in debugging random crashes users may be
experiencing where the check causing the crash is unknown as well as the source
location of the crash.
There are definitely some issues to iron out here as well as figuring out how
to effectively test an error state.
I did force a check to assert and got this output:
[build] Stack dump:
[build] 0. Program arguments: clang-tidy
llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-concat-nested-namespaces.cpp.tmp.cpp
-fix --checks=-*,modernize-concat-nested-namespaces -header-filter=.*
-config={} -- -I
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output
-std=c++17 -nostdinc++
[build] 1. Processing check modernize-concat-nested-namespaces
[build] Node 'namespace' - NamespaceDecl 0x58f780
<llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/modernize-concat-nested-namespaces.h:1:1,
line:6:1> line:1:11 nn1
[build] `-NamespaceDecl 0x58f7f0 <line:2:1, line:5:1> line:2:11 nn2
[build] `-FunctionDecl 0x58f8a8 <line:4:1, col:8> col:6 t 'void ()'
[build]
[build] 2. <eof> parser at end of file
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118520
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -24,6 +24,7 @@
class SourceManager;
namespace ast_matchers {
class MatchFinder;
+class BoundNodes;
} // namespace ast_matchers
namespace tooling {
class CompilationDatabase;
@@ -61,6 +62,8 @@
}
};
+class CheckStackTraceDebug;
+
/// Every \c ClangTidyCheck reports errors through a \c DiagnosticsEngine
/// provided by this context.
///
@@ -204,12 +207,17 @@
DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
}
+ void onResultEntry(StringRef CheckName,
+ const ast_matchers::BoundNodes &Result) const;
+ void onResultExit() const;
+
private:
// Writes to Stats.
friend class ClangTidyDiagnosticConsumer;
DiagnosticsEngine *DiagEngine;
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
+ std::unique_ptr<CheckStackTraceDebug> Debugger;
std::string CurrentFile;
ClangTidyOptions CurrentOptions;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -22,6 +22,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/Attr.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
@@ -35,6 +36,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Regex.h"
#include <tuple>
#include <utility>
@@ -151,6 +153,51 @@
};
} // end anonymous namespace
+namespace clang {
+namespace tidy {
+class CheckStackTraceDebug : public llvm::PrettyStackTraceEntry {
+public:
+ void print(raw_ostream &OS) const override {
+ if (CurrentCheckName.empty())
+ return;
+ // This should be an assert, but asserts shouldn't be used in signal
+ // handlers
+ if (!CurContext)
+ return;
+ OS << "Processing check " << CurrentCheckName << '\n';
+ CurrentCheckName = "";
+ const clang::ast_matchers::BoundNodes::IDToNodeMap &Map =
+ CurNodes->getMap();
+ if (Map.empty()) {
+ OS << "No bound nodes\n";
+ return;
+ }
+ for (const auto &Item : Map) {
+ OS << "Node '" << Item.first << "' - ";
+ Item.second.dump(OS, *CurContext);
+ OS << "\n";
+ }
+ }
+ void onResultEntry(StringRef CheckName,
+ const ast_matchers::BoundNodes &BoundNodes) {
+ CurrentCheckName = CheckName;
+ CurNodes = &BoundNodes;
+ }
+
+ void onResultLeave() {
+ CurrentCheckName = "";
+ CurNodes = nullptr;
+ }
+ void setContext(const ASTContext &Ctx) { CurContext = &Ctx; }
+ void clearContext() { CurContext = nullptr; }
+
+private:
+ mutable StringRef CurrentCheckName;
+ const ast_matchers::BoundNodes *CurNodes;
+ const ASTContext *CurContext;
+};
+} // namespace tidy
+} // namespace clang
ClangTidyError::ClangTidyError(StringRef CheckName,
ClangTidyError::Level DiagLevel,
StringRef BuildDirectory, bool IsWarningAsError)
@@ -161,7 +208,7 @@
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
bool AllowEnablingAnalyzerAlphaCheckers)
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
- Profile(false),
+ Debugger(std::make_unique<CheckStackTraceDebug>()), Profile(false),
AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
// Before the first translation unit we can get errors related to command-line
// parsing, use empty string for the file name in this case.
@@ -231,6 +278,7 @@
void ClangTidyContext::setASTContext(ASTContext *Context) {
DiagEngine->SetArgToStringFn(&FormatASTNodeDiagnosticArgument, Context);
LangOpts = Context->getLangOpts();
+ Debugger->setContext(*Context);
}
const ClangTidyGlobalOptions &ClangTidyContext::getGlobalOptions() const {
@@ -338,6 +386,11 @@
return Result;
}
+void ClangTidyContext::onResultEntry(
+ StringRef CheckName, const ast_matchers::BoundNodes &Result) const {
+ Debugger->onResultEntry(CheckName, Result);
+}
+void ClangTidyContext::onResultExit() const { Debugger->onResultLeave(); }
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -39,10 +39,12 @@
}
void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) {
+ Context->onResultEntry(CheckName, Result.Nodes);
// For historical reasons, checks don't implement the MatchFinder run()
// callback directly. We keep the run()/check() distinction to avoid interface
// churn, and to allow us to add cross-cutting logic in the future.
check(Result);
+ Context->onResultExit();
}
ClangTidyCheck::OptionsView::OptionsView(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
