[PATCH] Add method/format information to =record-started

[PATCH] Add method/format information to =record-started

[Simon Marchi]

Eclipse CDT now supports enabling execution recording using two methods
(full and btrace) and both formats for btrace (bts and pt).  In the
event that recording is enabled behind the back of the GUI (by the user
on the command line, or a script), we need to know which method/format
are being used, so it can be correctly reflected in the interface.  This
patch adds this information to the =record-started async record.

Before:

  =record-started,thread-group="i1"

After:

  =record-started,thread-group="i1",method="btrace",format="bts"
  =record-started,thread-group="i1",method="btrace",format="pt"
  =record-started,thread-group="i1",method="full"

The "format" field is only present when the current method supports
multiple formats (only the btrace method as of now).

gdb/ChangeLog:

	* NEWS: Mention the new fields in =record-started.
	* mi/mi-interp.c (mi_record_changed): Output method and format
	fields in the =record-started record.
	* record-btrace.c (record_btrace_open): Adapt record_changed
	notification.
	* record-full.c (record_full_open): Likewise.
	* record.c (cmd_record_stop): Likewise.

gdb/doc/ChangeLog:

	* gdb.texinfo (GDB/MI Async Records): Document method and
	format fields in =record-started.
	* observer.texi (record_changed): Add method and format
	parameters.

gdb/testsuite/ChangeLog:

	* gdb.mi/mi-record-changed.exp: Adjust =record-started output
	matching.
---
 gdb/NEWS                                   |  5 +++++
 gdb/doc/gdb.texinfo                        |  7 ++++++-
 gdb/doc/observer.texi                      |  7 ++++++-
 gdb/mi/mi-interp.c                         | 32 ++++++++++++++++++++++++++----
 gdb/record-btrace.c                        |  4 +++-
 gdb/record-full.c                          |  2 +-
 gdb/record.c                               |  2 +-
 gdb/testsuite/gdb.mi/mi-record-changed.exp |  4 ++--
 8 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index dce79a2..5d59bc7 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -67,6 +67,11 @@ maint selftest
   including JIT compiling fast tracepoint's conditional expression
   bytecode into native code.
 
+* MI async record =record-started new includes the method and format used for
+  recording.  For example:
+
+    =record-started,thread-group="i1",method="btrace",format="bts"
+
 *** Changes in GDB 7.11
 
 * GDB now supports debugging kernel-based threads on FreeBSD.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7e89003..5739d05 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26423,12 +26423,17 @@ breakpoint commands; @xref{GDB/MI Breakpoint Commands}.  The
 Note that if a breakpoint is emitted in the result record of a
 command, then it will not also be emitted in an async record.
 
-@item =record-started,thread-group="@var{id}"
+@item =record-started,thread-group="@var{id}",method="@var{method}"[,format="@var{format}"]
 @itemx =record-stopped,thread-group="@var{id}"
 Execution log recording was either started or stopped on an
 inferior.  The @var{id} is the @value{GDBN} identifier of the thread
 group corresponding to the affected inferior.
 
+The @var{method} field contains the method used to record execution.  If the
+method in use supports multiple recording formats, @var{format} will be present
+and contain the currently used format.  See @xref{Process Record and Replay}
+for existing method and format values.
+
 @item =cmd-param-changed,param=@var{param},value=@var{value}
 Reports that a parameter of the command @code{set @var{param}} is
 changed to @var{value}.  In the multi-word @code{set} command,
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index f4049ad..fc7aac4 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -141,11 +141,16 @@ at the entry-point instruction.  For @samp{attach} and @samp{core},
 inferior, and before any information on the inferior has been printed.
 @end deftypefun
 
-@deftypefun void record_changed (struct inferior *@var{inferior}, int @var{started})
+@deftypefun void record_changed (struct inferior *@var{inferior}, int @var{started}, const char *@var{method}, const char *@var{format})
 The status of process record for inferior @var{inferior} in
 @value{GDBN} has changed.  The process record is started if
 @var{started} is true, and the process record is stopped if
 @var{started} is false.
+
+When @var{started} is true, @var{method} indicates the short name of the method
+used for recording.  If the method supports multiple formats, @var{format}
+indicates which one is being used, otherwise it is NULL.  When @var{started} is
+false, they are both NULL.
 @end deftypefun
 
 @deftypefun void solib_loaded (struct so_list *@var{solib})
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 0fe19af..09de9fe 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -65,7 +65,8 @@ static void mi_on_no_history (void);
 
 static void mi_new_thread (struct thread_info *t);
 static void mi_thread_exit (struct thread_info *t, int silent);
-static void mi_record_changed (struct inferior*, int);
+static void mi_record_changed (struct inferior*, int, const char *,
+			       const char *);
 static void mi_inferior_added (struct inferior *inf);
 static void mi_inferior_appeared (struct inferior *inf);
 static void mi_inferior_exit (struct inferior *inf);
@@ -399,7 +400,8 @@ mi_thread_exit (struct thread_info *t, int silent)
 /* Emit notification on changing the state of record.  */
 
 static void
