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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked 4 inline comments as done.
SouraVX added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2435-2452
+  if (!DD.useSplitDwarf())
+    emitRangeList(DD, Asm, List.Label, DD.getDebugLocs().getEntries(List),
+                  *List.CU, dwarf::DW_LLE_base_addressx,
+                  dwarf::DW_LLE_offset_pair, dwarf::DW_LLE_startx_length,
+                  dwarf::DW_LLE_end_of_list, llvm::dwarf::LocListEncodingString,
+                  /* ShouldUseBaseAddress */ true,
+                  [&](const DebugLocStream::Entry &E) {
----------------
SouraVX wrote:
> dblaikie wrote:
> > I'm not sure I understand why this change is here - if I'm reading it right, the only difference between the two emitRangeList calls is "ShouldUseBaseAddress" - if that change is needed, perhaps it'd be better to have a single call that uses "!DD.useSplitDwarf()" as the "ShouldUseBaseAddress" parameter. But I don't think that's correct - base addresses should be used in split and non-split DWARF5, I think.
> > 
> > I believe base addresses should not be used in pre-DWARF5 split DWARF, but that codepath isn't using this function right now. (I think maybe it should, though)
> Thanks for noting this. Yeah, I was previously of holding on opposite thought, then
> 
> > base addresses should be used in split and non-split DWARF5
> 
> This seems fine, after referring back to Spec. 
> 
> But bear with me for a moment, I'm noting one further glitch-- if we do pass 
> ` ShouldUseBaseAddress ` , I'm getting loclist entries like.
> ` 0x00000018:
>             DW_LLE_base_addressx   (0x0000000000000000)  -- coming in every pair with 0
>             DW_LLE_offset_pair     (0x0000000000000000, 0x0000000000000004): DW_OP_reg5 RDI
>             DW_LLE_offset_pair     (0x0000000000000004, 0x0000000000000018): DW_OP_reg3 RBX
>             DW_LLE_end_of_list     () `
> 
> I'm guessing base is not their for split ?? Any thought on this ??
> 
> 
> 
> 
> 
> 
> 
> 
> 
+ Surprisingly this ` DW_LLE_base_addressx ` doesn't pops up in non-split case, Not sure if this is fine.
+ For ` ShouldUseBaseAddress ` {false}, we're producing / emitting
` DW_LLE_stratx_length ` while for {true} as mentioned above we're producing /emitting.
` DW_LLE_offset_pair `


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

https://reviews.llvm.org/D69462