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 { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
