[Lldb-commits] [lldb] b5ff511 - [lldb][NFC] Early-exit in DWARFASTParserClang::ParseSingleMember
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Sat Oct 9 05:41:02 PDT 2021
Author: Raphael Isemann
Date: 2021-10-09T14:40:39+02:00
New Revision: b5ff51104810d09071a5e6ac4c33a0c070f996ca
URL: https://github.com/llvm/llvm-project/commit/b5ff51104810d09071a5e6ac4c33a0c070f996ca
DIFF: https://github.com/llvm/llvm-project/commit/b5ff51104810d09071a5e6ac4c33a0c070f996ca.diff
LOG: [lldb][NFC] Early-exit in DWARFASTParserClang::ParseSingleMember
ParseSingleMember has two large ifs around the back of it's body:
`if (!is_artificial)` and `if (member_type)`. This patch just converts those
to early-exits. The patch is NFC. It even retains the curious fact that
Objective-C properties that fail to parse are silently ignored, but now there
is at least a FIXME that points this out.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 115047c97ca3f..1aa2c4f3b1ea4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2508,221 +2508,222 @@ void DWARFASTParserClang::ParseSingleMember(
return;
}
- if (!is_artificial) {
- Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
-
- clang::FieldDecl *field_decl = nullptr;
- const uint64_t character_width = 8;
- const uint64_t word_width = 32;
- if (tag == DW_TAG_member) {
- if (member_type) {
- CompilerType member_clang_type = member_type->GetLayoutCompilerType();
-
- if (accessibility == eAccessNone)
- accessibility = default_accessibility;
-
- uint64_t field_bit_offset =
- (member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
+ if (is_artificial)
+ return;
- if (bit_size > 0) {
- FieldInfo this_field_info;
- this_field_info.bit_offset = field_bit_offset;
- this_field_info.bit_size = bit_size;
+ Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
+ if (!member_type) {
+ // FIXME: This should not silently fail.
+ if (prop_name)
+ return;
+ if (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());
+ 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());
+ return;
+ }
- if (data_bit_offset != UINT64_MAX) {
- this_field_info.bit_offset = data_bit_offset;
- } else {
- if (!byte_size)
- byte_size = member_type->GetByteSize(nullptr);
+ clang::FieldDecl *field_decl = nullptr;
+ const uint64_t character_width = 8;
+ const uint64_t word_width = 32;
+ if (tag == DW_TAG_member) {
+ CompilerType member_clang_type = member_type->GetLayoutCompilerType();
- 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);
- } else {
- this_field_info.bit_offset += bit_offset;
- }
- }
+ if (accessibility == eAccessNone)
+ accessibility = default_accessibility;
- // The ObjC runtime knows the byte offset but we still need to provide
- // the bit-offset in the layout. It just means something
diff erent then
- // what it does in C and C++. So we skip this check for ObjC types.
- //
- // We also skip this for fields of a union since they will all have a
- // zero offset.
- if (!TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type) &&
- !(parent_die.Tag() == DW_TAG_union_type && this_field_info.bit_offset == 0) &&
- ((this_field_info.bit_offset >= parent_bit_size) ||
- (last_field_info.IsBitfield() &&
- !last_field_info.NextBitfieldOffsetIsValid(
- this_field_info.bit_offset)))) {
- ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
- objfile->GetModule()->ReportWarning(
- "0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid "
- "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,
- this_field_info.bit_offset, GetUnitName(parent_die).c_str());
- return;
- }
+ uint64_t field_bit_offset =
+ (member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
- // Update the field bit offset we will report for layout
- field_bit_offset = this_field_info.bit_offset;
+ if (bit_size > 0) {
+ FieldInfo this_field_info;
+ this_field_info.bit_offset = field_bit_offset;
+ this_field_info.bit_size = bit_size;
- // Objective-C has invalid DW_AT_bit_offset values in older
- // versions of clang, so we have to be careful and only insert
- // unnamed bitfields if we have a new enough clang.
- bool detect_unnamed_bitfields = true;
+ if (data_bit_offset != UINT64_MAX) {
+ this_field_info.bit_offset = data_bit_offset;
+ } else {
+ if (!byte_size)
+ byte_size = member_type->GetByteSize(nullptr);
- if (class_is_objc_object_or_interface)
- detect_unnamed_bitfields =
- die.GetCU()->Supports_unnamed_objc_bitfields();
+ 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);
+ } else {
+ this_field_info.bit_offset += bit_offset;
+ }
+ }
- if (detect_unnamed_bitfields) {
- clang::Optional<FieldInfo> unnamed_field_info;
- uint64_t last_field_end = 0;
+ // The ObjC runtime knows the byte offset but we still need to provide
+ // the bit-offset in the layout. It just means something
diff erent then
+ // what it does in C and C++. So we skip this check for ObjC types.
+ //
+ // We also skip this for fields of a union since they will all have a
+ // zero offset.
+ if (!TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type) &&
+ !(parent_die.Tag() == DW_TAG_union_type &&
+ this_field_info.bit_offset == 0) &&
+ ((this_field_info.bit_offset >= parent_bit_size) ||
+ (last_field_info.IsBitfield() &&
+ !last_field_info.NextBitfieldOffsetIsValid(
+ this_field_info.bit_offset)))) {
+ ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
+ objfile->GetModule()->ReportWarning(
+ "0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid "
+ "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,
+ this_field_info.bit_offset, GetUnitName(parent_die).c_str());
+ return;
+ }
- last_field_end =
- last_field_info.bit_offset + last_field_info.bit_size;
+ // Update the field bit offset we will report for layout
+ field_bit_offset = this_field_info.bit_offset;
- if (!last_field_info.IsBitfield()) {
- // The last field was not a bit-field...
- // but if it did take up the entire word then we need to extend
- // last_field_end so the bit-field does not step into the last
- // fields padding.
- if (last_field_end != 0 && ((last_field_end % word_width) != 0))
- last_field_end += word_width - (last_field_end % word_width);
- }
+ // Objective-C has invalid DW_AT_bit_offset values in older
+ // versions of clang, so we have to be careful and only insert
+ // unnamed bitfields if we have a new enough clang.
+ bool detect_unnamed_bitfields = true;
- // If we have a gap between the last_field_end and the current
- // field we have an unnamed bit-field.
- // If we have a base class, we assume there is no unnamed
- // bit-field if this is the first field since the gap can be
- // attributed to the members from the base class. This assumption
- // is not correct if the first field of the derived class is
- // indeed an unnamed bit-field. We currently do not have the
- // machinary to track the offset of the last field of classes we
- // have seen before, so we are not handling this case.
- if (this_field_info.bit_offset != last_field_end &&
- this_field_info.bit_offset > last_field_end &&
- !(last_field_info.bit_offset == 0 &&
- last_field_info.bit_size == 0 &&
- layout_info.base_offsets.size() != 0)) {
- unnamed_field_info = FieldInfo{};
- unnamed_field_info->bit_size =
- this_field_info.bit_offset - last_field_end;
- unnamed_field_info->bit_offset = last_field_end;
- }
+ if (class_is_objc_object_or_interface)
+ detect_unnamed_bitfields =
+ die.GetCU()->Supports_unnamed_objc_bitfields();
- if (unnamed_field_info) {
- clang::FieldDecl *unnamed_bitfield_decl =
- TypeSystemClang::AddFieldToRecordType(
- class_clang_type, llvm::StringRef(),
- m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
- word_width),
- accessibility, unnamed_field_info->bit_size);
+ if (detect_unnamed_bitfields) {
+ clang::Optional<FieldInfo> unnamed_field_info;
+ uint64_t last_field_end = 0;
- layout_info.field_offsets.insert(std::make_pair(
- unnamed_bitfield_decl, unnamed_field_info->bit_offset));
- }
- }
+ last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
- last_field_info = this_field_info;
- last_field_info.SetIsBitfield(true);
- } else {
- last_field_info.bit_offset = field_bit_offset;
-
- if (llvm::Optional<uint64_t> clang_type_size =
- member_type->GetByteSize(nullptr)) {
- last_field_info.bit_size = *clang_type_size * character_width;
- }
+ if (!last_field_info.IsBitfield()) {
+ // The last field was not a bit-field...
+ // but if it did take up the entire word then we need to extend
+ // last_field_end so the bit-field does not step into the last
+ // fields padding.
+ if (last_field_end != 0 && ((last_field_end % word_width) != 0))
+ last_field_end += word_width - (last_field_end % word_width);
+ }
- last_field_info.SetIsBitfield(false);
+ // If we have a gap between the last_field_end and the current
+ // field we have an unnamed bit-field.
+ // If we have a base class, we assume there is no unnamed
+ // bit-field if this is the first field since the gap can be
+ // attributed to the members from the base class. This assumption
+ // is not correct if the first field of the derived class is
+ // indeed an unnamed bit-field. We currently do not have the
+ // machinary to track the offset of the last field of classes we
+ // have seen before, so we are not handling this case.
+ if (this_field_info.bit_offset != last_field_end &&
+ this_field_info.bit_offset > last_field_end &&
+ !(last_field_info.bit_offset == 0 &&
+ last_field_info.bit_size == 0 &&
+ layout_info.base_offsets.size() != 0)) {
+ unnamed_field_info = FieldInfo{};
+ unnamed_field_info->bit_size =
+ this_field_info.bit_offset - last_field_end;
+ unnamed_field_info->bit_offset = last_field_end;
}
- if (!member_clang_type.IsCompleteType())
- member_clang_type.GetCompleteType();
-
- {
- // Older versions of clang emit array[0] and array[1] in the
- // same way (<rdar://problem/12566646>). If the current field
- // is at the end of the structure, then there is definitely no
- // room for extra elements and we override the type to
- // array[0].
-
- CompilerType member_array_element_type;
- uint64_t member_array_size;
- bool member_array_is_incomplete;
-
- if (member_clang_type.IsArrayType(&member_array_element_type,
- &member_array_size,
- &member_array_is_incomplete) &&
- !member_array_is_incomplete) {
- uint64_t parent_byte_size =
- parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size,
- UINT64_MAX);
-
- if (member_byte_offset >= parent_byte_size) {
- if (member_array_size != 1 &&
- (member_array_size != 0 ||
- 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(),
- parent_die.GetID());
- }
+ if (unnamed_field_info) {
+ clang::FieldDecl *unnamed_bitfield_decl =
+ TypeSystemClang::AddFieldToRecordType(
+ class_clang_type, llvm::StringRef(),
+ m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
+ word_width),
+ accessibility, unnamed_field_info->bit_size);
- member_clang_type =
- m_ast.CreateArrayType(member_array_element_type, 0, false);
- }
- }
+ layout_info.field_offsets.insert(std::make_pair(
+ unnamed_bitfield_decl, unnamed_field_info->bit_offset));
}
+ }
- RequireCompleteType(member_clang_type);
+ last_field_info = this_field_info;
+ last_field_info.SetIsBitfield(true);
+ } else {
+ last_field_info.bit_offset = field_bit_offset;
- field_decl = TypeSystemClang::AddFieldToRecordType(
- class_clang_type, name, member_clang_type, accessibility,
- bit_size);
+ if (llvm::Optional<uint64_t> clang_type_size =
+ member_type->GetByteSize(nullptr)) {
+ last_field_info.bit_size = *clang_type_size * character_width;
+ }
- m_ast.SetMetadataAsUserID(field_decl, die.GetID());
+ last_field_info.SetIsBitfield(false);
+ }
+
+ if (!member_clang_type.IsCompleteType())
+ member_clang_type.GetCompleteType();
+
+ {
+ // Older versions of clang emit array[0] and array[1] in the
+ // same way (<rdar://problem/12566646>). If the current field
+ // is at the end of the structure, then there is definitely no
+ // room for extra elements and we override the type to
+ // array[0].
+
+ CompilerType member_array_element_type;
+ uint64_t member_array_size;
+ bool member_array_is_incomplete;
+
+ if (member_clang_type.IsArrayType(&member_array_element_type,
+ &member_array_size,
+ &member_array_is_incomplete) &&
+ !member_array_is_incomplete) {
+ uint64_t parent_byte_size =
+ parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+
+ if (member_byte_offset >= parent_byte_size) {
+ if (member_array_size != 1 &&
+ (member_array_size != 0 ||
+ 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(),
+ parent_die.GetID());
+ }
- layout_info.field_offsets.insert(
- std::make_pair(field_decl, field_bit_offset));
- } else {
- if (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());
- 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());
+ member_clang_type =
+ m_ast.CreateArrayType(member_array_element_type, 0, false);
+ }
}
}
- if (prop_name != nullptr && member_type) {
- clang::ObjCIvarDecl *ivar_decl = nullptr;
+ RequireCompleteType(member_clang_type);
- if (field_decl) {
- ivar_decl = clang::dyn_cast<clang::ObjCIvarDecl>(field_decl);
- assert(ivar_decl != nullptr);
- }
+ field_decl = TypeSystemClang::AddFieldToRecordType(
+ class_clang_type, name, member_clang_type, accessibility, bit_size);
+
+ m_ast.SetMetadataAsUserID(field_decl, die.GetID());
+
+ layout_info.field_offsets.insert(
+ std::make_pair(field_decl, field_bit_offset));
+ }
- 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));
+ if (prop_name != nullptr) {
+ clang::ObjCIvarDecl *ivar_decl = nullptr;
- if (ivar_decl)
- m_ast.SetMetadataAsUserID(ivar_decl, die.GetID());
+ if (field_decl) {
+ ivar_decl = clang::dyn_cast<clang::ObjCIvarDecl>(field_decl);
+ assert(ivar_decl != nullptr);
}
+
+ 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));
+
+ if (ivar_decl)
+ m_ast.SetMetadataAsUserID(ivar_decl, die.GetID());
}
}
More information about the lldb-commits
mailing list