From 578a42589020f1b0ab5e4be458d55309d8a4add7 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 9 Nov 2017 10:43:16 +0000 Subject: [PATCH] Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder Summary: These functions used to return bool to signify whether they were able to retrieve the data. This is redundant because the ArchSpec and ByteOrder already have their own "invalid" states, *and* because both of the current implementations (linux, netbsd) can always provide a valid result. This allows us to simplify bits of the code handling these values. Reviewers: eugene, krytarowski Subscribers: javed.absar, lldb-commits Differential Revision: https://reviews.llvm.org/D39733 llvm-svn: 317779 --- lldb/include/lldb/Host/common/NativeProcessProtocol.h | 7 +++++-- lldb/source/Host/common/NativeProcessProtocol.cpp | 10 ---------- lldb/source/Host/common/NativeRegisterContext.cpp | 17 ++++------------- .../source/Plugins/Process/Linux/NativeProcessLinux.cpp | 5 ----- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h | 2 +- .../Process/Linux/NativeRegisterContextLinux.cpp | 10 +--------- .../Process/Linux/NativeRegisterContextLinux_arm64.cpp | 16 ++++------------ .../Process/Linux/NativeRegisterContextLinux_mips64.cpp | 12 +++++------- lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp | 6 +----- .../Plugins/Process/NetBSD/NativeProcessNetBSD.cpp | 5 ----- .../source/Plugins/Process/NetBSD/NativeProcessNetBSD.h | 2 +- .../Plugins/Process/NetBSD/NativeThreadNetBSD.cpp | 6 +----- .../gdb-remote/GDBRemoteCommunicationServerLLGS.cpp | 15 +++------------ 13 files changed, 26 insertions(+), 87 deletions(-) diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h index 554f53d..8f80ef1 100644 --- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -13,6 +13,7 @@ #include "NativeBreakpointList.h" #include "NativeThreadProtocol.h" #include "NativeWatchpointList.h" +#include "lldb/Core/ArchSpec.h" #include "lldb/Host/Host.h" #include "lldb/Host/MainLoop.h" #include "lldb/Utility/Status.h" @@ -100,7 +101,7 @@ public: virtual size_t UpdateThreads() = 0; - virtual bool GetArchitecture(ArchSpec &arch) const = 0; + virtual const ArchSpec &GetArchitecture() const = 0; //---------------------------------------------------------------------- // Breakpoint functions @@ -151,7 +152,9 @@ public: bool CanResume() const { return m_state == lldb::eStateStopped; } - bool GetByteOrder(lldb::ByteOrder &byte_order) const; + lldb::ByteOrder GetByteOrder() const { + return GetArchitecture().GetByteOrder(); + } virtual llvm::ErrorOr> GetAuxvData() const = 0; diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index b2cf7f4..0c51d50 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -8,8 +8,6 @@ //===----------------------------------------------------------------------===// #include "lldb/Host/common/NativeProcessProtocol.h" - -#include "lldb/Core/ArchSpec.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/State.h" #include "lldb/Host/Host.h" @@ -116,14 +114,6 @@ bool NativeProcessProtocol::IsAlive() const { m_state != eStateInvalid && m_state != eStateUnloaded; } -bool NativeProcessProtocol::GetByteOrder(lldb::ByteOrder &byte_order) const { - ArchSpec process_arch; - if (!GetArchitecture(process_arch)) - return false; - byte_order = process_arch.GetByteOrder(); - return true; -} - const NativeWatchpointList::WatchpointMap & NativeProcessProtocol::GetWatchpointMap() const { return m_watchpoint_list.GetWatchpointMap(); diff --git a/lldb/source/Host/common/NativeRegisterContext.cpp b/lldb/source/Host/common/NativeRegisterContext.cpp index 629b024..c0e2478 100644 --- a/lldb/source/Host/common/NativeRegisterContext.cpp +++ b/lldb/source/Host/common/NativeRegisterContext.cpp @@ -368,13 +368,8 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory( // TODO: we might need to add a parameter to this function in case the byte // order of the memory data doesn't match the process. For now we are assuming // they are the same. - lldb::ByteOrder byte_order; - if (process.GetByteOrder(byte_order)) { - error.SetErrorString("NativeProcessProtocol::GetByteOrder () failed"); - return error; - } - - reg_value.SetFromMemoryData(reg_info, src, src_len, byte_order, error); + reg_value.SetFromMemoryData(reg_info, src, src_len, process.GetByteOrder(), + error); return error; } @@ -393,12 +388,8 @@ Status NativeRegisterContext::WriteRegisterValueToMemory( // order of the memory data doesn't match the process. For now we are // assuming // they are the same. - lldb::ByteOrder byte_order; - if (!process.GetByteOrder(byte_order)) - return Status("NativeProcessProtocol::GetByteOrder () failed"); - - const size_t bytes_copied = - reg_value.GetAsMemoryData(reg_info, dst, dst_len, byte_order, error); + const size_t bytes_copied = reg_value.GetAsMemoryData( + reg_info, dst, dst_len, process.GetByteOrder(), error); if (error.Success()) { if (bytes_copied == 0) { diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index 150de3c..3f4842a 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1542,11 +1542,6 @@ size_t NativeProcessLinux::UpdateThreads() { return m_threads.size(); } -bool NativeProcessLinux::GetArchitecture(ArchSpec &arch) const { - arch = m_arch; - return true; -} - Status NativeProcessLinux::GetSoftwareBreakpointPCOffset( uint32_t &actual_opcode_size) { // FIXME put this behind a breakpoint protocol class that can be diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h index 3d86120..d213a81 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -87,7 +87,7 @@ public: size_t UpdateThreads() override; - bool GetArchitecture(ArchSpec &arch) const override; + const ArchSpec &GetArchitecture() const override { return m_arch; } Status SetBreakpoint(lldb::addr_t addr, uint32_t size, bool hardware) override; diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp index 30f09f0..91fbd22 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp @@ -26,15 +26,7 @@ NativeRegisterContextLinux::NativeRegisterContextLinux( reg_info_interface_p) {} lldb::ByteOrder NativeRegisterContextLinux::GetByteOrder() const { - // Get the target process whose privileged thread was used for the register - // read. - lldb::ByteOrder byte_order = lldb::eByteOrderInvalid; - - if (!m_thread.GetProcess().GetByteOrder(byte_order)) { - // FIXME log here - } - - return byte_order; + return m_thread.GetProcess().GetByteOrder(); } Status NativeRegisterContextLinux::ReadRegisterRaw(uint32_t reg_index, diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp index 7aad062..b66117b 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp @@ -872,12 +872,8 @@ Status NativeRegisterContextLinux_arm64::DoReadRegisterValue( error = NativeProcessLinux::PtraceWrapper( PTRACE_GETREGSET, m_thread.GetID(), ®set, &ioVec, sizeof regs); if (error.Success()) { - ArchSpec arch; - if (m_thread.GetProcess().GetArchitecture(arch)) - value.SetBytes((void *)(((unsigned char *)(®s)) + offset), 16, - arch.GetByteOrder()); - else - error.SetErrorString("failed to get architecture"); + value.SetBytes((void *)(((unsigned char *)(®s)) + offset), 16, + m_thread.GetProcess().GetByteOrder()); } } else { elf_gregset_t regs; @@ -889,12 +885,8 @@ Status NativeRegisterContextLinux_arm64::DoReadRegisterValue( error = NativeProcessLinux::PtraceWrapper( PTRACE_GETREGSET, m_thread.GetID(), ®set, &ioVec, sizeof regs); if (error.Success()) { - ArchSpec arch; - if (m_thread.GetProcess().GetArchitecture(arch)) - value.SetBytes((void *)(((unsigned char *)(regs)) + offset), 8, - arch.GetByteOrder()); - else - error.SetErrorString("failed to get architecture"); + value.SetBytes((void *)(((unsigned char *)(regs)) + offset), 8, + m_thread.GetProcess().GetByteOrder()); } } return error; diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp index f35ff2b..792f2de 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp @@ -1033,13 +1033,11 @@ Status NativeRegisterContextLinux_mips64::Read_SR_Config(uint32_t offset, Status error = NativeProcessLinux::PtraceWrapper( PTRACE_GETREGS, m_thread.GetID(), NULL, ®s, sizeof regs); if (error.Success()) { - lldb_private::ArchSpec arch; - if (m_thread.GetProcess().GetArchitecture(arch)) { - void *target_address = ((uint8_t *)®s) + offset + - 4 * (arch.GetMachine() == llvm::Triple::mips); - value.SetUInt(*(uint32_t *)target_address, size); - } else - error.SetErrorString("failed to get architecture"); + const lldb_private::ArchSpec &arch = + m_thread.GetProcess().GetArchitecture(); + void *target_address = ((uint8_t *)®s) + offset + + 4 * (arch.GetMachine() == llvm::Triple::mips); + value.SetUInt(*(uint32_t *)target_address, size); } return error; } diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp index 5cd4094..71324a4 100644 --- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp @@ -144,14 +144,10 @@ NativeRegisterContextSP NativeThreadLinux::GetRegisterContext() { if (m_reg_context_sp) return m_reg_context_sp; - ArchSpec target_arch; - if (!m_process.GetArchitecture(target_arch)) - return NativeRegisterContextSP(); - const uint32_t concrete_frame_idx = 0; m_reg_context_sp.reset( NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux( - target_arch, *this, concrete_frame_idx)); + m_process.GetArchitecture(), *this, concrete_frame_idx)); return m_reg_context_sp; } diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp index 4f9a696..40d0396 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -678,11 +678,6 @@ lldb::addr_t NativeProcessNetBSD::GetSharedLibraryInfoAddress() { size_t NativeProcessNetBSD::UpdateThreads() { return m_threads.size(); } -bool NativeProcessNetBSD::GetArchitecture(ArchSpec &arch) const { - arch = m_arch; - return true; -} - Status NativeProcessNetBSD::SetBreakpoint(lldb::addr_t addr, uint32_t size, bool hardware) { if (hardware) diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h index 5ecf462..8f34d96 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h @@ -77,7 +77,7 @@ public: size_t UpdateThreads() override; - bool GetArchitecture(ArchSpec &arch) const override; + const ArchSpec &GetArchitecture() const override { return m_arch; } Status SetBreakpoint(lldb::addr_t addr, uint32_t size, bool hardware) override; diff --git a/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp index 1fd7400..b64935a 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp +++ b/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp @@ -144,14 +144,10 @@ NativeRegisterContextSP NativeThreadNetBSD::GetRegisterContext() { if (m_reg_context_sp) return m_reg_context_sp; - ArchSpec target_arch; - if (!m_process.GetArchitecture(target_arch)) - return NativeRegisterContextSP(); - const uint32_t concrete_frame_idx = 0; m_reg_context_sp.reset( NativeRegisterContextNetBSD::CreateHostNativeRegisterContextNetBSD( - target_arch, *this, concrete_frame_idx)); + m_process.GetArchitecture(), *this, concrete_frame_idx)); return m_reg_context_sp; } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index d5b031b..3cbe717 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -2041,17 +2041,6 @@ GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) { return SendIllFormedResponse( packet, "P packet missing '=' char after register number"); - // Get process architecture. - ArchSpec process_arch; - if (!m_debugged_process_up || - !m_debugged_process_up->GetArchitecture(process_arch)) { - if (log) - log->Printf("GDBRemoteCommunicationServerLLGS::%s failed to retrieve " - "inferior architecture", - __FUNCTION__); - return SendErrorResponse(0x49); - } - // Parse out the value. uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register size_t reg_size = packet.GetHexBytesAvail(reg_bytes); @@ -2109,7 +2098,9 @@ GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) { // Build the reginfos response. StreamGDBRemote response; - RegisterValue reg_value(reg_bytes, reg_size, process_arch.GetByteOrder()); + RegisterValue reg_value( + reg_bytes, reg_size, + m_debugged_process_up->GetArchitecture().GetByteOrder()); Status error = reg_context_sp->WriteRegister(reg_info, reg_value); if (error.Fail()) { if (log) -- 2.7.4