[RFA] Expand "info record"

[RFA] Expand "info record"

[Michael Snyder]

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

Hi Hui,

This patch expands on the output of "info record" so that it looks like 
this:

(gdb) info rec
Lowest  recorded instruction number is 0.
Current instruction number is 46.
Highest recorded instruction number is 1016.
Max logged instructions is 200000


What do you think?
Michael


[-- Attachment #2: inforec.txt --]
[-- Type: text/plain, Size: 3615 bytes --]

2009-10-12  Michael Snyder  <msnyder@vmware.com>
	Elaborate "info record".
	* record.c (struct record_end_entry): New field 'insn_num'.
	(record_insn_count): New variable.
	(record_open): Initialize record_insn_count.
	(info_record_command): Display contents of record log as
	lowest, current, and highest instruction counts.
	(show_record_insn_number): Delete.
	(_initialize_record): Remove add_cmd show_record_insn_number.

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.20
diff -u -p -r1.20 record.c
--- record.c	27 Sep 2009 02:49:34 -0000	1.20
+++ record.c	12 Oct 2009 16:25:38 -0000
@@ -62,6 +62,7 @@ struct record_mem_entry
 struct record_end_entry
 {
   enum target_signal sigval;
+  unsigned long long insn_num;
 };
 
 enum record_type
@@ -100,6 +101,7 @@ static struct record_entry *record_arch_
 static int record_stop_at_limit = 1;
 static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
 static int record_insn_num = 0;
+static unsigned long long record_insn_count;
 
 /* The target_ops of process record.  */
 static struct target_ops record_ops;
@@ -322,6 +324,7 @@ record_arch_list_add_end (void)
   rec->next = NULL;
   rec->type = record_end;
   rec->u.end.sigval = TARGET_SIGNAL_0;
+  rec->u.end.insn_num = record_insn_count++;
 
   record_arch_list_add (rec);
 
@@ -552,6 +555,7 @@ record_open (char *name, int from_tty)
 
   /* Reset */
   record_insn_num = 0;
+  record_insn_count = 0;
   record_list = &record_first;
   record_list->next = NULL;
 }
@@ -1269,16 +1273,6 @@ set_record_insn_max_num (char *args, int
     }
 }
 
-/* Print the current index into the record log (number of insns recorded
-   so far).  */
-
-static void
-show_record_insn_number (char *ignore, int from_tty)
-{
-  printf_unfiltered (_("Record instruction number is %d.\n"),
-		     record_insn_num);
-}
-
 static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
 			       *show_record_cmdlist, *info_record_cmdlist;
 
@@ -1299,7 +1293,31 @@ show_record_command (char *args, int fro
 static void
 info_record_command (char *args, int from_tty)
 {
-  cmd_show_list (info_record_cmdlist, from_tty, "");
+  struct record_entry *p;
+
+  /* Find entry for first actual instruction in the log.  */
+  for (p = record_first.next;
+       (p != NULL) && (p->type != record_end);
+       p = p->next)
+    ;
+
+  /* Display instruction number for first instruction in the log.  */
+  if (p != NULL && p->type == record_end)
+    printf_filtered (_("Lowest  recorded instruction number is %llu.\n"),
+		     p->u.end.insn_num);
+
+  /* If we are not at the end of the log, display where we are.  */
+  if (record_list->next != NULL && record_list->type == record_end)
+    printf_filtered (_("Current instruction number is %llu.\n"),
+		     record_list->u.end.insn_num);
+
+  /* Display instruction number for last instruction in the log.  */
+  printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
+		   record_insn_count ? record_insn_count - 1 : 0);
+
+  /* Display max log size.  */
+  printf_filtered (_("Max logged instructions is %d\n"),
+		   record_insn_max_num);
 }
 
 void
@@ -1369,7 +1387,4 @@ Set the maximum number of instructions t
 record/replay buffer.  Zero means unlimited.  Default is 200000."),
 			    set_record_insn_max_num,
 			    NULL, &set_record_cmdlist, &show_record_cmdlist);
-  add_cmd ("insn-number", class_obscure, show_record_insn_number,
-	   _("Show the current number of instructions in the "
-	     "record/replay buffer."), &info_record_cmdlist);
 }

Re: [RFA] Expand "info record"

[Tom Tromey]

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> (gdb) info rec
Michael> Lowest  recorded instruction number is 0.

There is an extra space after "Lowest".

Tom

Re: [RFA] Expand "info record"

[Michael Snyder]

Tom Tromey wrote:
>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
> 
> Michael> (gdb) info rec
> Michael> Lowest  recorded instruction number is 0.
> 
> There is an extra space after "Lowest".

I know, I was just trying to line up the outputs.
Is this frowned upon?

Re: [RFA] Expand "info record"

[Tom Tromey]

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> Tom Tromey wrote:
>>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
>> 
Michael> (gdb) info rec
Michael> Lowest  recorded instruction number is 0.
>> 
>> There is an extra space after "Lowest".

Michael> I know, I was just trying to line up the outputs.
Michael> Is this frowned upon?

The example again:

(gdb) info rec
Lowest  recorded instruction number is 0.
Current instruction number is 46.
Highest recorded instruction number is 1016.
Max logged instructions is 200000


At first glance this did not seem to line up at all to me.
But now I see that the first and third lines to line up.

I think that is too subtle, but I suppose I don't really care that much
either way.

Tom

Re: [RFA] Expand "info record"

[Hui Zhu]

Thanks Michael,

I have 2 questions for this patch:

1.  info rec will not show record_insn_num?

2.  (gdb) info rec
Lowest  recorded instruction number is 0.
Current instruction number is 46.
Highest recorded instruction number is 1016.
Max logged instructions is 200000
Do you think the last line need a dot?

Hui

