From 7511c09fb965c582ef15075d6d3b7bf5e3fa303f Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Mon, 1 Feb 2021 16:06:17 -0800 Subject: [PATCH 1/2] Add missing nullptr checks after SwiftLanguageRuntimeImpl::GetTypeInfo() rdar://73749956 --- ...ftLanguageRuntimeDynamicTypeResolution.cpp | 42 +++++++++++-------- lldb/source/Target/SwiftLanguageRuntimeImpl.h | 5 ++- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp b/lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp index cb617ae4fd248..1364844dcf023 100644 --- a/lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp +++ b/lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp @@ -886,7 +886,7 @@ llvm::Optional SwiftLanguageRuntimeImpl::GetMemberVariableOffsetRemote auto frame = instance ? instance->GetExecutionContextRef().GetFrameSP().get() : nullptr; if (auto *ti = llvm::dyn_cast_or_null( - GetTypeInfo(instance_type, frame))) { + GetSwiftRuntimeTypeInfo(instance_type, frame))) { auto fields = ti->getFields(); LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES), "using record type info"); @@ -1040,10 +1040,11 @@ SwiftLanguageRuntimeImpl::GetNumChildren(CompilerType type, auto frame = valobj ? valobj->GetExecutionContextRef().GetFrameSP().get() : nullptr; const swift::reflection::TypeRef *tr = nullptr; - auto *ti = GetTypeInfo(type, frame, &tr); + auto *ti = GetSwiftRuntimeTypeInfo(type, frame, &tr); + if (!ti) + return {}; // Structs and Tuples. - if (auto *rti = - llvm::dyn_cast_or_null(ti)) { + if (auto *rti = llvm::dyn_cast(ti)) { switch (rti->getRecordKind()) { case swift::reflection::RecordKind::ExistentialMetatype: case swift::reflection::RecordKind::ThickFunction: @@ -1060,13 +1061,11 @@ SwiftLanguageRuntimeImpl::GetNumChildren(CompilerType type, return rti->getNumFields(); } } - if (auto *eti = llvm::dyn_cast_or_null(ti)) { + if (auto *eti = llvm::dyn_cast(ti)) { return eti->getNumPayloadCases(); } // Objects. - if (auto *rti = - llvm::dyn_cast_or_null(ti)) { - + if (auto *rti = llvm::dyn_cast(ti)) { switch (rti->getReferenceKind()) { case swift::reflection::ReferenceKind::Weak: case swift::reflection::ReferenceKind::Unowned: @@ -1113,7 +1112,9 @@ SwiftLanguageRuntimeImpl::GetNumFields(CompilerType type, using namespace swift::reflection; // Try the static type metadata. const TypeRef *tr = nullptr; - auto *ti = GetTypeInfo(type, exe_ctx->GetFramePtr(), &tr); + auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr(), &tr); + if (!ti) + return {}; // Structs and Tuples. switch (ti->getKind()) { case TypeInfoKind::Record: { @@ -1246,7 +1247,9 @@ llvm::Optional SwiftLanguageRuntimeImpl::GetEnumCaseName( CompilerType type, const DataExtractor &data, ExecutionContext *exe_ctx) { using namespace swift::reflection; using namespace swift::remote; - auto *ti = GetTypeInfo(type, exe_ctx->GetFramePtr()); + auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr()); + if (!ti) + return {}; if (ti->getKind() != TypeInfoKind::Enum) return {}; @@ -1278,7 +1281,9 @@ llvm::Optional SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName( using namespace swift::reflection; // Try the static type metadata. const TypeRef *tr = nullptr; - auto *ti = GetTypeInfo(type, exe_ctx->GetFramePtr(), &tr); + auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr(), &tr); + if (!ti) + return {}; switch (ti->getKind()) { case TypeInfoKind::Record: { // Structs and Tuples. @@ -1391,7 +1396,9 @@ CompilerType SwiftLanguageRuntimeImpl::GetChildCompilerTypeAtIndex( // Try the static type metadata. auto frame = valobj ? valobj->GetExecutionContextRef().GetFrameSP().get() : nullptr; - auto *ti = GetTypeInfo(type, frame); + auto *ti = GetSwiftRuntimeTypeInfo(type, frame); + if (!ti) + return {}; // Structs and Tuples. if (auto *rti = llvm::dyn_cast_or_null(ti)) { @@ -2754,7 +2761,8 @@ SwiftLanguageRuntimeImpl::GetTypeRef(CompilerType type, return type_ref; } -const swift::reflection::TypeInfo *SwiftLanguageRuntimeImpl::GetTypeInfo( +const swift::reflection::TypeInfo * +SwiftLanguageRuntimeImpl::GetSwiftRuntimeTypeInfo( CompilerType type, ExecutionContextScope *exe_scope, swift::reflection::TypeRef const **out_tr) { auto *ts = llvm::dyn_cast_or_null(type.GetTypeSystem()); @@ -2796,7 +2804,7 @@ const swift::reflection::TypeInfo *SwiftLanguageRuntimeImpl::GetTypeInfo( } bool SwiftLanguageRuntimeImpl::IsStoredInlineInBuffer(CompilerType type) { - if (auto *type_info = GetTypeInfo(type, nullptr)) + if (auto *type_info = GetSwiftRuntimeTypeInfo(type, nullptr)) return type_info->isBitwiseTakable() && type_info->getSize() <= 24; return true; } @@ -2804,14 +2812,14 @@ bool SwiftLanguageRuntimeImpl::IsStoredInlineInBuffer(CompilerType type) { llvm::Optional SwiftLanguageRuntimeImpl::GetBitSize(CompilerType type, ExecutionContextScope *exe_scope) { - if (auto *type_info = GetTypeInfo(type, exe_scope)) + if (auto *type_info = GetSwiftRuntimeTypeInfo(type, exe_scope)) return type_info->getSize() * 8; return {}; } llvm::Optional SwiftLanguageRuntimeImpl::GetByteStride(CompilerType type) { - if (auto *type_info = GetTypeInfo(type, nullptr)) + if (auto *type_info = GetSwiftRuntimeTypeInfo(type, nullptr)) return type_info->getStride(); return {}; } @@ -2819,7 +2827,7 @@ SwiftLanguageRuntimeImpl::GetByteStride(CompilerType type) { llvm::Optional SwiftLanguageRuntimeImpl::GetBitAlignment(CompilerType type, ExecutionContextScope *exe_scope) { - if (auto *type_info = GetTypeInfo(type, exe_scope)) + if (auto *type_info = GetSwiftRuntimeTypeInfo(type, exe_scope)) return type_info->getAlignment() * 8; return {}; } diff --git a/lldb/source/Target/SwiftLanguageRuntimeImpl.h b/lldb/source/Target/SwiftLanguageRuntimeImpl.h index 0ec6b4a358037..139a23db90cc6 100644 --- a/lldb/source/Target/SwiftLanguageRuntimeImpl.h +++ b/lldb/source/Target/SwiftLanguageRuntimeImpl.h @@ -77,9 +77,10 @@ class SwiftLanguageRuntimeImpl { Status &error); /// Ask Remote Mirrors for the type info about a Swift type. + /// This will return a nullptr if the lookup fails. const swift::reflection::TypeInfo * - GetTypeInfo(CompilerType type, ExecutionContextScope *exe_scope, - swift::reflection::TypeRef const **out_tr = nullptr); + GetSwiftRuntimeTypeInfo(CompilerType type, ExecutionContextScope *exe_scope, + swift::reflection::TypeRef const **out_tr = nullptr); llvm::Optional lookupClangTypeInfo(CompilerType clang_type); From dec585e9d9c56353b925e598d763e2fff98b5239 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Mon, 1 Feb 2021 16:45:54 -0800 Subject: [PATCH 2/2] Invoke SwiftASTContext::LogConfiguration() on all error paths (NFC outside of types log) The LogConfiguration() output is especially interesting if things aren't going as planned, so let's invoke it on all early exists as well. rdar://70008042 --- lldb/scripts/check-ast-context.py | 3 ++- .../TypeSystem/Swift/SwiftASTContext.cpp | 23 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lldb/scripts/check-ast-context.py b/lldb/scripts/check-ast-context.py index 676ca66b79f2c..e6de9d3852e99 100755 --- a/lldb/scripts/check-ast-context.py +++ b/lldb/scripts/check-ast-context.py @@ -421,7 +421,8 @@ def look_for_METHOD(c): 'GetFatalErrors', 'PrintDiagnostics', 'GetASTContext', - 'SetTriple' + 'SetTriple', + 'LogConfiguration' ] for method in methods: diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index f70f899baf4a0..d9b4e0f37fab9 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1694,6 +1694,14 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, m_description, target ? target->GetArchitecture().GetTriple() : triple, target))); + bool suppress_config_log = false; + auto defer_log = llvm::make_scope_exit([swift_ast_sp, &suppress_config_log] { + // To avoid spamming the log with useless info, we don't log the + // configuration if everything went fine and the current module + // doesn't have any Swift contents (i.e., the shared cache dylibs). + if (!suppress_config_log) + swift_ast_sp->LogConfiguration(); + }); // This is a module AST context, mark it as such. swift_ast_sp->m_is_scratch_context = false; @@ -1706,9 +1714,7 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, bool set_triple = false; bool found_swift_modules = false; - SymbolFile *sym_file = module.GetSymbolFile(); - std::string target_triple; if (sym_file) { @@ -1831,7 +1837,10 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, std::vector module_names; swift_ast_sp->RegisterSectionModules(module, module_names); - if (module_names.size()) { + if (!module_names.size()) { + // This dylib has no Swift contents; logging the configuration is pointless. + suppress_config_log = true; + } else { swift_ast_sp->ValidateSectionModules(module, module_names); if (lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES)) { std::lock_guard locker(g_log_mutex); @@ -1839,7 +1848,6 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, static_cast(&module), module.GetFileSpec().GetFilename().AsCString(""), static_cast(swift_ast_sp.get())); - swift_ast_sp->LogConfiguration(); } } @@ -1926,6 +1934,8 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, // detect if we have a iOS simulator. std::shared_ptr swift_ast_sp( new SwiftASTContextForExpressions(m_description, target)); + auto defer_log = llvm::make_scope_exit( + [swift_ast_sp] { swift_ast_sp->LogConfiguration(); }); LOG_PRINTF(LIBLLDB_LOG_TYPES, "(Target)"); @@ -2280,7 +2290,6 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, LOG_PRINTF(LIBLLDB_LOG_TYPES, "((Target*)%p) = %p", static_cast(&target), static_cast(swift_ast_sp.get())); - swift_ast_sp->LogConfiguration(); if (swift_ast_sp->HasFatalErrors()) { logError(swift_ast_sp->GetFatalErrors().AsCString()); @@ -4923,8 +4932,8 @@ void SwiftASTContext::ClearModuleDependentCaches() { } void SwiftASTContext::LogConfiguration() { - VALID_OR_RETURN_VOID(); - + // It makes no sense to call VALID_OR_RETURN here. We specifically + // want the logs in the error case! Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES)); if (!log) return;