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

[Todd Fiala]

Thanks!

And of course, Matthew, sorry for breaking you there.


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

>
> > On May 13, 2014, at 4:38 PM, Todd Fiala <tfiala at google.com> wrote:
> >
> > I should rephrase since we need context here:
> >
> > 1. TOT lldb <=> current debugserver/llgs
> >
> > Will use qProcessInfo and get the process id.
> >
> > 2. TOT lldb <=> older debugserver
> >
> > Will use qProcessINfo and get the process id.
> >
> > 3. TOT lldb <=> gdbserver
> >
> > Will fail to get the process id from qProcessInfo, will not ask for qC
> because it's not Apple/iOS.  That will trigger lldb to think its not
> attached properly.
> >
> > -- two fixes here:
> > 3.1 - fall back to qC as we would in the Apple/iOS case, and realize we
> wont' really have the real PID, which we don't really care about for the
> remote setup case,
> > 3.2 - rework and add a "do I have a process on the remote end"
> predicate, and rework attach logic to just ask this new question which
> doesn't promise a process id.
>
> I vote for 3.1 where we don't limit to iOS.
>
> > 4. old lldb <=> TOT debugserver
> >
> > Asks for qC, gets same behavior as it did before - gets a thread id,
> treats it as a process id, connection works.
>
> Yes.
> >
> >
> > So we really only have case 3 that is an issue, which is what Matthew
> hit since he has a different  gdbremote.
> >
> > Since I believe that is the only case we're trying to fix here, I'd be
> in favor of doing the simple fix of allowing $qC for a fake process id not
> limited to Apple/iOS.
>
> Agreed.
>
> >
> > That's my take.  I'm happy to check that change in if this seems
> reasonable.
> >
> > -Todd
> >
> >
> > On Tue, 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.
> >
> > I'd offer we consider falling back to qC and not limiting it to iOS 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).
> >
> > 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
> >
>
>


-- 
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/aa1d3c4c/attachment.html>