Add a new "thread-pcs" key-value pair to the T packet response from

debugserver.  thread-pcs has a comma separated list of base 16
addresses - the current pc value for every thread in the process.
It is a partner of the "threads:" key where a list of thread IDs
is given.  The pc values in thread-pcs correspond one-to-one with
the thread IDs in the threads list.

This is a part of performance work.  When lldb is instruction
stepping / fast stepping over a range of addresses for e.g. a "next"
command, and it steps in to another function, lldb will put a
breakpoint on the return address and continue the process.  Before
it calls continue, it calls Thread::SetupForResume on all the
threads, and SetupForResume needs to get the current pc value for
every thread to see if any are at a breakpoint site.

The result is that issuing a "c" continue requires that we send
"read pc register" packets for every thread.

We may do this sequence of step-into-function / continue-to-get-out
many times for a single user-visible "next" or "step" command, and
with highly multithreaded programs, we are sending many extra
packets to get all the thread values.

I looked at including this data in the "jstopinfo" JSON that
we already have in the T packet.  But there are three problems that
would make this increase the size of the T packet significantly.
First, numbers in JSON are base 10.  Second, a proper JSON would
have something like "thread_pcs": { "34224331112":383772734222, ...}
for thread-id 34224331112 and pc 383772734222 - so we're including
a whole extra copy of the thread id in addition to the pc.  Third,
the JSON text is hex-ascii'fied so the size of it is doubled.
In one example, 

threads:585db8,585dc7,585dc8,585dc9,585dca,585dce;thread-pcs:100001400,7fff8badc6de,7fff8badcff6,7fff8badc6de,7fff8badc6de,7fff8badc6de;

The "thread-pcs" adds 86 characters - 136 characters for both 
threads and thread-pcs.  Doing this in JSON would look like

threads={"5791160":4294972416,"5791175":140735536809694,"5791176":140735536812022,"5791177":140735536809694,"5791178":140735536809694,"5791182":140735536809694}

or 160 characters -- or 320 characters once it is hex-asciified.

Given that it's 86 characters vrs 320, I went with the old style
approach.  I've seen real world programs that have up to 60 threads
in them, so this could result in vastly larger packets if it
was all done in the JSON with hex-ascii expansion.

If we had an all-JSON T packet, where we didn't need to hex-ascii
encode anything, that would have been the better approach.  But
we'd already have a list of threads in JSON at that point so
the additional text wouldn't be too bad.

I'm working on finishing the patches to lldb to use this data;
will commit those once I've had a chance to test them more.  But
I wanted to commit the debugserver bits which are more
straightforward.


<rdar://problem/21963031> 

llvm-svn: 255711
This commit is contained in:
Jason Molenda 2015-12-15 23:47:44 +00:00
parent cf2626600c
commit 7e9bd599a2
2 changed files with 86 additions and 0 deletions

View File

@ -190,6 +190,24 @@ read packet: OK
This packet must be sent _prior_ to sending a "A" packet.
//----------------------------------------------------------------------
// QListThreadsInStopReply
//
// BRIEF
// Enable the threads: and thread-pcs: data in the question-mark packet
// ("T packet") responses when the stub reports that a program has
// stopped executing.
//
// PRIORITY TO IMPLEMENT
// Performance. This is a performance benefit to lldb if the thread id's
// and thread pc values are provided to lldb in the T stop packet -- if
// they are not provided to lldb, lldb will likely need to send one to
// two packets per thread to fetch the data at every private stop.
//----------------------------------------------------------------------
send packet: QListThreadsInStopReply
read packet: OK
//----------------------------------------------------------------------
// "qRegisterInfo<hex-reg-id>"
//
@ -201,6 +219,12 @@ This packet must be sent _prior_ to sending a "A" packet.
// This means if new registers are ever added to a remote target, they
// will get picked up automatically, and allows registers to change
// depending on the actual CPU type that is used.
//
// NB: As of summer 2015, lldb can get register information from the
// "qXfer:features:read:target.xml" FSF gdb standard register packet
// where the stub provides register definitions in an XML file.
// If qXfer:features:read:target.xml is supported, qRegisterInfo does
// not need to be implemented.
//----------------------------------------------------------------------
With LLDB, for register information, remote GDB servers can add
@ -1117,6 +1141,32 @@ for this region.
// if none of the key/value pairs are enough to
// describe why something stopped.
//
// "threads" comma-sep-base16 A list of thread ids for all threads (including
// the thread that we're reporting as stopped) that
// are live in the process right now. lldb may
// request that this be included in the T packet via
// the QListThreadsInStopReply packet earlier in
// the debug session.
//
// Example:
// threads:63387,633b2,63424,63462,63486;
//
// "thread-pcs" comma-sep-base16 A list of pc values for all threads that currently
// exist in the process, including the thread that
// this T packet is reporting as stopped.
// This key-value pair will only be emitted when the
// "threads" key is already included in the T packet.
// The pc values correspond to the threads reported
// in the "threads" list. The number of pcs in the
// "thread-pcs" list will be the same as the number of
// threads in the "threads" list.
// lldb may request that this be included in the T
// packet via the QListThreadsInStopReply packet
// earlier in the debug session.
//
// Example:
// thread-pcs:dec14,2cf872b0,2cf8681c,2d02d68c,2cf716a8;
//
// BEST PRACTICES:
// Since register values can be supplied with this packet, it is often useful
// to return the PC, SP, FP, LR (if any), and FLAGS registers so that separate

View File

@ -2743,6 +2743,7 @@ RNBRemote::SendStopReplyPacketForThread (nub_thread_t tid)
const nub_size_t numthreads = DNBProcessGetNumThreads (pid);
if (numthreads > 0)
{
std::vector<uint64_t> pc_values;
ostrm << std::hex << "threads:";
for (nub_size_t i = 0; i < numthreads; ++i)
{
@ -2750,8 +2751,43 @@ RNBRemote::SendStopReplyPacketForThread (nub_thread_t tid)
if (i > 0)
ostrm << ',';
ostrm << std::hex << th;
DNBRegisterValue pc_regval;
if (DNBThreadGetRegisterValueByID (pid, th, REGISTER_SET_GENERIC, GENERIC_REGNUM_PC, &pc_regval))
{
uint64_t pc = INVALID_NUB_ADDRESS;
if (pc_regval.value.uint64 != INVALID_NUB_ADDRESS)
{
if (pc_regval.info.size == 4)
{
pc = pc_regval.value.uint32;
}
else if (pc_regval.info.size == 8)
{
pc = pc_regval.value.uint64;
}
if (pc != INVALID_NUB_ADDRESS)
{
pc_values.push_back (pc);
}
}
}
}
ostrm << ';';
// If we failed to get any of the thread pc values, the size of our vector will not
// be the same as the # of threads. Don't provide any expedited thread pc values in
// that case. This should not happen.
if (pc_values.size() == numthreads)
{
ostrm << std::hex << "thread-pcs:";
for (nub_size_t i = 0; i < numthreads; ++i)
{
if (i > 0)
ostrm << ',';
ostrm << std::hex << pc_values[i];
}
ostrm << ';';
}
}
// Include JSON info that describes the stop reason for any threads