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

[Todd Fiala]

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.

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.


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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140513/eda2dff0/attachment.html>