llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) <details> <summary>Changes</summary> …eAtIndex Convert the function to early exits, and add sensible error messages for all failing checks. --- Patch is 33.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170932.diff 2 Files Affected: - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+336-340) - (modified) lldb/test/API/commands/frame/var-dil/basics/PointerArithmetic/TestFrameVarDILPointerArithmetic.py (+1-1) ``````````diff diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 2cb4a46130c84..d2606faeee8b8 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6241,7 +6241,7 @@ llvm::Expected<CompilerType> TypeSystemClang::GetChildCompilerTypeAtIndex( bool &child_is_base_class, bool &child_is_deref_of_parent, ValueObject *valobj, uint64_t &language_flags) { if (!type) - return CompilerType(); + return llvm::createStringError("invalid type"); auto get_exe_scope = [&exe_ctx]() { return exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; @@ -6265,356 +6265,277 @@ llvm::Expected<CompilerType> TypeSystemClang::GetChildCompilerTypeAtIndex( int32_t bit_offset; switch (parent_type_class) { case clang::Type::Builtin: - if (idx_is_valid) { - switch (llvm::cast<clang::BuiltinType>(parent_qual_type)->getKind()) { - case clang::BuiltinType::ObjCId: - case clang::BuiltinType::ObjCClass: - child_name = "isa"; - child_byte_size = - getASTContext().getTypeSize(getASTContext().ObjCBuiltinClassTy) / - CHAR_BIT; - return GetType(getASTContext().ObjCBuiltinClassTy); + if (!idx_is_valid) + return llvm::createStringError("invalid index"); - default: - break; - } + switch (llvm::cast<clang::BuiltinType>(parent_qual_type)->getKind()) { + case clang::BuiltinType::ObjCId: + case clang::BuiltinType::ObjCClass: + child_name = "isa"; + child_byte_size = + getASTContext().getTypeSize(getASTContext().ObjCBuiltinClassTy) / + CHAR_BIT; + return GetType(getASTContext().ObjCBuiltinClassTy); + + default: + break; } break; + case clang::Type::Record: { + if (!idx_is_valid) + return llvm::createStringError("invalid index"); + if (!GetCompleteType(type)) + return llvm::createStringError("cannot complete type"); - case clang::Type::Record: - if (idx_is_valid && GetCompleteType(type)) { - const clang::RecordType *record_type = - llvm::cast<clang::RecordType>(parent_qual_type.getTypePtr()); - const clang::RecordDecl *record_decl = - record_type->getDecl()->getDefinitionOrSelf(); - const clang::ASTRecordLayout &record_layout = - getASTContext().getASTRecordLayout(record_decl); - uint32_t child_idx = 0; + const clang::RecordType *record_type = + llvm::cast<clang::RecordType>(parent_qual_type.getTypePtr()); + const clang::RecordDecl *record_decl = + record_type->getDecl()->getDefinitionOrSelf(); + const clang::ASTRecordLayout &record_layout = + getASTContext().getASTRecordLayout(record_decl); + uint32_t child_idx = 0; - const clang::CXXRecordDecl *cxx_record_decl = - llvm::dyn_cast<clang::CXXRecordDecl>(record_decl); - if (cxx_record_decl) { - // We might have base classes to print out first - clang::CXXRecordDecl::base_class_const_iterator base_class, - base_class_end; - for (base_class = cxx_record_decl->bases_begin(), - base_class_end = cxx_record_decl->bases_end(); - base_class != base_class_end; ++base_class) { - const clang::CXXRecordDecl *base_class_decl = nullptr; + const clang::CXXRecordDecl *cxx_record_decl = + llvm::dyn_cast<clang::CXXRecordDecl>(record_decl); + if (cxx_record_decl) { + // We might have base classes to print out first + clang::CXXRecordDecl::base_class_const_iterator base_class, + base_class_end; + for (base_class = cxx_record_decl->bases_begin(), + base_class_end = cxx_record_decl->bases_end(); + base_class != base_class_end; ++base_class) { + const clang::CXXRecordDecl *base_class_decl = nullptr; - // Skip empty base classes - if (omit_empty_base_classes) { + // Skip empty base classes + if (omit_empty_base_classes) { + base_class_decl = + llvm::cast<clang::CXXRecordDecl>( + base_class->getType()->getAs<clang::RecordType>()->getDecl()) + ->getDefinitionOrSelf(); + if (!TypeSystemClang::RecordHasFields(base_class_decl)) + continue; + } + + if (idx == child_idx) { + if (base_class_decl == nullptr) base_class_decl = llvm::cast<clang::CXXRecordDecl>( base_class->getType() ->getAs<clang::RecordType>() ->getDecl()) ->getDefinitionOrSelf(); - if (!TypeSystemClang::RecordHasFields(base_class_decl)) - continue; - } - if (idx == child_idx) { - if (base_class_decl == nullptr) - base_class_decl = llvm::cast<clang::CXXRecordDecl>( - base_class->getType() - ->getAs<clang::RecordType>() - ->getDecl()) - ->getDefinitionOrSelf(); - - if (base_class->isVirtual()) { - bool handled = false; - if (valobj) { - clang::VTableContextBase *vtable_ctx = - getASTContext().getVTableContext(); - if (vtable_ctx) - handled = GetVBaseBitOffset(*vtable_ctx, *valobj, - record_layout, cxx_record_decl, - base_class_decl, bit_offset); - } - if (!handled) - bit_offset = record_layout.getVBaseClassOffset(base_class_decl) - .getQuantity() * - 8; - } else - bit_offset = record_layout.getBaseClassOffset(base_class_decl) + if (base_class->isVirtual()) { + bool handled = false; + if (valobj) { + clang::VTableContextBase *vtable_ctx = + getASTContext().getVTableContext(); + if (vtable_ctx) + handled = GetVBaseBitOffset(*vtable_ctx, *valobj, record_layout, + cxx_record_decl, base_class_decl, + bit_offset); + } + if (!handled) + bit_offset = record_layout.getVBaseClassOffset(base_class_decl) .getQuantity() * 8; - - // Base classes should be a multiple of 8 bits in size - child_byte_offset = bit_offset / 8; - CompilerType base_class_clang_type = GetType(base_class->getType()); - child_name = base_class_clang_type.GetTypeName().AsCString(""); - auto size_or_err = - base_class_clang_type.GetBitSize(get_exe_scope()); - if (!size_or_err) - return llvm::joinErrors( - llvm::createStringError("no size info for base class"), - size_or_err.takeError()); - - uint64_t base_class_clang_type_bit_size = *size_or_err; - - // Base classes bit sizes should be a multiple of 8 bits in size - assert(base_class_clang_type_bit_size % 8 == 0); - child_byte_size = base_class_clang_type_bit_size / 8; - child_is_base_class = true; - return base_class_clang_type; - } - // We don't increment the child index in the for loop since we might - // be skipping empty base classes - ++child_idx; - } - } - // Make sure index is in range... - uint32_t field_idx = 0; - clang::RecordDecl::field_iterator field, field_end; - for (field = record_decl->field_begin(), - field_end = record_decl->field_end(); - field != field_end; ++field, ++field_idx, ++child_idx) { - if (idx == child_idx) { - // Print the member type if requested - // Print the member name and equal sign - child_name.assign(field->getNameAsString()); - - // Figure out the type byte size (field_type_info.first) and - // alignment (field_type_info.second) from the AST context. - CompilerType field_clang_type = GetType(field->getType()); - assert(field_idx < record_layout.getFieldCount()); - auto size_or_err = field_clang_type.GetByteSize(get_exe_scope()); + } else + bit_offset = record_layout.getBaseClassOffset(base_class_decl) + .getQuantity() * + 8; + + // Base classes should be a multiple of 8 bits in size + child_byte_offset = bit_offset / 8; + CompilerType base_class_clang_type = GetType(base_class->getType()); + child_name = base_class_clang_type.GetTypeName().AsCString(""); + auto size_or_err = base_class_clang_type.GetBitSize(get_exe_scope()); if (!size_or_err) return llvm::joinErrors( - llvm::createStringError("no size info for field"), + llvm::createStringError("no size info for base class"), size_or_err.takeError()); - child_byte_size = *size_or_err; - const uint32_t child_bit_size = child_byte_size * 8; - - // Figure out the field offset within the current struct/union/class - // type - bit_offset = record_layout.getFieldOffset(field_idx); - if (FieldIsBitfield(*field, child_bitfield_bit_size)) { - child_bitfield_bit_offset = bit_offset % child_bit_size; - const uint32_t child_bit_offset = - bit_offset - child_bitfield_bit_offset; - child_byte_offset = child_bit_offset / 8; - } else { - child_byte_offset = bit_offset / 8; - } + uint64_t base_class_clang_type_bit_size = *size_or_err; - return field_clang_type; + // Base classes bit sizes should be a multiple of 8 bits in size + assert(base_class_clang_type_bit_size % 8 == 0); + child_byte_size = base_class_clang_type_bit_size / 8; + child_is_base_class = true; + return base_class_clang_type; } + // We don't increment the child index in the for loop since we might + // be skipping empty base classes + ++child_idx; } } - break; + // Make sure index is in range... + uint32_t field_idx = 0; + clang::RecordDecl::field_iterator field, field_end; + for (field = record_decl->field_begin(), + field_end = record_decl->field_end(); + field != field_end; ++field, ++field_idx, ++child_idx) { + if (idx == child_idx) { + // Print the member type if requested + // Print the member name and equal sign + child_name.assign(field->getNameAsString()); + + // Figure out the type byte size (field_type_info.first) and + // alignment (field_type_info.second) from the AST context. + CompilerType field_clang_type = GetType(field->getType()); + assert(field_idx < record_layout.getFieldCount()); + auto size_or_err = field_clang_type.GetByteSize(get_exe_scope()); + if (!size_or_err) + return llvm::joinErrors( + llvm::createStringError("no size info for field"), + size_or_err.takeError()); + child_byte_size = *size_or_err; + const uint32_t child_bit_size = child_byte_size * 8; + + // Figure out the field offset within the current struct/union/class + // type + bit_offset = record_layout.getFieldOffset(field_idx); + if (FieldIsBitfield(*field, child_bitfield_bit_size)) { + child_bitfield_bit_offset = bit_offset % child_bit_size; + const uint32_t child_bit_offset = + bit_offset - child_bitfield_bit_offset; + child_byte_offset = child_bit_offset / 8; + } else { + child_byte_offset = bit_offset / 8; + } + + return field_clang_type; + } + } + } break; case clang::Type::ObjCObject: - case clang::Type::ObjCInterface: - if (idx_is_valid && GetCompleteType(type)) { - const clang::ObjCObjectType *objc_class_type = - llvm::dyn_cast<clang::ObjCObjectType>(parent_qual_type.getTypePtr()); - assert(objc_class_type); - if (objc_class_type) { - uint32_t child_idx = 0; - clang::ObjCInterfaceDecl *class_interface_decl = - objc_class_type->getInterface(); + case clang::Type::ObjCInterface: { + if (!idx_is_valid) + return llvm::createStringError("invalid index"); + if (!GetCompleteType(type)) + return llvm::createStringError("cannot complete type"); - if (class_interface_decl) { + const clang::ObjCObjectType *objc_class_type = + llvm::dyn_cast<clang::ObjCObjectType>(parent_qual_type.getTypePtr()); + assert(objc_class_type); + if (!objc_class_type) + return llvm::createStringError("unexpected object type"); - const clang::ASTRecordLayout &interface_layout = - getASTContext().getASTObjCInterfaceLayout(class_interface_decl); - clang::ObjCInterfaceDecl *superclass_interface_decl = - class_interface_decl->getSuperClass(); - if (superclass_interface_decl) { - if (omit_empty_base_classes) { - CompilerType base_class_clang_type = - GetType(getASTContext().getObjCInterfaceType( - superclass_interface_decl)); - if (llvm::expectedToStdOptional( - base_class_clang_type.GetNumChildren( - omit_empty_base_classes, exe_ctx)) - .value_or(0) > 0) { - if (idx == 0) { - clang::QualType ivar_qual_type( - getASTContext().getObjCInterfaceType( - superclass_interface_decl)); - - child_name.assign( - superclass_interface_decl->getNameAsString()); - - clang::TypeInfo ivar_type_info = - getASTContext().getTypeInfo(ivar_qual_type.getTypePtr()); - - child_byte_size = ivar_type_info.Width / 8; - child_byte_offset = 0; - child_is_base_class = true; - - return GetType(ivar_qual_type); - } + uint32_t child_idx = 0; + clang::ObjCInterfaceDecl *class_interface_decl = + objc_class_type->getInterface(); - ++child_idx; - } - } else - ++child_idx; - } + if (!class_interface_decl) + return llvm::createStringError("cannot get interface decl"); - const uint32_t superclass_idx = child_idx; + const clang::ASTRecordLayout &interface_layout = + getASTContext().getASTObjCInterfaceLayout(class_interface_decl); + clang::ObjCInterfaceDecl *superclass_interface_decl = + class_interface_decl->getSuperClass(); + if (superclass_interface_decl) { + if (omit_empty_base_classes) { + CompilerType base_class_clang_type = GetType( + getASTContext().getObjCInterfaceType(superclass_interface_decl)); + if (llvm::expectedToStdOptional(base_class_clang_type.GetNumChildren( + omit_empty_base_classes, exe_ctx)) + .value_or(0) > 0) { + if (idx == 0) { + clang::QualType ivar_qual_type(getASTContext().getObjCInterfaceType( + superclass_interface_decl)); - if (idx < (child_idx + class_interface_decl->ivar_size())) { - clang::ObjCInterfaceDecl::ivar_iterator ivar_pos, - ivar_end = class_interface_decl->ivar_end(); + child_name.assign(superclass_interface_decl->getNameAsString()); - for (ivar_pos = class_interface_decl->ivar_begin(); - ivar_pos != ivar_end; ++ivar_pos) { - if (child_idx == idx) { - clang::ObjCIvarDecl *ivar_decl = *ivar_pos; - - clang::QualType ivar_qual_type(ivar_decl->getType()); - - child_name.assign(ivar_decl->getNameAsString()); - - clang::TypeInfo ivar_type_info = - getASTContext().getTypeInfo(ivar_qual_type.getTypePtr()); - - child_byte_size = ivar_type_info.Width / 8; - - // Figure out the field offset within the current - // struct/union/class type For ObjC objects, we can't trust the - // bit offset we get from the Clang AST, since that doesn't - // account for the space taken up by unbacked properties, or - // from the changing size of base classes that are newer than - // this class. So if we have a process around that we can ask - // about this object, do so. - child_byte_offset = LLDB_INVALID_IVAR_OFFSET; - Process *process = nullptr; - if (exe_ctx) - process = exe_ctx->GetProcessPtr(); - if (process) { - ObjCLanguageRuntime *objc_runtime = - ObjCLanguageRuntime::Get(*process); - if (objc_runtime != nullptr) { - CompilerType parent_ast_type = GetType(parent_qual_type); - child_byte_offset = objc_runtime->GetByteOffsetForIvar( - parent_ast_type, ivar_decl->getNameAsString().c_str()); - } - } + clang::TypeInfo ivar_type_info = + getASTContext().getTypeInfo(ivar_qual_type.getTypePtr()); - // Setting this to INT32_MAX to make sure we don't compute it - // twice... - bit_offset = INT32_MAX; + child_byte_size = ivar_type_info.Width / 8; + child_byte_offset = 0; + child_is_base_class = true; - if (child_byte_offset == - static_cast<int32_t>(LLDB_INVALID_IVAR_OFFSET)) { - bit_offset = interface_layout.getFieldOffset(child_idx - - superclass_idx); - child_byte_offset = bit_offset / 8; - } + return GetType(ivar_qual_type); + } - // Note, the ObjC Ivar Byte offset is just that, it doesn't - // account for the bit offset of a bitfield within its - // containing object. So regardless of where we get the byte - // offset from, we still need to get the bit offset for - // bitfields from the layout. + ++child_idx; + } + } else + ++child_idx; + } - if (FieldIsBitfield(ivar_decl, child_bitfield_bit_size)) { - if (bit_offset == INT32_MAX) - bit_offset = interface_layout.getFieldOffset( - child_idx - superclass_idx); + const uint32_t superclass_idx = child_idx; - child_bitfield_bit_offset = bit_offset % 8; - } - return GetType(ivar_qual_type); - } - ++child_idx; + if (idx < (child_idx + class_interface_decl->ivar_size())) { + clang::ObjCInterfaceDecl::ivar_iterator ivar_pos, + ivar_end = class_interface_decl->ivar_end(); + + for (ivar_pos = class_interface_decl->ivar_begin(); ivar_pos != ivar_end; + ++ivar_pos) { + if (child_idx == idx) { + clang::ObjCIvarDecl *ivar_decl = *ivar_pos; + + clang::QualType ivar_qual_type(ivar_decl->getType()); + + child_name.assign(ivar_decl->getNameAsString()); + + clang::TypeInfo ivar_type_info = + getASTContext().getTypeInfo(ivar_qual_type.getTypePtr()); + + child_byte_size = ivar_type_info.Width / 8; + + // Figure out the field offset within the current + // struct/union/class type For ObjC objects, we can't trust the + // bit offset we get from the Cla... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/170932 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
