[PATCH] D70081: DWARFDebugLoclists: add location list "interpretation" logic

Tue Nov 12 12:01:04 PST 2019

dblaikie added inline comments.

================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s:9
 # CHECK-NEXT:    0x00000000:
-# CHECK-NEXT:    Addr idx 1 (w/ length 16): DW_OP_reg5 RDI
+# CHECK-NEXT:    DW_LLE_startx_length (0x00000001, 0x00000010): DW_OP_reg5 RDI

----------------
SouraVX wrote:
> labath wrote:
> > SouraVX wrote:
> > > Hi Pavel, I'm not sure of this, here you're using "DW_LLE_startx_length" in a debug_loc.dwo -- any ideas ?
> > > My primary concern here, is the Debuggability impact.
> > > Though haven't, checked this? Did you get chance to debug these binaries{Containing above style location list} with LLDB or GDB ?? since predwarf(debug_loc and debug_loc.dwo) is debuggable with both debuggers.
> > I'm afraid I don't understand your concerns here. Can you elaborate?
> > 
> > This patch doesn't change how debug_loc.dwo sections are generated. It doesn't even change how they are parsed, for the most part. The only real difference in this test is that we now explicitly print the raw entry contents whereas previously we tried to "pretty print" it, which wasn't really useful without an address pool.
> > 
> > clang (and gcc, I believe) have been emitting this unstandardized .debug_loc.dwo format for a while. I haven't tried debugging these binaries lately, but I know that lldb has support for this format, but it is incomplete.
> Thanks Pavel, for clarifying this. Indeed it's not touching the emission / generating part.
> Obviously Debuggability is fine, checked with latest master.
> 
> This test is mostly about dwarfdump. Just curious here as to why, you've chosen "DW_LLE_*" representation here ?? those are [DWARF5] part.
The pre-standard/standard-proposal version of Fission used this same terminology: https://gcc.gnu.org/wiki/DebugFission so the names are suitable (& since they had no prior art in the standard there was no need to use an implementation namespace (DW_LLE_GNU_etc) first (well, kind of/might've been good to do that))

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70081