[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/69531

The method DWARFDebugInfoEntry::Extract needs to skip over all the data in the 
debug_info / debug_types section for each DIE. It had the logic to do so 
hardcoded inside a loop, when it already exists in a neatly isolated function.

>From eee1ecdd0a14c3349960cfbab3df7478a86bf06f Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Wed, 18 Oct 2023 14:23:48 -0700
Subject: [PATCH] [lldb][NFCI] Remove duplicated code in DWARFParser

The method DWARFDebugInfoEntry::Extract needs to skip over all the data in the
debug_info / debug_types section for each DIE. It had the logic to do so
hardcoded inside a loop, when it already exists in a neatly isolated function.
---
 .../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp  | 141 ++
 1 file changed, 9 insertions(+), 132 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index a6ab83700904cb9..46e5e209c7e13c6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -49,16 +49,12 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
-  // assert (fixed_form_sizes);  // For best performance this should be
-  // specified!
-
   if (m_abbr_idx == 0) {
 m_tag = llvm::dwarf::DW_TAG_null;
 m_has_children = false;
 return true; // NULL debug tag entry
   }
 
-  lldb::offset_t offset = *offset_ptr;
   const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl == nullptr) {
 cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);
-  break;
-
-case DW_FORM_indirect:
-

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-debuginfo

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

The method DWARFDebugInfoEntry::Extract needs to skip over all the data in the 
debug_info / debug_types section for each DIE. It had the logic to do so 
hardcoded inside a loop, when it already exists in a neatly isolated function.

---
Full diff: https://github.com/llvm/llvm-project/pull/69531.diff


1 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(+9-132) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index a6ab83700904cb9..46e5e209c7e13c6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -49,16 +49,12 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
-  // assert (fixed_form_sizes);  // For best performance this should be
-  // specified!
-
   if (m_abbr_idx == 0) {
 m_tag = llvm::dwarf::DW_TAG_null;
 m_has_children = false;
 return true; // NULL debug tag entry
   }
 
-  lldb::offset_t offset = *offset_ptr;
   const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl == nullptr) {
 cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);
-  break;
-
-case DW_FORM_indirect:
-  form_is_indirect = true;
-  form = static_cast(data.GetULEB128(&offset));
-  break;
-
-case DW_FORM_strp:
-case DW_FORM_line_strp:
-case DW_FORM_sec_offset:
-  data.GetU32(&offset);
-  break;
-
-case DW_FORM_implicit_const:
-  form_size = 0;
-  break;
-
-default:
-

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Greg Clayton via lldb-commits 

https://github.com/clayborg approved this pull request.

Please manually verify all cases are handled in the DWARFFormValue::SkipValue() 
function to ensure we are not losing functionality. LGTM.

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread David Blaikie via lldb-commits 


@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);

dwblaikie wrote:

One loss seems to be that the other code uses the `get` rather than `skip` 
version of the LEB parsing DataExtractor functions - so maybe those should be 
changed/improved (or, if that optimization isn't worthwhile, perhaps we can 
remove the skip versions)?

Otherwies all the cases seemed to be handled equivalently between the removed 
code and the new code path.

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

felipepiovezan wrote:

> Please manually verify all cases are handled in the 
> DWARFFormValue::SkipValue() function to ensure we are not losing 
> functionality. LGTM.

Yup, I've compared the two functions twice, so I'm fairly confident they are 
identical :) 

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Felipe de Azevedo Piovezan via lldb-commits 


@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);

felipepiovezan wrote:

mmm are we looking at the same place? This is what I see in the SkipValue 
function:

```
// signed or unsigned LEB 128 values
case DW_FORM_addrx:
case DW_FORM_loclistx:
case DW_FORM_rnglistx:
case DW_FORM_sdata:
case DW_FORM_udata:
case DW_FORM_ref_udata:
case DW_FORM_GNU_addr_index:
case DW_FORM_GNU_str_index:
case DW_FORM_strx:
  debug_info_data.Skip_LEB128(offset_ptr);
  return true;
```

For these cases, both version of the code use a "get":

```
case DW_FORM_exprloc:
 case DW_FORM_block:
   form_size = data.GetULEB128(&offset);
```

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord approved this pull request.

I looked at the implementations and I didn't notice anything too different 
either. This brings us from 3 implementations of this logic to 2. Hopefully we 
can make it 1 someday soon. 😄 

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-18 Thread Greg Clayton via lldb-commits 


@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);

clayborg wrote:

For exprloc and block, we need to get the length of the bytes to skip, and for 
that we do need to decode the size of the block/exprloc, so this is as intended.

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-19 Thread Jonas Devlieghere via lldb-commits 

JDevlieghere wrote:

🥳

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-19 Thread David Blaikie via lldb-commits 


@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
&data,
   m_tag = abbrevDecl->getTag();
   m_has_children = abbrevDecl->hasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
-  dw_form_t form;
   for (const auto &attribute : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
-  offset += *fixed_skip_size;
-else {
-  bool form_is_indirect = false;
-  do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
-  form_size = data.GetULEB128(&offset);
-  break;
-case DW_FORM_block1:
-  form_size = data.GetU8_unchecked(&offset);
-  break;
-case DW_FORM_block2:
-  form_size = data.GetU16_unchecked(&offset);
-  break;
-case DW_FORM_block4:
-  form_size = data.GetU32_unchecked(&offset);
-  break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
-  data.GetCStr(&offset);
-  break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
-  form_size = cu->GetAddressByteSize();
-  break;
-case DW_FORM_ref_addr:
-  if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
-  else
-form_size = 4;
-  break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+  continue;
 
-// 0 sized form
-case DW_FORM_flag_present:
-  form_size = 0;
-  break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
-  form_size = 1;
-  break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
-  form_size = 2;
-  break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
-  form_size = 3;
-  break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
-  form_size = 4;
-  break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
-  form_size = 8;
-  break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
-  data.Skip_LEB128(&offset);

dwblaikie wrote:

Ah, right - of course. Thanks for walking me through it!

https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-19 Thread David Blaikie via lldb-commits 

https://github.com/dwblaikie approved this pull request.


https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)

2023-10-19 Thread Felipe de Azevedo Piovezan via lldb-commits 

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits