This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v9 29/29] 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: Fri, 20 Dec 2013 16:07:19 +0000
- Subject: Re: [PATCH v9 29/29] record-btrace: add (reverse-)stepping support
- Authentication-results: sourceware.org; auth=none
- References: <1387471499-29444-1-git-send-email-markus dot t dot metzger at intel dot com> <1387471499-29444-30-git-send-email-markus dot t dot metzger at intel dot com> <52B3529E dot 70407 at redhat dot com> <A78C989F6D9628469189715575E55B230AA3BA2A at IRSMSX104 dot ger dot corp dot intel dot com>
On 12/20/2013 02:36 PM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Thursday, December 19, 2013 9:10 PM
>
>
>>> + if (non_stop)
>>> + error (_("Record btrace can't debug inferior in non-stop mode "
>>> + "(non-stop)."));
>>
>> What's the intent of saying non-stop twice, in:
>>
>> "in non-stop mode (non-stop)"
>
> I took this from s/w record without thinking. Fixed.
Ah, indeed I see it there now...
>>> + /* Stop all other threads. */
>>> + if (!non_stop)
>>> + ALL_THREADS (other)
>>> + other->btrace.flags &= ~BTHR_MOVE;
>>
>> (I know it doesn't work currently), but in non-stop, the
>> event thread should also get its BTHR_MOVE flag cleared.
>> I didn't spot where that was being done.
>
> It's done in record_btrace_step_thread right at the beginning.
>
>
>>> + /* GDB seems to need this. Without, a stale PC seems to be used
>> resulting in
>>> + the current location to be displayed incorrectly. */
>>> + registers_changed_ptid (tp->ptid);
>>
>> This really shouldn't be necessary, given target_resume does
>> it for you. If you still needed, you're papering over some
>> problem.
>
> If we start replaying in to_wait, we'll call get_current_frame
> to fix up some stepping related frames. This will be done on
> the current PC.
>
> When we step later on in record_btrace_step_thread, we change
> the replay position but not the PC.
Alright, that's the place to flush it then. It's just like
the registers_changed calls in linux-nat.c, whenever the
target resumes the thread behind the core's back:
registers_changed ();
if (linux_nat_prepare_to_resume != NULL)
linux_nat_prepare_to_resume (lp);
linux_ops->to_resume (linux_ops, pid_to_ptid (ptid_get_lwp (lp->ptid)),
lp->step, GDB_SIGNAL_0);
etc.
>
> I guess it will be more clear when I move this into
> record_btrace_step_thread and change the comment.
Thanks.
--
Pedro Alves