tberghammer added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:316-328
@@ +315,15 @@
+
+ const char* dwo_name = cu_die.GetAttributeValueAsString(m_dwarf2Data,
+ this,
+ DW_AT_GNU_dwo_name,
+ nullptr);
+ if (!dwo_name)
+ return;
+
+ const char* comp_dir = cu_die.GetAttributeValueAsString(m_dwarf2Data,
+ this,
+ DW_AT_comp_dir,
+ nullptr);
+ if (!comp_dir)
+ return;
+
----------------
clayborg wrote:
> Is DW_AT_GNU_dwo_name always a relative path?
I haven't seen absolute path in it so far, but it can be absolute. Added code
to handle it.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:660
@@ -564,3 +659,3 @@
{
- return m_dwarf2Data->DebugInfo()->GetDIE (die_offset);
+ return m_dwarf2Data->DebugInfo()->GetDIE (DIERef(die_offset));
}
----------------
clayborg wrote:
> Don't we need the compile unit to be encoded into the DIERef here?
No, we don't want to specify the compile unit here. We already checked that the
specified DIE offset isn't belongs to the current compile unit. In this case we
should try to get a DIE based on offset from the current symbol file. This can
happen when a DIE refers to a value in a different compile unit inside the same
file based on DIE offset
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:161
@@ -145,3 +160,3 @@
else
- return
DWARFDIE(dwarf->DebugInfo()->GetCompileUnitContainingDIE(block_die->GetOffset()),
block_die);
+ return
DWARFDIE(dwarf->DebugInfo()->GetCompileUnitContainingDIE(DIERef(cu->GetOffset(),
block_die->GetOffset())), block_die);
}
----------------
clayborg wrote:
> We probably should swift m_die->LookupAddress to take a "DWARFDIE
> *function_die, DWARFDIE *block_die" instead of "DWARFDebugInfoEntry*
> function_die, DWARFDebugInfoEntry* block_die". Then we can avoid doing the
> manual combination of CU + offset here.
I would prefer to leave it as it is now because we have to do the manual
combination at some point (because the returned DIE might be in a different CU
then the current DIE) and moving it into LookupAddress will make that function
even more complicated.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:184
@@ +183,3 @@
+ assert ((id&0xffffffff00000000ull) == 0 || m_cu->GetID() == 0);
+ id |= ((lldb::user_id_t)m_cu->GetID()) << 32;
+ }
----------------
clayborg wrote:
> The DWARFCompileUnit::GetID() already shifts the cu_offset over by 32, so we
> will just lose the compile unit bits here.
SymbolFileDWARF does it, but DWARFCompileUnit isn't. With shifting by 32 we
lose the SymbolFileDWARF id, but that one isn't considered to be part of the
DIE ID.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1250
@@ -1180,2 +1249,3 @@
dw_offset_t end_addr_offset = DW_INVALID_OFFSET;
- const dw_offset_t attr_offset = GetAttributeValue(dwarf2Data, cu, attr,
form_value, &end_addr_offset);
+ SymbolFileDWARF* attribute_symbol_file = nullptr;
+ const dw_offset_t attr_offset = GetAttributeValue(dwarf2Data,
----------------
clayborg wrote:
> Not sure what you were thinking here? You init this with nullptr and then try
> to use it below? Did you mean to init this with dwarf2Data? If so then we
> don't need this? Otherwise, please fix
The function isn't used at all now, so I just removed it completely.
(I made the change here to fetch the block data from the correct symbol file,
but one of the merge made it broken)
http://reviews.llvm.org/D12291
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
