From 4fd77668b2cc215f0605fe20bb989b90b29f4346 Mon Sep 17 00:00:00 2001 From: Muhammad Omair Javaid Date: Fri, 15 Jan 2021 15:33:31 +0500 Subject: [PATCH] [LLDB] Add per-thread register infos shared pointer in gdb-remote In gdb-remote process we have register infos defind as a refernce object of GDBRemoteDynamicRegisterInfo class. In past register infos have remained constant througout the life time of a process. This has changed after AArch64 SVE support where register infos will have per-thread configuration. SVE registers will have per-thread size and can be updated while running. This patch aims to build up for that support by changing GDBRemoteDynamicRegisterInfo reference to a shared pointer deinfed per-thread. Reviewed By: labath Differential Revision: https://reviews.llvm.org/D82857 --- .../Process/Utility/DynamicRegisterInfo.cpp | 2 ++ .../Plugins/Process/Utility/DynamicRegisterInfo.h | 10 +++++--- .../gdb-remote/GDBRemoteRegisterContext.cpp | 28 ++++++++++---------- .../Process/gdb-remote/GDBRemoteRegisterContext.h | 10 +++++--- .../Process/gdb-remote/ProcessGDBRemote.cpp | 30 +++++++++++----------- .../Plugins/Process/gdb-remote/ProcessGDBRemote.h | 2 +- .../Plugins/Process/gdb-remote/ThreadGDBRemote.cpp | 12 +++++++-- .../Plugins/Process/gdb-remote/ThreadGDBRemote.h | 4 +++ 8 files changed, 61 insertions(+), 37 deletions(-) diff --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp index 71e9b5e..411db05 100644 --- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp +++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp @@ -650,6 +650,8 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) { } } +bool DynamicRegisterInfo::IsReconfigurable() { return m_is_reconfigurable; } + size_t DynamicRegisterInfo::GetNumRegisters() const { return m_regs.size(); } size_t DynamicRegisterInfo::GetNumRegisterSets() const { return m_sets.size(); } diff --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h index d5e6f90..0938b47 100644 --- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h +++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h @@ -17,6 +17,10 @@ #include "lldb/lldb-private.h" class DynamicRegisterInfo { +protected: + DynamicRegisterInfo(DynamicRegisterInfo &) = default; + DynamicRegisterInfo &operator=(DynamicRegisterInfo &) = default; + public: DynamicRegisterInfo() = default; @@ -25,9 +29,6 @@ public: virtual ~DynamicRegisterInfo() = default; - DynamicRegisterInfo(DynamicRegisterInfo &) = delete; - void operator=(DynamicRegisterInfo &) = delete; - DynamicRegisterInfo(DynamicRegisterInfo &&info); DynamicRegisterInfo &operator=(DynamicRegisterInfo &&info); @@ -63,6 +64,8 @@ public: void Clear(); + bool IsReconfigurable(); + protected: // Classes that inherit from DynamicRegisterInfo can see and modify these typedef std::vector reg_collection; @@ -89,5 +92,6 @@ protected: size_t m_reg_data_byte_size = 0u; // The number of bytes required to store // all registers bool m_finalized = false; + bool m_is_reconfigurable = false; }; #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index 1f31b45..19bcac5 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -31,19 +31,20 @@ using namespace lldb_private::process_gdb_remote; // GDBRemoteRegisterContext constructor GDBRemoteRegisterContext::GDBRemoteRegisterContext( ThreadGDBRemote &thread, uint32_t concrete_frame_idx, - GDBRemoteDynamicRegisterInfo ®_info, bool read_all_at_once, + GDBRemoteDynamicRegisterInfoSP reg_info_sp, bool read_all_at_once, bool write_all_at_once) - : RegisterContext(thread, concrete_frame_idx), m_reg_info(reg_info), - m_reg_valid(), m_reg_data(), m_read_all_at_once(read_all_at_once), + : RegisterContext(thread, concrete_frame_idx), + m_reg_info_sp(std::move(reg_info_sp)), m_reg_valid(), m_reg_data(), + m_read_all_at_once(read_all_at_once), m_write_all_at_once(write_all_at_once) { // Resize our vector of bools to contain one bool for every register. We will // use these boolean values to know when a register value is valid in // m_reg_data. - m_reg_valid.resize(reg_info.GetNumRegisters()); + m_reg_valid.resize(m_reg_info_sp->GetNumRegisters()); // Make a heap based buffer that is big enough to store all registers DataBufferSP reg_data_sp( - new DataBufferHeap(reg_info.GetRegisterDataByteSize(), 0)); + new DataBufferHeap(m_reg_info_sp->GetRegisterDataByteSize(), 0)); m_reg_data.SetData(reg_data_sp); m_reg_data.SetByteOrder(thread.GetProcess()->GetByteOrder()); } @@ -62,12 +63,12 @@ void GDBRemoteRegisterContext::SetAllRegisterValid(bool b) { } size_t GDBRemoteRegisterContext::GetRegisterCount() { - return m_reg_info.GetNumRegisters(); + return m_reg_info_sp->GetNumRegisters(); } const RegisterInfo * GDBRemoteRegisterContext::GetRegisterInfoAtIndex(size_t reg) { - RegisterInfo *reg_info = m_reg_info.GetRegisterInfoAtIndex(reg); + RegisterInfo *reg_info = m_reg_info_sp->GetRegisterInfoAtIndex(reg); if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes) { const ArchSpec &arch = m_thread.GetProcess()->GetTarget().GetArchitecture(); @@ -78,11 +79,11 @@ GDBRemoteRegisterContext::GetRegisterInfoAtIndex(size_t reg) { } size_t GDBRemoteRegisterContext::GetRegisterSetCount() { - return m_reg_info.GetNumRegisterSets(); + return m_reg_info_sp->GetNumRegisterSets(); } const RegisterSet *GDBRemoteRegisterContext::GetRegisterSet(size_t reg_set) { - return m_reg_info.GetRegisterSet(reg_set); + return m_reg_info_sp->GetRegisterSet(reg_set); } bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info, @@ -209,9 +210,10 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, SetAllRegisterValid(true); return true; } else if (buffer_sp->GetByteSize() > 0) { - const int regcount = m_reg_info.GetNumRegisters(); + const int regcount = m_reg_info_sp->GetNumRegisters(); for (int i = 0; i < regcount; i++) { - struct RegisterInfo *reginfo = m_reg_info.GetRegisterInfoAtIndex(i); + struct RegisterInfo *reginfo = + m_reg_info_sp->GetRegisterInfoAtIndex(i); if (reginfo->byte_offset + reginfo->byte_size <= buffer_sp->GetByteSize()) { m_reg_valid[i] = true; @@ -506,7 +508,7 @@ bool GDBRemoteRegisterContext::ReadAllRegisterValues( // m_reg_data buffer } data_sp = std::make_shared( - m_reg_data.GetDataStart(), m_reg_info.GetRegisterDataByteSize()); + m_reg_data.GetDataStart(), m_reg_info_sp->GetRegisterDataByteSize()); return true; } else { @@ -708,7 +710,7 @@ bool GDBRemoteRegisterContext::WriteAllRegisterValues( uint32_t GDBRemoteRegisterContext::ConvertRegisterKindToRegisterNumber( lldb::RegisterKind kind, uint32_t num) { - return m_reg_info.ConvertRegisterKindToRegisterNumber(kind, num); + return m_reg_info_sp->ConvertRegisterKindToRegisterNumber(kind, num); } void GDBRemoteDynamicRegisterInfo::HardcodeARMRegisters(bool from_scratch) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h index 0158625..475e65a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -27,8 +27,12 @@ namespace process_gdb_remote { class ThreadGDBRemote; class ProcessGDBRemote; +class GDBRemoteDynamicRegisterInfo; -class GDBRemoteDynamicRegisterInfo : public DynamicRegisterInfo { +typedef std::shared_ptr + GDBRemoteDynamicRegisterInfoSP; + +class GDBRemoteDynamicRegisterInfo final : public DynamicRegisterInfo { public: GDBRemoteDynamicRegisterInfo() : DynamicRegisterInfo() {} @@ -40,7 +44,7 @@ public: class GDBRemoteRegisterContext : public RegisterContext { public: GDBRemoteRegisterContext(ThreadGDBRemote &thread, uint32_t concrete_frame_idx, - GDBRemoteDynamicRegisterInfo ®_info, + GDBRemoteDynamicRegisterInfoSP reg_info_sp, bool read_all_at_once, bool write_all_at_once); ~GDBRemoteRegisterContext() override; @@ -106,7 +110,7 @@ protected: m_reg_valid[reg] = valid; } - GDBRemoteDynamicRegisterInfo &m_reg_info; + GDBRemoteDynamicRegisterInfoSP m_reg_info_sp; std::vector m_reg_valid; DataExtractor m_reg_data; bool m_read_all_at_once; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 0d02acf..f9b0632 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -249,7 +249,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, ListenerSP listener_sp) : Process(target_sp, listener_sp), m_debugserver_pid(LLDB_INVALID_PROCESS_ID), m_last_stop_packet_mutex(), - m_register_info(), + m_register_info_sp(nullptr), m_async_broadcaster(nullptr, "lldb.process.gdb-remote.async-broadcaster"), m_async_listener_sp( Listener::MakeListener("lldb.process.gdb-remote.async-listener")), @@ -368,8 +368,8 @@ bool ProcessGDBRemote::ParsePythonTargetDefinition( m_breakpoint_pc_offset = breakpoint_pc_int_value->GetValue(); } - if (m_register_info.SetRegisterInfo(*target_definition_sp, - GetTarget().GetArchitecture()) > 0) { + if (m_register_info_sp->SetRegisterInfo( + *target_definition_sp, GetTarget().GetArchitecture()) > 0) { return true; } } @@ -396,10 +396,10 @@ static size_t SplitCommaSeparatedRegisterNumberString( } void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { - if (!force && m_register_info.GetNumRegisters() > 0) + if (!force && m_register_info_sp) return; - m_register_info.Clear(); + m_register_info_sp = std::make_shared(); // Check if qHostInfo specified a specific packet timeout for this // connection. If so then lets update our setting so the user knows what the @@ -581,7 +581,7 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use)) abi_sp->AugmentRegisterInfo(reg_info); - m_register_info.AddRegister(reg_info, reg_name, alt_name, set_name); + m_register_info_sp->AddRegister(reg_info, reg_name, alt_name, set_name); } else { break; // ensure exit before reg_num is incremented } @@ -590,8 +590,8 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { } } - if (m_register_info.GetNumRegisters() > 0) { - m_register_info.Finalize(GetTarget().GetArchitecture()); + if (m_register_info_sp->GetNumRegisters() > 0) { + m_register_info_sp->Finalize(GetTarget().GetArchitecture()); return; } @@ -600,21 +600,21 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { // updated debugserver down on the devices. On the other hand, if the // accumulated reg_num is positive, see if we can add composite registers to // the existing primordial ones. - bool from_scratch = (m_register_info.GetNumRegisters() == 0); + bool from_scratch = (m_register_info_sp->GetNumRegisters() == 0); if (!target_arch.IsValid()) { if (arch_to_use.IsValid() && (arch_to_use.GetMachine() == llvm::Triple::arm || arch_to_use.GetMachine() == llvm::Triple::thumb) && arch_to_use.GetTriple().getVendor() == llvm::Triple::Apple) - m_register_info.HardcodeARMRegisters(from_scratch); + m_register_info_sp->HardcodeARMRegisters(from_scratch); } else if (target_arch.GetMachine() == llvm::Triple::arm || target_arch.GetMachine() == llvm::Triple::thumb) { - m_register_info.HardcodeARMRegisters(from_scratch); + m_register_info_sp->HardcodeARMRegisters(from_scratch); } // At this point, we can finalize our register info. - m_register_info.Finalize(GetTarget().GetArchitecture()); + m_register_info_sp->Finalize(GetTarget().GetArchitecture()); } Status ProcessGDBRemote::WillLaunch(lldb_private::Module *module) { @@ -4567,7 +4567,7 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess( // ABI is also potentially incorrect. ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use); for (auto &feature_node : feature_nodes) { - ParseRegisters(feature_node, target_info, this->m_register_info, + ParseRegisters(feature_node, target_info, *this->m_register_info_sp, abi_to_use_sp, reg_num_remote, reg_num_local); } @@ -4597,9 +4597,9 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) { uint32_t reg_num_local = 0; if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml", reg_num_remote, reg_num_local)) - this->m_register_info.Finalize(arch_to_use); + this->m_register_info_sp->Finalize(arch_to_use); - return m_register_info.GetNumRegisters() > 0; + return m_register_info_sp->GetNumRegisters() > 0; } llvm::Expected ProcessGDBRemote::GetLoadedModuleList() { diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 8e0a04f..b07b2c9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -254,7 +254,7 @@ protected: // the last stop // packet variable std::recursive_mutex m_last_stop_packet_mutex; - GDBRemoteDynamicRegisterInfo m_register_info; + GDBRemoteDynamicRegisterInfoSP m_register_info_sp; Broadcaster m_async_broadcaster; lldb::ListenerSP m_async_listener_sp; HostThread m_async_thread; diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp index 6deabf8..2a9896e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp @@ -42,6 +42,14 @@ ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid) Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD)); LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(), GetID()); + // At this point we can clone reg_info for architectures supporting + // run-time update to register sizes and offsets.. + auto &gdb_process = static_cast(process); + if (!gdb_process.m_register_info_sp->IsReconfigurable()) + m_reg_info_sp = gdb_process.m_register_info_sp; + else + m_reg_info_sp = std::make_shared( + *gdb_process.m_register_info_sp); } ThreadGDBRemote::~ThreadGDBRemote() { @@ -307,8 +315,8 @@ ThreadGDBRemote::CreateRegisterContextForFrame(StackFrame *frame) { !pSupported || gdb_process->m_use_g_packet_for_reading; bool write_all_registers_at_once = !pSupported; reg_ctx_sp = std::make_shared( - *this, concrete_frame_idx, gdb_process->m_register_info, - read_all_registers_at_once, write_all_registers_at_once); + *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once, + write_all_registers_at_once); } } else { reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame); diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h index 5ad1117..b7d75021 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h @@ -14,6 +14,8 @@ #include "lldb/Target/Thread.h" #include "lldb/Utility/StructuredData.h" +#include "GDBRemoteRegisterContext.h" + class StringExtractor; namespace lldb_private { @@ -101,6 +103,8 @@ protected: m_queue_serial_number; // Queue info from stop reply/stop info for thread lldb_private::LazyBool m_associated_with_libdispatch_queue; + GDBRemoteDynamicRegisterInfoSP m_reg_info_sp; + bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data); bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval); -- 2.7.4