> On May 13, 2014, at 4:38 PM, Todd Fiala <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: > > > On May 13, 2014, at 7:56 AM, Todd Fiala <[email protected]> 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 <[email protected]> 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 | [email protected] | 650-943-3180 > > > > > -- > Todd Fiala | Software Engineer | [email protected] | 650-943-3180 > _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