-mi_record_changed (struct inferior *inferior, int started)
+mi_record_changed (struct inferior *inferior, int started, const char *method,
+		   const char *format)
 {
   struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
   struct cleanup *old_chain;
@@ -407,8 +409,30 @@ mi_record_changed (struct inferior *inferior, int started)
   old_chain = make_cleanup_restore_target_terminal ();
   target_terminal_ours_for_output ();
 
-  fprintf_unfiltered (mi->event_channel,  "record-%s,thread-group=\"i%d\"",
-		      started ? "started" : "stopped", inferior->num);
+  if (started)
+    {
+      if (format != NULL)
+	{
+	  fprintf_unfiltered (
+	    mi->event_channel,
+	    "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
+	    inferior->num, method, format);
+	}
+      else
+	{
+
+	  fprintf_unfiltered (
+	    mi->event_channel,
+	    "record-started,thread-group=\"i%d\",method=\"%s\"",
+	    inferior->num, method);
+	}
+    }
+  else
+    {
+      fprintf_unfiltered (mi->event_channel,
+			  "record-stopped,thread-group=\"i%d\"", inferior->num);
+    }
+
 
   gdb_flush (mi->event_channel);
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 77b5180..a1df75a 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -206,6 +206,7 @@ record_btrace_open (const char *args, int from_tty)
 {
   struct cleanup *disable_chain;
   struct thread_info *tp;
+  const char *format;
 
   DEBUG ("open");
 
@@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
 				  NULL);
   record_btrace_generating_corefile = 0;
 
-  observer_notify_record_changed (current_inferior (),  1);
+  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
+  observer_notify_record_changed (current_inferior (), 1, "btrace", format);
 
   discard_cleanups (disable_chain);
 }
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f307b48..0f61bcb 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -869,7 +869,7 @@ record_full_open (const char *name, int from_tty)
 
   record_full_init_record_breakpoints ();
 
-  observer_notify_record_changed (current_inferior (),  1);
+  observer_notify_record_changed (current_inferior (),  1, "full", NULL);
 }
 
 /* "to_close" target method.  Close the process record target.  */
diff --git a/gdb/record.c b/gdb/record.c
index 6190794..1af134f 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -268,7 +268,7 @@ cmd_record_stop (char *args, int from_tty)
   printf_unfiltered (_("Process record is stopped and all execution "
 		       "logs are deleted.\n"));
 
-  observer_notify_record_changed (current_inferior (), 0);
+  observer_notify_record_changed (current_inferior (), 0, NULL, NULL);
 }
 
 /* The "set record" command.  */
diff --git a/gdb/testsuite/gdb.mi/mi-record-changed.exp b/gdb/testsuite/gdb.mi/mi-record-changed.exp
index 2b5fcd6..22cf076 100644
--- a/gdb/testsuite/gdb.mi/mi-record-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-record-changed.exp
@@ -31,14 +31,14 @@ if [mi_gdb_start] {
 }
 mi_run_to_main
 
-mi_gdb_test "record" ".*=record-started,thread-group=\"i${decimal}\".*\\^done" \
+mi_gdb_test "record" ".*=record-started,thread-group=\"i${decimal}\",method=\"full\".*\\^done" \
     "record"
 mi_gdb_test "record stop" \
     ".*=record-stopped,thread-group=\"i${decimal}\".*\\^done" \
     "record end"
 
 mi_gdb_test "target record" \
-    ".*=record-started,thread-group=\"i${decimal}\".*\\^done" \
+    ".*=record-started,thread-group=\"i${decimal}\",method=\"full\".*\\^done" \
     "target record"
 
 return 0
-- 
2.8.3

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-03 11:52 AM, Simon Marchi wrote:
> diff --git a/gdb/NEWS b/gdb/NEWS
> index dce79a2..5d59bc7 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -67,6 +67,11 @@ maint selftest
>    including JIT compiling fast tracepoint's conditional expression
>    bytecode into native code.
>  
> +* MI async record =record-started new includes the method and format used for

Oops, typo here.  new -> now.

RE: [PATCH] Add method/format information to =record-started

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Simon Marchi
> Sent: Friday, June 3, 2016 5:52 PM
> To: gdb-patches@sourceware.org
> Cc: Simon Marchi <simon.marchi@ericsson.com>
> Subject: [PATCH] Add method/format information to =record-started
> 
> Eclipse CDT now supports enabling execution recording using two methods
> (full and btrace) and both formats for btrace (bts and pt).  In the
> event that recording is enabled behind the back of the GUI (by the user
> on the command line, or a script), we need to know which method/format
> are being used, so it can be correctly reflected in the interface.  This
> patch adds this information to the =record-started async record.
> 
> Before:
> 
>   =record-started,thread-group="i1"
> 
> After:
> 
>   =record-started,thread-group="i1",method="btrace",format="bts"
>   =record-started,thread-group="i1",method="btrace",format="pt"
>   =record-started,thread-group="i1",method="full"
> 
> The "format" field is only present when the current method supports
> multiple formats (only the btrace method as of now).

Thanks for adding this.


> @@ -407,8 +409,30 @@ mi_record_changed (struct inferior *inferior, int started)
>    old_chain = make_cleanup_restore_target_terminal ();
>    target_terminal_ours_for_output ();
> 
> -  fprintf_unfiltered (mi->event_channel,  "record-%s,thread-group=\"i%d\"",
> -		      started ? "started" : "stopped", inferior->num);
> +  if (started)
> +    {
> +      if (format != NULL)
> +	{

Do we really need braces, here...

> +	  fprintf_unfiltered (
> +	    mi->event_channel,
> +	    "record-started,thread-
> group=\"i%d\",method=\"%s\",format=\"%s\"",
> +	    inferior->num, method, format);
> +	}
> +      else
> +	{
> +

...and here...

> +	  fprintf_unfiltered (
> +	    mi->event_channel,
> +	    "record-started,thread-group=\"i%d\",method=\"%s\"",
> +	    inferior->num, method);
> +	}
> +    }
> +  else
> +    {

...and here?

> +      fprintf_unfiltered (mi->event_channel,
> +			  "record-stopped,thread-group=\"i%d\"", inferior-
> >num);
> +    }
> +


> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
>  				  NULL);
>    record_btrace_generating_corefile = 0;
> 
> -  observer_notify_record_changed (current_inferior (),  1);
> +  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";

I'd use a switch here and not assume that format != pt means bts.  If we added
another format (LBR maybe?) we might miss this.


Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add method/format information to =record-started

[Yao Qi]

"Metzger, Markus T" <markus.t.metzger@intel.com> writes:

Hi Markus,
Thanks for reviewing the patch....

>> +      if (format != NULL)
>> +	{
>
> Do we really need braces, here...
>

Yes, two or more lines in code should be wrapped in braces.
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards

>> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
>>  				  NULL);
>>    record_btrace_generating_corefile = 0;
>> 
>> -  observer_notify_record_changed (current_inferior (),  1);
>> +  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
>
> I'd use a switch here and not assume that format != pt means bts.  If we added
> another format (LBR maybe?) we might miss this.

I agree, otherwise, the patch is good to me.

-- 
Yao (齐尧)

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 09:03 AM, Yao Qi wrote:
> "Metzger, Markus T" <markus.t.metzger@intel.com> writes:
> 
> Hi Markus,
> Thanks for reviewing the patch....
> 
>>> +      if (format != NULL)
>>> +	{
>>
>> Do we really need braces, here...
>>
> 
> Yes, two or more lines in code should be wrapped in braces.
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards

Just to be clear, the inner if/else doesn't need them, but the outer one does.

Here's the result:


  if (started)
    {
      if (format != NULL)
	fprintf_unfiltered (
	  mi->event_channel,
	  "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
	  inferior->num, method, format);
      else
	fprintf_unfiltered (
	  mi->event_channel,
	  "record-started,thread-group=\"i%d\",method=\"%s\"",
	  inferior->num, method);
    }
  else
    fprintf_unfiltered (mi->event_channel,
			"record-stopped,thread-group=\"i%d\"", inferior->num);

Re: [PATCH] Add method/format information to =record-started

[Yao Qi]

Simon Marchi <simon.marchi@ericsson.com> writes:

> Just to be clear, the inner if/else doesn't need them, but the outer one does.
>

No, it is not "GDB C Coding Standard" compliant, IMO.  In the quoted url
I gave,

"Any two or more lines in code should be wrapped in braces, even if they
are comments, as they look like separate statements:"

so...

> Here's the result:
>
>
>   if (started)
>     {
>       if (format != NULL)

brace is needed here...

> 	fprintf_unfiltered (
> 	  mi->event_channel,
> 	  "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
> 	  inferior->num, method, format);

and here

>       else
> 	fprintf_unfiltered (
> 	  mi->event_channel,
> 	  "record-started,thread-group=\"i%d\",method=\"%s\"",
> 	  inferior->num, method);
>     }
>   else
>     fprintf_unfiltered (mi->event_channel,
> 			"record-stopped,thread-group=\"i%d\"", inferior->num);

-- 
Yao (齐尧)

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-07 05:32 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> Just to be clear, the inner if/else doesn't need them, but the outer one does.
>>
> 
> No, it is not "GDB C Coding Standard" compliant, IMO.  In the quoted url
> I gave,
> 
> "Any two or more lines in code should be wrapped in braces, even if they
> are comments, as they look like separate statements:"

Ahh ok, I thought braces were only needed in the case where you added a comment.

> so...
> 
>> Here's the result:
>>
>>
>>   if (started)
>>     {
>>       if (format != NULL)
> 
> brace is needed here...
> 
>> 	fprintf_unfiltered (
>> 	  mi->event_channel,
>> 	  "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
>> 	  inferior->num, method, format);
> 
> and here
> 
>>       else
>> 	fprintf_unfiltered (
>> 	  mi->event_channel,
>> 	  "record-started,thread-group=\"i%d\",method=\"%s\"",
>> 	  inferior->num, method);
>>     }
>>   else
>>     fprintf_unfiltered (mi->event_channel,
>> 			"record-stopped,thread-group=\"i%d\"", inferior->num);

I suppose here too?

Re: [PATCH] Add method/format information to =record-started

[Yao Qi]

Simon Marchi <simon.marchi@ericsson.com> writes:

>>>   else
>>>     fprintf_unfiltered (mi->event_channel,
>>> 			"record-stopped,thread-group=\"i%d\"", inferior->num);
>
> I suppose here too?

Yes, I think so.

-- 
Yao (齐尧)

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-07 11:38 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>>>>   else
>>>>     fprintf_unfiltered (mi->event_channel,
>>>> 			"record-stopped,thread-group=\"i%d\"", inferior->num);
>>
>> I suppose here too?
> 
> Yes, I think so.

Ok, I pushed this patch to fix it:


From 1aec0b6ad6eae1fa97bb1a4a47959ff204aa15a2 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 7 Jun 2016 10:02:42 -0400
Subject: [PATCH] mi/mi-interp.c: Add missing braces

gdb/ChangeLog:

	* mi/mi-interp.c (mi_record_changed): Add missing braces.
---
 gdb/ChangeLog      |  4 ++++
 gdb/mi/mi-interp.c | 26 ++++++++++++++++----------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bd2d5d2..9c09269 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2016-06-07  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* mi/mi-interp.c (mi_record_changed): Add missing braces.
+
 2016-06-07  Bernhard Heckel  <bernhard.heckel@intel.com>

 	* findvar.c (follow_static_link): Check for valid pointer.
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 8ce816c..3bbdb1f 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -412,19 +412,25 @@ mi_record_changed (struct inferior *inferior, int started, const char *method,
   if (started)
     {
       if (format != NULL)
-	fprintf_unfiltered (
-	  mi->event_channel,
-	  "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
-	  inferior->num, method, format);
+	{
+	  fprintf_unfiltered (
+	    mi->event_channel,
+	    "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
+	    inferior->num, method, format);
+	}
       else
-	fprintf_unfiltered (
-	  mi->event_channel,
-	  "record-started,thread-group=\"i%d\",method=\"%s\"",
-	  inferior->num, method);
+	{
+	  fprintf_unfiltered (
+	    mi->event_channel,
+	    "record-started,thread-group=\"i%d\",method=\"%s\"",
+	    inferior->num, method);
+	}
     }
   else
-    fprintf_unfiltered (mi->event_channel,
-			"record-stopped,thread-group=\"i%d\"", inferior->num);
+    {
+      fprintf_unfiltered (mi->event_channel,
+			  "record-stopped,thread-group=\"i%d\"", inferior->num);
+    }


   gdb_flush (mi->event_channel);
-- 
2.8.3

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 02:37 AM, Metzger, Markus T wrote:
>> @@ -407,8 +409,30 @@ mi_record_changed (struct inferior *inferior, int started)
>>    old_chain = make_cleanup_restore_target_terminal ();
>>    target_terminal_ours_for_output ();
>>
>> -  fprintf_unfiltered (mi->event_channel,  "record-%s,thread-group=\"i%d\"",
>> -		      started ? "started" : "stopped", inferior->num);
>> +  if (started)
>> +    {
>> +      if (format != NULL)
>> +	{
> 
> Do we really need braces, here...

Right, thanks.

>> +	  fprintf_unfiltered (
>> +	    mi->event_channel,
>> +	    "record-started,thread-
>> group=\"i%d\",method=\"%s\",format=\"%s\"",
>> +	    inferior->num, method, format);
>> +	}
>> +      else
>> +	{
>> +
> 
> ...and here...

Same.

>> +	  fprintf_unfiltered (
>> +	    mi->event_channel,
>> +	    "record-started,thread-group=\"i%d\",method=\"%s\"",
>> +	    inferior->num, method);
>> +	}
>> +    }
>> +  else
>> +    {
> 
> ...and here?

Ditto.

>> +      fprintf_unfiltered (mi->event_channel,
>> +			  "record-stopped,thread-group=\"i%d\"", inferior-
>>> num);
>> +    }
>> +
> 
> 
>> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
>>  				  NULL);
>>    record_btrace_generating_corefile = 0;
>>
>> -  observer_notify_record_changed (current_inferior (),  1);
>> +  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
> 
> I'd use a switch here and not assume that format != pt means bts.  If we added
> another format (LBR maybe?) we might miss this.

You're right.  What do you think of adding a convenience function like this?


diff --git a/gdb/common/btrace-common.c b/gdb/common/btrace-common.c
index eba3979..ebf3ed2 100644
--- a/gdb/common/btrace-common.c
+++ b/gdb/common/btrace-common.c
@@ -43,6 +43,23 @@ btrace_format_string (enum btrace_format format)

 /* See btrace-common.h.  */

+const char *
+btrace_format_short_string (enum btrace_format format)
+{
+  switch (format)
+    {
+    case BTRACE_FORMAT_BTS:
+      return "bts";
+
+    case BTRACE_FORMAT_PT:
+      return "pt";
+    }
+
+  internal_error (__FILE__, __LINE__, _("Unknown branch trace format"));
+}
+
+/* See btrace-common.h.  */
+
 void
 btrace_data_init (struct btrace_data *data)
 {
diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
index ad208cd..ba707a2 100644
--- a/gdb/common/btrace-common.h
+++ b/gdb/common/btrace-common.h
@@ -214,6 +214,10 @@ enum btrace_error
 /* Return a string representation of FORMAT.  */
 extern const char *btrace_format_string (enum btrace_format format);

+/* Return a short abbreviation string of FORMAT.  FORMAT can have the value
+   BTRACE_FORMAT_NONE.  */
+extern const char *btrace_format_short_string (enum btrace_format format);
+
 /* Initialize DATA.  */
 extern void btrace_data_init (struct btrace_data *data);

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index a1df75a..24594a9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -235,7 +235,7 @@ record_btrace_open (const char *args, int from_tty)
 				  NULL);
   record_btrace_generating_corefile = 0;

-  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
+  format = btrace_format_short_string (record_btrace_conf.format);
   observer_notify_record_changed (current_inferior (), 1, "btrace", format);

   discard_cleanups (disable_chain);

RE: [PATCH] Add method/format information to =record-started

[Metzger, Markus T]

> -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@ericsson.com]
> Sent: Monday, June 6, 2016 3:07 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] Add method/format information to =record-started

