This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch v8 24/24] record-btrace: add (reverse-)stepping support
- From: Pedro Alves <palves at redhat dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: "jan dot kratochvil at redhat dot com" <jan dot kratochvil at redhat dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Eli Zaretskii <eliz at gnu dot org>
- Date: Tue, 17 Dec 2013 20:07:10 +0000
- Subject: Re: [patch v8 24/24] record-btrace: add (reverse-)stepping support
- Authentication-results: sourceware.org; auth=none
- References: <1386839747-8860-1-git-send-email-markus dot t dot metzger at intel dot com> <1386839747-8860-25-git-send-email-markus dot t dot metzger at intel dot com> <52AB5E6A dot 1010105 at redhat dot com> <A78C989F6D9628469189715575E55B230AA38F1D at IRSMSX104 dot ger dot corp dot intel dot com> <52AF62CB dot 7010805 at redhat dot com> <A78C989F6D9628469189715575E55B230AA39796 at IRSMSX104 dot ger dot corp dot intel dot com>
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