<div dir="ltr"><div>Hi,</div><div><br></div>A diff against the previous patch, incorporating Abid's comments, follows inline.  A modified full patch is included as an attachment.<div><br></div><div>Thanks,</div><div>   Steve<br>
<div><br></div><div><div>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</div>
<div>index 2026e50..a1e982b 100644</div><div>--- a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h</div><div>+++ b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h</div>
<div>@@ -59,16 +59,22 @@ public:</div><div>                                   bool send_async);</div><div> </div><div>     // For packets which specify a range of output to be returned,</div><div>-    // return all of the output via a series of packets of the form</div>
<div>-    // <prefix>:000,<size></div><div>-    // <prefix>:<size>,<size></div><div>-    // <prefix>:<size>*2,<size></div><div>-    // <prefix>:<size>*3,<size></div>
<div>+    // return all of the output via a series of request packets of the form</div><div>+    // <prefix>0,<size></div><div>+    // <prefix><size>,<size></div><div>+    // <prefix><size>*2,<size></div>
<div>+    // <prefix><size>*3,<size></div><div>     // ...</div><div>     // until a "$l..." packet is received, indicating the end.</div><div>-    // Concatenate the results together and return in response.</div>
<div>-    // If any packet fails, the response indicates that failure</div><div>-    // and the returned string value is undefined.</div><div>+    // (size is in hex; this format is used by a standard gdbserver to</div><div>
+    // return the given portion of the output specified by <prefix>;</div><div>+    // for example, "qXfer:libraries-svr4:read::fff,1000" means</div><div>+    // "return a chunk of the xml description file for shared</div>
<div>+    // library load addresses, where the chunk starts at offset 0xfff</div><div>+    // and continues for 0x1000 bytes").</div><div>+    // Concatenate the resulting server response packets together and</div><div>
+    // return in response_string.  If any packet fails, the return value</div><div>+    // indicates that failure and the returned string value is undefined.</div><div>     PacketResult</div><div>     SendPacketsAndConcatenateResponses (const char *send_payload_prefix,</div>
<div>                                         std::string &response_string);</div><div>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<br>
</div><div>index 9f591c8..4a3292a 100644</div><div>--- a/usr/local/google/home/spucci/lldb/work/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp</div><div>+++ b/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp</div>
<div>@@ -341,7 +341,6 @@ GDBRemoteCommunicationClient::GetRemoteQSupported ()</div><div>         {</div><div>             StringExtractorGDBRemote packet_response(packet_size_str + strlen("PacketSize="));</div><div>
             m_max_packet_size = packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);</div><div>-            printf("max packet size %lu\n", (unsigned long)m_max_packet_size);</div><div>             if (m_max_packet_size == 0)</div>
<div>             {</div><div>                 m_max_packet_size = UINT64_MAX;  // Must have been a garbled response</div><div>spucci-linux %emacs ~/lldb/svn% 1497% </div></div><div><br></div><div><br></div></div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Jan 24, 2014 at 7:41 AM, Steve Pucci <span dir="ltr"><<a href="mailto:spucci@google.com" target="_blank">spucci@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Thanks, Abid.  Answers to your questions below.<div class="im"><div><br></div><div>On Fri, Jan 24, 2014 at 2:11 AM, Abid, Hafiz <span dir="ltr"><<a href="mailto:Hafiz_Abid@mentor.com" target="_blank">Hafiz_Abid@mentor.com</a>></span> wrote:<br>

</div></div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div lang="EN-GB" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Steve,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I had a quick look at the patch. It mostly looks good. Some small comments below.
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // For packets which specify a range of output to be returned,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // return all of the output via a series of packets of the form<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // <prefix>:000,<size><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // <prefix>:<size>,<size><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // <prefix>:<size>*2,<size><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // <prefix>:<size>*3,<size><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // ...<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // until a "$l..." packet is received, indicating the end.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    // Concatenate the results together and return in response.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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.</span></p>

</div></div></blockquote><div><br></div></div><div>OK, will do, thanks.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple">

<div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> </span><span style="color:rgb(31,73,125);font-family:Calibri,sans-serif;font-size:11pt">+            printf("max packet size %lu\n", (unsigned long)m_max_packet_size);</span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">You forgot to remove this printf.</span></p></div></div></blockquote><div><br></div></div><div>Right, thanks.</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> </span><span style="color:rgb(31,73,125);font-family:Calibri,sans-serif;font-size:11pt">+        snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size);</span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+        PacketResult result = SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(),<u></u><u></u></span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+                                                           this_response,<u></u><u></u></span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+                                                           /*send_async=*/false);<u></u><u></u></span></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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?</span></p></div></div></blockquote><div><br></div></div><div>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).</div>
<div class="im">
<div><span style="color:rgb(31,73,125);font-family:Calibri,sans-serif;font-size:11pt"> </span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple">

<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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?</span></p></div></div></blockquote><div><br></div></div><div>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.</div>

<div><br></div><div>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.</div>

<div><br></div><div>We are also interested in the xml target description but it's not a priority at the moment.</div><div><br></div><div>Thanks again for the review.  I'll send out the modified patch before committing.</div>
<span class="HOEnZb"><font color="#888888">
<div><br></div><div> - Steve</div></font></span><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple">
<div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>

<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Regards,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Abid<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:lldb-dev-bounces@cs.uiuc.edu" target="_blank">lldb-dev-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:lldb-dev-bounces@cs.uiuc.edu" target="_blank">lldb-dev-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Steve Pucci<br>
<b>Sent:</b> 23 January 2014 21:04<br>
<b>To:</b> <a href="mailto:lldb-dev@cs.uiuc.edu" target="_blank">lldb-dev@cs.uiuc.edu</a><br>
<b>Subject:</b> [lldb-dev] [PATCH] new utility methods in GDBRemoteCommunicationClient<u></u><u></u></span></p>
</div>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>


<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">There are a couple of pieces:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> * some lazy-evaluation members that store info listed in a qSupported response<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> * 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).<u></u><u></u></p>


</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">   Steve<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div>
</div>

</blockquote></div></div><br></div></div>
</blockquote></div><br></div>