[Lldb-commits] [PATCH] D73148: [lldb/Value] Report size of Value as size of underlying data buffer

Tue Jan 21 17:35:06 PST 2020

vsk added a comment.

In D73148#1832704 <https://reviews.llvm.org/D73148#1832704>, @clayborg wrote:

> Would it be better to just ensure that the buffer for DW_OP_piece is at least the size of the type instead? To do this right we would really need to track which bytes in the buffer are actually valid. This patch assumes we will always get the first N bytes of a value. Is it possible to describe bytes 4:7 of a 16 bit value where bits 0:3 and 8:15 are not valid? In this case, with this patch, we would think that bytes 0:3 are valid when they are just not specified, 4:7 would be valid, and 8:15 would not display because our value doesn't contain the value.

It's not necessary for DW_OP_piece to produce a type-sized result buffer, as the description of an object must start at byte offset 0, and we represent the undefined bits within the Value either with 0 or by leaving them out. E.g. `Evaluate({DW_OP_piece, 1, DW_OP_const1u, 0xff, DW_OP_piece, 1, DW_OP_piece, 1})` produces the 24-bit scalar `0x00ff00`, where the 0's correspond to the undefined low bits and the undefined high bits are left out. It /is/ a bug that we use 0 for undefined bits instead of `u` (or something), but we don't need a type-sized buffer to fix that (a bitset would suffice). Finding a way to improve the representation of undefined bits is something I'm interested in, but it seems like a separate problem, and it's more urgent for me to fix the memory smasher.

I'm also not sure that making DW_OP_piece produce a type-sized result would be a sufficient fix, as there (theoretically) may be clients of `Value::ResizeData()` other than the DW_OP_piece logic. I just count one, though (`ExpressionVariable::GetValueBytes()`), and I'm not sure whether it can resize the buffer to something other than the underlying type size.

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

https://reviews.llvm.org/D73148