tberghammer added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:729
@@ -727,3 +728,3 @@
if (attributes.ExtractFormValueAtIndex(m_dwarf2Data, i,
form_value))
- name = form_value.AsCString(debug_str);
+ name = form_value.AsCString(debug_str,
debug_str_offsets);
break;
----------------
clayborg wrote:
> Don't we need to pass both debug_str and debug_str_offsets here? Other places
> seem to pass both like on DWARFDebugInfoEntry.cpp:826
Both of them is passed in here. The only difference is that they are
pre-fetched in line 662-663.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:745
@@ -743,3 +744,3 @@
if (attributes.ExtractFormValueAtIndex(m_dwarf2Data, i,
form_value))
- mangled_cstr = form_value.AsCString(debug_str);
+ mangled_cstr = form_value.AsCString(debug_str,
debug_str_offsets);
break;
----------------
clayborg wrote:
> Don't we need to pass both debug_str and debug_str_offsets here? Other places
> seem to pass both like on DWARFDebugInfoEntry.cpp:826
>
Both of them is passed in here. The only difference is that they are
pre-fetched in line 662-663.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp:190
@@ -189,7 +189,3 @@
case DW_FORM_data8: m_value.value.uval = data.GetU64(offset_ptr);
break;
- case DW_FORM_string: m_value.value.cstr = data.GetCStr(offset_ptr);
- // Set the string value to also be the data
for inlined cstr form values only
- // so we can tell the difference between
DW_FORM_string and DW_FORM_strp form
- // values;
- m_value.data = (const
uint8_t*)m_value.value.cstr; break;
+ case DW_FORM_string: m_value.value.cstr = data.GetCStr(offset_ptr);
break;
case DW_FORM_exprloc:
----------------
clayborg wrote:
> We we not longer need "m_value.data" set to the cstring value to tell the
> difference between DW_FORM_string and DW_FORM_strp? This might have been left
> over from the code this originated from and might be able to be removed, but
> I just wanted to verify that you intended to remove this.
We store the information in the m_form field, so we don't have to rely on this
magic anymore (I don't see why we needed it at the first place).
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.h:68
@@ -68,1 +67,3 @@
+ const lldb_private::DWARFDataExtractor*
debug_str_offsets_data_ptr) const;
+ dw_addr_t Address(const lldb_private::DWARFDataExtractor*
debug_addr_data_ptr) const;
bool SkipValue(const lldb_private::DWARFDataExtractor&
debug_info_data, lldb::offset_t *offset_ptr) const;
----------------
clayborg wrote:
> Should be pass in a lldb::addr_t base_addr in case the DW_FORM is one where
> we need to add to the low_pc? Then code like this:
>
> ```
> dw_form_t form = form_value.Form();
> if (form == DW_FORM_addr || form == DW_FORM_GNU_addr_index)
> return form_value.Address(&dwarf2Data->get_debug_addr_data());
>
> // DWARF4 can specify the hi_pc as an <offset-from-lowpc>
> return lo_pc + form_value.Unsigned();
> ```
>
> Becomes:
>
> ```
> return form_value.Address(&dwarf2Data->get_debug_addr_data(),
> lo_pc);
> ```
>
> The "base_addr" could default to LLDB_INVALID_ADDRESS and if we get a
> relative address we should assert in debug builds using LLDB_ASSERT.
>
We have a dedicated function for that called
DWARFDebugInfoEntry::GetAttributeHighPC. The purpose of this function is to
fetch a value where the attribute encoding value class is "address"
(http://www.dwarfstd.org/doc/DWARF4.pdf page 155)
http://reviews.llvm.org/D12238
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
