[Lldb-commits] [lldb] 4edb7e3 - [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 10 07:10:19 PST 2020
Author: Pavel Labath
Date: 2020-11-10T16:10:08+01:00
New Revision: 4edb7e34f824ca4adaa55d3668e050166bae2ad4
URL: https://github.com/llvm/llvm-project/commit/4edb7e34f824ca4adaa55d3668e050166bae2ad4
DIFF: https://github.com/llvm/llvm-project/commit/4edb7e34f824ca4adaa55d3668e050166bae2ad4.diff
LOG: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results
Dwarf says (Section 2.5.1.1. of DWARF v5) that these operations should
push "generic" (pointer-sized) values. This was not the case for
DW_OP_const operations (which were pushing values based on the size of
arguments), nor DW_OP_litN (which were always pushing 64-bit values).
The practical effect of this that were were unable to display the values
of variables if the size of the DW_OP_const opcode was smaller than the
value of the variable it was describing. This would happen because we
would store this (small) result into a buffer and then would not be able
to read sufficient data out of it (in Value::GetValueAsData). Gcc emits
debug info like this.
Other (more subtle) effects are also possible.
The same fix should be applied to DW_OP_const[us] (leb128 versions), but
I'm not doing that right now, because that would cause us to display
wrong (truncated) values of variables on 32-bit targets (pr48087).
Differential Revision: https://reviews.llvm.org/D90840
Added:
Modified:
lldb/include/lldb/Utility/Scalar.h
lldb/source/Expression/DWARFExpression.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index 09d0f641e4d1..f797aaf99626 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -69,6 +69,8 @@ class Scalar {
}
Scalar(llvm::APInt v)
: m_type(e_int), m_integer(std::move(v), false), m_float(0.0f) {}
+ Scalar(llvm::APSInt v)
+ : m_type(e_int), m_integer(std::move(v)), m_float(0.0f) {}
bool SignExtend(uint32_t bit_pos);
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 9240956defc8..c30fdf565cd5 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -941,6 +941,16 @@ bool DWARFExpression::Evaluate(
Value pieces; // Used for DW_OP_piece
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+ // A generic type is "an integral type that has the size of an address and an
+ // unspecified signedness". For now, just use the signedness of the operand.
+ // TODO: Implement a real typed stack, and store the genericness of the value
+ // there.
+ auto to_generic = [&](auto v) {
+ bool is_signed = std::is_signed<decltype(v)>::value;
+ return Scalar(llvm::APSInt(
+ llvm::APInt(8 * opcodes.GetAddressByteSize(), v, is_signed),
+ !is_signed));
+ };
while (opcodes.ValidOffset(offset)) {
const lldb::offset_t op_offset = offset;
@@ -1251,37 +1261,42 @@ bool DWARFExpression::Evaluate(
// All DW_OP_constXXX opcodes have a single operand as noted below:
//
// Opcode Operand 1
- // DW_OP_const1u 1-byte unsigned integer constant DW_OP_const1s
- // 1-byte signed integer constant DW_OP_const2u 2-byte unsigned integer
- // constant DW_OP_const2s 2-byte signed integer constant DW_OP_const4u
- // 4-byte unsigned integer constant DW_OP_const4s 4-byte signed integer
- // constant DW_OP_const8u 8-byte unsigned integer constant DW_OP_const8s
- // 8-byte signed integer constant DW_OP_constu unsigned LEB128 integer
- // constant DW_OP_consts signed LEB128 integer constant
+ // DW_OP_const1u 1-byte unsigned integer constant
+ // DW_OP_const1s 1-byte signed integer constant
+ // DW_OP_const2u 2-byte unsigned integer constant
+ // DW_OP_const2s 2-byte signed integer constant
+ // DW_OP_const4u 4-byte unsigned integer constant
+ // DW_OP_const4s 4-byte signed integer constant
+ // DW_OP_const8u 8-byte unsigned integer constant
+ // DW_OP_const8s 8-byte signed integer constant
+ // DW_OP_constu unsigned LEB128 integer constant
+ // DW_OP_consts signed LEB128 integer constant
case DW_OP_const1u:
- stack.push_back(Scalar((uint8_t)opcodes.GetU8(&offset)));
+ stack.push_back(to_generic(opcodes.GetU8(&offset)));
break;
case DW_OP_const1s:
- stack.push_back(Scalar((int8_t)opcodes.GetU8(&offset)));
+ stack.push_back(to_generic((int8_t)opcodes.GetU8(&offset)));
break;
case DW_OP_const2u:
- stack.push_back(Scalar((uint16_t)opcodes.GetU16(&offset)));
+ stack.push_back(to_generic(opcodes.GetU16(&offset)));
break;
case DW_OP_const2s:
- stack.push_back(Scalar((int16_t)opcodes.GetU16(&offset)));
+ stack.push_back(to_generic((int16_t)opcodes.GetU16(&offset)));
break;
case DW_OP_const4u:
- stack.push_back(Scalar((uint32_t)opcodes.GetU32(&offset)));
+ stack.push_back(to_generic(opcodes.GetU32(&offset)));
break;
case DW_OP_const4s:
- stack.push_back(Scalar((int32_t)opcodes.GetU32(&offset)));
+ stack.push_back(to_generic((int32_t)opcodes.GetU32(&offset)));
break;
case DW_OP_const8u:
- stack.push_back(Scalar((uint64_t)opcodes.GetU64(&offset)));
+ stack.push_back(to_generic(opcodes.GetU64(&offset)));
break;
case DW_OP_const8s:
- stack.push_back(Scalar((int64_t)opcodes.GetU64(&offset)));
+ stack.push_back(to_generic((int64_t)opcodes.GetU64(&offset)));
break;
+ // These should also use to_generic, but we can't do that due to a
+ // producer-side bug in llvm. See llvm.org/pr48087.
case DW_OP_constu:
stack.push_back(Scalar(opcodes.GetULEB128(&offset)));
break;
@@ -1888,7 +1903,7 @@ bool DWARFExpression::Evaluate(
case DW_OP_lit29:
case DW_OP_lit30:
case DW_OP_lit31:
- stack.push_back(Scalar((uint64_t)(op - DW_OP_lit0)));
+ stack.push_back(to_generic(op - DW_OP_lit0));
break;
// OPCODE: DW_OP_regN
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 886a83de3eb5..37bd900cb9b5 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -75,6 +75,37 @@ TEST(DWARFExpression, DW_OP_pick) {
llvm::Failed());
}
+TEST(DWARFExpression, DW_OP_const) {
+ // Extend to address size.
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const1u, 0x88}), llvm::HasValue(0x88));
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const1s, 0x88}),
+ llvm::HasValue(0xffffff88));
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x47, 0x88}),
+ llvm::HasValue(0x8847));
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2s, 0x47, 0x88}),
+ llvm::HasValue(0xffff8847));
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const4u, 0x44, 0x42, 0x47, 0x88}),
+ llvm::HasValue(0x88474244));
+ EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const4s, 0x44, 0x42, 0x47, 0x88}),
+ llvm::HasValue(0x88474244));
+
+ // Truncate to address size.
+ EXPECT_THAT_EXPECTED(
+ Evaluate({DW_OP_const8u, 0x00, 0x11, 0x22, 0x33, 0x44, 0x42, 0x47, 0x88}),
+ llvm::HasValue(0x33221100));
+ EXPECT_THAT_EXPECTED(
+ Evaluate({DW_OP_const8s, 0x00, 0x11, 0x22, 0x33, 0x44, 0x42, 0x47, 0x88}),
+ llvm::HasValue(0x33221100));
+
+ // Don't truncate to address size for compatibility with clang (pr48087).
+ EXPECT_THAT_EXPECTED(
+ Evaluate({DW_OP_constu, 0x81, 0x82, 0x84, 0x88, 0x90, 0xa0, 0x40}),
+ llvm::HasValue(0x01010101010101));
+ EXPECT_THAT_EXPECTED(
+ Evaluate({DW_OP_consts, 0x81, 0x82, 0x84, 0x88, 0x90, 0xa0, 0x40}),
+ llvm::HasValue(0xffff010101010101));
+}
+
TEST(DWARFExpression, DW_OP_convert) {
/// Auxiliary debug info.
const char *yamldata = R"(
@@ -157,22 +188,15 @@ TEST(DWARFExpression, DW_OP_convert) {
// Positive tests.
//
- // Truncate to default unspecified (pointer-sized) type.
- EXPECT_THAT_EXPECTED(
- t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
- DW_OP_convert, 0x00}),
- llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
- // Truncate to 32 bits.
- EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const8u, //
- 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,//
+ // Leave as is.
+ EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //
DW_OP_convert, offs_uint32_t}),
- llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
+ llvm::HasValue(GetScalar(64, 0x44332211, not_signed)));
- // Leave as is.
- EXPECT_THAT_EXPECTED(
- t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
- DW_OP_convert, offs_uint64_t}),
- llvm::HasValue(GetScalar(64, 0x8877665544332211, not_signed)));
+ // Zero-extend to 64 bits.
+ EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //
+ DW_OP_convert, offs_uint64_t}),
+ llvm::HasValue(GetScalar(64, 0x44332211, not_signed)));
// Sign-extend to 64 bits.
EXPECT_THAT_EXPECTED(
@@ -180,6 +204,18 @@ TEST(DWARFExpression, DW_OP_convert) {
DW_OP_convert, offs_sint64_t}),
llvm::HasValue(GetScalar(64, 0xffffffffffeeddcc, is_signed)));
+ // Sign-extend, then truncate.
+ EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4s, 0xcc, 0xdd, 0xee, 0xff, //
+ DW_OP_convert, offs_sint64_t, //
+ DW_OP_convert, offs_uint32_t}),
+ llvm::HasValue(GetScalar(32, 0xffeeddcc, not_signed)));
+
+ // Truncate to default unspecified (pointer-sized) type.
+ EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4s, 0xcc, 0xdd, 0xee, 0xff, //
+ DW_OP_convert, offs_sint64_t, //
+ DW_OP_convert, 0x00}),
+ llvm::HasValue(GetScalar(32, 0xffeeddcc, not_signed)));
+
// Truncate to 8 bits.
EXPECT_THAT_EXPECTED(
t.Eval({DW_OP_const4s, 'A', 'B', 'C', 'D', DW_OP_convert, offs_uchar}),
More information about the lldb-commits
mailing list