Hi Simon,

Please see Yao's reply regarding formatting.

> >> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
> >>  				  NULL);
> >>    record_btrace_generating_corefile = 0;
> >>
> >> -  observer_notify_record_changed (current_inferior (),  1);
> >> +  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
> >
> > I'd use a switch here and not assume that format != pt means bts.  If we added
> > another format (LBR maybe?) we might miss this.
> 
> You're right.  What do you think of adding a convenience function like this?
> 
> 
> diff --git a/gdb/common/btrace-common.c b/gdb/common/btrace-common.c
> index eba3979..ebf3ed2 100644
> --- a/gdb/common/btrace-common.c
> +++ b/gdb/common/btrace-common.c
> @@ -43,6 +43,23 @@ btrace_format_string (enum btrace_format format)
> 
>  /* See btrace-common.h.  */
> 
> +const char *
> +btrace_format_short_string (enum btrace_format format)
> +{
> +  switch (format)
> +    {
> +    case BTRACE_FORMAT_BTS:
> +      return "bts";
> +
> +    case BTRACE_FORMAT_PT:
> +      return "pt";
> +    }
> +
> +  internal_error (__FILE__, __LINE__, _("Unknown branch trace format"));
> +}
> +
> +/* See btrace-common.h.  */
> +
>  void
>  btrace_data_init (struct btrace_data *data)
>  {
> diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
> index ad208cd..ba707a2 100644
> --- a/gdb/common/btrace-common.h
> +++ b/gdb/common/btrace-common.h
> @@ -214,6 +214,10 @@ enum btrace_error
>  /* Return a string representation of FORMAT.  */
>  extern const char *btrace_format_string (enum btrace_format format);
> 
> +/* Return a short abbreviation string of FORMAT.  FORMAT can have the value
> +   BTRACE_FORMAT_NONE.  */
> +extern const char *btrace_format_short_string (enum btrace_format format);

Looks good except that BTRACE_FORMAT_NONE is not handled in the switch.

We could leave it as a bad format string to be detected by the MI consumer.  This would
leave the short and long versions of btrace_format_string symmetric.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 09:20 AM, Metzger, Markus T wrote:
> Looks good except that BTRACE_FORMAT_NONE is not handled in the switch.
> 
> We could leave it as a bad format string to be detected by the MI consumer.  This would
> leave the short and long versions of btrace_format_string symmetric.

What do you mean by MI consumer?  The MI front-end (e.g. Eclipse), or the MI interpreter
code in gdb (mi-interp.c) ?

My thought was that not handling it in the switch (and let gdb implode) would help catch
a bug earlier, compared to emitting some invalid string.  But I don't really mind.  I can
add

    case BTRACE_FORMAT_NONE:
      return "unknown";

RE: [PATCH] Add method/format information to =record-started

[Metzger, Markus T]

> -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@ericsson.com]
> Sent: Monday, June 6, 2016 3:40 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] Add method/format information to =record-started
> 
> On 16-06-06 09:20 AM, Metzger, Markus T wrote:
> > Looks good except that BTRACE_FORMAT_NONE is not handled in the switch.
> >
> > We could leave it as a bad format string to be detected by the MI consumer.
> This would
> > leave the short and long versions of btrace_format_string symmetric.
> 
> What do you mean by MI consumer?  The MI front-end (e.g. Eclipse), or the MI
> interpreter
> code in gdb (mi-interp.c) ?

