Re: [RFA] let record_resume fail immediately on error

[Michael Snyder <msnyder@vmware.com>]

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.  */
>>