[Lldb-commits] [lldb] r255711 - Add a new "thread-pcs" key-value pair to the T packet response from

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Tue Dec 15 15:47:44 PST 2015


Author: jmolenda
Date: Tue Dec 15 17:47:44 2015
New Revision: 255711

URL: http://llvm.org/viewvc/llvm-project?rev=255711&view=rev
Log:
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> 




Modified:
    lldb/trunk/docs/lldb-gdb-remote.txt
    lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=255711&r1=255710&r2=255711&view=diff
==============================================================================
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Tue Dec 15 17:47:44 2015
@@ -191,6 +191,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>"
 //
 // BRIEF
@@ -201,6 +219,12 @@ This packet must be sent  _prior_ to sen
 //  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

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=255711&r1=255710&r2=255711&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Tue Dec 15 17:47:44 2015
@@ -2743,6 +2743,7 @@ RNBRemote::SendStopReplyPacketForThread
             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
                     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




More information about the lldb-commits mailing list