[PATCH] D69462: [DebugInfo]: Support for debug_loclists.dwo section in llvm and llvm-dwarfdump.

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:529
+      uint64_t Offset =
+          IsDWO ? HeaderSize + getLocSectionBase() : getLocSectionBase();
       DWARFDataExtractor Data(Context.getDWARFObj(), *LocSection,
----------------
SouraVX wrote:
> dblaikie wrote:
> > Maybe in the IsDWO case just set it to "HeaderSize"?
> > 
> > Hmm, @aprantl @probinson - I can't find the wording in the DWARF5 spec about the use of loclists_base and rnglists_base in split units. It's unclear if these need to be specified in the split unit, or if they're assumed to be zero (or, technically, zero + sizeof(list header))? Currently LLVM doesn't generate a rnglists_base for split DWARFv5 units (I haven't checked the history on that change - but I might've had a hand in it) & this change is going to propagate that choice & do similarly for loclists_base.
> > 
> > GCC 8.1 at least doesn't seem to give us any hint there - since they don't use rnglists.dwo so far as I can tell (I induced a function-local range list and it was put in the .o rnglists section, there was no rnglists.dwo section)
> > 
> > In some ways it'd be tidier to emit it, of course - consistent between .o and .dwo, but also it'd likely be redundant/always have the same value in every .dwo file.
> Anyways it's ` getLocSectionBase() ` value is ` 0 ` . 
> Since, we've set it up that way above --
> 
> ```
> if (IsDWO)
>        setLocSection(&Context.getDWARFObj().getLoclistsDWOSection(), 0);
> ```
> 
> Added this here just for readibility / uniformity sake.
Does `LocSectionBase` have to be zero? Could set the `LocSectionBase` to sizeof(header) instead? (If the consensus is that `DW_AT_loclists_base` is not needed in dwo files -- which seems reasonable to me.)

That way, you wouldn't need to do the `if(DWO)` dance here and also in DWARFDie.cpp:dumpLocation. I think that would actually mostly address my earlier comment about having a "central" place to compute the final location section offset.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69462/new/

https://reviews.llvm.org/D69462