[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b0d58e339b2: Add testing for DW_OP_piece and fix a bug with
small Scalar values. (authored by aprantl).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72879/new/
https://reviews.llvm.org/D72879
Files:
lldb/source/Expression/DWARFExpression.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@
/*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.
@@ -335,3 +350,9 @@
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)));
+}
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -2128,7 +2128,8 @@
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(
@@ -2139,7 +2140,14 @@
.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 buf{ap_int.getRawData(),
+ ap_int.getNumWords()};
+curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
} break;
case Value::eValueTypeVector: {
@@ -2161,7 +2169,7 @@
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");
@@ -2169,7 +2177,7 @@
}
} 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(
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@
/*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.
@@ -335,3 +350,9 @@
t.Eval({DW_OP_const1s, 'X', DW_OP_convert, 0x1d}).ta
[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.
vsk accepted this revision. vsk added a comment. LGTM. In the future it'd be nice to maybe split out piece/bit_piece handling into their own self-contained functions, and maybe share more code between them. But landing this narrow fix now sounds good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72879/new/ https://reviews.llvm.org/D72879 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.
aprantl added a comment. > My understanding is that DW_OP_piece is still incomplete, right? DW_OP_piece should work (with the documented caveat about the undefined values) fine, what definitely doesn't yet work is the combination of DW_OP_piece and DW_OP_bit_piece. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72879/new/ https://reviews.llvm.org/D72879 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Looks great! Thanks Adrian. My understanding is that `DW_OP_piece` is still incomplete, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72879/new/ https://reviews.llvm.org/D72879 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.
aprantl created this revision.
aprantl added reviewers: davide, friss, jasonmolenda.
Herald added a project: LLDB.
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".
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72879
Files:
lldb/source/Expression/DWARFExpression.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@
/*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.
@@ -335,3 +350,9 @@
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)));
+}
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -2128,7 +2128,8 @@
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(
@@ -2139,7 +2140,14 @@
.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 buf{ap_int.getRawData(),
+ ap_int.getNumWords()};
+curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
} break;
case Value::eValueTypeVector: {
@@ -2161,7 +2169,7 @@
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");
@@ -2169,7 +2177,7 @@
}
} 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(
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@
/*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
