Skip to content

Commit 7b0d58e

Browse files
committed
Add testing for DW_OP_piece and fix a bug with small Scalar values.
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
1 parent 2671df9 commit 7b0d58e

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

Diff for: lldb/source/Expression/DWARFExpression.cpp

+12-4
Original file line numberDiff line numberDiff line change
@@ -2128,7 +2128,8 @@ bool DWARFExpression::Evaluate(
21282128
case Value::eValueTypeScalar: {
21292129
uint32_t bit_size = piece_byte_size * 8;
21302130
uint32_t bit_offset = 0;
2131-
if (!curr_piece_source_value.GetScalar().ExtractBitfield(
2131+
Scalar &scalar = curr_piece_source_value.GetScalar();
2132+
if (!scalar.ExtractBitfield(
21322133
bit_size, bit_offset)) {
21332134
if (error_ptr)
21342135
error_ptr->SetErrorStringWithFormat(
@@ -2139,7 +2140,14 @@ bool DWARFExpression::Evaluate(
21392140
.GetByteSize());
21402141
return false;
21412142
}
2142-
curr_piece = curr_piece_source_value;
2143+
// Create curr_piece with bit_size. By default Scalar
2144+
// grows to the nearest host integer type.
2145+
llvm::APInt fail_value(1, 0, false);
2146+
llvm::APInt ap_int = scalar.UInt128(fail_value);
2147+
assert(ap_int.getBitWidth() >= bit_size);
2148+
llvm::ArrayRef<uint64_t> buf{ap_int.getRawData(),
2149+
ap_int.getNumWords()};
2150+
curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
21432151
} break;
21442152

21452153
case Value::eValueTypeVector: {
@@ -2161,15 +2169,15 @@ bool DWARFExpression::Evaluate(
21612169
if (op_piece_offset == 0) {
21622170
// This is the first piece, we should push it back onto the stack
21632171
// so subsequent pieces will be able to access this piece and add
2164-
// to it
2172+
// to it.
21652173
if (pieces.AppendDataToHostBuffer(curr_piece) == 0) {
21662174
if (error_ptr)
21672175
error_ptr->SetErrorString("failed to append piece data");
21682176
return false;
21692177
}
21702178
} else {
21712179
// If this is the second or later piece there should be a value on
2172-
// the stack
2180+
// the stack.
21732181
if (pieces.GetBuffer().GetByteSize() != op_piece_offset) {
21742182
if (error_ptr)
21752183
error_ptr->SetErrorStringWithFormat(

Diff for: lldb/unittests/Expression/DWARFExpressionTest.cpp

+22-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,22 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr,
3737
/*object_address_ptr*/ nullptr, result, &status))
3838
return status.ToError();
3939

40-
return result.GetScalar();
40+
switch (result.GetValueType()) {
41+
case Value::eValueTypeScalar:
42+
return result.GetScalar();
43+
case Value::eValueTypeHostAddress: {
44+
// Convert small buffers to scalars to simplify the tests.
45+
DataBufferHeap &buf = result.GetBuffer();
46+
if (buf.GetByteSize() <= 8) {
47+
uint64_t val = 0;
48+
memcpy(&val, buf.GetBytes(), buf.GetByteSize());
49+
return Scalar(llvm::APInt(buf.GetByteSize()*8, val, false));
50+
}
51+
}
52+
LLVM_FALLTHROUGH;
53+
default:
54+
return status.ToError();
55+
}
4156
}
4257

4358
/// A mock module holding an object file parsed from YAML.
@@ -335,3 +350,9 @@ TEST(DWARFExpression, DW_OP_convert) {
335350
t.Eval({DW_OP_const1s, 'X', DW_OP_convert, 0x1d}).takeError(),
336351
llvm::Failed());
337352
}
353+
354+
TEST(DWARFExpression, DW_OP_piece) {
355+
EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x11, 0x22, DW_OP_piece, 2,
356+
DW_OP_const2u, 0x33, 0x44, DW_OP_piece, 2}),
357+
llvm::HasValue(GetScalar(32, 0x44332211, true)));
358+
}

0 commit comments

Comments
 (0)