[lldb-dev] gdb-remote command broken in TOT

[Todd Fiala]

Ok - I'm testing that change now (which corresponds to that 3.1 solution in
the other fork of the thread).

Will have it in shortly.


On Tue, May 13, 2014 at 5:11 PM, Greg Clayton <gclayton at apple.com> wrote:

>
> > On May 13, 2014, at 4:25 PM, Todd Fiala <tfiala at google.com> wrote:
> >
> > You realize that qC's response is not stable as a pseudo pid, right?  So
> anything that asks for it, thinks they have it, and then asks again for it
> can get a different result depending on what the gdb remote thinks is the
> current thread.
>
> Yes I realize.
>
> > I'd offer we consider falling back to qC and not limiting it to iOS
>
> Agreed, don't limit to iOS. We just really need a pid that is not zero to
> indicate we have a valid process. We should always try qProcessInfo first
> and any new GDB servers we make or people work on should support that.
>
> > over reverting this entirely, unless you strongly object.  *Or* we can
> simply do exactly what you said - which is check if we're attached to
> something - by grabbing the qC result and just not storing it as a process
> id (i.e. essentially create a GDBRemoteCommunicationClient::HasProcess (),
> which happens to be implemented in terms of checking qProcessInfo, then qC,
> for anything).
>
> Yes, I don't like giving false info to the user either, but I would like
> to maintain backward compatibility.
>
> >
> > Just some thoughts.
> >
> > -Todd
> >
> >
> > On Tue, May 13, 2014 at 4:05 PM, Greg Clayton <gclayton at apple.com>
> wrote:
> >
> > > On May 13, 2014, at 7:56 AM, Todd Fiala <tfiala at google.com> wrote:
> > >
> > > Hey Matthew,
> > >
> > > > However is there any real need to restrict the fallback case to just
> Apple/iOS?
> > >
> > > Keep in mind that qC in gdbserver does implement the protocol
> correctly and returns the thread id, not the proc id. On Linux the first
> thread happens to get a thread id that is identical to the proc id.  So
> when launching an exe, the active thread id returned just happens to be the
> proc id as well out of happy coincidence. This is not true on other OSes
> and not guaranteed true when attaching to a Linux process if the active
> thread is not the first thread.  Given that lldb was not caching the proc
> id until my change last week, it means that lldb using qC against a
> gdbserver (or gdb remote protocol implementing qC per spec) is definitely
> getting the buggy behavior I was fixing (i.e. different proc ids throughout
> the life of the process when running against a multi-threaded inferior, and
> entirely invalid results for proc id when run on OSes where the thread ids
> have no correlation to the proc id, as on MacOSX).
> > >
> > > Thus, reverting to using qC when qProcessInfo isn't implemented is
> really just reverting to buggy and inconsistent behavior, although it may
> allow lldb to run where it can't now (and could before) and the proc id
> reported just happens to be right some of the time.
> >
> > I would revert this. The pid isn't really all that important, we just
> need to know that we are talking to something valid or not. We assume zero
> is invalid.
> > >
> > > I think the real fix here may be if $qProcessInfo isn't present, then
> we probably need to settle for not assuming we can get the process id, and
> just make sure lldb can handle running without it in that case.
> > >
> > > Greg, any thoughts on that?
> >
> > I would revert to getting the pid from qC. Lame, but it would make us
> more backward compatible. Leave a note in the code saying we really prefer
> stubs to implement the qProcessInfo packet.
> >
> > >
> > > -Todd
> > >
> > > On Tuesday, May 13, 2014, Matthew Gardiner <mg11 at csr.com> wrote:e
> thread if returned
> > > Todd Fiala wrote:
> > > This is a change I made.
> > >
> > > qC is supposed to return the thread id, not the process id.
> > >
> > > qProcessInfo is being used now to collect the process id.  We added a
> fallback so that if the process info request fails, on Apple/iOS we fall
> back to the older qC (which, as indicated before, used to return the
> process id erroneously, see the gdb remote documentation).
> > >
> > > After doing a bit more analysis, I see that you added a fallback such
> that if the process info request fails, we fall back to traditional
> gdbserver qC. This is completely reasonable, and permits our custom stubs
> to support qProcessInfo.
> > >
> > > However is there any real need to restrict the fallback case to just
> Apple/iOS? that is:
> > >
> > >     const llvm::Triple &triple = GetProcessArchitecture().GetTriple();
> > >     if ((triple.getVendor() == llvm::Triple::Apple) &&
> > >         (triple.getOS() == llvm::Triple::IOS))
> > >     {
> > >
> > > Surely, all stubs which fail to implement qProcessInfo should be
> permitted to fallback to qC?
> > >
> > > I've just commented out the above "if" in my working copy, and now the
> initial "gdb-remote host:port" works again - in so far as the current
> inferior process number is reported rather than 0. (Admittedly other things
> don't quite work as desired later on in the debug-session, e.g. "next". But
> presumably an Apple/iOS stub would either fail similarly, or work as
> expected due to some localised conditional treatment, (i.e. "if apple
> then...")).
> > >
> > > So, to continue to interoperate with traditional gdbserver
> implementations can we let all older stubs continue to fallback to qC?
> > >
> > > thanks
> > > Matt
> > >
> > >
> > >
> > >
> > >
> > > Member of the CSR plc group of companies. CSR plc registered in
> England and Wales, registered number 4187346, registered office Churchill
> House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United
> Kingdom
> > > More information can be found at www.csr.com. Keep up to date with
> CSR on our technical blog, www.csr.com/blog, CSR people blog,
> www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook,
> www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at
> www.twitter.com/CSR_plc.
> > > New for 2014, you can now access the wide range of products powered by
> aptX at www.aptx.com.
> >
> >
> >
> >
> > --
> > Todd Fiala |   Software Engineer |     tfiala at google.com |
> 650-943-3180
> >
>
>


-- 
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140513/bcdd635b/attachment.html>