I was thinking about Eclipse.  It has to handle unknown (e.g. new) formats, anyway.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 09:44 AM, Metzger, Markus T wrote:
> I was thinking about Eclipse.  It has to handle unknown (e.g. new) formats, anyway.

Right.

Here's a v2:


From ce80ad77f163700e51c1e640ceb933b33ca1d280 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 30 May 2016 17:29:39 -0400
Subject: [PATCH] Add method/format information to =record-started

Eclipse CDT now supports enabling execution recording using two methods
(full and btrace) and both formats for btrace (bts and pt).  In the
event that recording is enabled behind the back of the GUI (by the user
on the command line, or a script), we need to know which method/format
are being used, so it can be correctly reflected in the interface.  This
patch adds this information to the =record-started async record.

Before:

  =record-started,thread-group="i1"

After:

  =record-started,thread-group="i1",method="btrace",format="bts"
  =record-started,thread-group="i1",method="btrace",format="pt"
  =record-started,thread-group="i1",method="full"

The "format" field is only present when the current method supports
multiple formats (only the btrace method as of now).

gdb/ChangeLog:

	* NEWS: Mention the new fields in =record-started.
	* common/btrace-common.h (btrace_format_short_string): New function
	declaration.
	* common/btrace-common.c (btrace_format_short_string): New
	function.
	* mi/mi-interp.c (mi_record_changed): Output method and format
	fields in the =record-started record.
	* record-btrace.c (record_btrace_open): Adapt record_changed
	notification.
	* record-full.c (record_full_open): Likewise.
	* record.c (cmd_record_stop): Likewise.

gdb/doc/ChangeLog:

	* gdb.texinfo (GDB/MI Async Records): Document method and
	format fields in =record-started.
	* observer.texi (record_changed): Add method and format
	parameters.

gdb/testsuite/ChangeLog:

	* gdb.mi/mi-record-changed.exp: Adjust =record-started output
	matching.
---
 gdb/NEWS                                   |  5 +++++
 gdb/common/btrace-common.c                 | 20 ++++++++++++++++++++
 gdb/common/btrace-common.h                 |  3 +++
 gdb/doc/gdb.texinfo                        |  7 ++++++-
 gdb/doc/observer.texi                      |  7 ++++++-
 gdb/mi/mi-interp.c                         | 25 +++++++++++++++++++++----
 gdb/record-btrace.c                        |  4 +++-
 gdb/record-full.c                          |  2 +-
 gdb/record.c                               |  2 +-
 gdb/testsuite/gdb.mi/mi-record-changed.exp |  4 ++--
 10 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index dce79a2..253c8e5 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -67,6 +67,11 @@ maint selftest
   including JIT compiling fast tracepoint's conditional expression
   bytecode into native code.

