[Lldb-commits] [lldb] r292611 - Prevent client from querying each thread's PC at each stop.
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Fri Jan 20 06:17:16 PST 2017
Author: labath
Date: Fri Jan 20 08:17:16 2017
New Revision: 292611
URL: http://llvm.org/viewvc/llvm-project?rev=292611&view=rev
Log:
Prevent client from querying each thread's PC at each stop.
Summary:
The server was no longer sending the thread PCs the way the client
expected them.
I changed the server to send them back as a threadstop info field,
similar to the Apple version of the server.
I also changed the client to look for them there, before querying the
server.
I added a test to ensure the server doesn't stop sending them.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D28880
Author: Jason Majors
Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py?rev=292611&r1=292610&r2=292611&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py Fri Jan 20 08:17:16 2017
@@ -1,11 +1,13 @@
from __future__ import print_function
+import json
+import re
+
import gdbremote_testcase
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
-
class TestGdbRemoteThreadsInStopReply(
gdbremote_testcase.GdbRemoteTestCaseBase):
@@ -16,7 +18,8 @@ class TestGdbRemoteThreadsInStopReply(
"send packet: $OK#00",
]
- def gather_stop_reply_threads(self, post_startup_log_lines, thread_count):
+ def gather_stop_reply_fields(self, post_startup_log_lines, thread_count,
+ field_names):
# Set up the inferior args.
inferior_args = []
for i in range(thread_count - 1):
@@ -25,6 +28,9 @@ class TestGdbRemoteThreadsInStopReply(
procs = self.prep_debug_monitor_and_inferior(
inferior_args=inferior_args)
+ self.add_register_info_collection_packets()
+ self.add_process_info_collection_packets()
+
# Assumes test_sequence has anything added needed to setup the initial state.
# (Like optionally enabling QThreadsInStopReply.)
if post_startup_log_lines:
@@ -34,6 +40,7 @@ class TestGdbRemoteThreadsInStopReply(
], True)
context = self.expect_gdbremote_sequence()
self.assertIsNotNone(context)
+ hw_info = self.parse_hw_info(context)
# Give threads time to start up, then break.
time.sleep(1)
@@ -77,14 +84,89 @@ class TestGdbRemoteThreadsInStopReply(
kv_dict = self.parse_key_val_dict(key_vals_text)
self.assertIsNotNone(kv_dict)
+ result = dict();
+ result["pc_register"] = hw_info["pc_register"]
+ result["little_endian"] = hw_info["little_endian"]
+ for key_field in field_names:
+ result[key_field] = kv_dict.get(key_field)
+
+ return result
+
+ def gather_stop_reply_threads(self, post_startup_log_lines, thread_count):
# Pull out threads from stop response.
- stop_reply_threads_text = kv_dict.get("threads")
+ stop_reply_threads_text = self.gather_stop_reply_fields(
+ post_startup_log_lines, thread_count, ["threads"])["threads"]
if stop_reply_threads_text:
return [int(thread_id, 16)
for thread_id in stop_reply_threads_text.split(",")]
else:
return []
+ def gather_stop_reply_pcs(self, post_startup_log_lines, thread_count):
+ results = self.gather_stop_reply_fields( post_startup_log_lines,
+ thread_count, ["threads", "thread-pcs"])
+ if not results:
+ return []
+
+ threads_text = results["threads"]
+ pcs_text = results["thread-pcs"]
+ thread_ids = threads_text.split(",")
+ pcs = pcs_text.split(",")
+ self.assertTrue(len(thread_ids) == len(pcs))
+
+ thread_pcs = dict()
+ for i in range(0, len(pcs)):
+ thread_pcs[int(thread_ids[i], 16)] = pcs[i]
+
+ result = dict()
+ result["thread_pcs"] = thread_pcs
+ result["pc_register"] = results["pc_register"]
+ result["little_endian"] = results["little_endian"]
+ return result
+
+ def switch_endian(self, egg):
+ return "".join(reversed(re.findall("..", egg)))
+
+ def parse_hw_info(self, context):
+ self.assertIsNotNone(context)
+ process_info = self.parse_process_info_response(context)
+ endian = process_info.get("endian")
+ reg_info = self.parse_register_info_packets(context)
+ (pc_lldb_reg_index, pc_reg_info) = self.find_pc_reg_info(reg_info)
+
+ hw_info = dict()
+ hw_info["pc_register"] = pc_lldb_reg_index
+ hw_info["little_endian"] = (endian == "little")
+ return hw_info
+
+ def gather_threads_info_pcs(self, pc_register, little_endian):
+ self.reset_test_sequence()
+ self.test_sequence.add_log_lines(
+ [
+ "read packet: $jThreadsInfo#c1",
+ {
+ "direction": "send",
+ "regex": r"^\$(.*)#[0-9a-fA-F]{2}$",
+ "capture": {
+ 1: "threads_info"}},
+ ],
+ True)
+
+ context = self.expect_gdbremote_sequence()
+ self.assertIsNotNone(context)
+ threads_info = context.get("threads_info")
+ register = str(pc_register)
+ # The jThreadsInfo response is not valid JSON data, so we have to
+ # clean it up first.
+ jthreads_info = json.loads(re.sub(r"}]", "}", threads_info))
+ thread_pcs = dict()
+ for thread_info in jthreads_info:
+ tid = thread_info["tid"]
+ pc = thread_info["registers"][register]
+ thread_pcs[tid] = self.switch_endian(pc) if little_endian else pc
+
+ return thread_pcs
+
def QListThreadsInStopReply_supported(self):
procs = self.prep_debug_monitor_and_inferior()
self.test_sequence.add_log_lines(
@@ -183,3 +265,34 @@ class TestGdbRemoteThreadsInStopReply(
self.build()
self.set_inferior_startup_launch()
self.stop_reply_reports_correct_threads(5)
+
+ def stop_reply_contains_thread_pcs(self, thread_count):
+ results = self.gather_stop_reply_pcs(
+ self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, thread_count)
+ stop_reply_pcs = results["thread_pcs"]
+ pc_register = results["pc_register"]
+ little_endian = results["little_endian"]
+ self.assertEqual(len(stop_reply_pcs), thread_count)
+
+ threads_info_pcs = self.gather_threads_info_pcs(pc_register,
+ little_endian)
+
+ self.assertEqual(len(threads_info_pcs), thread_count)
+ for thread_id in stop_reply_pcs:
+ self.assertTrue(thread_id in threads_info_pcs)
+ self.assertTrue(int(stop_reply_pcs[thread_id], 16)
+ == int(threads_info_pcs[thread_id], 16))
+
+ @llgs_test
+ def test_stop_reply_contains_thread_pcs_llgs(self):
+ self.init_llgs_test()
+ self.build()
+ self.set_inferior_startup_launch()
+ self.stop_reply_contains_thread_pcs(5)
+
+ @debugserver_test
+ def test_stop_reply_contains_thread_pcs_debugserver(self):
+ self.init_debugserver_test()
+ self.build()
+ self.set_inferior_startup_launch()
+ self.stop_reply_contains_thread_pcs(5)
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=292611&r1=292610&r2=292611&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Fri Jan 20 08:17:16 2017
@@ -415,7 +415,8 @@ static void AppendHexValue(StreamString
static void WriteRegisterValueInHexFixedWidth(
StreamString &response, NativeRegisterContextSP ®_ctx_sp,
- const RegisterInfo ®_info, const RegisterValue *reg_value_p) {
+ const RegisterInfo ®_info, const RegisterValue *reg_value_p,
+ lldb::ByteOrder byte_order) {
RegisterValue reg_value;
if (!reg_value_p) {
Error error = reg_ctx_sp->ReadRegister(®_info, reg_value);
@@ -426,7 +427,8 @@ static void WriteRegisterValueInHexFixed
if (reg_value_p) {
AppendHexValue(response, (const uint8_t *)reg_value_p->GetBytes(),
- reg_value_p->GetByteSize(), false);
+ reg_value_p->GetByteSize(),
+ byte_order == lldb::eByteOrderLittle);
} else {
// Zero-out any unreadable values.
if (reg_info.byte_size > 0) {
@@ -436,8 +438,7 @@ static void WriteRegisterValueInHexFixed
}
}
-static JSONObject::SP GetRegistersAsJSON(NativeThreadProtocol &thread,
- bool abridged) {
+static JSONObject::SP GetRegistersAsJSON(NativeThreadProtocol &thread) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
NativeRegisterContextSP reg_ctx_sp = thread.GetRegisterContext();
@@ -462,11 +463,8 @@ static JSONObject::SP GetRegistersAsJSON
static const uint32_t k_expedited_registers[] = {
LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
LLDB_REGNUM_GENERIC_RA, LLDB_INVALID_REGNUM};
- static const uint32_t k_abridged_expedited_registers[] = {
- LLDB_REGNUM_GENERIC_PC, LLDB_INVALID_REGNUM};
- for (const uint32_t *generic_reg_p = abridged ? k_abridged_expedited_registers
- : k_expedited_registers;
+ for (const uint32_t *generic_reg_p = k_expedited_registers;
*generic_reg_p != LLDB_INVALID_REGNUM; ++generic_reg_p) {
uint32_t reg_num = reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
eRegisterKindGeneric, *generic_reg_p);
@@ -501,7 +499,7 @@ static JSONObject::SP GetRegistersAsJSON
StreamString stream;
WriteRegisterValueInHexFixedWidth(stream, reg_ctx_sp, *reg_info_p,
- ®_value);
+ ®_value, lldb::eByteOrderBig);
register_object_sp->SetObject(
llvm::to_string(reg_num),
@@ -567,8 +565,10 @@ static JSONArray::SP GetJSONThreadsInfo(
JSONObject::SP thread_obj_sp = std::make_shared<JSONObject>();
threads_array_sp->AppendObject(thread_obj_sp);
- if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp, abridged))
- thread_obj_sp->SetObject("registers", registers_sp);
+ if (!abridged) {
+ if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp))
+ thread_obj_sp->SetObject("registers", registers_sp);
+ }
thread_obj_sp->SetObject("tid", std::make_shared<JSONNumber>(tid));
if (signum != 0)
@@ -721,6 +721,41 @@ GDBRemoteCommunicationServerLLGS::SendSt
"jstopinfo field for pid %" PRIu64,
__FUNCTION__, m_debugged_process_sp->GetID());
}
+
+ uint32_t i = 0;
+ response.PutCString("thread-pcs");
+ char delimiter = ':';
+ for (NativeThreadProtocolSP thread_sp;
+ (thread_sp = m_debugged_process_sp->GetThreadAtIndex(i)) != nullptr;
+ ++i) {
+ NativeRegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
+ if (!reg_ctx_sp)
+ continue;
+
+ uint32_t reg_to_read = reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
+ eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
+ const RegisterInfo *const reg_info_p =
+ reg_ctx_sp->GetRegisterInfoAtIndex(reg_to_read);
+
+ RegisterValue reg_value;
+ Error error = reg_ctx_sp->ReadRegister(reg_info_p, reg_value);
+ if (error.Fail()) {
+ if (log)
+ log->Printf("%s failed to read register '%s' index %" PRIu32 ": %s",
+ __FUNCTION__,
+ reg_info_p->name ? reg_info_p->name
+ : "<unnamed-register>",
+ reg_to_read, error.AsCString());
+ continue;
+ }
+
+ response.PutChar(delimiter);
+ delimiter = ',';
+ WriteRegisterValueInHexFixedWidth(response, reg_ctx_sp, *reg_info_p,
+ ®_value, endian::InlHostByteOrder());
+ }
+
+ response.PutChar(';');
}
//
@@ -761,7 +796,7 @@ GDBRemoteCommunicationServerLLGS::SendSt
if (error.Success()) {
response.Printf("%.02x:", *reg_num_p);
WriteRegisterValueInHexFixedWidth(response, reg_ctx_sp, *reg_info_p,
- ®_value);
+ ®_value, lldb::eByteOrderBig);
response.PutChar(';');
} else {
if (log)
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=292611&r1=292610&r2=292611&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri Jan 20 08:17:16 2017
@@ -1649,27 +1649,8 @@ bool ProcessGDBRemote::UpdateThreadList(
__FUNCTION__, static_cast<void *>(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<ThreadGDBRemote *>(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]);
- }
- }
- }
+
+ SetThreadPc(thread_sp, i);
new_thread_list.AddThreadSortedByIndexID(thread_sp);
}
}
@@ -1689,6 +1670,22 @@ bool ProcessGDBRemote::UpdateThreadList(
return true;
}
+void ProcessGDBRemote::SetThreadPc(const ThreadSP &thread_sp, uint64_t index) {
+ if (m_thread_ids.size() == m_thread_pcs.size() && thread_sp.get() &&
+ GetByteOrder() != eByteOrderInvalid) {
+ ThreadGDBRemote *gdb_thread =
+ static_cast<ThreadGDBRemote *>(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[index]);
+ }
+ }
+ }
+}
+
bool ProcessGDBRemote::GetThreadStopInfoFromJSON(
ThreadGDBRemote *thread, const StructuredData::ObjectSP &thread_infos_sp) {
// See if we got thread stop infos for all threads via the "jThreadsInfo"
@@ -1774,6 +1771,11 @@ ThreadSP ProcessGDBRemote::SetThreadStop
static_cast<ThreadGDBRemote *>(thread_sp.get());
gdb_thread->GetRegisterContext()->InvalidateIfNeeded(true);
+ auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid);
+ if (iter != m_thread_ids.end()) {
+ SetThreadPc(thread_sp, iter - m_thread_ids.begin());
+ }
+
for (const auto &pair : expedited_register_map) {
StringExtractor reg_value_extractor;
reg_value_extractor.GetStringRef() = pair.second;
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=292611&r1=292610&r2=292611&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Fri Jan 20 08:17:16 2017
@@ -417,6 +417,7 @@ private:
void HandleStopReply() override;
void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
+ void SetThreadPc(const lldb::ThreadSP &thread_sp, uint64_t index);
using ModuleCacheKey = std::pair<std::string, std::string>;
// KeyInfo for the cached module spec DenseMap.
// The invariant is that all real keys will have the file and architecture
More information about the lldb-commits
mailing list