Re: [RFA] let record_resume fail immediately on error

[Hui Zhu <teawater@gmail.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.  */
>>>
>
>