Page MenuHomePhabricator

[lldb] Fix DW_OP_addrx uses.
ClosedPublic

Authored by tamur on Mar 5 2019, 3:37 PM.

Details

Summary

DW_OP_GNU_addr_index has been renamed as DW_OP_addrx in the standard. clang produces DW_OP_addrx tags and with this change lldb starts to process them.

Event Timeline

tamur created this revision.Mar 5 2019, 3:37 PM
shafik added a subscriber: shafik.Mar 5 2019, 3:44 PM
shafik added inline comments.
lldb/source/Expression/DWARFExpression.cpp
2910 ↗(On Diff #189652)

Should this comment be updated?

tamur updated this revision to Diff 189415.Mar 5 2019, 3:56 PM
tamur marked an inline comment as done.Mar 5 2019, 3:57 PM
tamur added inline comments.
lldb/source/Expression/DWARFExpression.cpp
2910 ↗(On Diff #189652)

Updated the comment.

The code looks fine, but this needs a testcase.

Is there a way to force clang to generate this and/or are existing tests failing because this support is missing?

tamur updated this revision to Diff 189423.EditedMar 5 2019, 4:27 PM

Added a test. (test with dwarf5 generation would fail without other changes).

labath added a subscriber: labath.Mar 6 2019, 3:33 AM

Would a test similar to lit/SymbolFile/DWARF/array-sizes.s be possible/appropriate?
I'm thinking of something like:

  • produce debug info in the form of a .s file (either with clang of via hand-tuning dwarf) which uses DW_OP_addrx
  • assemble&link it
  • run lldb, have it open the file and print the value of the global variable

The advantage of this would be that it's more explicit about what is being tested, and it would keep testing the same thing even if clang decides to encode variable locations differently in the future.

tamur updated this revision to Diff 189652.Mar 6 2019, 9:13 PM

Discarded changes to the unit test and added a lit test.

Thank you. The test looks good to me.

aprantl accepted this revision.Mar 7 2019, 9:30 AM
This revision is now accepted and ready to land.Mar 7 2019, 9:30 AM
Closed by commit rL355629: [lldb] Fix DW_OP_addrx uses. (authored by tamur, committed by ). · Explain WhyMar 7 2019, 11:40 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 7 2019, 11:40 AM