From ab1ec7cd3f611bfeeea05d85b276ca0025449300 Mon Sep 17 00:00:00 2001 From: Augusto Noronha Date: Fri, 27 Mar 2020 21:31:46 -0300 Subject: [PATCH 1/2] Fix crash when dyn_casting a potentially null value --- .../Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp index 3b4d3b764389d..26b819836d601 100644 --- a/lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp @@ -244,7 +244,7 @@ void SwiftUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) { self_type = ToCompilerType(object_type.getPointer()); // Handle weak self. - if (auto *ref_type = llvm::dyn_cast( + if (auto *ref_type = llvm::dyn_cast_or_null( GetSwiftType(self_type).getPointer())) { if (ref_type->getOwnership() == swift::ReferenceOwnership::Weak) { m_is_class = true; From 0718eea2e2e55f030616a00ab15bd2a2e7a17e08 Mon Sep 17 00:00:00 2001 From: Augusto Noronha Date: Tue, 31 Mar 2020 21:28:14 -0300 Subject: [PATCH 2/2] Improve reporting when type reconstruction fails --- lldb/include/lldb/Symbol/SwiftASTContext.h | 2 + lldb/source/Symbol/SwiftASTContext.cpp | 59 +++++++++++++++++----- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Symbol/SwiftASTContext.h b/lldb/include/lldb/Symbol/SwiftASTContext.h index 92f59db0011ee..919a29caeec54 100644 --- a/lldb/include/lldb/Symbol/SwiftASTContext.h +++ b/lldb/include/lldb/Symbol/SwiftASTContext.h @@ -612,6 +612,7 @@ class SwiftASTContext : public TypeSystemSwift { /// Reconstruct a Swift AST type from a mangled name by looking its /// components up in Swift modules. + swift::TypeBase *ReconstructType(ConstString mangled_typename); swift::TypeBase *ReconstructType(ConstString mangled_typename, Status &error); CompilerType GetTypeFromMangledTypename(ConstString mangled_typename); @@ -667,6 +668,7 @@ class SwiftASTContext : public TypeSystemSwift { void ClearDiagnostics(); bool SetColorizeDiagnostics(bool b); + void AddErrorStatusAsGenericDiagnostic(Status error); void PrintDiagnostics(DiagnosticManager &diagnostic_manager, uint32_t bufferID = UINT32_MAX, uint32_t first_line = 0, diff --git a/lldb/source/Symbol/SwiftASTContext.cpp b/lldb/source/Symbol/SwiftASTContext.cpp index 1f3ed99d21c5e..09456442cbfc7 100644 --- a/lldb/source/Symbol/SwiftASTContext.cpp +++ b/lldb/source/Symbol/SwiftASTContext.cpp @@ -191,11 +191,10 @@ swift::Type TypeSystemSwiftTypeRef::GetSwiftType(CompilerType compiler_type) { if (!ts) return {}; - Status error; // FIXME: Suboptimal performance, because the ConstString is looked up again. ConstString mangled_name( reinterpret_cast(compiler_type.GetOpaqueQualType())); - return ts->m_swift_ast_context->ReconstructType(mangled_name, error); + return ts->m_swift_ast_context->ReconstructType(mangled_name); } swift::Type SwiftASTContext::GetSwiftType(CompilerType compiler_type) { @@ -2853,8 +2852,8 @@ class ANSIColorStringStream : public llvm::raw_string_ostream { class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { public: StoringDiagnosticConsumer(SwiftASTContext &ast_context) - : m_ast_context(ast_context), m_diagnostics(), m_num_errors(0), - m_colorize(false) { + : m_ast_context(ast_context), m_raw_diagnostics(), m_diagnostics(), + m_num_errors(0), m_colorize(false) { m_ast_context.GetDiagnosticEngine().resetHadAnyError(); m_ast_context.GetDiagnosticEngine().addConsumer(*this); } @@ -2926,19 +2925,19 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { std::string &message_ref = os.str(); if (message_ref.empty()) - m_diagnostics.push_back(RawDiagnostic( + m_raw_diagnostics.push_back(RawDiagnostic( text.str(), info.Kind, bufferName, bufferID, line_col.first, line_col.second, use_fixits ? info.FixIts : llvm::ArrayRef())); else - m_diagnostics.push_back(RawDiagnostic( + m_raw_diagnostics.push_back(RawDiagnostic( message_ref, info.Kind, bufferName, bufferID, line_col.first, line_col.second, use_fixits ? info.FixIts : llvm::ArrayRef())); } else { - m_diagnostics.push_back(RawDiagnostic( + m_raw_diagnostics.push_back(RawDiagnostic( text.str(), info.Kind, bufferName, bufferID, line_col.first, line_col.second, llvm::ArrayRef())); } @@ -2949,6 +2948,7 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { void Clear() { m_ast_context.GetDiagnosticEngine().resetHadAnyError(); + m_raw_diagnostics.clear(); m_diagnostics.clear(); m_num_errors = 0; } @@ -2980,8 +2980,13 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { void PrintDiagnostics(DiagnosticManager &diagnostic_manager, uint32_t bufferID = UINT32_MAX, uint32_t first_line = 0, uint32_t last_line = UINT32_MAX) { - bool added_one_diagnostic = false; - for (const RawDiagnostic &diagnostic : m_diagnostics) { + bool added_one_diagnostic = !m_diagnostics.empty(); + + for (std::unique_ptr &diagnostic : m_diagnostics) { + diagnostic_manager.AddDiagnostic(std::move(diagnostic)); + } + + for (const RawDiagnostic &diagnostic : m_raw_diagnostics) { // We often make expressions and wrap them in some code. When // we see errors we want the line numbers to be correct so we // correct them below. LLVM stores in SourceLoc objects as @@ -3053,7 +3058,7 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { // This will report diagnostic errors from outside the // expression's source range. Those are not interesting to // users, so we only emit them in debug builds. - for (const RawDiagnostic &diagnostic : m_diagnostics) { + for (const RawDiagnostic &diagnostic : m_raw_diagnostics) { const DiagnosticSeverity severity = SeverityForKind(diagnostic.kind); const DiagnosticOrigin origin = eDiagnosticOriginSwift; diagnostic_manager.AddDiagnostic(diagnostic.description.c_str(), @@ -3070,6 +3075,10 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { return old; } + void AddDiagnostic(std::unique_ptr diagnostic) { + m_diagnostics.push_back(std::move(diagnostic)); + } + private: // We don't currently use lldb_private::Diagostic or any of the lldb // DiagnosticManager machinery to store diagnostics as they @@ -3095,9 +3104,12 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer { std::vector fixits; }; typedef std::vector RawDiagnosticBuffer; + typedef std::vector> DiagnosticList; SwiftASTContext &m_ast_context; - RawDiagnosticBuffer m_diagnostics; + RawDiagnosticBuffer m_raw_diagnostics; + DiagnosticList m_diagnostics; + unsigned m_num_errors = 0; bool m_colorize; }; @@ -4360,9 +4372,8 @@ swift::Type convertSILFunctionTypesToASTFunctionTypes(swift::Type t) { CompilerType SwiftASTContext::GetTypeFromMangledTypename(ConstString mangled_typename) { - Status error; if (llvm::isa(this)) - return GetCompilerType(ReconstructType(mangled_typename, error)); + return GetCompilerType(ReconstructType(mangled_typename)); return GetCompilerType(mangled_typename); } @@ -4410,6 +4421,17 @@ CompilerType SwiftASTContext::GetAsClangType(ConstString mangled_name) { return clang_type; } +swift::TypeBase *SwiftASTContext::ReconstructType(ConstString mangled_typename) { + Status error; + + auto reconstructed_type = + this->ReconstructType(mangled_typename, error); + if (!error.Success()) { + this->AddErrorStatusAsGenericDiagnostic(error); + } + return reconstructed_type; +} + swift::TypeBase *SwiftASTContext::ReconstructType(ConstString mangled_typename, Status &error) { VALID_OR_RETURN(nullptr); @@ -5001,6 +5023,17 @@ bool SwiftASTContext::SetColorizeDiagnostics(bool b) { return false; } +void SwiftASTContext::AddErrorStatusAsGenericDiagnostic(Status error) { + assert(!error.Success() && "status should be in an error state"); + + auto diagnostic = std::make_unique( + error.AsCString(), eDiagnosticSeverityError, eDiagnosticOriginLLDB, + LLDB_INVALID_COMPILER_ID); + if (m_diagnostic_consumer_ap.get()) + static_cast(m_diagnostic_consumer_ap.get()) + ->AddDiagnostic(std::move(diagnostic)); +} + void SwiftASTContext::PrintDiagnostics(DiagnosticManager &diagnostic_manager, uint32_t bufferID, uint32_t first_line, uint32_t last_line) {