This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Support .debug_rnglists.dwo sections in dwp file
ClosedPublic

Authored by kimanh on Aug 4 2021, 6:15 AM.

Details

Summary

This patch considers the CU index entry
when reading the .debug_rnglists.dwo section.

Diff Detail

Event Timeline

kimanh created this revision.Aug 4 2021, 6:15 AM
kimanh updated this revision to Diff 364081.Aug 4 2021, 6:16 AM

Include all commits.

kimanh updated this revision to Diff 364364.Aug 4 2021, 11:52 PM

Simplifying test and add comments.

kimanh published this revision for review.Aug 4 2021, 11:53 PM
kimanh added reviewers: labath, jankratochvil.
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2021, 11:53 PM
jankratochvil added inline comments.Aug 11 2021, 7:45 AM
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
524

Here could be an error message.

546

I have found this getContribution adjustment duplication, are there some reasons it is not unified? https://www.jankratochvil.net/t/D107456-unify.patch
The issue is the code then handles two different .debug_rnglists DWARFDataExtractors with different offsets.

553

One such reason can be missing DWP absolute offset for the error report. That could be returned from GetRnglistData().

kimanh updated this revision to Diff 367758.Aug 20 2021, 3:49 AM
kimanh marked 2 inline comments as done.
  • Incorporating Jan's patch
  • Adding error message

Thanks a lot for the review! Very sorry for my late update, I was out on vacation and then had to catch up with mails. Updated the revision now.

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
546

Thanks a lot for the patch, I unified it in the latest update. IIRC (some time ago, so I don't recall completely) there was a mismatch in offsets maybe because I missed to update GetRnglistOffset to use getRnglistData too.

553

If I understand your comment correctly, you are suggesting to incorporate the contribution offset into the error message of GetRnglistData, is that correct? However, GetRnglistData will only return an error message if the contribution offset cannot be read, so the contribution offset cannot be specified in the error message (and thus the absolute offset cannot be inferred) . Or maybe I'm misunderstanding your comment here?

Friendly ping!

jankratochvil accepted this revision.Aug 27 2021, 1:26 PM
jankratochvil added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
553

Not so correct. The callers of GetRnglistData need to know the contribution offset to properly report it during parsing errors in the callers. GetRnglistTable can report error if extractHeaderAndOffsets finds invalid header data. In such case LLDB will report (with https://www.jankratochvil.net/t/debug_rnglists-dwp.s.patch):

error: debug_rnglists-dwp.s.tmp Failed to extract range list table at offset 0xc: parsing .debug_rnglists table at offset 0x0: unsupported reserved unit length of value 0xfffffff0

And one has no idea which DWO in that DWP file was invalid, that 0xc is relative to start of contribution, not to start of the .debug_rnglists.dwo section. GetRnglistData knows the contribution offset but its caller DWARFDebugRnglistTable does not.
I am not sure how perfect the error messages should be so giving an approval (if I am authorized to give one) but it would be nice to improve the error reporting.

This revision is now accepted and ready to land.Aug 27 2021, 1:26 PM
kimanh updated this revision to Diff 369547.Aug 30 2021, 2:11 PM
  • Updating error messages
kimanh added inline comments.Aug 30 2021, 2:12 PM
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
553

Ahh, okay, thanks a lot for the clarification! I updated a patch to address the error message. Please take a look if the updated patch is what you'd like to see.

kimanh updated this revision to Diff 370562.Sep 3 2021, 5:47 AM
  • splitting up new code (for error handling) into a different CL
  • keep code that was already accepted

I've split up the code that addresses the .debug_rnglists from the code that takes care of better error handling (https://reviews.llvm.org/D109231). I'll commit the reviewed part (this).

This revision was landed with ongoing or failed builds.Sep 3 2021, 6:21 AM
This revision was automatically updated to reflect the committed changes.