[Lldb-commits] [PATCH] D61502: Permit cross-CU references

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed May 15 01:57:34 PDT 2019

labath added a comment.

I like this patch in a lot of ways, because it makes the code cleaner, and prepares us for DWARF5, which can reference entries in other files in a lot of interesting ways.

The part I am not totally sure if is the alignment of lldb's DWARFFormValue with it's llvm counterpart. The llvm version does not have this fancy cross-unit resolution capability, but it just returns the raw value and leaves it up to the user to make sense of it (much like our class does now). However, it's possible that the llvm version does not have this functionality because there wasn't need for it, and it already has access to the DWARFContext, so it is certainly prepared to do more complex lookups, so it's possible it could be extended similarly once we go around to merging the two. And even if we go back to the world where `form.Reference()` returns a raw value, this may serve as a good intermediate step, because otherwise we'd have a hard time catching all the cases where someone calls `Reference`, and expects it to return an simple section offset.

So, overall, I'm inclined to accept this patch, but I'd like to hear what @JDevlieghere and @clayborg think.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:114-115
 DWARFDIE::GetReferencedDIE(const dw_attr_t attr) const {
-  const dw_offset_t die_offset =
-      GetAttributeValueAsReference(attr, DW_INVALID_OFFSET);
-  if (die_offset != DW_INVALID_OFFSET)
-    return GetDIE(die_offset);
-  else
-    return DWARFDIE();
+  return GetAttributeValueAsReference(attr);
It looks like these two functions are equivalent now and we can remove one of them (I vote to keep this one).




More information about the lldb-commits mailing list