On Tue, Oct 13, 2009 at 00:27, Michael Snyder <msnyder@vmware.com> wrote:
> Hi Hui,
>
> This patch expands on the output of "info record" so that it looks like
> this:
>
> (gdb) info rec
> Lowest  recorded instruction number is 0.
> Current instruction number is 46.
> Highest recorded instruction number is 1016.
> Max logged instructions is 200000
>
>
> What do you think?
> Michael
>
>
> 2009-10-12  Michael Snyder  <msnyder@vmware.com>
>        Elaborate "info record".
>        * record.c (struct record_end_entry): New field 'insn_num'.
>        (record_insn_count): New variable.
>        (record_open): Initialize record_insn_count.
>        (info_record_command): Display contents of record log as
>        lowest, current, and highest instruction counts.
>        (show_record_insn_number): Delete.
>        (_initialize_record): Remove add_cmd show_record_insn_number.
>
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 record.c
> --- record.c    27 Sep 2009 02:49:34 -0000      1.20
> +++ record.c    12 Oct 2009 16:25:38 -0000
> @@ -62,6 +62,7 @@ struct record_mem_entry
>  struct record_end_entry
>  {
>   enum target_signal sigval;
> +  unsigned long long insn_num;
>  };
>
>  enum record_type
> @@ -100,6 +101,7 @@ static struct record_entry *record_arch_
>  static int record_stop_at_limit = 1;
>  static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
>  static int record_insn_num = 0;
> +static unsigned long long record_insn_count;
>
>  /* The target_ops of process record.  */
>  static struct target_ops record_ops;
> @@ -322,6 +324,7 @@ record_arch_list_add_end (void)
>   rec->next = NULL;
>   rec->type = record_end;
>   rec->u.end.sigval = TARGET_SIGNAL_0;
> +  rec->u.end.insn_num = record_insn_count++;
>
>   record_arch_list_add (rec);
>
> @@ -552,6 +555,7 @@ record_open (char *name, int from_tty)
>
>   /* Reset */
>   record_insn_num = 0;
> +  record_insn_count = 0;
>   record_list = &record_first;
>   record_list->next = NULL;
>  }
> @@ -1269,16 +1273,6 @@ set_record_insn_max_num (char *args, int
>     }
>  }
>
> -/* Print the current index into the record log (number of insns recorded
> -   so far).  */
> -
> -static void
> -show_record_insn_number (char *ignore, int from_tty)
> -{
> -  printf_unfiltered (_("Record instruction number is %d.\n"),
> -                    record_insn_num);
> -}
> -
>  static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
>                               *show_record_cmdlist, *info_record_cmdlist;
>
> @@ -1299,7 +1293,31 @@ show_record_command (char *args, int fro
>  static void
>  info_record_command (char *args, int from_tty)
>  {
> -  cmd_show_list (info_record_cmdlist, from_tty, "");
> +  struct record_entry *p;
> +
> +  /* Find entry for first actual instruction in the log.  */
> +  for (p = record_first.next;
> +       (p != NULL) && (p->type != record_end);
> +       p = p->next)
> +    ;
> +
> +  /* Display instruction number for first instruction in the log.  */
> +  if (p != NULL && p->type == record_end)
> +    printf_filtered (_("Lowest  recorded instruction number is %llu.\n"),
> +                    p->u.end.insn_num);
> +
> +  /* If we are not at the end of the log, display where we are.  */
> +  if (record_list->next != NULL && record_list->type == record_end)
> +    printf_filtered (_("Current instruction number is %llu.\n"),
> +                    record_list->u.end.insn_num);
> +
> +  /* Display instruction number for last instruction in the log.  */
> +  printf_filtered (_("Highest recorded instruction number is %llu.\n"),
> +                  record_insn_count ? record_insn_count - 1 : 0);
> +
> +  /* Display max log size.  */
> +  printf_filtered (_("Max logged instructions is %d\n"),
> +                  record_insn_max_num);
>  }
>
>  void
> @@ -1369,7 +1387,4 @@ Set the maximum number of instructions t
>  record/replay buffer.  Zero means unlimited.  Default is 200000."),
>                            set_record_insn_max_num,
>                            NULL, &set_record_cmdlist, &show_record_cmdlist);
> -  add_cmd ("insn-number", class_obscure, show_record_insn_number,
> -          _("Show the current number of instructions in the "
> -            "record/replay buffer."), &info_record_cmdlist);
>  }
>
>

Re: [RFA] Expand "info record"

[Pedro Alves]

On Monday 12 October 2009 19:10:44, Tom Tromey wrote:

> The example again:
> 
> (gdb) info rec
> Lowest  recorded instruction number is 0.
> Current instruction number is 46.
> Highest recorded instruction number is 1016.
> Max logged instructions is 200000
> 
> At first glance this did not seem to line up at all to me.
> But now I see that the first and third lines to line up.

I just saw a similar paste in another message, and it looked
like a bug to me as well.  The subtle vertical alignment here is
only distracting, and serves no useful purpose, IMO.

-- 
Pedro Alves

Re: [RFA] Expand "info record"

[Michael Snyder]

Hui Zhu wrote:
> Thanks Michael,
> 
> I have 2 questions for this patch:
> 
> 1.  info rec will not show record_insn_num?

It does!  See below...

> 
> 2.  (gdb) info rec
> Lowest  recorded instruction number is 0.
> Current instruction number is 46.
> Highest recorded instruction number is 1016.

This is your record_insn_num -- the highest recorded insn num.

> Max logged instructions is 200000

And this is your record_insn_max_num.

> Do you think the last line need a dot?

Yes, thank you for catching it.

OK to check in with that fixed?

Re: [RFA] Expand "info record"

[Michael Snyder]

Michael Snyder wrote:
> Hui Zhu wrote:
>> Thanks Michael,
>>
>> I have 2 questions for this patch:
>>
>> 1.  info rec will not show record_insn_num?
> 
> It does!  See below...
> 
>> 2.  (gdb) info rec
>> Lowest  recorded instruction number is 0.
>> Current instruction number is 46.
>> Highest recorded instruction number is 1016.
> 
> This is your record_insn_num -- the highest recorded insn num.

Oh, sorry, I'm wrong about that.
I'll post a new patch.

> 
>> Max logged instructions is 200000
> 
> And this is your record_insn_max_num.
> 
>> Do you think the last line need a dot?
> 
> Yes, thank you for catching it.
> 
> OK to check in with that fixed?
> 

Re: [RFA] Expand "info record"

[Michael Snyder]

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

Michael Snyder wrote:
> Michael Snyder wrote:
>> Hui Zhu wrote:
>>> Thanks Michael,
>>>
>>> I have 2 questions for this patch:
>>>
>>> 1.  info rec will not show record_insn_num?
>> It does!  See below...
>>
>>> 2.  (gdb) info rec
>>> Lowest  recorded instruction number is 0.
>>> Current instruction number is 46.
>>> Highest recorded instruction number is 1016.
>> This is your record_insn_num -- the highest recorded insn num.
> 
> Oh, sorry, I'm wrong about that.
> I'll post a new patch.

Looks like I forgot to post a new patch!  Here...