+* MI async record =record-started now includes the method and format used for
+  recording.  For example:
+
+    =record-started,thread-group="i1",method="btrace",format="bts"
+
 *** Changes in GDB 7.11

 * GDB now supports debugging kernel-based threads on FreeBSD.
diff --git a/gdb/common/btrace-common.c b/gdb/common/btrace-common.c
index eba3979..90aef76 100644
--- a/gdb/common/btrace-common.c
+++ b/gdb/common/btrace-common.c
@@ -43,6 +43,26 @@ btrace_format_string (enum btrace_format format)

 /* See btrace-common.h.  */

+const char *
+btrace_format_short_string (enum btrace_format format)
+{
+  switch (format)
+    {
+    case BTRACE_FORMAT_NONE:
+      return "unknown";
+
+    case BTRACE_FORMAT_BTS:
+      return "bts";
+
+    case BTRACE_FORMAT_PT:
+      return "pt";
+    }
+
+  internal_error (__FILE__, __LINE__, _("Unknown branch trace format"));
+}
+
+/* See btrace-common.h.  */
+
 void
 btrace_data_init (struct btrace_data *data)
 {
diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
index ad208cd..d702bb8 100644
--- a/gdb/common/btrace-common.h
+++ b/gdb/common/btrace-common.h
@@ -214,6 +214,9 @@ enum btrace_error
 /* Return a string representation of FORMAT.  */
 extern const char *btrace_format_string (enum btrace_format format);

+/* Return an abbreviation string representation of FORMAT.  */
+extern const char *btrace_format_short_string (enum btrace_format format);
+
 /* Initialize DATA.  */
 extern void btrace_data_init (struct btrace_data *data);

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7e89003..f50582a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26423,12 +26423,17 @@ breakpoint commands; @xref{GDB/MI Breakpoint Commands}.  The
 Note that if a breakpoint is emitted in the result record of a
 command, then it will not also be emitted in an async record.

-@item =record-started,thread-group="@var{id}"
+@item =record-started,thread-group="@var{id}",method="@var{method}"[,format="@var{format}"]
 @itemx =record-stopped,thread-group="@var{id}"
 Execution log recording was either started or stopped on an
 inferior.  The @var{id} is the @value{GDBN} identifier of the thread
 group corresponding to the affected inferior.

+The @var{method} field indicates the method used to record execution.  If the
+method in use supports multiple recording formats, @var{format} will be present
+and contain the currently used format.  See @xref{Process Record and Replay}
+for existing method and format values.
+
 @item =cmd-param-changed,param=@var{param},value=@var{value}
 Reports that a parameter of the command @code{set @var{param}} is
 changed to @var{value}.  In the multi-word @code{set} command,
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index f4049ad..fc7aac4 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -141,11 +141,16 @@ at the entry-point instruction.  For @samp{attach} and @samp{core},
 inferior, and before any information on the inferior has been printed.
 @end deftypefun

-@deftypefun void record_changed (struct inferior *@var{inferior}, int @var{started})
+@deftypefun void record_changed (struct inferior *@var{inferior}, int @var{started}, const char *@var{method}, const char *@var{format})
 The status of process record for inferior @var{inferior} in
 @value{GDBN} has changed.  The process record is started if
 @var{started} is true, and the process record is stopped if
 @var{started} is false.
+
+When @var{started} is true, @var{method} indicates the short name of the method
+used for recording.  If the method supports multiple formats, @var{format}
+indicates which one is being used, otherwise it is NULL.  When @var{started} is
+false, they are both NULL.
 @end deftypefun

 @deftypefun void solib_loaded (struct so_list *@var{solib})
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 0fe19af..8ce816c 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -65,7 +65,8 @@ static void mi_on_no_history (void);

 static void mi_new_thread (struct thread_info *t);
 static void mi_thread_exit (struct thread_info *t, int silent);
-static void mi_record_changed (struct inferior*, int);
+static void mi_record_changed (struct inferior*, int, const char *,
+			       const char *);
 static void mi_inferior_added (struct inferior *inf);
 static void mi_inferior_appeared (struct inferior *inf);
 static void mi_inferior_exit (struct inferior *inf);
@@ -399,7 +400,8 @@ mi_thread_exit (struct thread_info *t, int silent)
 /* Emit notification on changing the state of record.  */

 static void
-mi_record_changed (struct inferior *inferior, int started)
+mi_record_changed (struct inferior *inferior, int started, const char *method,
+		   const char *format)
 {
   struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
   struct cleanup *old_chain;
@@ -407,8 +409,23 @@ mi_record_changed (struct inferior *inferior, int started)
   old_chain = make_cleanup_restore_target_terminal ();
   target_terminal_ours_for_output ();

-  fprintf_unfiltered (mi->event_channel,  "record-%s,thread-group=\"i%d\"",
-		      started ? "started" : "stopped", inferior->num);
+  if (started)
+    {
+      if (format != NULL)
+	fprintf_unfiltered (
+	  mi->event_channel,
+	  "record-started,thread-group=\"i%d\",method=\"%s\",format=\"%s\"",
+	  inferior->num, method, format);
+      else
+	fprintf_unfiltered (
+	  mi->event_channel,
+	  "record-started,thread-group=\"i%d\",method=\"%s\"",
+	  inferior->num, method);
+    }
+  else
+    fprintf_unfiltered (mi->event_channel,
+			"record-stopped,thread-group=\"i%d\"", inferior->num);
+

   gdb_flush (mi->event_channel);

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 77b5180..24594a9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -206,6 +206,7 @@ record_btrace_open (const char *args, int from_tty)
 {
   struct cleanup *disable_chain;
   struct thread_info *tp;
+  const char *format;

   DEBUG ("open");

@@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
 				  NULL);
   record_btrace_generating_corefile = 0;

-  observer_notify_record_changed (current_inferior (),  1);
+  format = btrace_format_short_string (record_btrace_conf.format);
+  observer_notify_record_changed (current_inferior (), 1, "btrace", format);

   discard_cleanups (disable_chain);
 }
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f307b48..0f61bcb 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -869,7 +869,7 @@ record_full_open (const char *name, int from_tty)

   record_full_init_record_breakpoints ();

-  observer_notify_record_changed (current_inferior (),  1);
+  observer_notify_record_changed (current_inferior (),  1, "full", NULL);
 }

 /* "to_close" target method.  Close the process record target.  */
