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

[Greg Clayton]

> 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
>