https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/94026
We received a bug report where someone was trying to print a global variable
without a process. This would succeed in a debug build but fail in a on
optimized build. We traced the issue back to the location being described by a
DW_OP_addr + DW_OP_piece.
The issue is that the DWARF expression evaluator only support reading pieces
from a load address. There's no reason it cannot do the same for a file
address, and indeed, that solves the problem.
I unsuccessfully tried to craft a test case to illustrate the original example,
using a global struct and trying to trick the compiler into breaking it apart
with SROA. Instead I wrote a unit test that uses a mock target to read memory
from.
rdar://127435923
>From 0487856604f97d94d1be2502d9c41080064e2a64 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Fri, 31 May 2024 12:21:28 -0700
Subject: [PATCH] [lldb] Support reading DW_OP_piece from file address
We received a bug report where someone was trying to print a global
variable without a process. This would succeed in a debug build but fail
in a on optimized build. We traced the issue back to the location being
described by a DW_OP_addr + DW_OP_piece.
The issue is that the DWARF expression evaluator only support reading
pieces from a load address. There's no reason it cannot do the same for
a file address, and indeed, that solves the problem.
I unsuccessfully tried to craft a test case to illustrate the original
example, using a global struct and trying to trick the compiler into
breaking it apart with SROA. Instead I wrote a unit test that uses a
mock target to read memory from.
rdar://127435923
---
lldb/include/lldb/Target/Target.h | 8 +--
lldb/source/Expression/DWARFExpression.cpp | 46 ++++++++++----
.../Expression/DWARFExpressionTest.cpp | 60 +++++++++++++++++++
3 files changed, 97 insertions(+), 17 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 7ad9f33586054..792a4caa76e2d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1077,9 +1077,9 @@ class Target : public
std::enable_shared_from_this<Target>,
// section, then read from the file cache
// 2 - if there is a process, then read from memory
// 3 - if there is no process, then read from the file cache
- size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
- Status &error, bool force_live_memory = false,
- lldb::addr_t *load_addr_ptr = nullptr);
+ virtual size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
+ Status &error, bool force_live_memory = false,
+ lldb::addr_t *load_addr_ptr = nullptr);
size_t ReadCStringFromMemory(const Address &addr, std::string &out_str,
Status &error, bool force_live_memory = false);
@@ -1615,7 +1615,7 @@ class Target : public
std::enable_shared_from_this<Target>,
TargetStats &GetStatistics() { return m_stats; }
-private:
+protected:
/// Construct with optional file and arch.
///
/// This member is private. Clients must use
diff --git a/lldb/source/Expression/DWARFExpression.cpp
b/lldb/source/Expression/DWARFExpression.cpp
index c061fd1140fff..aed550e52c579 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -2153,21 +2153,41 @@ bool DWARFExpression::Evaluate(
}
break;
- case Value::ValueType::FileAddress:
- case Value::ValueType::HostAddress:
- if (error_ptr) {
- lldb::addr_t addr =
curr_piece_source_value.GetScalar().ULongLong(
- LLDB_INVALID_ADDRESS);
+ case Value::ValueType::FileAddress: {
+ lldb::addr_t addr = curr_piece_source_value.GetScalar().ULongLong(
+ LLDB_INVALID_ADDRESS);
+ if (target) {
+ if (curr_piece.ResizeData(piece_byte_size) == piece_byte_size) {
+ if (target->ReadMemory(addr, curr_piece.GetBuffer().GetBytes(),
+ piece_byte_size, error,
+ /*force_live_memory=*/false) !=
+ piece_byte_size) {
+ if (error_ptr)
+ error_ptr->SetErrorStringWithFormat(
+ "failed to read memory DW_OP_piece(%" PRIu64
+ ") from load address 0x%" PRIx64,
+ piece_byte_size, addr);
+ return false;
+ }
+ } else {
+ if (error_ptr)
+ error_ptr->SetErrorStringWithFormat(
+ "failed to read memory DW_OP_piece(%" PRIu64
+ ") from load address 0x%" PRIx64,
+ piece_byte_size, addr);
+ return false;
+ }
+ }
+ } break;
+ case Value::ValueType::HostAddress: {
+ lldb::addr_t addr = curr_piece_source_value.GetScalar().ULongLong(
+ LLDB_INVALID_ADDRESS);
+ if (error_ptr)
error_ptr->SetErrorStringWithFormat(
"failed to read memory DW_OP_piece(%" PRIu64
- ") from %s address 0x%" PRIx64,
- piece_byte_size, curr_piece_source_value.GetValueType() ==
- Value::ValueType::FileAddress
- ? "file"
- : "host",
- addr);
- }
- return false;
+ ") from host address 0x%" PRIx64,
+ piece_byte_size, addr);
+ } break;
case Value::ValueType::Scalar: {
uint32_t bit_size = piece_byte_size * 8;
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 8d77d6b2585f1..7b20c603889b2 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -768,3 +768,63 @@ TEST(DWARFExpression, ExtensionsDWO) {
testExpressionVendorExtensions(dwo_module_sp, *dwo_dwarf_unit);
}
+
+TEST_F(DWARFExpressionMockProcessTest, DW_OP_piece_file_addr) {
+ struct MockTarget : Target {
+ MockTarget(Debugger &debugger, const ArchSpec &target_arch,
+ const lldb::PlatformSP &platform_sp)
+ : Target(debugger, target_arch, platform_sp, true) {}
+
+ size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
+ Status &error, bool force_live_memory = false,
+ lldb::addr_t *load_addr_ptr = nullptr) override {
+ // We expected to be called in a very specific way.
+ assert(dst_len == 1);
+ assert(addr.GetOffset() == 0x40 || addr.GetOffset() == 0x50);
+
+ if (addr.GetOffset() == 0x40)
+ ((uint8_t *)dst)[0] = 0x11;
+
+ if (addr.GetOffset() == 0x50)
+ ((uint8_t *)dst)[0] = 0x22;
+
+ return 1;
+ }
+ };
+
+ // Set up a mock process.
+ ArchSpec arch("i386-pc-linux");
+ Platform::SetHostPlatform(
+ platform_linux::PlatformLinux::CreateInstance(true, &arch));
+ lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
+ ASSERT_TRUE(debugger_sp);
+ lldb::PlatformSP platform_sp;
+ lldb::TargetSP target_sp =
+ std::make_shared<MockTarget>(*debugger_sp, arch, platform_sp);
+ ASSERT_TRUE(target_sp);
+ ASSERT_TRUE(target_sp->GetArchitecture().IsValid());
+
+ ExecutionContext exe_ctx(target_sp, false);
+
+ uint8_t expr[] = {DW_OP_addr, 0x40, 0x0, 0x0, 0x0, DW_OP_piece, 1,
+ DW_OP_addr, 0x50, 0x0, 0x0, 0x0, DW_OP_piece, 1};
+ DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle,
+ /*addr_size*/ 4);
+ Value result;
+ Status status;
+ ASSERT_TRUE(DWARFExpression::Evaluate(
+ &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor,
+ /*unit*/ nullptr, lldb::eRegisterKindLLDB,
+ /*initial_value_ptr*/ nullptr,
+ /*object_address_ptr*/ nullptr, result, &status))
+ << status.ToError();
+
+ ASSERT_EQ(result.GetValueType(), Value::ValueType::HostAddress);
+
+ DataBufferHeap &buf = result.GetBuffer();
+ ASSERT_EQ(buf.GetByteSize(), 2U);
+
+ const uint8_t *bytes = buf.GetBytes();
+ EXPECT_EQ(bytes[0], 0x11);
+ EXPECT_EQ(bytes[1], 0x22);
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
