Re: [RFA] let record_resume fail immediately on error

[Hui Zhu <teawater@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3882 bytes --]

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

[-- Attachment #2: prec-fix-error-handler.txt --]
[-- Type: text/plain, Size: 1166 bytes --]

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