diff --git a/gdb/record.c b/gdb/record.c
index 6190794..1af134f 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -268,7 +268,7 @@ cmd_record_stop (char *args, int from_tty)
   printf_unfiltered (_("Process record is stopped and all execution "
 		       "logs are deleted.\n"));

-  observer_notify_record_changed (current_inferior (), 0);
+  observer_notify_record_changed (current_inferior (), 0, NULL, NULL);
 }

 /* The "set record" command.  */
diff --git a/gdb/testsuite/gdb.mi/mi-record-changed.exp b/gdb/testsuite/gdb.mi/mi-record-changed.exp
index 2b5fcd6..22cf076 100644
--- a/gdb/testsuite/gdb.mi/mi-record-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-record-changed.exp
@@ -31,14 +31,14 @@ if [mi_gdb_start] {
 }
 mi_run_to_main

-mi_gdb_test "record" ".*=record-started,thread-group=\"i${decimal}\".*\\^done" \
+mi_gdb_test "record" ".*=record-started,thread-group=\"i${decimal}\",method=\"full\".*\\^done" \
     "record"
 mi_gdb_test "record stop" \
     ".*=record-stopped,thread-group=\"i${decimal}\".*\\^done" \
     "record end"

 mi_gdb_test "target record" \
-    ".*=record-started,thread-group=\"i${decimal}\".*\\^done" \
+    ".*=record-started,thread-group=\"i${decimal}\",method=\"full\".*\\^done" \
     "target record"

 return 0
-- 
2.8.3

RE: [PATCH] Add method/format information to =record-started

[Metzger, Markus T]

> -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@ericsson.com]
> Sent: Monday, June 6, 2016 3:49 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] Add method/format information to =record-started


> From ce80ad77f163700e51c1e640ceb933b33ca1d280 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Mon, 30 May 2016 17:29:39 -0400
> Subject: [PATCH] Add method/format information to =record-started
> 
> Eclipse CDT now supports enabling execution recording using two methods
> (full and btrace) and both formats for btrace (bts and pt).  In the
> event that recording is enabled behind the back of the GUI (by the user
> on the command line, or a script), we need to know which method/format
> are being used, so it can be correctly reflected in the interface.  This
> patch adds this information to the =record-started async record.
> 
> Before:
> 
>   =record-started,thread-group="i1"
> 
> After:
> 
>   =record-started,thread-group="i1",method="btrace",format="bts"
>   =record-started,thread-group="i1",method="btrace",format="pt"
>   =record-started,thread-group="i1",method="full"
> 
> The "format" field is only present when the current method supports
> multiple formats (only the btrace method as of now).

The btrace bits look good to me.

I'm wondering if we want the short or the long form of the format.  If we
expect MI front-ends to just print it as-is, the long form may be more useful.
That's also what's printed by "info record".  I'm fine either way.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 10:02 AM, Metzger, Markus T wrote:
> I'm wondering if we want the short or the long form of the format.  If we
> expect MI front-ends to just print it as-is, the long form may be more useful.
> That's also what's printed by "info record".  I'm fine either way.

The main goal for this is to communicate to Eclipse which method is being used,
so it can update its UI as if the user had done the same action using the UI.
It's really for program/program communication, not program/human.  If frontends
wanted a nice human readable string, we could always add it as an extra.

RE: [PATCH] Add method/format information to =record-started

[Metzger, Markus T]

> -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@ericsson.com]
> Sent: Monday, June 6, 2016 4:28 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH] Add method/format information to =record-started
> 
> On 16-06-06 10:02 AM, Metzger, Markus T wrote:
> > I'm wondering if we want the short or the long form of the format.  If we
> > expect MI front-ends to just print it as-is, the long form may be more useful.
> > That's also what's printed by "info record".  I'm fine either way.
> 
> The main goal for this is to communicate to Eclipse which method is being used,
> so it can update its UI as if the user had done the same action using the UI.
> It's really for program/program communication, not program/human.  If
> frontends
> wanted a nice human readable string, we could always add it as an extra.

OK.

Thanks,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] Add method/format information to =record-started

[Eli Zaretskii]

> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Mon, 6 Jun 2016 09:49:01 -0400
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index dce79a2..253c8e5 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -67,6 +67,11 @@ maint selftest
>    including JIT compiling fast tracepoint's conditional expression
>    bytecode into native code.
> 
> +* MI async record =record-started now includes the method and format used for
> +  recording.  For example:
> +
> +    =record-started,thread-group="i1",method="btrace",format="bts"
> +
>  *** Changes in GDB 7.11