[-- Attachment #2: info.txt --]
[-- Type: text/plain, Size: 4997 bytes --]

2009-10-12  Michael Snyder  <msnyder@vmware.com>

	Elaborate "info record".
	* record.c (struct record_end_entry): New field 'insn_num'.
	(record_insn_count): New variable.
	(record_open): Initialize record_insn_count.
	(info_record_command): Display contents of record log as
	lowest, current, and highest instruction counts.
	(show_record_insn_number): Delete.
	(_initialize_record): Remove add_cmd show_record_insn_number.

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.25
diff -u -p -b -w -r1.25 record.c
--- record.c	19 Oct 2009 09:51:41 -0000	1.25
+++ record.c	20 Oct 2009 19:38:55 -0000
@@ -71,6 +71,7 @@ struct record_reg_entry
 struct record_end_entry
 {
   enum target_signal sigval;
+  unsigned long long insn_num;
 };
 
 enum record_type
@@ -109,6 +110,7 @@ static struct record_entry *record_arch_
 static int record_stop_at_limit = 1;
 static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
 static int record_insn_num = 0;
+static unsigned long long record_insn_count;
 
 /* The target_ops of process record.  */
 static struct target_ops record_ops;
@@ -279,7 +281,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
+	{
 	record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -308,7 +313,6 @@ record_list_release_first (void)
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
 	{
-	  record_insn_num--;
 	  break;	/* End loop at first record_end.  */
 	}
 
@@ -435,6 +439,7 @@ record_arch_list_add_end (void)
 
   rec = record_end_alloc ();
   rec->u.end.sigval = TARGET_SIGNAL_0;
+  rec->u.end.insn_num = record_insn_count++;
 
   record_arch_list_add (rec);
 
@@ -665,6 +670,7 @@ record_open (char *name, int from_tty)
 
   /* Reset */
   record_insn_num = 0;
+  record_insn_count = 0;
   record_list = &record_first;
   record_list->next = NULL;
 }
@@ -1362,8 +1368,8 @@ cmd_record_stop (char *args, int from_tt
   if (current_target.to_stratum == record_stratum)
     {
       unpush_target (&record_ops);
-      printf_unfiltered (_("Process record is stoped and all execution "
-                           "log is deleted.\n"));
+      printf_unfiltered (_("Process record is stopped and all execution "
+                           "logs are deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));
@@ -1385,16 +1391,6 @@ set_record_insn_max_num (char *args, int
     }
 }
 
-/* Print the current index into the record log (number of insns recorded
-   so far).  */
-
-static void
-show_record_insn_number (char *ignore, int from_tty)
-{
-  printf_unfiltered (_("Record instruction number is %d.\n"),
-		     record_insn_num);
-}
-
 static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
 			       *show_record_cmdlist, *info_record_cmdlist;
 
@@ -1412,10 +1408,47 @@ show_record_command (char *args, int fro
   cmd_show_list (show_record_cmdlist, from_tty, "");
 }
 
+/* Display some statistics about the execution log.  */
+
 static void
 info_record_command (char *args, int from_tty)
 {
-  cmd_show_list (info_record_cmdlist, from_tty, "");
+  struct record_entry *p;
+
+  /* Find entry for first actual instruction in the log.  */
+  for (p = record_first.next;
+       (p != NULL) && (p->type != record_end);
+       p = p->next)
+    ;
+
+  /* Do we have a log at all?  */
+  if (p != NULL && p->type == record_end)
+    {
+      /* Display instruction number for first instruction in the log.  */
+      printf_filtered (_("Lowest recorded instruction number is %llu.\n"),
+		       p->u.end.insn_num);
+
+      /* If we are not at the end of the log, display where we are.  */
+      if (record_list->next != NULL && record_list->type == record_end)
+	printf_filtered (_("Current instruction number is %llu.\n"),
+			 record_list->u.end.insn_num);
+
+      /* Display instruction number for last instruction in the log.  */
+      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
+		       record_insn_count ? record_insn_count - 1 : 0);
+
+      /* Display log count.  */
+      printf_filtered (_("Log contains %d instructions.\n"), record_insn_num);
+    }
+  else
+    {
+      printf_filtered (_("Process record is not active, or "
+			 "there is no log yet.\n"));
+    }
+
+  /* Display max log size.  */
+  printf_filtered (_("Max logged instructions is %d.\n"),
+		   record_insn_max_num);
 }
 
 void
@@ -1485,7 +1518,4 @@ Set the maximum number of instructions t
 record/replay buffer.  Zero means unlimited.  Default is 200000."),
 			    set_record_insn_max_num,
 			    NULL, &set_record_cmdlist, &show_record_cmdlist);
-  add_cmd ("insn-number", class_obscure, show_record_insn_number,
-	   _("Show the current number of instructions in the "
-	     "record/replay buffer."), &info_record_cmdlist);
 }

Re: [RFA] Expand "info record"

[Pedro Alves]

On Tuesday 20 October 2009 20:44:16, Michael Snyder wrote:
> +  unsigned long long insn_num;

ULONGEST

> +       (p != NULL) && (p->type != record_end);

Superfluous parens.

> +      printf_filtered (_("Lowest recorded instruction number is %llu.\n"),
> +                      p->u.end.insn_num);

pulongest

> +      /* Display instruction number for last instruction in the log.  */
> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
> +                      record_insn_count ? record_insn_count - 1 : 0);

Why the conditional subtraction?

Given this post inc:
> +  rec->u.end.insn_num = record_insn_count++;

The subtraction looks suspicious.

Could you add a comment to record_insn_num and
record_insn_count's definitions explaining what they are and
how they're different, if it doesn't become obvious?

-- 
Pedro Alves

Re: [RFA] Expand "info record"

[Michael Snyder]

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

Pedro Alves wrote:
> On Tuesday 20 October 2009 20:44:16, Michael Snyder wrote:
>> +  unsigned long long insn_num;
> 
> ULONGEST
> 
>> +       (p != NULL) && (p->type != record_end);
> 
> Superfluous parens.
> 
>> +      printf_filtered (_("Lowest recorded instruction number is %llu.\n"),
>> +                      p->u.end.insn_num);
> 
> pulongest

OK to all.


>> +      /* Display instruction number for last instruction in the log.  */
>> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
>> +                      record_insn_count ? record_insn_count - 1 : 0);
> 
> Why the conditional subtraction?

Because I don't want it to say "-1".

> Given this post inc:
>> +  rec->u.end.insn_num = record_insn_count++;

No, the post inc implies that the count is actually one greater
than the last insn that used it.  Yes?

> The subtraction looks suspicious.
> 
> Could you add a comment to record_insn_num and
> record_insn_count's definitions explaining what they are and
> how they're different, if it doesn't become obvious?

Yep, done, see attached.

Thanks!




