Re: [patch v8 24/24] record-btrace: add (reverse-)stepping support

Pedro Alves
2013-12-17T20:07:17Z

On 12/17/2013 02:14 PM, Metzger, Markus T wrote:

> Does scheduler locking set inferior_ptid?
> If it does, we should already get the desired behaviour.

It does, but only by chance. The thread to resume will
be in ptid passed to target_resume. But I confirm
the code already does the right thing.

>
>
>>>>> + /* Find the thread to move. */
>>>>> + if (ptid_equal (minus_one_ptid, ptid) || ptid_is_pid (ptid))
>>>>> + {
>>>>> + ALL_THREADS (tp)
>>>>> + record_btrace_resume_thread (tp, flag);
>>>>
>>>> Seems like this steps all threads, when gdb only wants to
>>>> step inferior_ptid and continue others?
>>>
>>> Only if gdb passes -1 or the ptid of the process.
>>
>> Yes, but -1 or "ptid of the process" means "step inferior_ptid
>> and let others run free".
>
> See below.
>
>
>>> In the end, we will move exactly one thread and keep all
>>> others where they are. This one thread will hit a breakpoint
>>> or run out of execution history.
>>
>> Some of the other threads can hit a breakpoint before the
>> current thread hits one too.
>
> That depends on thread interleaving. There is no guarantee that
> the other threads make progress. The way it is currently
> implemented, we step the chosen thread until the next event.
> This event is either a breakpoint, step completed, or out of
> execution history.
>
> We're effectively only stepping a single thread. I believe this
> is also how the s/w record target works.

I'd understand that if the code looked obviously like but,
how the quoted code above seems to be resuming/stepping all
threads?

/* Find the thread to move. */
if (ptid_equal (minus_one_ptid, ptid) || ptid_is_pid (ptid))
{
ALL_THREADS (tp)
record_btrace_resume_thread (tp, flag);

> But this means that I would rely on inferior_ptid in to_resume
> if -1 or the process id is passed.

That's fine. target_resume is telling the target "resume _this_",
albeit getting what "this" is has a weird interface. It's
target_wait that's the problem. The user can have e.g., thread 3
selected (meaning inferior_ptid points at thread 3), while
target_wait returns an event for some other thread. There's
really no connection here.

--
Pedro Alves