dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D106466#2950268 <https://reviews.llvm.org/D106466#2950268>, @jankratochvil 
wrote:

> In D106466#2947710 <https://reviews.llvm.org/D106466#2947710>, @dblaikie 
> wrote:
>
>> I assume there's already test coverage for rnglistx in debug_info.dwo/split 
>> unit? (because in that case there's no rnglists_base, but rnglistx is usable)
>
> I admit I did not check it, thanks for catching it. But it is already tested 
> by lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s 
> <https://github.com/llvm/llvm-project/blob/f58a642da19c64cb6ee1badb0f176a872c1d7a0a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s>
>  implemented by Pavel Labath in 2019 
> <https://github.com/llvm/llvm-project/commit/5ee8e673135891072789f0f9bf14a5d82f0f8e01>.
>  It is using DWO `DW_TAG_lexical_block->DW_AT_ranges->DW_FORM_rnglistx`.
>
>> - could you explain how this code avoids treating the split unit 
>> rnglists_base == 0 case as "there is no rnglists_base and so rnglistx isn't 
>> usable"?
>
> `m_ranges_base` is not zero in such case as it has been set from the skeleton 
> <https://github.com/llvm/llvm-project/blob/f58a642da19c64cb6ee1badb0f176a872c1d7a0a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp#L105>.

ah, OK - was going to say that setting it from the skeleton would be incorrect 
(for DWARF 5, at least) - but I see that's handled a few lines down: 
https://github.com/llvm/llvm-project/blob/f58a642da19c64cb6ee1badb0f176a872c1d7a0a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp#L110

Looks all good to me, then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to