This is OK.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7e89003..f50582a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26423,12 +26423,17 @@ breakpoint commands; @xref{GDB/MI Breakpoint Commands}.  The
>  Note that if a breakpoint is emitted in the result record of a
>  command, then it will not also be emitted in an async record.
> 
> -@item =record-started,thread-group="@var{id}"
> +@item =record-started,thread-group="@var{id}",method="@var{method}"[,format="@var{format}"]
>  @itemx =record-stopped,thread-group="@var{id}"
>  Execution log recording was either started or stopped on an
>  inferior.  The @var{id} is the @value{GDBN} identifier of the thread
>  group corresponding to the affected inferior.
> 
> +The @var{method} field indicates the method used to record execution.  If the
> +method in use supports multiple recording formats, @var{format} will be present
> +and contain the currently used format.  See @xref{Process Record and Replay}
> +for existing method and format values.  ^^^

No need for "See" there, as @xref generates that automatically.  Or
use "See @ref".

OK for the documentation parts, with the above gotcha fixed.

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-06-06 11:13 AM, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Mon, 6 Jun 2016 09:49:01 -0400
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index dce79a2..253c8e5 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -67,6 +67,11 @@ maint selftest
>>    including JIT compiling fast tracepoint's conditional expression
>>    bytecode into native code.
>>
>> +* MI async record =record-started now includes the method and format used for
>> +  recording.  For example:
>> +
>> +    =record-started,thread-group="i1",method="btrace",format="bts"
>> +
>>  *** Changes in GDB 7.11
> 
> This is OK.
> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 7e89003..f50582a 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -26423,12 +26423,17 @@ breakpoint commands; @xref{GDB/MI Breakpoint Commands}.  The
>>  Note that if a breakpoint is emitted in the result record of a
>>  command, then it will not also be emitted in an async record.
>>
>> -@item =record-started,thread-group="@var{id}"
>> +@item =record-started,thread-group="@var{id}",method="@var{method}"[,format="@var{format}"]
>>  @itemx =record-stopped,thread-group="@var{id}"
>>  Execution log recording was either started or stopped on an
>>  inferior.  The @var{id} is the @value{GDBN} identifier of the thread
>>  group corresponding to the affected inferior.
>>
>> +The @var{method} field indicates the method used to record execution.  If the
>> +method in use supports multiple recording formats, @var{format} will be present
>> +and contain the currently used format.  See @xref{Process Record and Replay}
>> +for existing method and format values.  ^^^
> 
> No need for "See" there, as @xref generates that automatically.  Or
> use "See @ref".
> 
> OK for the documentation parts, with the above gotcha fixed.

Oh it slipped passed my eyes when reading, but you're right.

Thanks, this is now pushed.

Re: [PATCH] Add method/format information to =record-started

[Yao Qi]

Simon Marchi <simon.marchi@ericsson.com> writes:

> gdb/testsuite/ChangeLog:
>
> 	* gdb.mi/mi-record-changed.exp: Adjust =record-started output
> 	matching.

We need to update gdb.mi/mi-reverse.exp too.  Patch below does it.
Patch is pushed in.

-- 
Yao (齐尧)
From 647c264cb2c60c90ee2d09edb6bd001ff357306d Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 5 Jul 2016 14:46:21 +0100
Subject: [PATCH] Fix fail in gdb.mi/mi-reverse.exp

Commit 38b022b4452f996fb5a8598f80d850b594621bcf adds "method" and
"format" fields in =record-started, but doesn't update test case
gdb.mi/mi-reverse.exp, so it causes the fail like this,

PASS: gdb.mi/mi-reverse.exp: mi runto main
Expecting: ^(-interpreter-exec console record[^M
]+)?(=record-started,thread-group="i1"^M
\^done[^M
]+[(]gdb[)] ^M
[ ]*)
-interpreter-exec console record^M
=record-started,thread-group="i1",method="full"^M
^done^M
(gdb) ^M
FAIL: gdb.mi/mi-reverse.exp: Turn on process record

and regression was found by buildbot too
https://sourceware.org/ml/gdb-testers/2016-q2/msg04492.html

gdb/testsuite:

2016-07-05  Yao Qi  <yao.qi@linaro.org>

	* gdb.mi/mi-reverse.exp: Match =record-started output.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bcbffaa..be0cf9d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2016-07-05  Yao Qi  <yao.qi@linaro.org>
+
+	* gdb.mi/mi-reverse.exp: Match =record-started output.
+
 2016-07-01  Pedro Alves  <palves@redhat.com>
 
 	* gdb.base/jit-reader.exp (info_registers_current_frame): New
diff --git a/gdb/testsuite/gdb.mi/mi-reverse.exp b/gdb/testsuite/gdb.mi/mi-reverse.exp
index d8f3844..5afeece 100644
--- a/gdb/testsuite/gdb.mi/mi-reverse.exp
+++ b/gdb/testsuite/gdb.mi/mi-reverse.exp
@@ -51,7 +51,7 @@ mi_run_to_main
 if [supports_process_record] {
     # Activate process record/replay
     if [mi_gdb_test "-interpreter-exec console record" \
-	    "=record-started,thread-group=\"i1\"\r\n\\^done" \
+	    "=record-started,thread-group=\"i1\",method=\"full\"\r\n\\^done" \
 	    "Turn on process record"] {
 	warning "Fail to activate process record/replay, tests in this group will not be performed.\n"
 	return -1

Re: [PATCH] Add method/format information to =record-started

[Simon Marchi]

On 16-07-05 09:50 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.mi/mi-record-changed.exp: Adjust =record-started output
>> 	matching.
> 
> We need to update gdb.mi/mi-reverse.exp too.  Patch below does it.
> Patch is pushed in.
> 

Thanks!