Skip to content

Commit

Permalink
Add testing for DW_OP_piece and fix a bug with small Scalar values.
Browse files Browse the repository at this point in the history
By switching to Scalars that are backed by explicitly-sized APInts we
can avoid a bug that increases the buffer reserved for a small piece
to the next-largest host integer type.

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

Differential Revision: https://reviews.llvm.org/D72879

(cherry picked from commit 7b0d58e)
  • Loading branch information
adrian-prantl authored and zmodem committed Feb 19, 2020
1 parent d5f8656 commit 7751f0c
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
16 changes: 12 additions & 4 deletions lldb/source/Expression/DWARFExpression.cpp
Expand Up @@ -2128,7 +2128,8 @@ bool DWARFExpression::Evaluate(
case Value::eValueTypeScalar: {
uint32_t bit_size = piece_byte_size * 8;
uint32_t bit_offset = 0;
if (!curr_piece_source_value.GetScalar().ExtractBitfield(
Scalar &scalar = curr_piece_source_value.GetScalar();
if (!scalar.ExtractBitfield(
bit_size, bit_offset)) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat(
Expand All @@ -2139,7 +2140,14 @@ bool DWARFExpression::Evaluate(
.GetByteSize());
return false;
}
curr_piece = curr_piece_source_value;
// Create curr_piece with bit_size. By default Scalar
// grows to the nearest host integer type.
llvm::APInt fail_value(1, 0, false);
llvm::APInt ap_int = scalar.UInt128(fail_value);
assert(ap_int.getBitWidth() >= bit_size);
llvm::ArrayRef<uint64_t> buf{ap_int.getRawData(),
ap_int.getNumWords()};
curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
} break;

case Value::eValueTypeVector: {
Expand All @@ -2161,15 +2169,15 @@ bool DWARFExpression::Evaluate(
if (op_piece_offset == 0) {
// This is the first piece, we should push it back onto the stack
// so subsequent pieces will be able to access this piece and add
// to it
// to it.
if (pieces.AppendDataToHostBuffer(curr_piece) == 0) {
if (error_ptr)
error_ptr->SetErrorString("failed to append piece data");
return false;
}
} else {
// If this is the second or later piece there should be a value on
// the stack
// the stack.
if (pieces.GetBuffer().GetByteSize() != op_piece_offset) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat(
Expand Down
23 changes: 22 additions & 1 deletion lldb/unittests/Expression/DWARFExpressionTest.cpp
Expand Up @@ -37,7 +37,22 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr,
/*object_address_ptr*/ nullptr, result, &status))
return status.ToError();

return result.GetScalar();
switch (result.GetValueType()) {
case Value::eValueTypeScalar:
return result.GetScalar();
case Value::eValueTypeHostAddress: {
// Convert small buffers to scalars to simplify the tests.
DataBufferHeap &buf = result.GetBuffer();
if (buf.GetByteSize() <= 8) {
uint64_t val = 0;
memcpy(&val, buf.GetBytes(), buf.GetByteSize());
return Scalar(llvm::APInt(buf.GetByteSize()*8, val, false));
}
}
LLVM_FALLTHROUGH;
default:
return status.ToError();
}
}

/// A mock module holding an object file parsed from YAML.
Expand Down Expand Up @@ -335,3 +350,9 @@ TEST(DWARFExpression, DW_OP_convert) {
t.Eval({DW_OP_const1s, 'X', DW_OP_convert, 0x1d}).takeError(),
llvm::Failed());
}

TEST(DWARFExpression, DW_OP_piece) {
EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x11, 0x22, DW_OP_piece, 2,
DW_OP_const2u, 0x33, 0x44, DW_OP_piece, 2}),
llvm::HasValue(GetScalar(32, 0x44332211, true)));
}

0 comments on commit 7751f0c

Please sign in to comment.