[Lldb-commits] [PATCH] D98996: Teach DWARFExpression about DWARF 4+ Location Descriptions

Mon Mar 22 17:31:29 PDT 2021

aprantl added a comment.

@dblaikie wrote:

> (idle question: Is this code remotely related to any code in LLVM's libDebugInfoDWARF? Does this patch take us closer to or further away from unifying them? If it takes us further away, any chance of designing it in a direction that points towards each other rather than away from?)

There's definitely an opportunity to refactor the main loop of Evaluate() to use llvm::DWARFExpression::iterator. I would say The impact of this patch towards that goal is neutral. The iterator (and all of libDebugInfo) doesn't care about the semantics of DWARF expressions.

================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2615

   if (stack.empty()) {
     // Nothing on the stack, check if we created a piece value from DW_OP_piece
----------------
@labath wrote:

> Are you sure that this logic is correct in presence of DW_OP(_bit)_piece? If I follow this right, then the final value type will be determined by the last operand. That sounds like it could be right for regular dwarf expressions, but I'm not sure about those with pieces. The classification function will mark those as "memory", and that doesn't sound right...

DW_OP_pieces are handled in this block.

For your immediate question, this means that this patch is safe. However, it also highlights that this patch isn't fixing the same bug in a composite location description. I'll address this in my next revision of the patch.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98996/new/

https://reviews.llvm.org/D98996