[-- Attachment #2: info2.txt --]
[-- Type: text/plain, Size: 4877 bytes --]

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.25
diff -u -p -b -w -r1.25 record.c
--- record.c	19 Oct 2009 09:51:41 -0000	1.25
+++ record.c	20 Oct 2009 22:19:19 -0000
@@ -71,6 +71,7 @@ struct record_reg_entry
 struct record_end_entry
 {
   enum target_signal sigval;
+  ULONGEST insn_num;
 };
 
 enum record_type
@@ -107,8 +108,13 @@ static struct record_entry *record_arch_
 
 /* 1 ask user. 0 auto delete the last struct record_entry.  */
 static int record_stop_at_limit = 1;
+/* Maximum allowed number of insns in execution log.  */
 static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
+/* Actual count of insns presently in execution log.  */
 static int record_insn_num = 0;
+/* Count of insns logged so far (may be larger
+   than count of insns presently in execution log).  */
+static ULONGEST record_insn_count;
 
 /* The target_ops of process record.  */
 static struct target_ops record_ops;
@@ -279,7 +285,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
+	{
 	record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -308,7 +317,6 @@ record_list_release_first (void)
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
 	{
-	  record_insn_num--;
 	  break;	/* End loop at first record_end.  */
 	}
 
@@ -435,6 +443,7 @@ record_arch_list_add_end (void)
 
   rec = record_end_alloc ();
   rec->u.end.sigval = TARGET_SIGNAL_0;
+  rec->u.end.insn_num = record_insn_count++;
 
   record_arch_list_add (rec);
 
@@ -665,6 +674,7 @@ record_open (char *name, int from_tty)
 
   /* Reset */
   record_insn_num = 0;
+  record_insn_count = 0;
   record_list = &record_first;
   record_list->next = NULL;
 }
@@ -1362,8 +1372,8 @@ cmd_record_stop (char *args, int from_tt
   if (current_target.to_stratum == record_stratum)
     {
       unpush_target (&record_ops);
-      printf_unfiltered (_("Process record is stoped and all execution "
-                           "log is deleted.\n"));
+      printf_unfiltered (_("Process record is stopped and all execution "
+                           "logs are deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));
@@ -1385,16 +1395,6 @@ set_record_insn_max_num (char *args, int
     }
 }
 
-/* Print the current index into the record log (number of insns recorded
-   so far).  */
-
-static void
-show_record_insn_number (char *ignore, int from_tty)
-{
-  printf_unfiltered (_("Record instruction number is %d.\n"),
-		     record_insn_num);
-}
-
 static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
 			       *show_record_cmdlist, *info_record_cmdlist;
 
@@ -1412,10 +1412,48 @@ show_record_command (char *args, int fro
   cmd_show_list (show_record_cmdlist, from_tty, "");
 }
 
+/* Display some statistics about the execution log.  */
+
 static void
 info_record_command (char *args, int from_tty)
 {
-  cmd_show_list (info_record_cmdlist, from_tty, "");
+  struct record_entry *p;
+
+  /* Find entry for first actual instruction in the log.  */
+  for (p = record_first.next;
+       p != NULL && p->type != record_end;
+       p = p->next)
+    ;
+
+  /* Do we have a log at all?  */
+  if (p != NULL && p->type == record_end)
+    {
+      /* Display instruction number for first instruction in the log.  */
+      printf_filtered (_("Lowest recorded instruction number is %s.\n"),
+		       pulongest (p->u.end.insn_num));
+
+      /* If we are not at the end of the log, display where we are.  */
+      if (record_list->next != NULL && record_list->type == record_end)
+	printf_filtered (_("Current instruction number is %s.\n"),
+			 pulongest (record_list->u.end.insn_num));
+
+      /* Display instruction number for last instruction in the log.  */
+      printf_filtered (_("Highest recorded instruction number is %s.\n"), 
+		       record_insn_count ? 
+		       pulongest (record_insn_count - 1) : "0");
+
+      /* Display log count.  */
+      printf_filtered (_("Log contains %d instructions.\n"), record_insn_num);
+    }
+  else
+    {
+      printf_filtered (_("Process record is not active, or "
+			 "there is no log yet.\n"));
+    }
+
+  /* Display max log size.  */
+  printf_filtered (_("Max logged instructions is %d.\n"),
+		   record_insn_max_num);
 }
 
 void
@@ -1485,7 +1523,4 @@ Set the maximum number of instructions t
 record/replay buffer.  Zero means unlimited.  Default is 200000."),
 			    set_record_insn_max_num,
 			    NULL, &set_record_cmdlist, &show_record_cmdlist);
-  add_cmd ("insn-number", class_obscure, show_record_insn_number,
-	   _("Show the current number of instructions in the "
-	     "record/replay buffer."), &info_record_cmdlist);
 }

Re: [RFA] Expand "info record"

[Pedro Alves]

On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:

> >> +      /* Display instruction number for last instruction in the log.  */
> >> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
> >> +                      record_insn_count ? record_insn_count - 1 : 0);
> > 
> > Why the conditional subtraction?
> 
> Because I don't want it to say "-1".

Okay, that much is obvious, but how can you reach here
with record_insn_count == 0, given that you check if you
have a log at all a bit above?

> > Given this post inc:
> >> +  rec->u.end.insn_num = record_insn_count++;
> 
> No, the post inc implies that the count is actually one greater
> than the last insn that used it.  Yes?

Yes.

> > Could you add a comment to record_insn_num and
> > record_insn_count's definitions explaining what they are and
> > how they're different, if it doesn't become obvious?
> 
> Yep, done, see attached.

Thanks.

May I add that I think it's a shame that "info record"
doesn't distinguish precord not being active from no log yet:

 (gdb) info record
 Process record is not active, or there is no log yet.
 Max logged instructions is 200000.

I played with the patch 5 minutes, and I immediately felt that
this bit not having an else branch was surprising:

     /* If we are not at the end of the log, display where we are.  */
      if (record_list->next != NULL && record_list->type == record_end)
	printf_filtered (_("Current instruction number is %s.\n"),
			 pulongest (record_list->u.end.insn_num));

E.g.:

 (top-gdb) info record
 Lowest recorded instruction number is 14.
 Current instruction number is 17.
 Highest recorded instruction number is 18.
 Log contains 5 instructions.
 Max logged instructions is 5.
 (top-gdb) si

 No more reverse-execution history.
 0x00007ffff7df3e1e in ?? () from /lib64/ld-linux-x86-64.so.2
 (top-gdb) info record
 Lowest recorded instruction number is 14.
 Highest recorded instruction number is 18.
 Log contains 5 instructions.
 Max logged instructions is 5.
 (top-gdb)                     

Can't we explicitly say that we're in recording/live vs replay
mode, or something?

-- 
Pedro Alves

Re: [RFA] Expand "info record"

[Michael Snyder]

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

Pedro Alves wrote:
> On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:
> 
>>>> +      /* Display instruction number for last instruction in the log.  */
>>>> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
>>>> +                      record_insn_count ? record_insn_count - 1 : 0);
>>> Why the conditional subtraction?
>> Because I don't want it to say "-1".
> 
> Okay, that much is obvious, but how can you reach here
> with record_insn_count == 0, given that you check if you
> have a log at all a bit above?

Maybe not -- but I'm a belt-and-suspenders guy.
I don't believe in not checking for something just because
it "can't happen".  What if somebody changed the check above?

[...]
> 
> Can't we explicitly say that we're in recording/live vs replay
> mode, or something?

Yeah, you've convinced me.  Thanks for the prodding,
and please see new revision.  I'll make that conditional
go away too.   ;-)


