https://github.com/igorkudrin created https://github.com/llvm/llvm-project/pull/169150
Both `Target::ReadSignedIntegerFromMemory()` and `Process::ReadSignedIntegerFromMemory()` internally created an unsigned scalar, so extending the value later did not duplicate the sign bit. >From 2e5aa6f603adac6bfe5900d873900992102f933f Mon Sep 17 00:00:00 2001 From: Igor Kudrin <[email protected]> Date: Fri, 21 Nov 2025 22:07:01 -0800 Subject: [PATCH] [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<TargetHack *>(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<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast<DummyProcess *>(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<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + OverrideTargetProcess(target_sp.get(), process_sp); + + DummyProcess *process = static_cast<DummyProcess *>(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 { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
