This is an archive of the discontinued LLVM Phabricator instance.

[debug_loc] Fix typo in DWARFExpression constructor
ClosedPublic

Authored by labath on Apr 5 2018, 8:08 AM.

Details

Summary

The positions of the DwarfVersion and AddressSize arguments were
reversed, which caused parsing for dwarf opcodes which contained
address-size-dependent operands (such as DW_OP_addr). Amusingly enough,
none of the address-size asserts fired, as dwarf version was always 4,
which is a valid address size.

I ran into this when constructing weird inputs for the DWARF verifier. I
I add a test case as hand-written dwarf -- I am not sure how to trigger
this differently, as having a DW_OP_addr inside a location list is a
fairly non-standard thing to do.

Fixing this error exposed a bug in the debug_loc.dwo parser, which was
always being constructed with an address size of 0. I fix that as well
by following the pattern in the non-dwo parser of picking up the address
size from the first compile unit (which is technically not correct, but
probably good enough in practice).

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Apr 5 2018, 8:08 AM
aprantl accepted this revision.Apr 5 2018, 8:46 AM
aprantl added inline comments.
lib/DebugInfo/DWARF/DWARFContext.cpp
689 ↗(On Diff #141157)

LLVM always prefers full sentences:

// Assume all compile units have the same address byte size.
lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
36 ↗(On Diff #141157)

Ouch.

This revision is now accepted and ready to land.Apr 5 2018, 8:46 AM
JDevlieghere accepted this revision.Apr 5 2018, 11:15 AM

Wow... great thing you caught this.

This revision was automatically updated to reflect the committed changes.
labath added inline comments.Apr 6 2018, 1:53 AM
lib/DebugInfo/DWARF/DWARFContext.cpp
689 ↗(On Diff #141157)

I thought you might say that. :)
I will fix also the place where I copied this from.