[-- Attachment #2: info4.txt --]
[-- Type: text/plain, Size: 5441 bytes --]

2009-10-12  Michael Snyder  <msnyder@vmware.com>

	Elaborate "info record".
	* record.c (struct record_end_entry): New field 'insn_num'.
	(record_insn_count): New variable.
	(record_open): Initialize record_insn_count.
	(info_record_command): Display contents of record log as
	lowest, current, and highest instruction counts.
	(show_record_insn_number): Delete.
	(_initialize_record): Remove add_cmd show_record_insn_number.

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.27
diff -u -p -b -w -r1.27 record.c
--- record.c	20 Oct 2009 23:06:13 -0000	1.27
+++ record.c	21 Oct 2009 01:33:30 -0000
@@ -91,6 +91,7 @@ struct record_reg_entry
 struct record_end_entry
 {
   enum target_signal sigval;
+  ULONGEST insn_num;
 };
 
 enum record_type
@@ -166,8 +167,13 @@ static struct record_entry *record_arch_
 
 /* 1 ask user. 0 auto delete the last struct record_entry.  */
 static int record_stop_at_limit = 1;
+/* Maximum allowed number of insns in execution log.  */
 static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
+/* Actual count of insns presently in execution log.  */
 static int record_insn_num = 0;
+/* Count of insns logged so far (may be larger
+   than count of insns presently in execution log).  */
+static ULONGEST record_insn_count;
 
 /* The target_ops of process record.  */
 static struct target_ops record_ops;
@@ -338,7 +344,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
+	{
 	record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -367,7 +376,6 @@ record_list_release_first (void)
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
 	{
-	  record_insn_num--;
 	  break;	/* End loop at first record_end.  */
 	}
 
@@ -494,6 +502,7 @@ record_arch_list_add_end (void)
 
   rec = record_end_alloc ();
   rec->u.end.sigval = TARGET_SIGNAL_0;
+  rec->u.end.insn_num = ++record_insn_count;
 
   record_arch_list_add (rec);
 
@@ -800,6 +809,7 @@ record_open (char *name, int from_tty)
 
   /* Reset */
   record_insn_num = 0;
+  record_insn_count = 0;
   record_list = &record_first;
   record_list->next = NULL;
 }
@@ -1458,8 +1468,8 @@ cmd_record_stop (char *args, int from_tt
   if (current_target.to_stratum == record_stratum)
     {
       unpush_target (&record_ops);
-      printf_unfiltered (_("Process record is stoped and all execution "
-                           "log is deleted.\n"));
+      printf_unfiltered (_("Process record is stopped and all execution "
+                           "logs are deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));
@@ -1481,16 +1491,6 @@ set_record_insn_max_num (char *args, int
     }
 }
 
-/* Print the current index into the record log (number of insns recorded
-   so far).  */
-
-static void
-show_record_insn_number (char *ignore, int from_tty)
-{
-  printf_unfiltered (_("Record instruction number is %d.\n"),
-		     record_insn_num);
-}
-
 static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
 			       *show_record_cmdlist, *info_record_cmdlist;
 
@@ -1508,10 +1508,59 @@ show_record_command (char *args, int fro
   cmd_show_list (show_record_cmdlist, from_tty, "");
 }
 
+/* Display some statistics about the execution log.  */
+
 static void
 info_record_command (char *args, int from_tty)
 {
-  cmd_show_list (info_record_cmdlist, from_tty, "");
+  struct record_entry *p;
+
+  if (current_target.to_stratum == record_stratum)
+    {
+      if (RECORD_IS_REPLAY)
+	printf_filtered (_("Replay mode:\n"));
+      else
+	printf_filtered (_("Record mode:\n"));
+
+      /* Find entry for first actual instruction in the log.  */
+      for (p = record_first.next;
+	   p != NULL && p->type != record_end;
+	   p = p->next)
+	;
+
+      /* Do we have a log at all?  */
+      if (p != NULL && p->type == record_end)
+	{
+	  /* Display instruction number for first instruction in the log.  */
+	  printf_filtered (_("Lowest recorded instruction number is %s.\n"),
+			   pulongest (p->u.end.insn_num));
+
+	  /* If in replay mode, display where we are in the log.  */
+	  if (RECORD_IS_REPLAY)
+	    printf_filtered (_("Current instruction number is %s.\n"),
+			     pulongest (record_list->u.end.insn_num));
+
+	  /* Display instruction number for last instruction in the log.  */
+	  printf_filtered (_("Highest recorded instruction number is %s.\n"), 
+			   pulongest (record_insn_count));
+
+	  /* Display log count.  */
+	  printf_filtered (_("Log contains %d instructions.\n"), 
+			   record_insn_num);
+	}
+      else
+	{
+	  printf_filtered (_("No instructions have been logged.\n"));
+	}
+    }
+  else
+    {
+      printf_filtered (_("target record is not active.\n"));
+    }
+
+  /* Display max log size.  */
+  printf_filtered (_("Max logged instructions is %d.\n"),
+		   record_insn_max_num);
 }
 
 void
@@ -1581,7 +1630,4 @@ Set the maximum number of instructions t
 record/replay buffer.  Zero means unlimited.  Default is 200000."),
 			    set_record_insn_max_num,
 			    NULL, &set_record_cmdlist, &show_record_cmdlist);
-  add_cmd ("insn-number", class_obscure, show_record_insn_number,
-	   _("Show the current number of instructions in the "
-	     "record/replay buffer."), &info_record_cmdlist);
 }

Re: [RFA] Expand "info record"

[Hui Zhu]

Thanks Michael.

With change:
 static struct target_ops record_ops;
@@ -338,7 +344,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
+	{
 	record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -367,7 +376,6 @@ record_list_release_first (void)
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
 	{
-	  record_insn_num--;
 	  break;	/* End loop at first record_end.  */
 	}
To:
 static struct target_ops record_ops;
@@ -338,7 +344,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
+	{
 	  record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -367,7 +376,6 @@ record_list_release_first (void)
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
- 	{
-	  record_insn_num--;
 	  break;	/* End loop at first record_end.  */
- 	}

Just some format change.

I am OK with this patch.

Thanks.
Hui


On Wed, Oct 21, 2009 at 09:31, Michael Snyder <msnyder@vmware.com> wrote:
> Pedro Alves wrote:
>>
>> On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:
>>
>>>>> +      /* Display instruction number for last instruction in the log.
>>>>>  */
>>>>> +      printf_filtered (_("Highest recorded instruction number is
>>>>> %llu.\n"), +                      record_insn_count ? record_insn_count - 1
>>>>> : 0);
>>>>
>>>> Why the conditional subtraction?
>>>
>>> Because I don't want it to say "-1".
>>
>> Okay, that much is obvious, but how can you reach here
>> with record_insn_count == 0, given that you check if you
>> have a log at all a bit above?
>
> Maybe not -- but I'm a belt-and-suspenders guy.
> I don't believe in not checking for something just because
> it "can't happen".  What if somebody changed the check above?
>
> [...]
>>
>> Can't we explicitly say that we're in recording/live vs replay
>> mode, or something?
>
> Yeah, you've convinced me.  Thanks for the prodding,
> and please see new revision.  I'll make that conditional
> go away too.   ;-)
>
>
> 2009-10-12  Michael Snyder  <msnyder@vmware.com>
>
>        Elaborate "info record".
>        * record.c (struct record_end_entry): New field 'insn_num'.
>        (record_insn_count): New variable.
>        (record_open): Initialize record_insn_count.
>        (info_record_command): Display contents of record log as
>        lowest, current, and highest instruction counts.
>        (show_record_insn_number): Delete.
>        (_initialize_record): Remove add_cmd show_record_insn_number.
>
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.27
> diff -u -p -b -w -r1.27 record.c
> --- record.c    20 Oct 2009 23:06:13 -0000      1.27
> +++ record.c    21 Oct 2009 01:33:30 -0000
> @@ -91,6 +91,7 @@ struct record_reg_entry
>  struct record_end_entry
>  {
>   enum target_signal sigval;
> +  ULONGEST insn_num;
>  };
>
>  enum record_type
> @@ -166,8 +167,13 @@ static struct record_entry *record_arch_
>
>  /* 1 ask user. 0 auto delete the last struct record_entry.  */
>  static int record_stop_at_limit = 1;
> +/* Maximum allowed number of insns in execution log.  */
>  static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
> +/* Actual count of insns presently in execution log.  */
>  static int record_insn_num = 0;
> +/* Count of insns logged so far (may be larger
> +   than count of insns presently in execution log).  */
> +static ULONGEST record_insn_count;
>
>  /* The target_ops of process record.  */
>  static struct target_ops record_ops;
> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>     {
>       rec = tmp->next;
>       if (record_entry_release (tmp) == record_end)
> +       {
>        record_insn_num--;
> +         record_insn_count--;
> +       }
>       tmp = rec;
>     }
>  }
> @@ -367,7 +376,6 @@ record_list_release_first (void)
>       /* tmp is now isolated, and can be deleted.  */
>       if (record_entry_release (tmp) == record_end)
>        {
> -         record_insn_num--;
>          break;        /* End loop at first record_end.  */
>        }
>
> @@ -494,6 +502,7 @@ record_arch_list_add_end (void)
>
>   rec = record_end_alloc ();
>   rec->u.end.sigval = TARGET_SIGNAL_0;
> +  rec->u.end.insn_num = ++record_insn_count;
>
>   record_arch_list_add (rec);
>
> @@ -800,6 +809,7 @@ record_open (char *name, int from_tty)
>
>   /* Reset */
>   record_insn_num = 0;
> +  record_insn_count = 0;
>   record_list = &record_first;
>   record_list->next = NULL;
>  }
> @@ -1458,8 +1468,8 @@ cmd_record_stop (char *args, int from_tt
>   if (current_target.to_stratum == record_stratum)
>     {
>       unpush_target (&record_ops);
> -      printf_unfiltered (_("Process record is stoped and all execution "
> -                           "log is deleted.\n"));
> +      printf_unfiltered (_("Process record is stopped and all execution "
> +                           "logs are deleted.\n"));
>     }
>   else
>     printf_unfiltered (_("Process record is not started.\n"));
> @@ -1481,16 +1491,6 @@ set_record_insn_max_num (char *args, int
>     }
>  }
>
> -/* Print the current index into the record log (number of insns recorded
> -   so far).  */
> -
> -static void
> -show_record_insn_number (char *ignore, int from_tty)
> -{
> -  printf_unfiltered (_("Record instruction number is %d.\n"),
> -                    record_insn_num);
> -}
> -
>  static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
>                               *show_record_cmdlist, *info_record_cmdlist;
>
> @@ -1508,10 +1508,59 @@ show_record_command (char *args, int fro
>   cmd_show_list (show_record_cmdlist, from_tty, "");
>  }
>
> +/* Display some statistics about the execution log.  */
> +
>  static void
>  info_record_command (char *args, int from_tty)
>  {
> -  cmd_show_list (info_record_cmdlist, from_tty, "");
> +  struct record_entry *p;
> +
> +  if (current_target.to_stratum == record_stratum)
> +    {
> +      if (RECORD_IS_REPLAY)
> +       printf_filtered (_("Replay mode:\n"));
> +      else
> +       printf_filtered (_("Record mode:\n"));
> +
> +      /* Find entry for first actual instruction in the log.  */
> +      for (p = record_first.next;
> +          p != NULL && p->type != record_end;
> +          p = p->next)
> +       ;
> +
> +      /* Do we have a log at all?  */
> +      if (p != NULL && p->type == record_end)
> +       {
> +         /* Display instruction number for first instruction in the log.
>  */
> +         printf_filtered (_("Lowest recorded instruction number is %s.\n"),
> +                          pulongest (p->u.end.insn_num));
> +
> +         /* If in replay mode, display where we are in the log.  */
> +         if (RECORD_IS_REPLAY)
> +           printf_filtered (_("Current instruction number is %s.\n"),
> +                            pulongest (record_list->u.end.insn_num));
> +
> +         /* Display instruction number for last instruction in the log.  */
> +         printf_filtered (_("Highest recorded instruction number is
> %s.\n"),
> +                          pulongest (record_insn_count));
> +
> +         /* Display log count.  */
> +         printf_filtered (_("Log contains %d instructions.\n"),
> +                          record_insn_num);
> +       }
> +      else
> +       {
> +         printf_filtered (_("No instructions have been logged.\n"));
> +       }
> +    }
> +  else
> +    {
> +      printf_filtered (_("target record is not active.\n"));
> +    }
> +
> +  /* Display max log size.  */
> +  printf_filtered (_("Max logged instructions is %d.\n"),
> +                  record_insn_max_num);
>  }
>
>  void
> @@ -1581,7 +1630,4 @@ Set the maximum number of instructions t
>  record/replay buffer.  Zero means unlimited.  Default is 200000."),
>                            set_record_insn_max_num,
>                            NULL, &set_record_cmdlist, &show_record_cmdlist);
> -  add_cmd ("insn-number", class_obscure, show_record_insn_number,
> -          _("Show the current number of instructions in the "
> -            "record/replay buffer."), &info_record_cmdlist);
>  }
>
>

Re: [RFA] Expand "info record"

[Pedro Alves]

On Wednesday 21 October 2009 02:31:41, Michael Snyder wrote:
> Pedro Alves wrote:
> > On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:
> > 
> >>>> +      /* Display instruction number for last instruction in the log.  */
> >>>> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
> >>>> +                      record_insn_count ? record_insn_count - 1 : 0);
> >>> Why the conditional subtraction?
> >> Because I don't want it to say "-1".
> > 
> > Okay, that much is obvious, but how can you reach here
> > with record_insn_count == 0, given that you check if you
> > have a log at all a bit above?
> 
> Maybe not -- but I'm a belt-and-suspenders guy.
> I don't believe in not checking for something just because
> it "can't happen".  What if somebody changed the check above?

