From 545304d323d6bbebdddcd93a7b657e761b8df168 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Fri, 18 Dec 2015 00:45:35 +0000 Subject: [PATCH] The lldb side changes to go along with r255711 where a new "thread-pcs" key is added to the T (questionmark) packet in gdb-remote protocol so that lldb doesn't need to query the pc values of every thread before it resumes a process. The only odd part with this is that I'm sending the pc values in big endian order, so we need to know the endianness of the remote process before we can use them. All other register values in gdb-remote protocol are sent in native-endian format so this requirement doesn't exist. This addition is a performance enhancement -- lldb will fall back to querying the pc of each thread individually if it needs to -- so when we don't have the byte order for the process yet, we don't use these values. Practically speaking, the only way I've been able to elicit this condition is for the first T packet when we attach to a process. llvm-svn: 255942 --- .../gdb-remote/GDBRemoteRegisterContext.cpp | 46 +++++++++++ .../gdb-remote/GDBRemoteRegisterContext.h | 3 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 79 +++++++++++++++++++ .../Process/gdb-remote/ProcessGDBRemote.h | 4 + .../Process/gdb-remote/ThreadGDBRemote.cpp | 8 ++ .../Process/gdb-remote/ThreadGDBRemote.h | 4 + 6 files changed, 144 insertions(+) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index 7b9471e92251..b0a1eaaeb79c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -147,6 +147,52 @@ GDBRemoteRegisterContext::PrivateSetRegisterValue (uint32_t reg, StringExtractor return success; } +bool +GDBRemoteRegisterContext::PrivateSetRegisterValue (uint32_t reg, uint64_t new_reg_val) +{ + const RegisterInfo *reg_info = GetRegisterInfoAtIndex (reg); + if (reg_info == NULL) + return false; + + // Early in process startup, we can get a thread that has an invalid byte order + // because the process hasn't been completely set up yet (see the ctor where the + // byte order is setfrom the process). If that's the case, we can't set the + // value here. + if (m_reg_data.GetByteOrder() == eByteOrderInvalid) + { + return false; + } + + // Invalidate if needed + InvalidateIfNeeded (false); + + DataBufferSP buffer_sp (new DataBufferHeap (&new_reg_val, sizeof (new_reg_val))); + DataExtractor data (buffer_sp, endian::InlHostByteOrder(), sizeof (void*)); + + // If our register context and our register info disagree, which should never happen, don't + // overwrite past the end of the buffer. + if (m_reg_data.GetByteSize() < reg_info->byte_offset + reg_info->byte_size) + return false; + + // Grab a pointer to where we are going to put this register + uint8_t *dst = const_cast(m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size)); + + if (dst == NULL) + return false; + + + if (data.CopyByteOrderedData (0, // src offset + reg_info->byte_size, // src length + dst, // dst + reg_info->byte_size, // dst length + m_reg_data.GetByteOrder())) // dst byte order + { + SetRegisterIsValid (reg, true); + return true; + } + return false; +} + // Helper function for GDBRemoteRegisterContext::ReadRegisterBytes(). bool GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h index 2383b374dafb..0e26c69eb2a9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -109,6 +109,9 @@ protected: bool PrivateSetRegisterValue (uint32_t reg, StringExtractor &response); + bool + PrivateSetRegisterValue (uint32_t reg, uint64_t val); + void SetAllRegisterValid (bool b); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 87d1f12008ea..2e7a5b5384f4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -390,6 +390,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, Listener &listener) m_async_listener("lldb.process.gdb-remote.async-listener"), m_async_thread_state_mutex(Mutex::eMutexTypeRecursive), m_thread_ids (), + m_thread_pcs (), m_jstopinfo_sp (), m_jthreadsinfo_sp (), m_continue_c_tids (), @@ -1751,12 +1752,14 @@ ProcessGDBRemote::ClearThreadIDList () { Mutex::Locker locker(m_thread_list_real.GetMutex()); m_thread_ids.clear(); + m_thread_pcs.clear(); } size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue (std::string &value) { m_thread_ids.clear(); + m_thread_pcs.clear(); size_t comma_pos; lldb::tid_t tid; while ((comma_pos = value.find(',')) != std::string::npos) @@ -1774,6 +1777,26 @@ ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue (std::string &value) return m_thread_ids.size(); } +size_t +ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue (std::string &value) +{ + m_thread_pcs.clear(); + size_t comma_pos; + lldb::addr_t pc; + while ((comma_pos = value.find(',')) != std::string::npos) + { + value[comma_pos] = '\0'; + pc = StringConvert::ToUInt64 (value.c_str(), LLDB_INVALID_ADDRESS, 16); + if (pc != LLDB_INVALID_ADDRESS) + m_thread_pcs.push_back (pc); + value.erase(0, comma_pos + 1); + } + pc = StringConvert::ToUInt64 (value.c_str(), LLDB_INVALID_ADDRESS, 16); + if (pc != LLDB_INVALID_THREAD_ID) + m_thread_pcs.push_back (pc); + return m_thread_pcs.size(); +} + bool ProcessGDBRemote::UpdateThreadIDList () { @@ -1786,6 +1809,7 @@ ProcessGDBRemote::UpdateThreadIDList () if (thread_infos && thread_infos->GetSize() > 0) { m_thread_ids.clear(); + m_thread_pcs.clear(); thread_infos->ForEach([this](StructuredData::Object* object) -> bool { StructuredData::Dictionary *thread_dict = object->GetAsDictionary(); if (thread_dict) @@ -1820,6 +1844,20 @@ ProcessGDBRemote::UpdateThreadIDList () // Get the thread stop info StringExtractorGDBRemote &stop_info = m_stop_packet_stack[i]; const std::string &stop_info_str = stop_info.GetStringRef(); + + m_thread_pcs.clear(); + const size_t thread_pcs_pos = stop_info_str.find(";thread-pcs:"); + if (thread_pcs_pos != std::string::npos) + { + const size_t start = thread_pcs_pos + strlen(";thread-pcs:"); + const size_t end = stop_info_str.find(';', start); + if (end != std::string::npos) + { + std::string value = stop_info_str.substr(start, end - start); + UpdateThreadPCsFromStopReplyThreadsValue(value); + } + } + const size_t threads_pos = stop_info_str.find(";threads:"); if (threads_pos != std::string::npos) { @@ -1887,6 +1925,25 @@ ProcessGDBRemote::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new __FUNCTION__, static_cast(thread_sp.get()), thread_sp->GetID()); } + // The m_thread_pcs vector has pc values in big-endian order, not target-endian, unlike most + // of the register read/write packets in gdb-remote protocol. + // Early in the process startup, we may not yet have set the process ByteOrder so we ignore these; + // they are a performance improvement over fetching thread register values individually, the + // method we will fall back to if needed. + if (m_thread_ids.size() == m_thread_pcs.size() && thread_sp.get() && GetByteOrder() != eByteOrderInvalid) + { + ThreadGDBRemote *gdb_thread = static_cast (thread_sp.get()); + RegisterContextSP reg_ctx_sp (thread_sp->GetRegisterContext()); + if (reg_ctx_sp) + { + uint32_t pc_regnum = reg_ctx_sp->ConvertRegisterKindToRegisterNumber + (eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC); + if (pc_regnum != LLDB_INVALID_REGNUM) + { + gdb_thread->PrivateSetRegisterValue (pc_regnum, m_thread_pcs[i]); + } + } + } new_thread_list.AddThread(thread_sp); } } @@ -2447,6 +2504,27 @@ ProcessGDBRemote::SetThreadStopInfo (StringExtractor& stop_packet) if (tid != LLDB_INVALID_THREAD_ID) m_thread_ids.push_back (tid); } + else if (key.compare("thread-pcs") == 0) + { + m_thread_pcs.clear(); + // A comma separated list of all threads in the current + // process that includes the thread for this stop reply + // packet + size_t comma_pos; + lldb::addr_t pc; + while ((comma_pos = value.find(',')) != std::string::npos) + { + value[comma_pos] = '\0'; + // thread in big endian hex + pc = StringConvert::ToUInt64 (value.c_str(), LLDB_INVALID_ADDRESS, 16); + if (pc != LLDB_INVALID_ADDRESS) + m_thread_pcs.push_back (pc); + value.erase(0, comma_pos + 1); + } + pc = StringConvert::ToUInt64 (value.c_str(), LLDB_INVALID_ADDRESS, 16); + if (pc != LLDB_INVALID_ADDRESS) + m_thread_pcs.push_back (pc); + } else if (key.compare("jstopinfo") == 0) { StringExtractor json_extractor; @@ -2624,6 +2702,7 @@ ProcessGDBRemote::RefreshStateAfterStop () { Mutex::Locker locker(m_thread_list_real.GetMutex()); m_thread_ids.clear(); + m_thread_pcs.clear(); // Set the thread stop info. It might have a "threads" key whose value is // a list of all thread IDs in the current process, so m_thread_ids might // get set. diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 12094e786128..54749827d6ac 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -288,6 +288,7 @@ protected: typedef std::map MMapMap; typedef std::map ExpeditedRegisterMap; tid_collection m_thread_ids; // Thread IDs for all threads. This list gets updated after stopping + std::vector m_thread_pcs; // PC values for all the threads. StructuredData::ObjectSP m_jstopinfo_sp; // Stop info only for any threads that have valid stop infos StructuredData::ObjectSP m_jthreadsinfo_sp; // Full stop info, expedited registers and memory for all threads if "jThreadsInfo" packet is supported tid_collection m_continue_c_tids; // 'c' for continue @@ -383,6 +384,9 @@ protected: bool CalculateThreadStopInfo (ThreadGDBRemote *thread); + size_t + UpdateThreadPCsFromStopReplyThreadsValue (std::string &value); + size_t UpdateThreadIDsFromStopReplyThreadsValue (std::string &value); diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp index d2a6503caf8e..9b410d8b5b8c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp @@ -312,6 +312,14 @@ ThreadGDBRemote::PrivateSetRegisterValue (uint32_t reg, StringExtractor &respons return gdb_reg_ctx->PrivateSetRegisterValue (reg, response); } +bool +ThreadGDBRemote::PrivateSetRegisterValue (uint32_t reg, uint64_t regval) +{ + GDBRemoteRegisterContext *gdb_reg_ctx = static_cast(GetRegisterContext ().get()); + assert (gdb_reg_ctx); + return gdb_reg_ctx->PrivateSetRegisterValue (reg, regval); +} + bool ThreadGDBRemote::CalculateStopInfo () { diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h index d7e8e4e57aa3..24693ba891cc 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h @@ -116,6 +116,10 @@ protected: PrivateSetRegisterValue (uint32_t reg, StringExtractor &response); + bool + PrivateSetRegisterValue (uint32_t reg, + uint64_t regval); + bool CachedQueueInfoIsValid() const {