llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Igor Kudrin (igorkudrin) <details> <summary>Changes</summary> Both `Target::ReadSignedIntegerFromMemory()` and `Process::ReadSignedIntegerFromMemory()` internally created an unsigned scalar, so extending the value later did not duplicate the sign bit. --- Full diff: https://github.com/llvm/llvm-project/pull/169150.diff 3 Files Affected: - (modified) lldb/source/Target/Process.cpp (+3-1) - (modified) lldb/source/Target/Target.cpp (+4-2) - (modified) lldb/unittests/Target/MemoryTest.cpp (+61-1) ``````````diff 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 { `````````` </details> https://github.com/llvm/llvm-project/pull/169150 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