That's why we comment non-obvious code, or add
gdb_asserts (could be overkill), or warnings.  The worse that can
happen if somebody changes the check above, is the new bug is
quickly exposed because gdb starting printing '(ULONGEST) -1'.
Masking a buglet like that is wrong, IMO.

> Yeah, you've convinced me.  Thanks for the prodding,
> and please see new revision.  I'll make that conditional
> go away too.   ;-)

Thanks.  I could nit some more, but the all the info seems
there.  ;-)  I'm fine with the patch.  

I don't see "info record" mentioned in the docs.  Only:
 @kindex info record insn-number
 @item info record insn-number
 Show the current number of recorded instructions.

Was there a corresponding documentation patch?

FTR, it still pains me when I see this:
> +  if (current_target.to_stratum == record_stratum)
> +    {

especially more so now that we have a stratum higher
than record_statum...

-- 
Pedro Alves

Re: [RFA] Expand "info record"

[Michael Snyder]

Pedro Alves wrote:
> On Wednesday 21 October 2009 02:31:41, Michael Snyder wrote:
>> Pedro Alves wrote:
>>> On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:
>>>
>>>>>> +      /* Display instruction number for last instruction in the log.  */
>>>>>> +      printf_filtered (_("Highest recorded instruction number is %llu.\n"), 
>>>>>> +                      record_insn_count ? record_insn_count - 1 : 0);
>>>>> Why the conditional subtraction?
>>>> Because I don't want it to say "-1".
>>> Okay, that much is obvious, but how can you reach here
>>> with record_insn_count == 0, given that you check if you
>>> have a log at all a bit above?
>> Maybe not -- but I'm a belt-and-suspenders guy.
>> I don't believe in not checking for something just because
>> it "can't happen".  What if somebody changed the check above?
> 
> That's why we comment non-obvious code, or add
> gdb_asserts (could be overkill), or warnings.  The worse that can
> happen if somebody changes the check above, is the new bug is
> quickly exposed because gdb starting printing '(ULONGEST) -1'.
> Masking a buglet like that is wrong, IMO.
> 
>> Yeah, you've convinced me.  Thanks for the prodding,
>> and please see new revision.  I'll make that conditional
>> go away too.   ;-)
> 
> Thanks.  I could nit some more, but the all the info seems
> there.  ;-)  I'm fine with the patch.  
> 
> I don't see "info record" mentioned in the docs.  Only:
>  @kindex info record insn-number
>  @item info record insn-number
>  Show the current number of recorded instructions.
> 
> Was there a corresponding documentation patch?
> 
> FTR, it still pains me when I see this:
>> +  if (current_target.to_stratum == record_stratum)
>> +    {
> 
> especially more so now that we have a stratum higher
> than record_statum...

We do?  ...

OK -- how would you suggest doing this sort of test?

I'll get right to work on the docs patch, thanks for reminding me.

Re: [RFA] Expand "info record"

[Michael Snyder]

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

Hui Zhu wrote:
> Thanks Michael.
> 
> With change:
>  static struct target_ops record_ops;
> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>      {
>        rec = tmp->next;
>        if (record_entry_release (tmp) == record_end)
> +	{
>  	record_insn_num--;
> +	  record_insn_count--;
> +	}
>        tmp = rec;
>      }
>  }
> @@ -367,7 +376,6 @@ record_list_release_first (void)
>        /* tmp is now isolated, and can be deleted.  */
>        if (record_entry_release (tmp) == record_end)
>  	{
> -	  record_insn_num--;
>  	  break;	/* End loop at first record_end.  */
>  	}
> To:
>  static struct target_ops record_ops;
> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>      {
>        rec = tmp->next;
>        if (record_entry_release (tmp) == record_end)
> +	{
>  	  record_insn_num--;
> +	  record_insn_count--;
> +	}
>        tmp = rec;
>      }
>  }
> @@ -367,7 +376,6 @@ record_list_release_first (void)
>        /* tmp is now isolated, and can be deleted.  */
>        if (record_entry_release (tmp) == record_end)
> - 	{
> -	  record_insn_num--;
>  	  break;	/* End loop at first record_end.  */
> - 	}
> 
> Just some format change.
> 
> I am OK with this patch.

