llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> Here's an example crash that we've seen sporadically over the years: ``` 0 libsystem_kernel.dylib 0x19d392388 __pthread_kill + 8 1 libsystem_pthread.dylib 0x19d3cb88c pthread_kill + 296 2 libsystem_c.dylib 0x19d29cd04 raise + 32 3 LLDB 0x112cbbc94 SignalHandler(int, __siginfo*, void*) + 324 4 libsystem_platform.dylib 0x19d4056a4 _sigtramp + 56 5 LLDB 0x110dcbd38 clang::TextDiagnosticPrinter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 1216 6 LLDB 0x110dcbd38 clang::TextDiagnosticPrinter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 1216 7 LLDB 0x10de12128 ClangDiagnosticManagerAdapter::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) + 332 8 LLDB 0x1121eb3dc clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const + 200 9 LLDB 0x1121e67a0 clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) + 100 10 LLDB 0x111d766cc IsStructurallyEquivalent(clang::StructuralEquivalenceContext&, clang::FieldDecl*, clang::FieldDecl*, clang::QualType) + 1568 11 LLDB 0x111d71b54 IsStructurallyEquivalent(clang::StructuralEquivalenceContext&, clang::RecordDecl*, clang::RecordDecl*) + 2076 12 LLDB 0x111d6e448 clang::StructuralEquivalenceContext::Finish() + 204 13 LLDB 0x111d6e1e0 clang::StructuralEquivalenceContext::IsEquivalent(clang::Decl*, clang::Decl*) + 32 14 LLDB 0x111d3b788 clang::ASTNodeImporter::IsStructuralMatch(clang::Decl*, clang::Decl*, bool, bool) + 168 15 LLDB 0x111d404e0 clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*) + 1300 16 LLDB 0x111d5cae0 clang::ASTImporter::ImportImpl(clang::Decl*) + 24 17 LLDB 0x10ddf30bc lldb_private::ClangASTImporter::ASTImporterDelegate::ImportImpl(clang::Decl*) + 308 18 LLDB 0x111d48140 clang::ASTImporter::Import(clang::Decl*) + 984 19 LLDB 0x10ddee9dc lldb_private::ClangASTImporter::CopyDecl(clang::ASTContext*, clang::Decl*) + 100 20 LLDB 0x10ddfab40 lldb_private::ClangASTSource::FindExternalLexicalDecls(clang::DeclContext const*, llvm::function_ref<bool (clang::Decl::Kind)>, llvm::SmallVectorImpl<clang::Decl*>&) + 1692 21 LLDB 0x111e1cb84 clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const + 180 22 LLDB 0x111e1df50 clang::DeclContext::buildLookup() + 204 23 LLDB 0x111e1dcf4 clang::DeclContext::makeDeclVisibleInContextWithFlags(clang::NamedDecl*, bool, bool) + 504 24 LLDB 0x111d3b01c clang::ASTNodeImporter::ImportDeclContext(clang::DeclContext*, bool) + 724 25 LLDB 0x111d62d10 clang::ASTImporter::ImportDefinition(clang::Decl*) + 428 26 LLDB 0x10ddf1cb0 lldb_private::ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(clang::Decl*, clang::Decl*) + 524 27 LLDB 0x10ddef3c8 (anonymous namespace)::CompleteTagDeclsScope::~CompleteTagDeclsScope() + 616 28 LLDB 0x10ddef6dc lldb_private::ClangASTImporter::DeportDecl(clang::ASTContext*, clang::Decl*) + 436 29 LLDB 0x10ddec3dc lldb_private::ASTResultSynthesizer::CommitPersistentDecls() + 236 30 LLDB 0x10de1091c lldb_private::ClangExpressionParser::ParseInternal(lldb_private::DiagnosticManager&, clang::CodeCompleteConsumer*, unsigned int, unsigned int) + 2292 31 LLDB 0x10de21238 lldb_private::ClangUserExpression::TryParse(lldb_private::DiagnosticManager&, lldb_private::ExecutionContext&, lldb_private::ExecutionPolicy, bool, bool) + 328 ... ``` Here `ASTImporter::IsStructurallyEquivalent` is trying to emit a diagnostic using `ClangExpressionParser`'s `ClangDiagnosticManagerAdapter`. But `TextDiagnosticPrinter::TextDiag` seems to be `nullptr`. This can only happen when we call `HandleDiagnostic` on `ClangDiagnosticManagerAdapter` after we called `EndSourceFile`. Specifically, it looks like when moving a type from `Expression` AST to `Scratch` AST (in `CommitPersistentDecls`), something went wrong, so the ASTImporter tried to emit a diagnostic, but we already called `EndSourceFile` at that point. This patch moves the call to `ResetManager` to before `CommitPersistentDecls`, so if we called `HandleDiagnostic`, we would correctly short-circuit out of it. This seems to have been intended anyway based on this comment: https://github.com/llvm/llvm-project/blob/cecdff92838f3c049548e3445a15d8c9c7a49205/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp#L200-L204 But something must have broken that during a refactor. I'm not 100% sure how best to test this because we need a scenario where moving a type into the scratch AST fails, but the expression itself succeeded. rdar://159647906 --- Full diff: https://github.com/llvm/llvm-project/pull/160074.diff 1 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+4-2) ``````````diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 6885977baa24e..924953cc43fa2 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -1307,6 +1307,10 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager, m_compiler->setSema(nullptr); adapter->EndSourceFile(); + // Creating persistent variables can trigger diagnostic emission. + // Make sure we reset the manager so we don't get asked to handle + // diagnostics after we finished parsing. + adapter->ResetManager(); unsigned num_errors = adapter->getNumErrors(); @@ -1322,8 +1326,6 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager, type_system_helper->CommitPersistentDecls(); } - adapter->ResetManager(); - return num_errors; } `````````` </details> https://github.com/llvm/llvm-project/pull/160074 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
