From 2e5aa6f603adac6bfe5900d873900992102f933f Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Fri, 21 Nov 2025 22:07:01 -0800 Subject: [PATCH 1/2] [lldb] Fix reading 32-bit signed integers Both `Target::ReadSignedIntegerFromMemory()` and `Process::ReadSignedIntegerFromMemory()` internally created an unsigned scalar, so extending the value later did not duplicate the sign bit. --- lldb/source/Target/Process.cpp | 4 +- lldb/source/Target/Target.cpp | 6 ++- lldb/unittests/Target/MemoryTest.cpp | 62 +++++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 4 deletions(-) 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 6fc8053038d10..a06626e4e3044 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2282,8 +2282,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 { @@ -2298,7 +2300,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..c095e7c5753f0 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -61,7 +61,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 +72,11 @@ 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; } + using Process::SetPrivateState; }; } // namespace @@ -85,6 +88,13 @@ TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { return target_sp; } +static void OverrideTargetProcess(Target *target, lldb::ProcessSP process) { + struct TargetHack : public Target { + void SetProcess(lldb::ProcessSP process) { m_process_sp = process; } + }; + static_cast(target)->SetProcess(process); +} + TEST_F(MemoryTest, TesetMemoryCacheRead) { ArchSpec arch("x86_64-apple-macosx-"); @@ -227,6 +237,56 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) { // old cache } +TEST_F(MemoryTest, TestProcessReadSignedInteger) { + 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); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast(process_sp.get()); + process->SetFiller(0xff); + process->SetMaxReadSize(4); + + Status error; + int64_t val = process->ReadSignedIntegerFromMemory(0, 4, 0, error); + EXPECT_EQ(val, -1); +} + +TEST_F(MemoryTest, TestTargetReadSignedInteger) { + 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); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + OverrideTargetProcess(target_sp.get(), process_sp); + + DummyProcess *process = static_cast(process_sp.get()); + process->SetFiller(0xff); + process->SetMaxReadSize(4); + process->SetPrivateState(eStateStopped); + + Status error; + int64_t val = target_sp->ReadSignedIntegerFromMemory(Address(0), 4, 0, error); + EXPECT_EQ(val, -1); +} + /// A process class that, when asked to read memory from some address X, returns /// the least significant byte of X. class DummyReaderProcess : public Process { From 44bc3b63c423240a91c01f531b44426609a829d4 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Mon, 24 Nov 2025 19:58:31 -0800 Subject: [PATCH 2/2] fixup! Update tests --- lldb/unittests/Target/MemoryTest.cpp | 83 +++++++++++++++------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index c095e7c5753f0..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) @@ -76,7 +78,6 @@ class DummyProcess : public Process { MemoryCache &GetMemoryCache() { return m_memory_cache; } void SetMaxReadSize(size_t size) { m_bytes_left = size; } void SetFiller(int filler) { m_filler = filler; } - using Process::SetPrivateState; }; } // namespace @@ -88,11 +89,16 @@ TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { return target_sp; } -static void OverrideTargetProcess(Target *target, lldb::ProcessSP process) { +static ProcessSP CreateProcess(lldb::TargetSP target_sp) { + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared(target_sp, listener_sp); + struct TargetHack : public Target { - void SetProcess(lldb::ProcessSP process) { m_process_sp = process; } + void SetProcess(ProcessSP process) { m_process_sp = process; } }; - static_cast(target)->SetProcess(process); + static_cast(target_sp.get())->SetProcess(process_sp); + + return process_sp; } TEST_F(MemoryTest, TesetMemoryCacheRead) { @@ -106,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(target_sp, listener_sp); + ProcessSP process_sp = CreateProcess(target_sp); ASSERT_TRUE(process_sp); DummyProcess *process = static_cast(process_sp.get()); @@ -237,7 +242,7 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) { // old cache } -TEST_F(MemoryTest, TestProcessReadSignedInteger) { +TEST_F(MemoryTest, TestReadInteger) { ArchSpec arch("x86_64-apple-macosx-"); Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); @@ -248,43 +253,45 @@ TEST_F(MemoryTest, TestProcessReadSignedInteger) { TargetSP target_sp = CreateTarget(debugger_sp, arch); ASSERT_TRUE(target_sp); - ListenerSP listener_sp(Listener::MakeListener("dummy")); - ProcessSP process_sp = std::make_shared(target_sp, listener_sp); + ProcessSP process_sp = CreateProcess(target_sp); ASSERT_TRUE(process_sp); DummyProcess *process = static_cast(process_sp.get()); - process->SetFiller(0xff); - process->SetMaxReadSize(4); - Status error; - int64_t val = process->ReadSignedIntegerFromMemory(0, 4, 0, error); - EXPECT_EQ(val, -1); -} - -TEST_F(MemoryTest, TestTargetReadSignedInteger) { - 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); - ListenerSP listener_sp(Listener::MakeListener("dummy")); - ProcessSP process_sp = std::make_shared(target_sp, listener_sp); - ASSERT_TRUE(process_sp); - OverrideTargetProcess(target_sp.get(), process_sp); - - DummyProcess *process = static_cast(process_sp.get()); process->SetFiller(0xff); - process->SetMaxReadSize(4); - process->SetPrivateState(eStateStopped); - - Status error; - int64_t val = target_sp->ReadSignedIntegerFromMemory(Address(0), 4, 0, error); - EXPECT_EQ(val, -1); + 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