Page MenuHomePhabricator

Fix a buffer-size bug when the first DW_OP_piece is undefined
ClosedPublic

Authored by aprantl on Jan 16 2020, 2:29 PM.

Details

Summary
Fix a buffer-size bug when the first DW_OP_piece is undefined and document the shortcomings of LLDB's partially defined DW_OP_piece  handling.

This would manifest as "DW_OP_piece for offset foo but top of stack is of size bar".

rdar://problem/46262998

Diff Detail

Event Timeline

aprantl created this revision.Jan 16 2020, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2020, 2:29 PM
vsk accepted this revision.Jan 16 2020, 3:56 PM
vsk added a subscriber: vsk.

LGTM, this seems like a clear improvement.

This revision is now accepted and ready to land.Jan 16 2020, 3:56 PM
vsk added inline comments.Jan 16 2020, 4:03 PM
lldb/source/Expression/DWARFExpression.cpp
2077

FTR I'm not really sure this case is allowed by the dwarf standard. Under DW_OP_piece the standard references a "preceding simple location description" -- istm like an error if the stack is empty. My $0.0002 :)

This revision was automatically updated to reflect the committed changes.
labath added a subscriber: labath.Jan 17 2020, 12:48 AM
labath added inline comments.
lldb/source/Expression/DWARFExpression.cpp
2077

I think this is fine -- an "empty location description" (section 2.6.1.1.4 of DWARF4) is a kind of a "simple location description" (sect. 2.6.1.1).

The part that worries me more is this: "Each simple location description that is a DWARF expression is evaluated independently of any others (as though on its own separate stack, if any).". I think that means we should reset/clear the stack after each DW_OP_piece, but we don't currently do that (we just pop the topmost value). However, I am not sure if this has any impact on valid DWARF expressions (though it can cause us to produce bogus values for invalid ones).

aprantl marked an inline comment as done.Jan 17 2020, 11:18 AM
aprantl added inline comments.
lldb/source/Expression/DWARFExpression.cpp
2077

That's cute, so with our current implementation one could write a communicating DWARF expression like:

DW_OP_lit1 DW_OP_dup DW_OP_piece 1 DW_OP_piece 1

Is the second byte supposed to be 1 or undef?

;-)

I agree, we should probably clear the stack.