Thanks, changes attached and committed.


[-- Attachment #2: info5.txt --]
[-- Type: text/plain, Size: 5542 bytes --]

2009-10-12  Michael Snyder  <msnyder@vmware.com>

	Elaborate "info record".
	* record.c (struct record_end_entry): New field 'insn_num'.
	(record_insn_count): New variable.
	(record_open): Initialize record_insn_count.
	(info_record_command): Display contents of record log as
	lowest, current, and highest instruction counts.
	(show_record_insn_number): Delete.
	(_initialize_record): Remove add_cmd show_record_insn_number.

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.27
diff -u -p -r1.27 record.c
--- record.c	20 Oct 2009 23:06:13 -0000	1.27
+++ record.c	21 Oct 2009 15:16:47 -0000
@@ -91,6 +91,7 @@ struct record_reg_entry
 struct record_end_entry
 {
   enum target_signal sigval;
+  ULONGEST insn_num;
 };
 
 enum record_type
@@ -166,8 +167,13 @@ static struct record_entry *record_arch_
 
 /* 1 ask user. 0 auto delete the last struct record_entry.  */
 static int record_stop_at_limit = 1;
+/* Maximum allowed number of insns in execution log.  */
 static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
+/* Actual count of insns presently in execution log.  */
 static int record_insn_num = 0;
+/* Count of insns logged so far (may be larger
+   than count of insns presently in execution log).  */
+static ULONGEST record_insn_count;
 
 /* The target_ops of process record.  */
 static struct target_ops record_ops;
@@ -338,7 +344,10 @@ record_list_release_following (struct re
     {
       rec = tmp->next;
       if (record_entry_release (tmp) == record_end)
-	record_insn_num--;
+	{
+	  record_insn_num--;
+	  record_insn_count--;
+	}
       tmp = rec;
     }
 }
@@ -366,10 +375,7 @@ record_list_release_first (void)
 
       /* tmp is now isolated, and can be deleted.  */
       if (record_entry_release (tmp) == record_end)
-	{
-	  record_insn_num--;
-	  break;	/* End loop at first record_end.  */
-	}
+	break;	/* End loop at first record_end.  */
 
       if (!record_first.next)
 	{
@@ -494,6 +500,7 @@ record_arch_list_add_end (void)
 
   rec = record_end_alloc ();
   rec->u.end.sigval = TARGET_SIGNAL_0;
+  rec->u.end.insn_num = ++record_insn_count;
 
   record_arch_list_add (rec);
 
@@ -800,6 +807,7 @@ record_open (char *name, int from_tty)
 
   /* Reset */
   record_insn_num = 0;
+  record_insn_count = 0;
   record_list = &record_first;
   record_list->next = NULL;
 }
@@ -1458,8 +1466,8 @@ cmd_record_stop (char *args, int from_tt
   if (current_target.to_stratum == record_stratum)
     {
       unpush_target (&record_ops);
-      printf_unfiltered (_("Process record is stoped and all execution "
-                           "log is deleted.\n"));
+      printf_unfiltered (_("Process record is stopped and all execution "
+                           "logs are deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));
@@ -1481,16 +1489,6 @@ set_record_insn_max_num (char *args, int
     }
 }
 
-/* Print the current index into the record log (number of insns recorded
-   so far).  */
-
-static void
-show_record_insn_number (char *ignore, int from_tty)
-{
-  printf_unfiltered (_("Record instruction number is %d.\n"),
-		     record_insn_num);
-}
-
 static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
 			       *show_record_cmdlist, *info_record_cmdlist;
 
@@ -1508,10 +1506,59 @@ show_record_command (char *args, int fro
   cmd_show_list (show_record_cmdlist, from_tty, "");
 }
 
+/* Display some statistics about the execution log.  */
+
 static void
 info_record_command (char *args, int from_tty)
 {
-  cmd_show_list (info_record_cmdlist, from_tty, "");
+  struct record_entry *p;
+
+  if (current_target.to_stratum == record_stratum)
+    {
+      if (RECORD_IS_REPLAY)
+	printf_filtered (_("Replay mode:\n"));
+      else
+	printf_filtered (_("Record mode:\n"));
+
+      /* Find entry for first actual instruction in the log.  */
+      for (p = record_first.next;
+	   p != NULL && p->type != record_end;
+	   p = p->next)
+	;
+
+      /* Do we have a log at all?  */
+      if (p != NULL && p->type == record_end)
+	{
+	  /* Display instruction number for first instruction in the log.  */
+	  printf_filtered (_("Lowest recorded instruction number is %s.\n"),
+			   pulongest (p->u.end.insn_num));
+
+	  /* If in replay mode, display where we are in the log.  */
+	  if (RECORD_IS_REPLAY)
+	    printf_filtered (_("Current instruction number is %s.\n"),
+			     pulongest (record_list->u.end.insn_num));
+
+	  /* Display instruction number for last instruction in the log.  */
+	  printf_filtered (_("Highest recorded instruction number is %s.\n"), 
+			   pulongest (record_insn_count));
+
+	  /* Display log count.  */
+	  printf_filtered (_("Log contains %d instructions.\n"), 
+			   record_insn_num);
+	}
+      else
+	{
+	  printf_filtered (_("No instructions have been logged.\n"));
+	}
+    }
+  else
+    {
+      printf_filtered (_("target record is not active.\n"));
+    }
+
+  /* Display max log size.  */
+  printf_filtered (_("Max logged instructions is %d.\n"),
+		   record_insn_max_num);
 }
 
 void
@@ -1581,7 +1628,4 @@ Set the maximum number of instructions t
 record/replay buffer.  Zero means unlimited.  Default is 200000."),
 			    set_record_insn_max_num,
 			    NULL, &set_record_cmdlist, &show_record_cmdlist);
-  add_cmd ("insn-number", class_obscure, show_record_insn_number,
-	   _("Show the current number of instructions in the "
-	     "record/replay buffer."), &info_record_cmdlist);
 }

Re: [RFA] Expand "info record"

[Hui Zhu]

Thanks for your works,  Michael.

Hui

On Wed, Oct 21, 2009 at 23:17, Michael Snyder <msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Thanks Michael.
>>
>> With change:
>>  static struct target_ops record_ops;
>> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>>     {
>>       rec = tmp->next;
>>       if (record_entry_release (tmp) == record_end)
>> +       {
>>        record_insn_num--;
>> +         record_insn_count--;
>> +       }
>>       tmp = rec;
>>     }
>>  }
>> @@ -367,7 +376,6 @@ record_list_release_first (void)
>>       /* tmp is now isolated, and can be deleted.  */
>>       if (record_entry_release (tmp) == record_end)
>>        {
>> -         record_insn_num--;
>>          break;        /* End loop at first record_end.  */
>>        }
>> To:
>>  static struct target_ops record_ops;
>> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>>     {
>>       rec = tmp->next;
>>       if (record_entry_release (tmp) == record_end)
>> +       {
>>          record_insn_num--;
>> +         record_insn_count--;
>> +       }
>>       tmp = rec;
>>     }
>>  }
>> @@ -367,7 +376,6 @@ record_list_release_first (void)
>>       /* tmp is now isolated, and can be deleted.  */
>>       if (record_entry_release (tmp) == record_end)
>> -       {
>> -         record_insn_num--;
>>          break;        /* End loop at first record_end.  */
>> -       }
>>
>> Just some format change.
>>
>> I am OK with this patch.
>
> Thanks, changes attached and committed.
>
>
> 2009-10-12  Michael Snyder  <msnyder@vmware.com>
>
>        Elaborate "info record".
>        * record.c (struct record_end_entry): New field 'insn_num'.
>        (record_insn_count): New variable.
>        (record_open): Initialize record_insn_count.
>        (info_record_command): Display contents of record log as
>        lowest, current, and highest instruction counts.
>        (show_record_insn_number): Delete.
>        (_initialize_record): Remove add_cmd show_record_insn_number.
>
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 record.c
> --- record.c    20 Oct 2009 23:06:13 -0000      1.27
> +++ record.c    21 Oct 2009 15:16:47 -0000
> @@ -91,6 +91,7 @@ struct record_reg_entry
>  struct record_end_entry
>  {
>   enum target_signal sigval;
> +  ULONGEST insn_num;
>  };
>
>  enum record_type
> @@ -166,8 +167,13 @@ static struct record_entry *record_arch_
>
>  /* 1 ask user. 0 auto delete the last struct record_entry.  */
>  static int record_stop_at_limit = 1;
> +/* Maximum allowed number of insns in execution log.  */
>  static unsigned int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
> +/* Actual count of insns presently in execution log.  */
>  static int record_insn_num = 0;
> +/* Count of insns logged so far (may be larger
> +   than count of insns presently in execution log).  */
> +static ULONGEST record_insn_count;
>
>  /* The target_ops of process record.  */
>  static struct target_ops record_ops;
> @@ -338,7 +344,10 @@ record_list_release_following (struct re
>     {
>       rec = tmp->next;
>       if (record_entry_release (tmp) == record_end)
> -       record_insn_num--;
> +       {
> +         record_insn_num--;
> +         record_insn_count--;
> +       }
>       tmp = rec;
>     }
>  }
> @@ -366,10 +375,7 @@ record_list_release_first (void)
>
>       /* tmp is now isolated, and can be deleted.  */
>       if (record_entry_release (tmp) == record_end)
> -       {
> -         record_insn_num--;
> -         break;        /* End loop at first record_end.  */
> -       }
> +       break;  /* End loop at first record_end.  */
>
>       if (!record_first.next)
>        {
> @@ -494,6 +500,7 @@ record_arch_list_add_end (void)
>
>   rec = record_end_alloc ();
>   rec->u.end.sigval = TARGET_SIGNAL_0;
> +  rec->u.end.insn_num = ++record_insn_count;
>
>   record_arch_list_add (rec);
>
> @@ -800,6 +807,7 @@ record_open (char *name, int from_tty)
>
>   /* Reset */
>   record_insn_num = 0;
> +  record_insn_count = 0;
>   record_list = &record_first;
>   record_list->next = NULL;
>  }
> @@ -1458,8 +1466,8 @@ cmd_record_stop (char *args, int from_tt
>   if (current_target.to_stratum == record_stratum)
>     {
>       unpush_target (&record_ops);
> -      printf_unfiltered (_("Process record is stoped and all execution "
> -                           "log is deleted.\n"));
> +      printf_unfiltered (_("Process record is stopped and all execution "
> +                           "logs are deleted.\n"));
>     }
>   else
>     printf_unfiltered (_("Process record is not started.\n"));
> @@ -1481,16 +1489,6 @@ set_record_insn_max_num (char *args, int
>     }
>  }
>
> -/* Print the current index into the record log (number of insns recorded
> -   so far).  */
> -
> -static void
> -show_record_insn_number (char *ignore, int from_tty)
> -{
> -  printf_unfiltered (_("Record instruction number is %d.\n"),
> -                    record_insn_num);
> -}
> -
>  static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
>                               *show_record_cmdlist, *info_record_cmdlist;
>
> @@ -1508,10 +1506,59 @@ show_record_command (char *args, int fro
>   cmd_show_list (show_record_cmdlist, from_tty, "");
>  }
>
> +/* Display some statistics about the execution log.  */
> +
>  static void
>  info_record_command (char *args, int from_tty)
>  {
> -  cmd_show_list (info_record_cmdlist, from_tty, "");
> +  struct record_entry *p;
> +
> +  if (current_target.to_stratum == record_stratum)
> +    {
> +      if (RECORD_IS_REPLAY)
> +       printf_filtered (_("Replay mode:\n"));
> +      else
> +       printf_filtered (_("Record mode:\n"));
> +
> +      /* Find entry for first actual instruction in the log.  */
> +      for (p = record_first.next;
> +          p != NULL && p->type != record_end;
> +          p = p->next)
> +       ;
> +
> +      /* Do we have a log at all?  */
> +      if (p != NULL && p->type == record_end)
> +       {
> +         /* Display instruction number for first instruction in the log.
>  */
> +         printf_filtered (_("Lowest recorded instruction number is %s.\n"),
> +                          pulongest (p->u.end.insn_num));
> +
> +         /* If in replay mode, display where we are in the log.  */
> +         if (RECORD_IS_REPLAY)
> +           printf_filtered (_("Current instruction number is %s.\n"),
> +                            pulongest (record_list->u.end.insn_num));
> +
> +         /* Display instruction number for last instruction in the log.  */
> +         printf_filtered (_("Highest recorded instruction number is
> %s.\n"),
> +                          pulongest (record_insn_count));
> +
> +         /* Display log count.  */
> +         printf_filtered (_("Log contains %d instructions.\n"),
> +                          record_insn_num);
> +       }
> +      else
> +       {
> +         printf_filtered (_("No instructions have been logged.\n"));
> +       }
> +    }
> +  else
> +    {
> +      printf_filtered (_("target record is not active.\n"));
> +    }
> +
> +  /* Display max log size.  */
> +  printf_filtered (_("Max logged instructions is %d.\n"),
> +                  record_insn_max_num);
>  }
>
>  void
> @@ -1581,7 +1628,4 @@ Set the maximum number of instructions t
>  record/replay buffer.  Zero means unlimited.  Default is 200000."),
>                            set_record_insn_max_num,
>                            NULL, &set_record_cmdlist, &show_record_cmdlist);
> -  add_cmd ("insn-number", class_obscure, show_record_insn_number,
> -          _("Show the current number of instructions in the "
> -            "record/replay buffer."), &info_record_cmdlist);
>  }
>
>

Re: [RFA] Expand "info record"

[Pedro Alves]

On Wednesday 21 October 2009 16:17:08, Michael Snyder wrote:

> > FTR, it still pains me when I see this:
> >> +  if (current_target.to_stratum == record_stratum)
> >> +    {
> > 
> > especially more so now that we have a stratum higher
> > than record_statum...
> 
> We do?  ...
> 
> OK -- how would you suggest doing this sort of test?

In common code, test for target feature or property of
interest to the caller code in question (target_has_execution,
_has_stack, _has_foo), instead of checking if the current
target is target foo or statum foo.

In record.c itself, iff you want to know if precord is
activated, either iterate over the pushed targets
stack looking for &record_ops:

static int
precord_active (void)
{
  foreach (ops, current_target.beneath)
    if (ops == &record_ops)
     return 1;
  return 0;
}

... or, simply keep a global similar to exec.c:using_exec_ops,
and check for that.

(If you want to consider multi-process at some point,
you'll probably move the log-related globals to a
per-inferior structure, and presumably keep a list of
such objects around in record.c.  Then, in record.c, when
checking if recording is activated for a given inferior,
you'd then check if there's an instance of such an object
for the current inferior in that list.)

-- 
Pedro Alves