[Lldb-commits] [lldb] a059afa - [lldb] Fix reading 32-bit signed integers (#169150)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Nov 26 10:49:53 PST 2025
Author: Igor Kudrin
Date: 2025-11-26T10:49:49-08:00
New Revision: a059afafde068773693c1fab4d89c208b1437f76
URL: https://github.com/llvm/llvm-project/commit/a059afafde068773693c1fab4d89c208b1437f76
DIFF: https://github.com/llvm/llvm-project/commit/a059afafde068773693c1fab4d89c208b1437f76.diff
LOG: [lldb] Fix reading 32-bit signed integers (#169150)
Both `Target::ReadSignedIntegerFromMemory()` and
`Process::ReadSignedIntegerFromMemory()` internally created an unsigned
scalar, so extending the value later did not duplicate the sign bit.
Added:
Modified:
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/unittests/Target/MemoryTest.cpp
Removed:
################################################################################
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 69edea503002e..5879b8f4795ab 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2452,8 +2452,10 @@ size_t Process::ReadScalarIntegerFromMemory(addr_t addr, uint32_t byte_size,
scalar = data.GetMaxU32(&offset, byte_size);
else
scalar = data.GetMaxU64(&offset, byte_size);
- if (is_signed)
+ if (is_signed) {
+ scalar.MakeSigned();
scalar.SignExtend(byte_size * 8);
+ }
return bytes_read;
}
} else {
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 12c653c0eb8cf..3a936b85f6339 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2283,8 +2283,10 @@ size_t Target::ReadScalarIntegerFromMemory(const Address &addr, uint32_t byte_si
else
scalar = data.GetMaxU64(&offset, byte_size);
- if (is_signed)
+ if (is_signed) {
+ scalar.MakeSigned();
scalar.SignExtend(byte_size * 8);
+ }
return bytes_read;
}
} else {
@@ -2299,7 +2301,7 @@ int64_t Target::ReadSignedIntegerFromMemory(const Address &addr,
int64_t fail_value, Status &error,
bool force_live_memory) {
Scalar scalar;
- if (ReadScalarIntegerFromMemory(addr, integer_byte_size, false, scalar, error,
+ if (ReadScalarIntegerFromMemory(addr, integer_byte_size, true, scalar, error,
force_live_memory))
return scalar.SLongLong(fail_value);
return fail_value;
diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index e444f68dc4871..131a3cabdd896 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -48,6 +48,8 @@ class DummyProcess : public Process {
}
Status DoDestroy() override { return {}; }
void RefreshStateAfterStop() override {}
+ // Required by Target::ReadMemory() to call Process::ReadMemory()
+ bool IsAlive() override { return true; }
size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
Status &error) override {
if (m_bytes_left == 0)
@@ -61,7 +63,7 @@ class DummyProcess : public Process {
m_bytes_left -= size;
}
- memset(buf, 'B', num_bytes_to_write);
+ memset(buf, m_filler, num_bytes_to_write);
return num_bytes_to_write;
}
bool DoUpdateThreadList(ThreadList &old_thread_list,
@@ -72,8 +74,10 @@ class DummyProcess : public Process {
// Test-specific additions
size_t m_bytes_left;
+ int m_filler = 'B';
MemoryCache &GetMemoryCache() { return m_memory_cache; }
void SetMaxReadSize(size_t size) { m_bytes_left = size; }
+ void SetFiller(int filler) { m_filler = filler; }
};
} // namespace
@@ -85,6 +89,18 @@ TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) {
return target_sp;
}
+static ProcessSP CreateProcess(lldb::TargetSP target_sp) {
+ ListenerSP listener_sp(Listener::MakeListener("dummy"));
+ ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp);
+
+ struct TargetHack : public Target {
+ void SetProcess(ProcessSP process) { m_process_sp = process; }
+ };
+ static_cast<TargetHack *>(target_sp.get())->SetProcess(process_sp);
+
+ return process_sp;
+}
+
TEST_F(MemoryTest, TesetMemoryCacheRead) {
ArchSpec arch("x86_64-apple-macosx-");
@@ -96,8 +112,7 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
TargetSP target_sp = CreateTarget(debugger_sp, arch);
ASSERT_TRUE(target_sp);
- ListenerSP listener_sp(Listener::MakeListener("dummy"));
- ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp);
+ ProcessSP process_sp = CreateProcess(target_sp);
ASSERT_TRUE(process_sp);
DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
@@ -227,6 +242,58 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
// old cache
}
+TEST_F(MemoryTest, TestReadInteger) {
+ ArchSpec arch("x86_64-apple-macosx-");
+
+ Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+ DebuggerSP debugger_sp = Debugger::CreateInstance();
+ ASSERT_TRUE(debugger_sp);
+
+ TargetSP target_sp = CreateTarget(debugger_sp, arch);
+ ASSERT_TRUE(target_sp);
+
+ ProcessSP process_sp = CreateProcess(target_sp);
+ ASSERT_TRUE(process_sp);
+
+ DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
+ Status error;
+
+ process->SetFiller(0xff);
+ process->SetMaxReadSize(256);
+ // The ReadSignedIntegerFromMemory() methods return int64_t. Check that they
+ // extend the sign correctly when reading 32-bit values.
+ EXPECT_EQ(-1,
+ target_sp->ReadSignedIntegerFromMemory(Address(0), 4, 0, error));
+ EXPECT_EQ(-1, process->ReadSignedIntegerFromMemory(0, 4, 0, error));
+ // Check reading 64-bit values as well.
+ EXPECT_EQ(-1,
+ target_sp->ReadSignedIntegerFromMemory(Address(0), 8, 0, error));
+ EXPECT_EQ(-1, process->ReadSignedIntegerFromMemory(0, 8, 0, error));
+
+ // ReadUnsignedIntegerFromMemory() should not extend the sign.
+ EXPECT_EQ(0xffffffffULL,
+ target_sp->ReadUnsignedIntegerFromMemory(Address(0), 4, 0, error));
+ EXPECT_EQ(0xffffffffULL,
+ process->ReadUnsignedIntegerFromMemory(0, 4, 0, error));
+ EXPECT_EQ(0xffffffffffffffffULL,
+ target_sp->ReadUnsignedIntegerFromMemory(Address(0), 8, 0, error));
+ EXPECT_EQ(0xffffffffffffffffULL,
+ process->ReadUnsignedIntegerFromMemory(0, 8, 0, error));
+
+ // Check reading positive values.
+ process->GetMemoryCache().Clear();
+ process->SetFiller(0x7f);
+ process->SetMaxReadSize(256);
+ EXPECT_EQ(0x7f7f7f7fLL,
+ target_sp->ReadSignedIntegerFromMemory(Address(0), 4, 0, error));
+ EXPECT_EQ(0x7f7f7f7fLL, process->ReadSignedIntegerFromMemory(0, 4, 0, error));
+ EXPECT_EQ(0x7f7f7f7f7f7f7f7fLL,
+ target_sp->ReadSignedIntegerFromMemory(Address(0), 8, 0, error));
+ EXPECT_EQ(0x7f7f7f7f7f7f7f7fLL,
+ process->ReadSignedIntegerFromMemory(0, 8, 0, error));
+}
+
/// A process class that, when asked to read memory from some address X, returns
/// the least significant byte of X.
class DummyReaderProcess : public Process {
More information about the lldb-commits
mailing list