<div dir="ltr">Revised patch per Greg's request to remove interface from base class Process.h/cpp.  I also caught and removed the added field m_breakpoint_trap_opcode_size I had accidentally left in the sandbox I used to create the original patch.<div>
<br></div><div>Thanks,</div><div>   Steve</div><div class="gmail_extra"><br></div><div class="gmail_extra">PS, sorry, forgot to CC lldb-dev on my reply to Greg.  That exchange is included below.<br><br><div class="gmail_quote">
On Tue, Mar 4, 2014 at 10:57 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks for the info. I would prefer to remove the code from Process.h/cpp and resubmit the patch.<br>
<div class="HOEnZb"><div class="h5"><br>
On Mar 4, 2014, at 7:31 AM, Steve Pucci <<a href="mailto:spucci@google.com">spucci@google.com</a>> wrote:<br>
<br>
> Hi Greg,<br>
><br>
> Thanks for the review.  The auxv data structure (which is apparently a feature of ELF-derived programs) seems to be common to at least linux and freebsd, as there are implementations of each. It's used to discover process information such as the program entry point and the program headers.<br>

><br>
> To be honest I'm uncertain of the ultimate use of the code in this patch.  In one of my sandboxes I have it called from DynamicLoaderPOSIXDYLD because that seemed to be the place that wants to know the auxv, and it doesn't know the type of Process it has (I've run into issues with the lack of target architectural knowledge in my proposed DynamicLoaderGDBRemote class and this second sandbox was an experiment to see if it would be cleaner implemented without the custom dynamic loader for remote).<br>

><br>
> I didn't change Host; it already had GetAuxvData() as an API, implemented only on the linux and freebsd versions. [ This seemed an odd place to have that function to me, as it seems to be a Target function rather than a Host one, but I might be missing something. ]  So my new API on Process delegates to the existing functionality except when working remotely.<br>

><br>
> That said, I have no objection to removing the API on Process.cpp/h if you think that's best.  Mostly I just want this code in place for future use as I think it will ultimately be useful in some form for remote debugging on linux/freebsd; I'm going to be working on a different project for the forseeable future and I don't want it to be lost altogether.<br>

><br>
> Let me know what you think.<br>
><br>
>  - Steve<br>
><br>
><br>
> On Mon, Mar 3, 2014 at 5:02 PM, Greg Clayton <<a href="mailto:gclayton@apple.com">gclayton@apple.com</a>> wrote:<br>
> Steve, what is the rationale for having Process::GetAuxvData()? Can we not just keep this as a ProcessGDBRemote only function? Is anyone else besides ProcessGDBRemote going to be able to do anything with this data? If the answer is no, I would prefer to keep this out of Process.cpp/Process.h. If it is only in there to keep from having to cast Process to ProcessGDBRemote, then it definitely should be removed from Process.cpp/.h. I am guessing you have some other idea for this because you added it to the host layer. I don't the see the host layer patch for those parts of the changes. I am not really familiar with the content of the auxv data and I don't know how standardized it is or isn't?<br>

><br>
> Greg<br>
><br>
> On Mar 3, 2014, at 3:44 PM, Steve Pucci <<a href="mailto:spucci@google.com">spucci@google.com</a>> wrote:<br>
><br>
> > Hi all,<br>
> ><br>
> > This is a patch which allows ProcessGDBServer to obtain the auxv from a remote gdbserver via GDBServerProcessRemote, returning the data as a DataBufferSP.<br>
> ><br>
> > The patch includes a small fix to GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses() to support binary file format packet returns (by not assuming each binary packet is a null-terminated string when concatenating them).<br>

> ><br>
> > I've also added a base-class Process::GetAuxvData which delegates to the Host.<br>
> ><br>
> > No code currently calls this method.<br>
> ><br>
> >  - Steve<br>
> ><br>
> > Index: include/lldb/Target/Process.h<br>
> > ===================================================================<br>
> > --- include/lldb/Target/Process.h     (revision 202766)<br>
> > +++ include/lldb/Target/Process.h     (working copy)<br>
> > @@ -2063,6 +2063,9 @@<br>
> >          return m_unix_signals;<br>
> >      }<br>
> ><br>
> > +    virtual const lldb::DataBufferSP<br>
> > +    GetAuxvData();<br>
> > +<br>
> >      //==================================================================<br>
> >      // Plug-in Process Control Overrides<br>
> >      //==================================================================<br>
> > Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp<br>
> > ===================================================================<br>
> > --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp        (revision 202766)<br>
> > +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp        (working copy)<br>
> > @@ -66,6 +66,7 @@<br>
> >      m_prepare_for_reg_writing_reply (eLazyBoolCalculate),<br>
> >      m_supports_p (eLazyBoolCalculate),<br>
> >      m_supports_QSaveRegisterState (eLazyBoolCalculate),<br>
> > +    m_supports_qXfer_auxv_read (eLazyBoolCalculate),<br>
> >      m_supports_qXfer_libraries_read (eLazyBoolCalculate),<br>
> >      m_supports_qXfer_libraries_svr4_read (eLazyBoolCalculate),<br>
> >      m_supports_augmented_libraries_svr4_read (eLazyBoolCalculate),<br>
> > @@ -187,6 +188,16 @@<br>
> >      return (m_supports_qXfer_libraries_read == eLazyBoolYes);<br>
> >  }<br>
> ><br>
> > +bool<br>
> > +GDBRemoteCommunicationClient::GetQXferAuxvReadSupported ()<br>
> > +{<br>
> > +    if (m_supports_qXfer_auxv_read == eLazyBoolCalculate)<br>
> > +    {<br>
> > +        GetRemoteQSupported();<br>
> > +    }<br>
> > +    return (m_supports_qXfer_auxv_read == eLazyBoolYes);<br>
> > +}<br>
> > +<br>
> >  uint64_t<br>
> >  GDBRemoteCommunicationClient::GetRemoteMaxPacketSize()<br>
> >  {<br>
> > @@ -294,6 +305,7 @@<br>
> >      m_supports_memory_region_info = eLazyBoolCalculate;<br>
> >      m_prepare_for_reg_writing_reply = eLazyBoolCalculate;<br>
> >      m_attach_or_wait_reply = eLazyBoolCalculate;<br>
> > +    m_supports_qXfer_auxv_read = eLazyBoolCalculate;<br>
> >      m_supports_qXfer_libraries_read = eLazyBoolCalculate;<br>
> >      m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;<br>
> >      m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;<br>
> > @@ -320,8 +332,9 @@<br>
> >  GDBRemoteCommunicationClient::GetRemoteQSupported ()<br>
> >  {<br>
> >      // Clear out any capabilities we expect to see in the qSupported response<br>
> > +    m_supports_qXfer_auxv_read = eLazyBoolNo;<br>
> > +    m_supports_qXfer_libraries_read = eLazyBoolNo;<br>
> >      m_supports_qXfer_libraries_svr4_read = eLazyBoolNo;<br>
> > -    m_supports_qXfer_libraries_read = eLazyBoolNo;<br>
> >      m_supports_augmented_libraries_svr4_read = eLazyBoolNo;<br>
> >      m_max_packet_size = UINT64_MAX;  // It's supposed to always be there, but if not, we assume no limit<br>
> ><br>
> > @@ -331,6 +344,8 @@<br>
> >                                       /*send_async=*/false) == PacketResult::Success)<br>
> >      {<br>
> >          const char *response_cstr = response.GetStringRef().c_str();<br>
> > +        if (::strstr (response_cstr, "qXfer:auxv:read+"))<br>
> > +            m_supports_qXfer_auxv_read = eLazyBoolYes;<br>
> >          if (::strstr (response_cstr, "qXfer:libraries-svr4:read+"))<br>
> >              m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;<br>
> >          if (::strstr (response_cstr, "augmented-libraries-svr4-read"))<br>
> > @@ -502,11 +517,8 @@<br>
> >          {<br>
> >              return PacketResult::ErrorReplyInvalid;<br>
> >          }<br>
> > -        // Skip past m or l<br>
> > -        const char *s = this_string.c_str() + 1;<br>
> > -<br>
> > -        // Concatenate the result so far<br>
> > -        response_string += s;<br>
> > +        // Concatenate the result so far (skipping 'm' or 'l')<br>
> > +        response_string.append(this_string, 1, std::string::npos);<br>
> >          if (first_char == 'l')<br>
> >              // We're done<br>
> >              return PacketResult::Success;<br>
> > Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h<br>
> > ===================================================================<br>
> > --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h  (revision 202766)<br>
> > +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h  (working copy)<br>
> > @@ -384,6 +384,9 @@<br>
> >      SetCurrentThreadForRun (uint64_t tid);<br>
> ><br>
> >      bool<br>
> > +    GetQXferAuxvReadSupported ();<br>
> > +<br>
> > +    bool<br>
> >      GetQXferLibrariesReadSupported ();<br>
> ><br>
> >      bool<br>
> > @@ -525,6 +528,7 @@<br>
> >      lldb_private::LazyBool m_prepare_for_reg_writing_reply;<br>
> >      lldb_private::LazyBool m_supports_p;<br>
> >      lldb_private::LazyBool m_supports_QSaveRegisterState;<br>
> > +    lldb_private::LazyBool m_supports_qXfer_auxv_read;<br>
> >      lldb_private::LazyBool m_supports_qXfer_libraries_read;<br>
> >      lldb_private::LazyBool m_supports_qXfer_libraries_svr4_read;<br>
> >      lldb_private::LazyBool m_supports_augmented_libraries_svr4_read;<br>
> > Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp<br>
> > ===================================================================<br>
> > --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp    (revision 202766)<br>
> > +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp    (working copy)<br>
> > @@ -281,7 +281,8 @@<br>
> >      m_waiting_for_attach (false),<br>
> >      m_destroy_tried_resuming (false),<br>
> >      m_command_sp (),<br>
> > -    m_breakpoint_pc_offset (0)<br>
> > +    m_breakpoint_pc_offset (0),<br>
> > +    m_breakpoint_trap_opcode_size (0)<br>
> >  {<br>
> >      m_async_broadcaster.SetEventName (eBroadcastBitAsyncThreadShouldExit,   "async thread should exit");<br>
> >      m_async_broadcaster.SetEventName (eBroadcastBitAsyncContinue,           "async thread continue");<br>
> > @@ -3060,7 +3061,22 @@<br>
> >      return m_dyld_ap.get();<br>
> >  }<br>
> ><br>
> > +const DataBufferSP<br>
> > +ProcessGDBRemote::GetAuxvData()<br>
> > +{<br>
> > +    DataBufferSP buf;<br>
> > +    if (m_gdb_comm.GetQXferAuxvReadSupported())<br>
> > +    {<br>
> > +        std::string response_string;<br>
> > +        if (m_gdb_comm.SendPacketsAndConcatenateResponses("qXfer:auxv:read::", response_string) == GDBRemoteCommunication::PacketResult::Success)<br>
> > +            buf.reset(new DataBufferHeap(response_string.c_str(), response_string.length()));<br>
> > +    }<br>
> > +    if (buf.get() != NULL)<br>
> > +        return buf;<br>
> > +    return Process::GetAuxvData();<br>
> > +}<br>
> ><br>
> > +<br>
> >  class CommandObjectProcessGDBRemotePacketHistory : public CommandObjectParsed<br>
> >  {<br>
> >  private:<br>
> > Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h<br>
> > ===================================================================<br>
> > --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h      (revision 202766)<br>
> > +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h      (working copy)<br>
> > @@ -299,6 +299,9 @@<br>
> >      bool<br>
> >      ParseRegisters(lldb_private::ScriptInterpreterObject *registers_array);<br>
> ><br>
> > +    virtual const lldb::DataBufferSP<br>
> > +    GetAuxvData();<br>
> > +<br>
> >      //------------------------------------------------------------------<br>
> >      /// Broadcaster event bits definitions.<br>
> >      //------------------------------------------------------------------<br>
> > @@ -341,7 +344,8 @@<br>
> >      bool m_destroy_tried_resuming;<br>
> >      lldb::CommandObjectSP m_command_sp;<br>
> >      int64_t m_breakpoint_pc_offset;<br>
> > -<br>
> > +    size_t m_breakpoint_trap_opcode_size;  // If set by Python<br>
> > +<br>
> >      bool<br>
> >      StartAsyncThread ();<br>
> ><br>
> > Index: source/Target/Process.cpp<br>
> > ===================================================================<br>
> > --- source/Target/Process.cpp (revision 202766)<br>
> > +++ source/Target/Process.cpp (working copy)<br>
> > @@ -5973,6 +5973,12 @@<br>
> >      target.DidExec();<br>
> >  }<br>
> ><br>
> > +const DataBufferSP<br>
> > +Process::GetAuxvData()<br>
> > +{<br>
> > +    return lldb_private::Host::GetAuxvData(this);<br>
> > +}<br>
> > +<br>
> >  addr_t<br>
> >  Process::ResolveIndirectFunction(const Address *address, Error &error)<br>
> >  {<br>
> ><br>
> > <patch-auxv-viaPacket.txt>_______________________________________________<br>
> > lldb-dev mailing list<br>
> > <a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>