[Lldb-commits] [PATCH] D12291: Add split dwarf support to SymbolFileDWARF

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few changes.

I think this might create problems for DWARF in .o files on MacOSX. Are you able to test on MacOSX? We used to store the compile unit index in the upper 32 bits of the type ID and all of our compile units from each .o file have 0xb as their cu_offset, so I would think that DIERef would always cause us to grab the first compile unit...


================
Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:34-44
@@ +33,13 @@
+
+DIERef::DIERef(const DWARFFormValue& form_value) :
+    cu_offset(DW_INVALID_OFFSET),
+    die_offset(DW_INVALID_OFFSET)
+{
+    if (form_value.IsValid())
+    {
+        if (form_value.GetCompileUnit())
+            cu_offset = form_value.GetCompileUnit()->GetOffset();
+        die_offset = form_value.Reference();
+    }
+}
+
----------------
For DWO files, won't every compile unit have offset 0x0000000b? Or do you use the actual compile unit DIE from the main executable?

I have doubts this will work for DWARF in .o files. We used to encode the compile unit index in the upper 32 bits because all of our compile units have a dw_offset_t that is 0xb..

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:603-605
@@ -581,7 +602,5 @@
     {
-        std::vector<dw_offset_t>::const_iterator pos;
-        std::vector<dw_offset_t>::const_iterator end = die_offsets.end();
-        for (pos = die_offsets.begin(); pos != end; ++pos)
+        for (auto pos = die_refs.begin(), end = die_refs.end(); pos != end; ++pos)
         {
-            dw_offset_t die_offset = *pos;
-            if (die_offset != DW_INVALID_OFFSET)
+            const DIERef& die_ref = *pos;
+            if (die_ref.die_offset != DW_INVALID_OFFSET)
----------------
These 3 lines would be nicer as:

```
for (const auto &die_ref : die_refs)
{
```

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1025-1036
@@ +1024,14 @@
+    {
+        if (GetAttributeValue(dwarf2Data, cu, DW_AT_specification, form_value) ||
+            GetAttributeValue(dwarf2Data, cu, DW_AT_abstract_origin, form_value))
+        {
+            DWARFDIE die = const_cast<DWARFCompileUnit*>(cu)->GetDIE(form_value.Reference());
+            if (die)
+                return die.GetDIE()->GetAttributeValue(die.GetDWARF(),
+                                                       die.GetCU(),
+                                                       attr,
+                                                       form_value,
+                                                       end_attr_offset_ptr,
+                                                       false);
+        }
+    }
----------------
This will fail if a DIE can have both a DW_AT_specification and a DW_AT_abstract_origin. Not sure if that can happen. If it can, we will need to check each one individually. Probably best to just check both individually.

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1039-1055
@@ -985,1 +1038,19 @@
+
+    if (!dwo_symbol_file)
+        return 0;
+
+    DWARFCompileUnit* dwo_cu = dwo_symbol_file->GetCompileUnit();
+    if (!dwo_cu)
+        return 0;
+
+    DWARFDIE dwo_cu_die = dwo_cu->GetCompileUnitDIEOnly();
+    if (!dwo_cu_die.IsValid())
+        return 0;
+
+    return dwo_cu_die.GetDIE()->GetAttributeValue(dwo_symbol_file,
+                                                  dwo_cu,
+                                                  attr,
+                                                  form_value,
+                                                  end_attr_offset_ptr,
+                                                  check_specification_or_abstract_origin);
 }
----------------
Don't you want to only do all of this code if:
```
if (Tag() == DW_TAG_compile_unit)
```


http://reviews.llvm.org/D12291