[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[David Blaikie via Phabricator via lldb-commits]

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