This is an archive of the discontinued LLVM Phabricator instance.

DWZ 01/07: Support reading section ".gnu_debugaltlink"
ClosedPublic

Authored by jankratochvil on Nov 26 2017, 5:09 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

jankratochvil created this revision.Nov 26 2017, 5:09 AM
clayborg requested changes to this revision.Nov 27 2017, 10:24 AM

Just move the eSectionTypeDWARFGNUDebugAltLink enumerator to the end of the enum and this is good to go.

include/lldb/lldb-enumerations.h
647 ↗(On Diff #124294)

Since this enumeration is in the public API, it should be added to the end of the enum. Anyone that builds a new liblldb.so and then uses it with an binary compiled before could run into issues. But I noticed eSectionTypeDWARFDebugCuIndex was added in the middle in August, but we need to avoid these kinds of API issues, so lets add this to the end just to be safe.

This revision now requires changes to proceed.Nov 27 2017, 10:24 AM
jankratochvil marked an inline comment as done.
jankratochvil retitled this revision from DWZ 05/12: Support reading section ".gnu_debugaltlink" to DWZ 04/11: Support reading section ".gnu_debugaltlink".
jankratochvil planned changes to this revision.Apr 14 2018, 4:21 AM
jankratochvil retitled this revision from DWZ 04/11: Support reading section ".gnu_debugaltlink" to DWZ 01/07: Support reading section ".gnu_debugaltlink".

It is now reworked without FileOffset and the remapping to unique DIE offsets for each DW_TAG_partial_unit inclusion by DW_TAG_imported_unit, as discussed in: https://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180409/040324.html

Could you add a "section type" line in the dumpModules in lldb-test.cpp. Then we can write a test for this. (not that this is a extremely dangerous change, but it seems a shame not to test it when it is so easy).

Implemented that section type line in the dumpModules in lldb-test.cpp.

Thanks, that's great, but I don't see you using that functionality to test the debug-alt-link classification (I guess I wasn't clear about that, but that is the reason why I asked you to add it). Could you add a test with the new section as well?

Added lit/Modules/dwarf-gnu-debugaltlink.yaml. I do not see its test success anywhere but when I make it fail I see its failure in make check-lldb stdout/stderr. Curiously its simulated failure does not appear in make check-lit.

labath accepted this revision.Apr 27 2018, 2:02 AM

Awesome, thanks. I think we can get this one out of the way.

Added lit/Modules/dwarf-gnu-debugaltlink.yaml. I do not see its test success anywhere but when I make it fail I see its failure in make check-lldb stdout/stderr.

That's supposed to happen. To see passing tests you need to run lit in the verbose mode.

Curiously its simulated failure does not appear in make check-lit.

check-lit is a lit-selftest. This test should not be run as a part of that. Did you perhaps mean check-lldb-lit (which should probably be removed because it's now practically an alias for check-lldb)?

Awesome, thanks. I think we can get this one out of the way.

Thanks, I will check it in next days.

check-lit is a lit-selftest. This test should not be run as a part of that. Did you perhaps mean check-lldb-lit (which should probably be removed because it's now practically an alias for check-lldb)?

Yes, I was wrongly running check-lit instead of check-lldb-lit, thanks for the correction.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 29 2018, 12:51 PM
This revision was automatically updated to reflect the committed changes.