This is an archive of the discontinued LLVM Phabricator instance.

Make a function to correctly extract the DW_AT_high_pc given the low pc value.
ClosedPublic

Authored by clayborg on Dec 17 2016, 4:06 PM.

Details

Summary

DWARF 4 and later supports encoding the PC as an address or as as offset from the low PC. Clients using DWARFDie should be insulated from how to extract the high PC value. This function takes care of extracting the form value and looking for the correct form.

Diff Detail

Event Timeline

clayborg updated this revision to Diff 81855.Dec 17 2016, 4:06 PM
clayborg retitled this revision from to Make a function to correctly extract the DW_AT_high_pc given the low pc value..
clayborg updated this object.
davide added a subscriber: davide.Dec 17 2016, 4:15 PM

is there a way to test this?

include/llvm/DebugInfo/DWARF/DWARFDie.h
285

The typo fix may be committed separately, maybe?

aprantl added inline comments.Dec 18 2016, 8:22 AM
lib/DebugInfo/DWARF/DWARFDie.cpp
266

should the update be atomically (ie. only update if highpc is available, too)?

tools/dsymutil/DwarfLinker.cpp
2145

I think MSVC now also understands
Ranges[*LowPc] = { *HighPc, MyInfo.AddrAdjust };

I will remove the typo fix and move the assignment of the low PC into the if.

include/llvm/DebugInfo/DWARF/DWARFDie.h
285

Yeah, I can remove this.

lib/DebugInfo/DWARF/DWARFDie.cpp
266

I can move the assignment into the if statement below.

I can add a test as well.

clayborg updated this revision to Diff 81976.Dec 19 2016, 11:07 AM

Addressed all comments and added a full suite of low/high PC unit tests.

probinson added inline comments.Dec 19 2016, 11:37 AM
include/llvm/DebugInfo/DWARF/DWARFDie.h
288

Comment should end with a period.

unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
796 ↗(On Diff #81976)

Comment should end with a period. Also 'grabs' not 'grab'.

aprantl accepted this revision.Dec 19 2016, 11:44 AM
aprantl edited edge metadata.

LGTM after addressing all outstanding issues. Thanks!

This revision is now accepted and ready to land.Dec 19 2016, 11:44 AM
This revision was automatically updated to reflect the committed changes.