Re: [RFA] let record_resume fail immediately on error

[Hui Zhu <teawater at gmail dot com>]

OK.  Thanks, Michael.

Hui

On Sun, Sep 27, 2009 at 02:33, Michael Snyder <msnyder@vmware.com> wrote:
> Personally, I think this one isn't critical for 7.0.
> Waiting for 7.1 will allow us more time to assess it.
>
> Hui Zhu wrote:
>>
>> Hi Joel,
>>
>> Sorry to disturb you. ?Ping
>> http://sourceware.org/ml/gdb-patches/2009-09/msg00231.html
>>
>> Thanks,
>> Hui
>>
>> On Wed, Sep 9, 2009 at 10:05, Hui Zhu <teawater@gmail.com> wrote:
>>>
>>> On Wed, Sep 9, 2009 at 00:55, Michael Snyder<msnyder@vmware.com> wrote:
>>>>
>>>> Hui Zhu wrote:
>>>>>
>>>>> On Tue, Sep 8, 2009 at 15:25, Hui Zhu<teawater@gmail.com> wrote:
>>>>>>
>>>>>> If GDB call error in record_resume, user cannot keep debug the
>>>>>> inferior.
>>>>>>
>>>>>> Hui
>>>>>>
>>>>>> On Tue, Sep 8, 2009 at 15:23, Hui Zhu<teawater@gmail.com> wrote:
>>>>>>>
>>>>>>> The "record_resume_error" in gdb-cvs is to make user after get a
>>>>>>> error
>>>>>>> of record_message, they can "record stop" close the record and keep
>>>>>>> debug the inferior.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Hui
>>>>>>>
>>>>>>> On Tue, Sep 8, 2009 at 14:58, Joel Brobecker<brobecker@adacore.com>
>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> ?if (!RECORD_IS_REPLAY)
>>>>>>>>> ? ?{
>>>>>>>>> ? ? ?if (do_record_message (get_current_regcache ()))
>>>>>>>>> - ? ? ? ?{
>>>>>>>>> - ? ? ? ? ?record_resume_error = 0;
>>>>>>>>> - ? ? ? ?}
>>>>>>>>> - ? ? ?else
>>>>>>>>> - ? ? ? ?{
>>>>>>>>> - ? ? ? ? ?record_resume_error = 1;
>>>>>>>>> - ? ? ? ? ?return;
>>>>>>>>> - ? ? ? ?}
>>>>>>>>> + ? ? internal_error (__FILE__, __LINE__,
>>>>>>>>> + ? ? ? ? ? ? ? ? ? ? _("record_resume: do_record_message
>>>>>>>>> failed."));
>>>>>>>>> +
>>>>>>>>
>>>>>>>> Forgive me if I'm wrong, as I don't know the record.c code at all,
>>>>>>>> but
>>>>>>>> I cannot help but think that the internal_error is suspicious here.
>>>>>>>> Why is this an internal_error?
>>>>>>>>
>>>>>>>> --
>>>>>>>> Joel
>>>>>>>>
>>>>> Hi guys,
>>>>>
>>>>> I make a patch that make "record_resume_error" work better. ?I did
>>>>> some test. ?It seems better than before.
>>>>
>>>> Could you explain what you mean by better?
>>>> I mean, what behavior are you looking for here?
>>>>
>>>> Here is the behavior that I see --- I am making a recording,
>>>> I say "continue", and after a while this "record_resume_error"
>>>> is triggered, and gdb stops and says "No more reverse-execution
>>>> history."
>>>>
>>>> I would never expect to see that message during recording.
>>>
>>> In before, GDB cannot throw error in record_resume (It just in my
>>> memory, maybe I am wrong).
>>> If we try to do it, it will get:
>>> (gdb) c
>>> Continuing.
>>> Cannot execute this command while the selected thread is running.
>>> So I add record_resume_error to let record_wait to handle it.
>>>
>>> But record_wait cannot call error too. ?So I let it:
>>> ? ? ? ? /* If record_resume get error, return directly. ?*/
>>> ? ? ? ? status->kind = TARGET_WAITKIND_STOPPED;
>>> ? ? ? ? status->value.sig = TARGET_SIGNAL_0;
>>> ? ? ? ? return inferior_ptid;
>>>
>>> But I found that record_resume can call error now. ?So I make a new
>>> patch for it.
>>>
>>> Could you help me try it?
>>>
>>>
>>> Thanks,
>>> Hui
>>>
>>> 2009-09-09 ?Hui Zhu ?<teawater@gmail.com>
>>>
>>> ? ? ? * record.c (record_resume_error): Deleted.
>>> ? ? ? (record_resume): Call record_message.
>>> ? ? ? (record_wait): Deleted record_resume_error.
>>>
>>> ---
>>> ?record.c | ? 19 +------------------
>>> ?1 file changed, 1 insertion(+), 18 deletions(-)
>>>
>>> --- a/record.c
>>> +++ b/record.c
>>> @@ -516,7 +516,6 @@ record_close (int quitting)
>>> ?}
>>>
>>> ?static int record_resume_step = 0;
>>> -static int record_resume_error;
>>>
>>> ?static void
>>> ?record_resume (struct target_ops *ops, ptid_t ptid, int step,
>>> @@ -526,15 +525,7 @@ record_resume (struct target_ops *ops, p
>>>
>>> ?if (!RECORD_IS_REPLAY)
>>> ? ?{
>>> - ? ? ?if (do_record_message (get_current_regcache ()))
>>> - ? ? ? ?{
>>> - ? ? ? ? ?record_resume_error = 0;
>>> - ? ? ? ?}
>>> - ? ? ?else
>>> - ? ? ? ?{
>>> - ? ? ? ? ?record_resume_error = 1;
>>> - ? ? ? ? ?return;
>>> - ? ? ? ?}
>>> + ? ? ?record_message (get_current_regcache ());
>>> ? ? ?record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?siggnal);
>>> ? ?}
>>> @@ -586,14 +577,6 @@ record_wait (struct target_ops *ops,
>>>
>>> ?if (!RECORD_IS_REPLAY)
>>> ? ?{
>>> - ? ? ?if (record_resume_error)
>>> - ? ? ? {
>>> - ? ? ? ? /* If record_resume get error, return directly. ?*/
>>> - ? ? ? ? status->kind = TARGET_WAITKIND_STOPPED;
>>> - ? ? ? ? status->value.sig = TARGET_SIGNAL_ABRT;
>>> - ? ? ? ? return inferior_ptid;
>>> - ? ? ? }
>>> -
>>> ? ? ?if (record_resume_step)
>>> ? ? ? {
>>> ? ? ? ? /* This is a single step. ?*/
>>>
>
>