https://github.com/dmpots created
https://github.com/llvm/llvm-project/pull/169587
This commit unifies the code in the dwarf expression evaluator that handles
these two deref operations. Previously we had similar, but not identical code
for handling them.
The implementation was taken from the DW_OP_deref_size code path since that
handles the general case. We put that code into an Evaluate_DW_OP_deref_size
function and call it with the address size when evaluating DW_OP_deref.
There were initially two test failures when I made the change. The
`DW_op_deref_no_ptr_fixing` unittest failed because we were not correctly
setting the address size when we created the DataExtractor. The `DW_OP_deref
test` failed because previously the expression `DW_OP_lit4, DW_OP_deref` would
evaluate to a LoadAddress, but the code for deref_size was evaluating it to a
scalar.
The difference was in how it handled a deref of a scalar value type. In the
deref code path we convert a scalar to a load address, but this was not done in
the deref_size code path.
```
case Value::ValueType::Scalar:
stack.back().SetValueType(Value::ValueType::LoadAddress);
```
I decided to modify the deref_size code to also convert the value to a load
address to keep the test passing.
There is no functional change intended here. The motivation is to reduce the
number of code paths that implement the deref operation.
>From 8f6d74a968e377d926d69eb95929ee7bbf62db50 Mon Sep 17 00:00:00 2001
From: David Peixotto <[email protected]>
Date: Tue, 25 Nov 2025 16:36:06 -0800
Subject: [PATCH] [lldb] Unify DW_OP_deref and DW_OP_deref_size implementations
This commit unifies the code in the dwarf expression evaluator that
handles these two deref operations. Previously we had similar, but not
identical code for handling them.
The implementation was taken from the DW_OP_deref_size code path since
that handles the general case. We put that code into an
Evaluate_DW_OP_deref_size function and call it with the address size
when evaluating DW_OP_deref.
There were initially two test failures when I made the change. The
`DW_op_deref_no_ptr_fixing` unittest failed because we were not
correctly setting the address size when we created the DataExtractor.
The DW_OP_deref test failed because previously the expression
`DW_OP_lit4, DW_OP_deref` would evaluate to a LoadAddress, but the code
for deref_size was evaluating it to a scalar.
The difference was in how it handled a deref of a scalar value type. In
the deref code path we convert a scalar to a load address, but this was
not done in the deref_size code path.
```
case Value::ValueType::Scalar:
stack.back().SetValueType(Value::ValueType::LoadAddress);
```
I decided to modify the deref_size code to also convert the value to a
load address to keep the test passing.
There is no functional change intended here. The motivation is to reduce
the number of code paths that implement the deref operation.
---
lldb/source/Expression/DWARFExpression.cpp | 311 ++++++++----------
.../Expression/DWARFExpressionTest.cpp | 5 +-
2 files changed, 136 insertions(+), 180 deletions(-)
diff --git a/lldb/source/Expression/DWARFExpression.cpp
b/lldb/source/Expression/DWARFExpression.cpp
index 4f9d6ebf27bf0..842874b3302df 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -861,84 +861,159 @@ ResolveLoadAddress(ExecutionContext *exe_ctx,
lldb::ModuleSP &module_sp,
return load_addr;
}
-static llvm::Error Evaluate_DW_OP_deref(DWARFExpression::Stack &stack,
- ExecutionContext *exe_ctx,
- lldb::ModuleSP module_sp,
- Process *process) {
- if (stack.empty())
- return llvm::createStringError("expression stack empty for DW_OP_deref");
-
- const Value::ValueType value_type = stack.back().GetValueType();
+/// Helper function to move common code used to load sized data from a uint8_t
+/// buffer.
+///
+/// \param addr_bytes uint8_t buffer containg raw data
+/// \param size_addr_bytes how large is the underlying raw data
+/// \param byte_order what is the byter order of the underlyig data
+/// \param size How much of the underlying data we want to use
+/// \return The underlying data converted into a Scalar
+static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
+ size_t size_addr_bytes,
+ ByteOrder byte_order, size_t size) {
+ DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
+
+ lldb::offset_t addr_data_offset = 0;
+ if (size <= 8)
+ return addr_data.GetMaxU64(&addr_data_offset, size);
+ else
+ return addr_data.GetAddress(&addr_data_offset);
+}
+
+static llvm::Error Evaluate_DW_OP_deref_size(DWARFExpression::Stack &stack,
+ ExecutionContext *exe_ctx,
+ lldb::ModuleSP module_sp,
+ Process *process, Target *target,
+ uint8_t size) {
+ if (stack.empty()) {
+ return llvm::createStringError(
+ "expression stack empty for DW_OP_deref_size");
+ }
+
+ if (size > 8) {
+ return llvm::createStringError(
+ "Invalid address size for DW_OP_deref_size: %d\n", size);
+ }
+ Value::ValueType value_type = stack.back().GetValueType();
switch (value_type) {
case Value::ValueType::HostAddress: {
void *src = (void *)stack.back().GetScalar().ULongLong();
intptr_t ptr;
::memcpy(&ptr, src, sizeof(void *));
+ // I can't decide whether the size operand should apply to the bytes in
+ // their
+ // lldb-host endianness or the target endianness.. I doubt this'll ever
+ // come up but I'll opt for assuming big endian regardless.
+ switch (size) {
+ case 1:
+ ptr = ptr & 0xff;
+ break;
+ case 2:
+ ptr = ptr & 0xffff;
+ break;
+ case 3:
+ ptr = ptr & 0xffffff;
+ break;
+ case 4:
+ ptr = ptr & 0xffffffff;
+ break;
+ // the casts are added to work around the case where intptr_t is a 32
+ // bit quantity;
+ // presumably we won't hit the 5..7 cases if (void*) is 32-bits in this
+ // program.
+ case 5:
+ ptr = (intptr_t)ptr & 0xffffffffffULL;
+ break;
+ case 6:
+ ptr = (intptr_t)ptr & 0xffffffffffffULL;
+ break;
+ case 7:
+ ptr = (intptr_t)ptr & 0xffffffffffffffULL;
+ break;
+ default:
+ break;
+ }
stack.back().GetScalar() = ptr;
stack.back().ClearContext();
} break;
case Value::ValueType::FileAddress: {
auto file_addr = stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
Address so_addr;
- auto maybe_load_addr = ResolveLoadAddress(exe_ctx, module_sp,
"DW_OP_deref",
- file_addr, so_addr);
+ auto maybe_load_addr = ResolveLoadAddress(
+ exe_ctx, module_sp, "DW_OP_deref_size", file_addr, so_addr,
+ /*check_sectionoffset=*/true);
+
if (!maybe_load_addr)
return maybe_load_addr.takeError();
- stack.back().GetScalar() = *maybe_load_addr;
+
+ addr_t load_addr = *maybe_load_addr;
+
+ if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
+ uint8_t addr_bytes[8];
+ Status error;
+
+ if (target && target->ReadMemory(so_addr, &addr_bytes, size, error,
+ /*force_live_memory=*/false) == size) {
+ ObjectFile *objfile = module_sp->GetObjectFile();
+
+ stack.back().GetScalar() = DerefSizeExtractDataHelper(
+ addr_bytes, size, objfile->GetByteOrder(), size);
+ stack.back().ClearContext();
+ break;
+ } else {
+ return llvm::createStringError(
+ "Failed to dereference pointer for DW_OP_deref_size: "
+ "%s\n",
+ error.AsCString());
+ }
+ }
+ stack.back().GetScalar() = load_addr;
// Fall through to load address promotion code below.
}
+
[[fallthrough]];
case Value::ValueType::Scalar:
// Promote Scalar to LoadAddress and fall through.
stack.back().SetValueType(Value::ValueType::LoadAddress);
[[fallthrough]];
- case Value::ValueType::LoadAddress: {
- if (!exe_ctx)
- return llvm::createStringError("NULL execution context for DW_OP_deref");
- if (!process)
- return llvm::createStringError("NULL process for DW_OP_deref");
- lldb::addr_t pointer_addr =
- stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
- Status error;
- lldb::addr_t pointer_value =
- process->ReadPointerFromMemory(pointer_addr, error);
- if (pointer_value == LLDB_INVALID_ADDRESS)
- return llvm::joinErrors(
- llvm::createStringError(
+ case Value::ValueType::LoadAddress:
+ if (exe_ctx) {
+ if (process) {
+ lldb::addr_t pointer_addr =
+ stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
+ uint8_t addr_bytes[sizeof(lldb::addr_t)];
+ Status error;
+ if (process->ReadMemory(pointer_addr, &addr_bytes, size, error) ==
+ size) {
+
+ stack.back().GetScalar() = DerefSizeExtractDataHelper(
+ addr_bytes, sizeof(addr_bytes), process->GetByteOrder(), size);
+ stack.back().ClearContext();
+ } else {
+ return llvm::createStringError(
"Failed to dereference pointer from 0x%" PRIx64
- " for DW_OP_deref",
- pointer_addr),
- error.takeError());
- stack.back().GetScalar() = pointer_value;
- stack.back().ClearContext();
- } break;
+ " for DW_OP_deref: %s\n",
+ pointer_addr, error.AsCString());
+ }
+ } else {
+
+ return llvm::createStringError("NULL process for DW_OP_deref_size");
+ }
+ } else {
+ return llvm::createStringError(
+ "NULL execution context for DW_OP_deref_size");
+ }
+ break;
+
case Value::ValueType::Invalid:
- return llvm::createStringError("invalid value type for DW_OP_deref");
+
+ return llvm::createStringError("invalid value for DW_OP_deref_size");
}
return llvm::Error::success();
}
-/// Helper function to move common code used to load sized data from a uint8_t
-/// buffer.
-///
-/// \param addr_bytes uint8_t buffer containg raw data
-/// \param size_addr_bytes how large is the underlying raw data
-/// \param byte_order what is the byter order of the underlyig data
-/// \param size How much of the underlying data we want to use
-/// \return The underlying data converted into a Scalar
-static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
- size_t size_addr_bytes,
- ByteOrder byte_order, size_t size) {
- DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
-
- lldb::offset_t addr_data_offset = 0;
- if (size <= 8)
- return addr_data.GetMaxU64(&addr_data_offset, size);
- else
- return addr_data.GetAddress(&addr_data_offset);
-}
-
llvm::Expected<Value> DWARFExpression::Evaluate(
ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
lldb::ModuleSP module_sp, const DataExtractor &opcodes,
@@ -1079,8 +1154,9 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
// retrieved from the dereferenced address is the size of an address on the
// target machine.
case DW_OP_deref: {
- if (llvm::Error err =
- Evaluate_DW_OP_deref(stack, exe_ctx, module_sp, process))
+ size_t size = opcodes.GetAddressByteSize();
+ if (llvm::Error err = Evaluate_DW_OP_deref_size(stack, exe_ctx,
module_sp,
+ process, target, size))
return err;
} break;
@@ -1097,131 +1173,10 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
// the size of an address on the target machine before being pushed on the
// expression stack.
case DW_OP_deref_size: {
- if (stack.empty()) {
- return llvm::createStringError(
- "expression stack empty for DW_OP_deref_size");
- }
- uint8_t size = opcodes.GetU8(&offset);
- if (size > 8) {
- return llvm::createStringError(
- "Invalid address size for DW_OP_deref_size: %d\n", size);
- }
- Value::ValueType value_type = stack.back().GetValueType();
- switch (value_type) {
- case Value::ValueType::HostAddress: {
- void *src = (void *)stack.back().GetScalar().ULongLong();
- intptr_t ptr;
- ::memcpy(&ptr, src, sizeof(void *));
- // I can't decide whether the size operand should apply to the bytes in
- // their
- // lldb-host endianness or the target endianness.. I doubt this'll ever
- // come up but I'll opt for assuming big endian regardless.
- switch (size) {
- case 1:
- ptr = ptr & 0xff;
- break;
- case 2:
- ptr = ptr & 0xffff;
- break;
- case 3:
- ptr = ptr & 0xffffff;
- break;
- case 4:
- ptr = ptr & 0xffffffff;
- break;
- // the casts are added to work around the case where intptr_t is a 32
- // bit quantity;
- // presumably we won't hit the 5..7 cases if (void*) is 32-bits in this
- // program.
- case 5:
- ptr = (intptr_t)ptr & 0xffffffffffULL;
- break;
- case 6:
- ptr = (intptr_t)ptr & 0xffffffffffffULL;
- break;
- case 7:
- ptr = (intptr_t)ptr & 0xffffffffffffffULL;
- break;
- default:
- break;
- }
- stack.back().GetScalar() = ptr;
- stack.back().ClearContext();
- } break;
- case Value::ValueType::FileAddress: {
- auto file_addr =
- stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
- Address so_addr;
- auto maybe_load_addr = ResolveLoadAddress(
- exe_ctx, module_sp, "DW_OP_deref_size", file_addr, so_addr,
- /*check_sectionoffset=*/true);
-
- if (!maybe_load_addr)
- return maybe_load_addr.takeError();
-
- addr_t load_addr = *maybe_load_addr;
-
- if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
- uint8_t addr_bytes[8];
- Status error;
-
- if (target &&
- target->ReadMemory(so_addr, &addr_bytes, size, error,
- /*force_live_memory=*/false) == size) {
- ObjectFile *objfile = module_sp->GetObjectFile();
-
- stack.back().GetScalar() = DerefSizeExtractDataHelper(
- addr_bytes, size, objfile->GetByteOrder(), size);
- stack.back().ClearContext();
- break;
- } else {
- return llvm::createStringError(
- "Failed to dereference pointer for DW_OP_deref_size: "
- "%s\n",
- error.AsCString());
- }
- }
- stack.back().GetScalar() = load_addr;
- // Fall through to load address promotion code below.
- }
-
- [[fallthrough]];
- case Value::ValueType::Scalar:
- case Value::ValueType::LoadAddress:
- if (exe_ctx) {
- if (process) {
- lldb::addr_t pointer_addr =
- stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
- uint8_t addr_bytes[sizeof(lldb::addr_t)];
- Status error;
- if (process->ReadMemory(pointer_addr, &addr_bytes, size, error) ==
- size) {
-
- stack.back().GetScalar() =
- DerefSizeExtractDataHelper(addr_bytes, sizeof(addr_bytes),
- process->GetByteOrder(), size);
- stack.back().ClearContext();
- } else {
- return llvm::createStringError(
- "Failed to dereference pointer from 0x%" PRIx64
- " for DW_OP_deref: %s\n",
- pointer_addr, error.AsCString());
- }
- } else {
-
- return llvm::createStringError("NULL process for
DW_OP_deref_size");
- }
- } else {
- return llvm::createStringError(
- "NULL execution context for DW_OP_deref_size");
- }
- break;
-
- case Value::ValueType::Invalid:
-
- return llvm::createStringError("invalid value for DW_OP_deref_size");
- }
-
+ size_t size = opcodes.GetU8(&offset);
+ if (llvm::Error err = Evaluate_DW_OP_deref_size(stack, exe_ctx,
module_sp,
+ process, target, size))
+ return err;
} break;
// OPCODE: DW_OP_xderef_size
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index e0c2193d27c36..0126c408d8696 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -237,8 +237,9 @@ static llvm::Expected<Value>
Evaluate(llvm::ArrayRef<uint8_t> expr,
DWARFExpression::Delegate *unit =
nullptr,
ExecutionContext *exe_ctx = nullptr,
RegisterContext *reg_ctx = nullptr) {
- DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle,
- /*addr_size*/ 4);
+ DataExtractor extractor(
+ expr.data(), expr.size(), lldb::eByteOrderLittle,
+ /*addr_size*/ exe_ctx ? exe_ctx->GetAddressByteSize() : 4);
return DWARFExpression::Evaluate(exe_ctx, reg_ctx, module_sp, extractor,
unit,
lldb::eRegisterKindLLDB,
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
