Author: teemperor
Date: Wed Oct 9 01:30:06 2019
New Revision: 374145
URL: http://llvm.org/viewvc/llvm-project?rev=374145&view=rev
Log:
[lldb] Don't crash when the ASTImporter produces diagnostics but instead log
them.
When playing with the C++ module prototype I noticed I can get LLDB to crash
by making a result type that depends on __make_integer_seq (a BuiltinTemplate)
which is not supported by the ASTImporter yet. This causes the ASTImporter to
emit
a diagnostic when copying the type to the ScratchASTContext. As deporting the
result
type is done after we are done parsing and the Clang's diagnostic engine
asserts that
it can only be used during parsing, it crashes LLDB while trying to render the
diagnostic
in the HandleDiagnostic method of ClangDiagnosticManagerAdapter.
This patch just moves the HandleDiagnostic call to Clang behind our check that
we still
have a DiagnosticManager (which we remove after parsing) which prevents the
assert
from firing. We also shouldn't ignore such diagnostics, so I added a log
statement for
them.
There doesn't seem to way to test this as these diagnostic only happen when we
copy
a node that's not supported by the ASTImporter which should never happen once
we can copy everything with the ASTImporter, so every test case we add here will
eventually become invalid.
(Note that most of this diff is just whitespace changes as we now use an early
exit
instead of a huge 'if' block).
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=374145&r1=374144&r2=374145&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Wed Oct 9 01:30:06 2019
@@ -162,51 +162,66 @@ public:
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) override {
+ if (!m_manager) {
+ // We have no DiagnosticManager before/after parsing but we still could
+ // receive diagnostics (e.g., by the ASTImporter failing to copy decls
+ // when we move the expression result ot the ScratchASTContext). Let's at
+ // least log these diagnostics until we find a way to properly render
+ // them and display them to the user.
+ Log
*log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+ if (log) {
+ llvm::SmallVector<char, 32> diag_str;
+ Info.FormatDiagnostic(diag_str);
+ diag_str.push_back('\0');
+ const char *plain_diag = diag_str.data();
+ LLDB_LOG(log, "Received diagnostic outside parsing: {0}", plain_diag);
+ }
+ return;
+ }
+
// Render diagnostic message to m_output.
m_output.clear();
m_passthrough->HandleDiagnostic(DiagLevel, Info);
m_os->flush();
- if (m_manager) {
- lldb_private::DiagnosticSeverity severity;
- bool make_new_diagnostic = true;
-
- switch (DiagLevel) {
- case DiagnosticsEngine::Level::Fatal:
- case DiagnosticsEngine::Level::Error:
- severity = eDiagnosticSeverityError;
- break;
- case DiagnosticsEngine::Level::Warning:
- severity = eDiagnosticSeverityWarning;
- break;
- case DiagnosticsEngine::Level::Remark:
- case DiagnosticsEngine::Level::Ignored:
- severity = eDiagnosticSeverityRemark;
- break;
- case DiagnosticsEngine::Level::Note:
- m_manager->AppendMessageToDiagnostic(m_output);
- make_new_diagnostic = false;
- }
- if (make_new_diagnostic) {
- // ClangDiagnostic messages are expected to have no whitespace/newlines
- // around them.
- std::string stripped_output = llvm::StringRef(m_output).trim();
-
- ClangDiagnostic *new_diagnostic =
- new ClangDiagnostic(stripped_output, severity, Info.getID());
- m_manager->AddDiagnostic(new_diagnostic);
-
- // Don't store away warning fixits, since the compiler doesn't have
- // enough context in an expression for the warning to be useful.
- // FIXME: Should we try to filter out FixIts that apply to our
generated
- // code, and not the user's expression?
- if (severity == eDiagnosticSeverityError) {
- size_t num_fixit_hints = Info.getNumFixItHints();
- for (size_t i = 0; i < num_fixit_hints; i++) {
- const clang::FixItHint &fixit = Info.getFixItHint(i);
- if (!fixit.isNull())
- new_diagnostic->AddFixitHint(fixit);
- }
+ lldb_private::DiagnosticSeverity severity;
+ bool make_new_diagnostic = true;
+
+ switch (DiagLevel) {
+ case DiagnosticsEngine::Level::Fatal:
+ case DiagnosticsEngine::Level::Error:
+ severity = eDiagnosticSeverityError;
+ break;
+ case DiagnosticsEngine::Level::Warning:
+ severity = eDiagnosticSeverityWarning;
+ break;
+ case DiagnosticsEngine::Level::Remark:
+ case DiagnosticsEngine::Level::Ignored:
+ severity = eDiagnosticSeverityRemark;
+ break;
+ case DiagnosticsEngine::Level::Note:
+ m_manager->AppendMessageToDiagnostic(m_output);
+ make_new_diagnostic = false;
+ }
+ if (make_new_diagnostic) {
+ // ClangDiagnostic messages are expected to have no whitespace/newlines
+ // around them.
+ std::string stripped_output = llvm::StringRef(m_output).trim();
+
+ ClangDiagnostic *new_diagnostic =
+ new ClangDiagnostic(stripped_output, severity, Info.getID());
+ m_manager->AddDiagnostic(new_diagnostic);
+
+ // Don't store away warning fixits, since the compiler doesn't have
+ // enough context in an expression for the warning to be useful.
+ // FIXME: Should we try to filter out FixIts that apply to our generated
+ // code, and not the user's expression?
+ if (severity == eDiagnosticSeverityError) {
+ size_t num_fixit_hints = Info.getNumFixItHints();
+ for (size_t i = 0; i < num_fixit_hints; i++) {
+ const clang::FixItHint &fixit = Info.getFixItHint(i);
+ if (!fixit.isNull())
+ new_diagnostic->AddFixitHint(fixit);
}
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
