[Lldb-commits] [PATCH] D11415: Add jstopinfo support to llgs

[Jim Ingham]

Jason & Greg looked at this too, and I think they were playing with solutions along the lines of your #1. I think that's really the only safe way to do it.  For instance, on OS X, when a thread is sitting in a syscall, and the program receives a signal, as part of interrupting the syscall the current register state operating in the kernel will get copied out from the kernel and overwrite the register state you got from the thread_info call before the signal, even though that thread formally hasn't gotten a chance to run.  That's very uncommon, of course, but still possible.  

We learned this originally because every so often in gdb we would call a function by hand, and then continue and the program would later on crash in some nonsensical way.  Turns out we were calling a function on a thread doing a syscall, so we saved the user-visible thread state before the function, but since the thread hadn't sync'ed the user & kernel states, that was an incorrect register state.  Then when we restored the register state, we changed the actual (in the kernel) register state to that incorrect state, and when the syscall continued it did so with an incorrect register state and crashed.  That bug took a bunch of effort to track down, and in general the times where this sort of assumption proves not to be true will cause failures that are subtle and hard to diagnose.

Anyway, that story doesn't bear directly on this issue, except to urge extreme caution in assuming anything you don't need to assume about what happens when you let a program run under the debugger.  Out of this abundance of caution, I'd really rather not bake into lldb the assumption that a thread that was suspended over a step won't have changed its state, and certainly not if we can find some other way to get the same performance speedups by some straightforward mechanism like telling ourselves the things we're likely to need to know when we stop.

Jim


> On Jul 30, 2015, at 5:09 AM, Pavel Labath <labath at google.com> wrote:
> 
> 
> Hello Greg, Jim,
> 
> after adding jstopinfo support to LLGS, i am still getting a bunch of
> register read packets which access the program counter. These are
> coming from Thread::SetupForResume(), where we need to check whether a
> thread is on a breakpoint before letting it run. I would like to get
> rid of those packets (my preliminary tests show I can gain about 10%
> stepping speed improvement on my test app with 20 threads). I see two
> ways to go about this:
> 1. send the program counters as a part of the jstopinfo field. I will
> need to change the format of the field a bit, because the current
> assumption is that you do not include the threads which don't have a
> stop reason there, but we need the registers for every thread.
> 2. cache the registers on the client side. These queries happen after
> we have previously done a vCont:s (as a part of ThreadPlanStepOver),
> so the client can determine that the other threads have not executed,
> and the registers are unchanged. We would still avoid caching the stop
> reason, since on OSX this can change even if the thread is not
> running, but I hope the registers remain the same.
> 
> All in all, the first approach is probably more easier to implement,
> but the second one sounds better to me architecturally, and has the
> benefit of caching all registers, and not just the PC.
> 
> Do you have any thoughts on this?
> 
> cheers,
> pl
> 
> 
> On 23 July 2015 at 10:10, Pavel Labath <labath at google.com> wrote:
>> This revision was automatically updated to reflect the committed changes.
>> labath marked 2 inline comments as done.
>> Closed by commit rL242997: Add jstopinfo support to llgs (authored by labath).
>> 
>> Changed prior to commit:
>>  http://reviews.llvm.org/D11415?vs=30348&id=30464#toc
>> 
>> Repository:
>>  rL LLVM
>> 
>> http://reviews.llvm.org/D11415
>> 
>> Files:
>>  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
>>  lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp
>>  lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h
>>  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>>