This revision was automatically updated to reflect the committed changes.
Closed by commit rGf110999bf6b5: [lldb][NFCI] Refactor out attribute parsing
from DWARFASTParserClang… (authored by teemperor).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111494/new/
https://reviews.llvm.org/D111494
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2344,56 +2344,52 @@
return nullptr;
}
-void DWARFASTParserClang::ParseSingleMember(
- const DWARFDIE &die, const DWARFDIE &parent_die,
- const lldb_private::CompilerType &class_clang_type,
- lldb::AccessType default_accessibility,
- DelayedPropertyList &delayed_properties,
- lldb_private::ClangASTImporter::LayoutInfo &layout_info,
- FieldInfo &last_field_info) {
- ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
- const dw_tag_t tag = die.Tag();
- // Get the parent byte size so we can verify any members will fit
- const uint64_t parent_byte_size =
- parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
- const uint64_t parent_bit_size =
- parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
-
- DWARFAttributes attributes;
- const size_t num_attributes = die.GetAttributes(attributes);
- if (num_attributes == 0)
- return;
-
+namespace {
+/// Parsed form of all attributes that are relevant for parsing type members.
+struct MemberAttributes {
+ explicit MemberAttributes(const DWARFDIE &die, const DWARFDIE &parent_die,
+ ModuleSP module_sp);
const char *name = nullptr;
+ /// Indicates how many bits into the word (according to the host endianness)
+ /// the low-order bit of the field starts. Can be negative.
+ int64_t bit_offset = 0;
+ /// Indicates the size of the field in bits.
+ size_t bit_size = 0;
+ uint64_t data_bit_offset = UINT64_MAX;
+ AccessType accessibility = eAccessNone;
+ llvm::Optional<uint64_t> byte_size;
+ DWARFFormValue encoding_form;
+ /// Indicates the byte offset of the word from the base address of the
+ /// structure.
+ uint32_t member_byte_offset;
+ bool is_artificial = false;
+ /// On DW_TAG_members, this means the member is static.
+ bool is_external = false;
+};
+
+/// Parsed form of all attributes that are relevant for parsing Objective-C
+/// properties.
+struct PropertyAttributes {
+ explicit PropertyAttributes(const DWARFDIE &die);
const char *prop_name = nullptr;
const char *prop_getter_name = nullptr;
const char *prop_setter_name = nullptr;
+ /// \see clang::ObjCPropertyAttribute
uint32_t prop_attributes = 0;
+};
+} // namespace
- bool is_artificial = false;
- DWARFFormValue encoding_form;
- AccessType accessibility = eAccessNone;
- uint32_t member_byte_offset =
- (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
- llvm::Optional<uint64_t> byte_size;
- int64_t bit_offset = 0;
- uint64_t data_bit_offset = UINT64_MAX;
- size_t bit_size = 0;
- bool is_external =
- false; // On DW_TAG_members, this means the member is static
- uint32_t i;
- for (i = 0; i < num_attributes && !is_artificial; ++i) {
+MemberAttributes::MemberAttributes(const DWARFDIE &die,
+ const DWARFDIE &parent_die,
+ ModuleSP module_sp) {
+ member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
+
+ DWARFAttributes attributes;
+ const size_t num_attributes = die.GetAttributes(attributes);
+ for (std::size_t i = 0; i < num_attributes; ++i) {
const dw_attr_t attr = attributes.AttributeAtIndex(i);
DWARFFormValue form_value;
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
- // DW_AT_data_member_location indicates the byte offset of the
- // word from the base address of the structure.
- //
- // DW_AT_bit_offset indicates how many bits into the word
- // (according to the host endianness) the low-order bit of the
- // field starts. AT_bit_offset can be negative.
- //
- // DW_AT_bit_size indicates the size of the field in bits.
switch (attr) {
case DW_AT_name:
name = form_value.AsCString();
@@ -2444,6 +2440,42 @@
case DW_AT_artificial:
is_artificial = form_value.Boolean();
break;
+ case DW_AT_external:
+ is_external = form_value.Boolean();
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ // Clang has a DWARF generation bug where sometimes it represents
+ // fields that are references with bad byte size and bit size/offset
+ // information such as:
+ //
+ // DW_AT_byte_size( 0x00 )
+ // DW_AT_bit_size( 0x40 )
+ // DW_AT_bit_offset( 0xffffffffffffffc0 )
+ //
+ // So check the bit offset to make sure it is sane, and if the values
+ // are not sane, remove them. If we don't do this then we will end up
+ // with a crash if we try to use this type in an expression when clang
+ // becomes unhappy with its recycled debug info.
+ if (byte_size.getValueOr(0) == 0 && bit_offset < 0) {
+ bit_size = 0;
+ bit_offset = 0;
+ }
+}
+
+PropertyAttributes::PropertyAttributes(const DWARFDIE &die) {
+
+ DWARFAttributes attributes;
+ const size_t num_attributes = die.GetAttributes(attributes);
+ for (size_t i = 0; i < num_attributes; ++i) {
+ const dw_attr_t attr = attributes.AttributeAtIndex(i);
+ DWARFFormValue form_value;
+ if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+ switch (attr) {
case DW_AT_APPLE_property_name:
prop_name = form_value.AsCString();
break;
@@ -2456,110 +2488,103 @@
case DW_AT_APPLE_property_attribute:
prop_attributes = form_value.Unsigned();
break;
- case DW_AT_external:
- is_external = form_value.Boolean();
- break;
-
default:
- case DW_AT_declaration:
- case DW_AT_description:
- case DW_AT_mutable:
- case DW_AT_visibility:
- case DW_AT_sibling:
break;
}
}
}
- if (prop_name) {
- ConstString fixed_setter;
-
- // Check if the property getter/setter were provided as full names.
- // We want basenames, so we extract them.
-
- if (prop_getter_name && prop_getter_name[0] == '-') {
- ObjCLanguage::MethodName prop_getter_method(prop_getter_name, true);
- prop_getter_name = prop_getter_method.GetSelector().GetCString();
- }
+ if (!prop_name)
+ return;
+ ConstString fixed_setter;
- if (prop_setter_name && prop_setter_name[0] == '-') {
- ObjCLanguage::MethodName prop_setter_method(prop_setter_name, true);
- prop_setter_name = prop_setter_method.GetSelector().GetCString();
- }
+ // Check if the property getter/setter were provided as full names.
+ // We want basenames, so we extract them.
+ if (prop_getter_name && prop_getter_name[0] == '-') {
+ ObjCLanguage::MethodName prop_getter_method(prop_getter_name, true);
+ prop_getter_name = prop_getter_method.GetSelector().GetCString();
+ }
- // If the names haven't been provided, they need to be filled in.
+ if (prop_setter_name && prop_setter_name[0] == '-') {
+ ObjCLanguage::MethodName prop_setter_method(prop_setter_name, true);
+ prop_setter_name = prop_setter_method.GetSelector().GetCString();
+ }
- if (!prop_getter_name) {
- prop_getter_name = prop_name;
- }
- if (!prop_setter_name && prop_name[0] &&
- !(prop_attributes & DW_APPLE_PROPERTY_readonly)) {
- StreamString ss;
+ // If the names haven't been provided, they need to be filled in.
+ if (!prop_getter_name)
+ prop_getter_name = prop_name;
+ if (!prop_setter_name && prop_name[0] &&
+ !(prop_attributes & DW_APPLE_PROPERTY_readonly)) {
+ StreamString ss;
- ss.Printf("set%c%s:", toupper(prop_name[0]), &prop_name[1]);
+ ss.Printf("set%c%s:", toupper(prop_name[0]), &prop_name[1]);
- fixed_setter.SetString(ss.GetString());
- prop_setter_name = fixed_setter.GetCString();
- }
+ fixed_setter.SetString(ss.GetString());
+ prop_setter_name = fixed_setter.GetCString();
}
+}
- // Clang has a DWARF generation bug where sometimes it represents
- // fields that are references with bad byte size and bit size/offset
- // information such as:
- //
- // DW_AT_byte_size( 0x00 )
- // DW_AT_bit_size( 0x40 )
- // DW_AT_bit_offset( 0xffffffffffffffc0 )
- //
- // So check the bit offset to make sure it is sane, and if the values
- // are not sane, remove them. If we don't do this then we will end up
- // with a crash if we try to use this type in an expression when clang
- // becomes unhappy with its recycled debug info.
+void DWARFASTParserClang::ParseSingleMember(
+ const DWARFDIE &die, const DWARFDIE &parent_die,
+ const lldb_private::CompilerType &class_clang_type,
+ lldb::AccessType default_accessibility,
+ DelayedPropertyList &delayed_properties,
+ lldb_private::ClangASTImporter::LayoutInfo &layout_info,
+ FieldInfo &last_field_info) {
+ ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
+ const dw_tag_t tag = die.Tag();
+ // Get the parent byte size so we can verify any members will fit
+ const uint64_t parent_byte_size =
+ parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+ const uint64_t parent_bit_size =
+ parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
- if (byte_size.getValueOr(0) == 0 && bit_offset < 0) {
- bit_size = 0;
- bit_offset = 0;
- }
+ // FIXME: Remove the workarounds below and make this const.
+ MemberAttributes attrs(die, parent_die, module_sp);
+ // Skip artificial members such as vtable pointers.
+ // FIXME: This check should verify that this is indeed an artificial member
+ // we are supposed to ignore.
+ if (attrs.is_artificial)
+ return;
+
+ const PropertyAttributes propAttrs(die);
const bool class_is_objc_object_or_interface =
TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
// FIXME: Make Clang ignore Objective-C accessibility for expressions
if (class_is_objc_object_or_interface)
- accessibility = eAccessNone;
+ attrs.accessibility = eAccessNone;
// Handle static members
- if (is_external && member_byte_offset == UINT32_MAX) {
- Type *var_type = die.ResolveTypeUID(encoding_form.Reference());
+ if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+ Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
if (var_type) {
- if (accessibility == eAccessNone)
- accessibility = eAccessPublic;
+ if (attrs.accessibility == eAccessNone)
+ attrs.accessibility = eAccessPublic;
TypeSystemClang::AddVariableToRecordType(
- class_clang_type, name, var_type->GetForwardCompilerType(),
- accessibility);
+ class_clang_type, attrs.name, var_type->GetForwardCompilerType(),
+ attrs.accessibility);
}
return;
}
- if (is_artificial)
- return;
-
- Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
+ Type *member_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
if (!member_type) {
// FIXME: This should not silently fail.
- if (prop_name)
+ if (propAttrs.prop_name)
return;
- if (name)
+ if (attrs.name)
module_sp->ReportError(
"0x%8.8" PRIx64 ": DW_TAG_member '%s' refers to type 0x%8.8x"
" which was unable to be parsed",
- die.GetID(), name, encoding_form.Reference().GetOffset());
+ die.GetID(), attrs.name, attrs.encoding_form.Reference().GetOffset());
else
module_sp->ReportError(
"0x%8.8" PRIx64 ": DW_TAG_member refers to type 0x%8.8x"
" which was unable to be parsed",
- die.GetID(), encoding_form.Reference().GetOffset());
+ die.GetID(), attrs.encoding_form.Reference().GetOffset());
return;
}
@@ -2569,29 +2594,30 @@
if (tag == DW_TAG_member) {
CompilerType member_clang_type = member_type->GetLayoutCompilerType();
- if (accessibility == eAccessNone)
- accessibility = default_accessibility;
+ if (attrs.accessibility == eAccessNone)
+ attrs.accessibility = default_accessibility;
- uint64_t field_bit_offset =
- (member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
+ uint64_t field_bit_offset = (attrs.member_byte_offset == UINT32_MAX
+ ? 0
+ : (attrs.member_byte_offset * 8));
- if (bit_size > 0) {
+ if (attrs.bit_size > 0) {
FieldInfo this_field_info;
this_field_info.bit_offset = field_bit_offset;
- this_field_info.bit_size = bit_size;
+ this_field_info.bit_size = attrs.bit_size;
- if (data_bit_offset != UINT64_MAX) {
- this_field_info.bit_offset = data_bit_offset;
+ if (attrs.data_bit_offset != UINT64_MAX) {
+ this_field_info.bit_offset = attrs.data_bit_offset;
} else {
- if (!byte_size)
- byte_size = member_type->GetByteSize(nullptr);
+ if (!attrs.byte_size)
+ attrs.byte_size = member_type->GetByteSize(nullptr);
ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
if (objfile->GetByteOrder() == eByteOrderLittle) {
- this_field_info.bit_offset += byte_size.getValueOr(0) * 8;
- this_field_info.bit_offset -= (bit_offset + bit_size);
+ this_field_info.bit_offset += attrs.byte_size.getValueOr(0) * 8;
+ this_field_info.bit_offset -= (attrs.bit_offset + attrs.bit_size);
} else {
- this_field_info.bit_offset += bit_offset;
+ this_field_info.bit_offset += attrs.bit_offset;
}
}
@@ -2614,7 +2640,7 @@
"bit offset (0x%8.8" PRIx64
") member will be ignored. Please file a bug against the "
"compiler and include the preprocessed output for %s\n",
- die.GetID(), DW_TAG_value_to_name(tag), name,
+ die.GetID(), DW_TAG_value_to_name(tag), attrs.name,
this_field_info.bit_offset, GetUnitName(parent_die).c_str());
return;
}
@@ -2672,7 +2698,7 @@
class_clang_type, llvm::StringRef(),
m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
word_width),
- accessibility, unnamed_field_info->bit_size);
+ attrs.accessibility, unnamed_field_info->bit_size);
layout_info.field_offsets.insert(std::make_pair(
unnamed_bitfield_decl, unnamed_field_info->bit_offset));
@@ -2713,14 +2739,15 @@
uint64_t parent_byte_size =
parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
- if (member_byte_offset >= parent_byte_size) {
+ if (attrs.member_byte_offset >= parent_byte_size) {
if (member_array_size != 1 &&
(member_array_size != 0 ||
- member_byte_offset > parent_byte_size)) {
+ attrs.member_byte_offset > parent_byte_size)) {
module_sp->ReportError(
"0x%8.8" PRIx64 ": DW_TAG_member '%s' refers to type 0x%8.8x"
" which extends beyond the bounds of 0x%8.8" PRIx64,
- die.GetID(), name, encoding_form.Reference().GetOffset(),
+ die.GetID(), attrs.name,
+ attrs.encoding_form.Reference().GetOffset(),
parent_die.GetID());
}
@@ -2733,7 +2760,8 @@
RequireCompleteType(member_clang_type);
field_decl = TypeSystemClang::AddFieldToRecordType(
- class_clang_type, name, member_clang_type, accessibility, bit_size);
+ class_clang_type, attrs.name, member_clang_type, attrs.accessibility,
+ attrs.bit_size);
m_ast.SetMetadataAsUserID(field_decl, die.GetID());
@@ -2741,7 +2769,7 @@
std::make_pair(field_decl, field_bit_offset));
}
- if (prop_name != nullptr) {
+ if (propAttrs.prop_name != nullptr) {
clang::ObjCIvarDecl *ivar_decl = nullptr;
if (field_decl) {
@@ -2752,9 +2780,10 @@
ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
delayed_properties.push_back(DelayedAddObjCClassProperty(
- class_clang_type, prop_name, member_type->GetLayoutCompilerType(),
- ivar_decl, prop_setter_name, prop_getter_name, prop_attributes,
- &metadata));
+ class_clang_type, propAttrs.prop_name,
+ member_type->GetLayoutCompilerType(), ivar_decl,
+ propAttrs.prop_setter_name, propAttrs.prop_getter_name,
+ propAttrs.prop_attributes, &metadata));
if (ivar_decl)
m_ast.SetMetadataAsUserID(ivar_decl, die.GetID());
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
