[lldb-dev] [PATCH] new utility methods in GDBRemoteCommunicationClient
Steve Pucci
spucci at google.com
Fri Jan 24 08:13:18 PST 2014
Hi,
A diff against the previous patch, incorporating Abid's comments, follows
inline. A modified full patch is included as an attachment.
Thanks,
Steve
diff --git
a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 2026e50..a1e982b 100644
---
a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++
b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -59,16 +59,22 @@ public:
bool send_async);
// For packets which specify a range of output to be returned,
- // return all of the output via a series of packets of the form
- // <prefix>:000,<size>
- // <prefix>:<size>,<size>
- // <prefix>:<size>*2,<size>
- // <prefix>:<size>*3,<size>
+ // return all of the output via a series of request packets of the form
+ // <prefix>0,<size>
+ // <prefix><size>,<size>
+ // <prefix><size>*2,<size>
+ // <prefix><size>*3,<size>
// ...
// until a "$l..." packet is received, indicating the end.
- // Concatenate the results together and return in response.
- // If any packet fails, the response indicates that failure
- // and the returned string value is undefined.
+ // (size is in hex; this format is used by a standard gdbserver to
+ // return the given portion of the output specified by <prefix>;
+ // for example, "qXfer:libraries-svr4:read::fff,1000" means
+ // "return a chunk of the xml description file for shared
+ // library load addresses, where the chunk starts at offset 0xfff
+ // and continues for 0x1000 bytes").
+ // Concatenate the resulting server response packets together and
+ // return in response_string. If any packet fails, the return value
+ // indicates that failure and the returned string value is undefined.
PacketResult
SendPacketsAndConcatenateResponses (const char *send_payload_prefix,
std::string &response_string);
diff --git
a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 9f591c8..4a3292a 100644
---
a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++
b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -341,7 +341,6 @@ GDBRemoteCommunicationClient::GetRemoteQSupported ()
{
StringExtractorGDBRemote packet_response(packet_size_str +
strlen("PacketSize="));
m_max_packet_size =
packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
- printf("max packet size %lu\n", (unsigned
long)m_max_packet_size);
if (m_max_packet_size == 0)
{
m_max_packet_size = UINT64_MAX; // Must have been a
garbled response
spucci-linux %emacs ~/lldb/svn% 1497%
On Fri, Jan 24, 2014 at 7:41 AM, Steve Pucci <spucci at google.com> wrote:
> Thanks, Abid. Answers to your questions below.
>
> On Fri, Jan 24, 2014 at 2:11 AM, Abid, Hafiz <Hafiz_Abid at mentor.com>wrote:
>
>> Hi Steve,
>>
>> I had a quick look at the patch. It mostly looks good. Some small
>> comments below.
>>
>>
>>
>> + // For packets which specify a range of output to be returned,
>>
>> + // return all of the output via a series of packets of the form
>>
>> + // <prefix>:000,<size>
>>
>> + // <prefix>:<size>,<size>
>>
>> + // <prefix>:<size>*2,<size>
>>
>> + // <prefix>:<size>*3,<size>
>>
>> + // ...
>>
>> + // until a "$l..." packet is received, indicating the end.
>>
>> + // Concatenate the results together and return in response.
>>
>> I think that perhaps you can make this comment a bit more clear. Just
>> separate what the remote target will do and what this function will do.
>>
>
> OK, will do, thanks.
>
>
>> + printf("max packet size %lu\n", (unsigned
>> long)m_max_packet_size);
>>
>> You forgot to remove this printf.
>>
>
> Right, thanks.
>
>
>> + snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x",
>> offset, response_size);
>>
>> + PacketResult result =
>> SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(),
>>
>> + this_response,
>>
>> +
>> /*send_async=*/false);
>>
>> When I see such packets in GDB remote log (e.g.
>> $qXfer:features:read:64bit-core.xml:0,fff), there is a colon before offset.
>> I don’t see it being added here. Is it supposed to be in payload_prefix_str?
>>
>
> Yes, that's the way I have it currently in my calling code. It's been
> tested in that environment and works but I'm not ready to check in the
> calling code yet. (I've also tested it in an environment without my
> calling code to ensure this works as a standalone patch, though it's
> unused).
>
>
>> I suppose after this is approved, you would be working on parsing the
>> xml that came from the gdbserver. Would it be only for shared libraries or
>> you are also interested in xml target description?
>>
>
> Yes, my calling code parses using libxml2, and the plan is to have libxml2
> conditionally compiled and linked in (using the same mechanism that clang
> uses for the same purpose). I have that working also in the calling code.
>
> I considered putting the xml parser in GDBRemoteCommunicationClient also
> but it seemed a bit higher-level than that class operates at. But I'm open
> to whatever people think here. One possibility would be to split the
> creation of the XML DOM and the walking of the DOM tree, and put the
> creation of the DOM in GDBRemoteCommunicationClient.
>
> We are also interested in the xml target description but it's not a
> priority at the moment.
>
> Thanks again for the review. I'll send out the modified patch before
> committing.
>
> - Steve
>
>
>>
>>
>> Regards,
>>
>> Abid
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> *From:* lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu]
>> *On Behalf Of *Steve Pucci
>> *Sent:* 23 January 2014 21:04
>> *To:* lldb-dev at cs.uiuc.edu
>> *Subject:* [lldb-dev] [PATCH] new utility methods in
>> GDBRemoteCommunicationClient
>>
>>
>>
>> I'm working on better support for debugging against a remote stock
>> gdbserver, and these are some new support methods I'm using. I'm sending
>> this separately for easier digestion.
>>
>>
>>
>> There are a couple of pieces:
>>
>> * some lazy-evaluation members that store info listed in a qSupported
>> response
>>
>> * new method SendPacketsAndConcatenateResponses which is used for
>> fetching fixed-size objects from the remote gdbserver by using multiple
>> packets if necessary (first use will be to fetch shared-library XML files).
>>
>>
>>
>> Thanks,
>>
>> Steve
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140124/60461f01/attachment.html>
-------------- next part --------------
diff --git a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 9f591c8..4a3292a 100644
--- a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -341,7 +341,6 @@ GDBRemoteCommunicationClient::GetRemoteQSupported ()
{
StringExtractorGDBRemote packet_response(packet_size_str + strlen("PacketSize="));
m_max_packet_size = packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
- printf("max packet size %lu\n", (unsigned long)m_max_packet_size);
if (m_max_packet_size == 0)
{
m_max_packet_size = UINT64_MAX; // Must have been a garbled response
spucci-linux %emacs ~/lldb/svn% 1497% ls -la
total 20
drwxr-x--- 4 spucci eng 4096 Jan 23 12:32 .
drwxr-x--- 10 spucci eng 4096 Jan 22 19:07 ..
drwxr-x--- 13 spucci eng 4096 Jan 23 12:35 build
drwxr-x--- 15 spucci eng 4096 Jan 23 11:27 llvm
-rw-r----- 1 spucci eng 61 Jan 17 10:09 make.log
spucci-linux %emacs ~/lldb/svn% 1498% cd llvm
spucci-linux %emacs ~/lldb/svn/llvm% 1499% cd tools/lldb
spucci-linux %emacs ~/lldb/svn/llvm/tools/lldb% 1500% ls
CMakeLists.txt examples INSTALL.txt LICENSE.TXT lldb.xcworkspace resources source test utils
docs include lib lldb.xcodeproj Makefile scripts svn-commit.tmp~ tools www
spucci-linux %emacs ~/lldb/svn/llvm/tools/lldb% 1501% svn diff
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (revision 199934)
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (working copy)
@@ -66,6 +66,9 @@
m_prepare_for_reg_writing_reply (eLazyBoolCalculate),
m_supports_p (eLazyBoolCalculate),
m_supports_QSaveRegisterState (eLazyBoolCalculate),
+ m_supports_qXfer_libraries_read (eLazyBoolCalculate),
+ m_supports_qXfer_libraries_svr4_read (eLazyBoolCalculate),
+ m_supports_augmented_libraries_svr4_read (eLazyBoolCalculate),
m_supports_qProcessInfoPID (true),
m_supports_qfProcessInfo (true),
m_supports_qUserName (true),
@@ -96,7 +99,8 @@
m_os_build (),
m_os_kernel (),
m_hostname (),
- m_default_packet_timeout (0)
+ m_default_packet_timeout (0),
+ m_max_packet_size (0)
{
}
@@ -149,6 +153,46 @@
}
bool
+GDBRemoteCommunicationClient::GetAugmentedLibrariesSVR4ReadSupported ()
+{
+ if (m_supports_augmented_libraries_svr4_read == eLazyBoolCalculate)
+ {
+ GetRemoteQSupported();
+ }
+ return (m_supports_augmented_libraries_svr4_read == eLazyBoolYes);
+}
+
+bool
+GDBRemoteCommunicationClient::GetQXferLibrariesSVR4ReadSupported ()
+{
+ if (m_supports_qXfer_libraries_svr4_read == eLazyBoolCalculate)
+ {
+ GetRemoteQSupported();
+ }
+ return (m_supports_qXfer_libraries_svr4_read == eLazyBoolYes);
+}
+
+bool
+GDBRemoteCommunicationClient::GetQXferLibrariesReadSupported ()
+{
+ if (m_supports_qXfer_libraries_read == eLazyBoolCalculate)
+ {
+ GetRemoteQSupported();
+ }
+ return (m_supports_qXfer_libraries_read == eLazyBoolYes);
+}
+
+uint64_t
+GDBRemoteCommunicationClient::GetRemoteMaxPacketSize()
+{
+ if (m_max_packet_size == 0)
+ {
+ GetRemoteQSupported();
+ }
+ return m_max_packet_size;
+}
+
+bool
GDBRemoteCommunicationClient::QueryNoAckModeSupported ()
{
if (m_supports_not_sending_acks == eLazyBoolCalculate)
@@ -245,6 +289,9 @@
m_supports_memory_region_info = eLazyBoolCalculate;
m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
m_attach_or_wait_reply = eLazyBoolCalculate;
+ m_supports_qXfer_libraries_read = eLazyBoolCalculate;
+ m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
+ m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
m_supports_qProcessInfoPID = true;
m_supports_qfProcessInfo = true;
@@ -260,9 +307,51 @@
m_supports_QEnvironmentHexEncoded = true;
m_host_arch.Clear();
m_process_arch.Clear();
+
+ m_max_packet_size = 0;
}
+void
+GDBRemoteCommunicationClient::GetRemoteQSupported ()
+{
+ // Clear out any capabilities we expect to see in the qSupported response
+ m_supports_qXfer_libraries_svr4_read = eLazyBoolNo;
+ m_supports_qXfer_libraries_read = eLazyBoolNo;
+ m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
+ m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if not, we assume no limit
+ StringExtractorGDBRemote response;
+ if (SendPacketAndWaitForResponse("qSupported",
+ response,
+ /*send_async=*/false) == PacketResult::Success)
+ {
+ const char *response_cstr = response.GetStringRef().c_str();
+ if (::strstr (response_cstr, "qXfer:libraries-svr4:read+"))
+ m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
+ if (::strstr (response_cstr, "augmented-libraries-svr4-read"))
+ {
+ m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
+ m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
+ }
+ if (::strstr (response_cstr, "qXfer:libraries:read+"))
+ m_supports_qXfer_libraries_read = eLazyBoolYes;
+
+ const char *packet_size_str = ::strstr (response_cstr, "PacketSize=");
+ if (packet_size_str)
+ {
+ StringExtractorGDBRemote packet_response(packet_size_str + strlen("PacketSize="));
+ m_max_packet_size = packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
+ if (m_max_packet_size == 0)
+ {
+ m_max_packet_size = UINT64_MAX; // Must have been a garbled response
+ Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
+ if (log)
+ log->Printf ("Garbled PacketSize spec in qSupported response");
+ }
+ }
+ }
+}
+
bool
GDBRemoteCommunicationClient::GetThreadSuffixSupported ()
{
@@ -364,6 +453,51 @@
}
GDBRemoteCommunicationClient::PacketResult
+GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses
+(
+ const char *payload_prefix,
+ std::string &response_string
+)
+{
+ response_string = "";
+ std::string payload_prefix_str(payload_prefix);
+ unsigned int response_size = 0x1000;
+ if (response_size > GetRemoteMaxPacketSize()) { // May send qSupported packet
+ response_size = GetRemoteMaxPacketSize();
+ }
+
+ for (unsigned int offset = 0; true; offset += response_size)
+ {
+ StringExtractorGDBRemote this_response;
+ // Construct payload
+ char sizeDescriptor[128];
+ snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size);
+ PacketResult result = SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(),
+ this_response,
+ /*send_async=*/false);
+ if (result != PacketResult::Success)
+ return result;
+
+ const std::string &this_string = this_response.GetStringRef();
+
+ // Check for m or l as first character; l seems to mean this is the last chunk
+ char first_char = *this_string.c_str();
+ if (first_char != 'm' && first_char != 'l')
+ {
+ return PacketResult::ErrorReplyInvalid;
+ }
+ // Skip past m or l
+ const char *s = this_string.c_str() + 1;
+
+ // Concatenate the result so far
+ response_string += s;
+ if (first_char == 'l')
+ // We're done
+ return PacketResult::Success;
+ }
+}
+
+GDBRemoteCommunicationClient::PacketResult
GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
(
const char *payload,
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (revision 199934)
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (working copy)
@@ -58,6 +58,27 @@
StringExtractorGDBRemote &response,
bool send_async);
+ // For packets which specify a range of output to be returned,
+ // return all of the output via a series of request packets of the form
+ // <prefix>0,<size>
+ // <prefix><size>,<size>
+ // <prefix><size>*2,<size>
+ // <prefix><size>*3,<size>
+ // ...
+ // until a "$l..." packet is received, indicating the end.
+ // (size is in hex; this format is used by a standard gdbserver to
+ // return the given portion of the output specified by <prefix>;
+ // for example, "qXfer:libraries-svr4:read::fff,1000" means
+ // "return a chunk of the xml description file for shared
+ // library load addresses, where the chunk starts at offset 0xfff
+ // and continues for 0x1000 bytes").
+ // Concatenate the resulting server response packets together and
+ // return in response_string. If any packet fails, the return value
+ // indicates that failure and the returned string value is undefined.
+ PacketResult
+ SendPacketsAndConcatenateResponses (const char *send_payload_prefix,
+ std::string &response_string);
+
lldb::StateType
SendContinuePacketAndWaitForResponse (ProcessGDBRemote *process,
const char *packet_payload,
@@ -248,6 +269,9 @@
const lldb_private::ArchSpec &
GetProcessArchitecture ();
+ void
+ GetRemoteQSupported();
+
bool
GetVContSupported (char flavor);
@@ -359,6 +383,18 @@
bool
SetCurrentThreadForRun (uint64_t tid);
+ bool
+ GetQXferLibrariesReadSupported ();
+
+ bool
+ GetQXferLibrariesSVR4ReadSupported ();
+
+ uint64_t
+ GetRemoteMaxPacketSize();
+
+ bool
+ GetAugmentedLibrariesSVR4ReadSupported ();
+
lldb_private::LazyBool
SupportsAllocDeallocMemory () // const
{
@@ -489,7 +525,10 @@
lldb_private::LazyBool m_prepare_for_reg_writing_reply;
lldb_private::LazyBool m_supports_p;
lldb_private::LazyBool m_supports_QSaveRegisterState;
-
+ lldb_private::LazyBool m_supports_qXfer_libraries_read;
+ lldb_private::LazyBool m_supports_qXfer_libraries_svr4_read;
+ lldb_private::LazyBool m_supports_augmented_libraries_svr4_read;
+
bool
m_supports_qProcessInfoPID:1,
m_supports_qfProcessInfo:1,
@@ -532,6 +571,7 @@
std::string m_os_kernel;
std::string m_hostname;
uint32_t m_default_packet_timeout;
+ uint64_t m_max_packet_size; // as returned by qSupported
bool
DecodeProcessInfoResponse (StringExtractorGDBRemote &response,
More information about the lldb-dev
mailing list