[PATCH 0/2] Reimplement redirection for MI

[PATCH 0/2] Reimplement redirection for MI

[Adrian Sendroiu]

The current implementation uses a "saved_buffer" field to save the current
stream when doing a redirection, which will not function correctly in the case
of nested mi_redirect calls. These patches reimplement the redirection using a
stack of streams, in a manner similar with the one in the cli interpreter.

Adrian Sendroiu (2):
  cli/cli-logging.c: don't call ui_out_redirect for MI when disabling
    logging
  mi-out: Implement mi redirection using a stack.

 gdb/cli/cli-logging.c |    3 +-
 gdb/mi/mi-out.c       |   75 +++++++++++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 32 deletions(-)

-- 
1.7.9.5

[PATCH 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* mi/mi-out.c (ui_filep): New typedef.
	(DEF_VEC_P (ui_filep)): New type.
	(struct ui_out_data): <buffer> Remove field.
	<original_buffer> Remove field.
	<streams>: New field.
	(mi_field_string): Get the output stream from the top of the
	stack of streams.
	(mi_field_fmt): Likewise.
	(mi_flush): Likewise.
	(field_separator): Likewise.
	(mi_open): Likewise.
	(mi_close): Likewise.
	(mi_out_buffered): Likewise.
	(mi_out_rewind): Likewise.
	(mi_out_put): Likewise.
	(mi_out_new): Initialize the stack of streams. Push a newly
	created stream on the stack.
	(mi_redirect): Reimplement using push and pop operations.
---
 gdb/mi/mi-out.c |   75 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 6ec41e6..4452080 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -22,14 +22,17 @@
 #include "defs.h"
 #include "ui-out.h"
 #include "mi-out.h"
+#include "vec.h"
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
 
 struct ui_out_data
   {
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
-    struct ui_file *buffer;
-    struct ui_file *original_buffer;
+    VEC (ui_filep) *streams;
   };
 typedef struct ui_out_data mi_out_data;
 
@@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
 		 enum ui_align align, const char *fldname, const char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=", fldname);
-  fprintf_unfiltered (data->buffer, "\"");
+    fprintf_unfiltered (stream, "%s=", fldname);
+  fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', data->buffer);
-  fprintf_unfiltered (data->buffer, "\"");
+    fputstr_unfiltered (string, '"', stream);
+  fprintf_unfiltered (stream, "\"");
 }
 
 /* This is the only field function that does not align.  */
@@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 	      const char *format, va_list args)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
+    fprintf_unfiltered (stream, "%s=\"", fldname);
   else
-    fputs_unfiltered ("\"", data->buffer);
-  vfprintf_unfiltered (data->buffer, format, args);
-  fputs_unfiltered ("\"", data->buffer);
+    fputs_unfiltered ("\"", stream);
+  vfprintf_unfiltered (stream, format, args);
+  fputs_unfiltered ("\"", stream);
 }
 
 void
@@ -275,8 +284,9 @@ void
 mi_flush (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  gdb_flush (data->buffer);
+  gdb_flush (stream);
 }
 
 int
@@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
   mi_out_data *data = ui_out_data (uiout);
 
   if (outstream != NULL)
-    {
-      data->original_buffer = data->buffer;
-      data->buffer = outstream;
-    }
-  else if (data->original_buffer != NULL)
-    {
-      data->buffer = data->original_buffer;
-      data->original_buffer = NULL;
-    }
+    VEC_safe_push (ui_filep, data->streams, outstream);
+  else
+    VEC_pop (ui_filep, data->streams);
 
   return 0;
 }
@@ -306,29 +310,31 @@ static void
 field_separator (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   if (data->suppress_field_separator)
     data->suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', data->buffer);
+    fputc_unfiltered (',', stream);
 }
 
 static void
 mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   field_separator (uiout);
   data->suppress_field_separator = 1;
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    fprintf_unfiltered (stream, "%s=", name);
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('{', data->buffer);
+      fputc_unfiltered ('{', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      fputc_unfiltered ('[', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -339,14 +345,15 @@ static void
 mi_close (struct ui_out *uiout, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('}', data->buffer);
+      fputc_unfiltered ('}', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      fputc_unfiltered (']', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -360,8 +367,9 @@ void
 mi_out_buffered (struct ui_out *uiout, char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  fprintf_unfiltered (data->buffer, "%s", string);
+  fprintf_unfiltered (stream, "%s", string);
 }
 
 /* Clear the buffer.  */
@@ -370,8 +378,9 @@ void
 mi_out_rewind (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_rewind (data->buffer);
+  ui_file_rewind (stream);
 }
 
 /* Dump the buffer onto the specified stream.  */
@@ -386,9 +395,10 @@ void
 mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_put (data->buffer, do_write, stream);
-  ui_file_rewind (data->buffer);
+  ui_file_put (current_stream, do_write, stream);
+  ui_file_rewind (current_stream);
 }
 
 /* Return the current MI version.  */
@@ -407,6 +417,7 @@ struct ui_out *
 mi_out_new (int mi_version)
 {
   int flags = 0;
+  struct ui_file *new_stream;
 
   mi_out_data *data = XNEW (mi_out_data);
   data->suppress_field_separator = 0;
@@ -414,6 +425,8 @@ mi_out_new (int mi_version)
   data->mi_version = mi_version;
   /* FIXME: This code should be using a ``string_file'' and not the
      TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  data->streams = NULL;
+  new_stream = mem_fileopen();
+  VEC_safe_push (ui_filep, data->streams, new_stream);
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
-- 
1.7.9.5

[PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging

[Adrian Sendroiu]

When logging is set up, the code skips the call to ui_out_redirect if the
current interpreter is MI, but still calls it unconditionally when logging
is turned off. Since the number of "redirect"/"unredirect" calls must match,
this patch ensures that the ui_out_redirect call is also skipped when logging
is turned off.

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* cli/cli-logging.c (pop_output_files): Don't call ui_out_redirect
	if the current interpreter is MI.
---
 gdb/cli/cli-logging.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index aae0039..e51bb95 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -180,7 +180,8 @@ pop_output_files (void)
   saved_output.targ = NULL;
   saved_output.targerr = NULL;
 
-  ui_out_redirect (current_uiout, NULL);
+  if (!ui_out_is_mi_like_p (current_uiout))
+    ui_out_redirect (current_uiout, NULL);
 }
 
 /* This is a helper for the `set logging' command.  */
-- 
1.7.9.5

Re: [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging

[Tom Tromey]

>>>>> "Adrian" == Adrian Sendroiu <adrian.sendroiu@freescale.com> writes:

Adrian> 2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

Adrian> 	* cli/cli-logging.c (pop_output_files): Don't call ui_out_redirect
Adrian> 	if the current interpreter is MI.

Thanks, this is ok.

Tom

Re: [PATCH 2/2] mi-out: Implement mi redirection using a stack.

[Tom Tromey]

>>>>> "Adrian" == Adrian Sendroiu <adrian.sendroiu@freescale.com> writes:

Adrian> gdb/
Adrian> 2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

Adrian> 	* mi/mi-out.c (ui_filep): New typedef.
[...]

I think it would be good to have some text in the message describing the
rationale for this patch.

One nit below.

Adrian>    /* FIXME: This code should be using a ``string_file'' and not the
Adrian>       TUI buffer hack. */

Not your problem but I wonder what this FIXME comment means.
I suspect it's obsolete since as far as I know mem_fileopen does return
a "string_file".

Adrian> +  new_stream = mem_fileopen();

Space before the open paren.

The patch is ok with this fixed and with some rationale text.

thanks,
Tom

[PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

The current implementation doesn't support nested redirections because it only
has an "original_buffer" where it saves the current stream when a redirection is
done. To overcome this limitation, this patch reimplements it using a stack of
streams.

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* mi/mi-out.c (ui_filep): New typedef.
	(DEF_VEC_P (ui_filep)): New type.
	(struct ui_out_data): <buffer> Remove field.
	<original_buffer> Remove field.
	<streams>: New field.
	(mi_field_string): Get the output stream from the top of the
	stack of streams.
	(mi_field_fmt): Likewise.
	(mi_flush): Likewise.
	(field_separator): Likewise.
	(mi_open): Likewise.
	(mi_close): Likewise.
	(mi_out_buffered): Likewise.
	(mi_out_rewind): Likewise.
	(mi_out_put): Likewise.
	(mi_out_new): Initialize the stack of streams. Push a newly
	created stream on the stack. Remove obsolete comment.
	(mi_redirect): Reimplement using push and pop operations.
---
 gdb/mi/mi-out.c |   77 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 33 deletions(-)

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 6ec41e6..6731509 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -22,14 +22,17 @@
 #include "defs.h"
 #include "ui-out.h"
 #include "mi-out.h"
+#include "vec.h"
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
 
 struct ui_out_data
   {
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
-    struct ui_file *buffer;
-    struct ui_file *original_buffer;
+    VEC (ui_filep) *streams;
   };
 typedef struct ui_out_data mi_out_data;
 
@@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
 		 enum ui_align align, const char *fldname, const char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=", fldname);
-  fprintf_unfiltered (data->buffer, "\"");
+    fprintf_unfiltered (stream, "%s=", fldname);
+  fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', data->buffer);
-  fprintf_unfiltered (data->buffer, "\"");
+    fputstr_unfiltered (string, '"', stream);
+  fprintf_unfiltered (stream, "\"");
 }
 
 /* This is the only field function that does not align.  */
@@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 	      const char *format, va_list args)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
+    fprintf_unfiltered (stream, "%s=\"", fldname);
   else
-    fputs_unfiltered ("\"", data->buffer);
-  vfprintf_unfiltered (data->buffer, format, args);
-  fputs_unfiltered ("\"", data->buffer);
+    fputs_unfiltered ("\"", stream);
+  vfprintf_unfiltered (stream, format, args);
+  fputs_unfiltered ("\"", stream);
 }
 
 void
@@ -275,8 +284,9 @@ void
 mi_flush (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  gdb_flush (data->buffer);
+  gdb_flush (stream);
 }
 
 int
@@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
   mi_out_data *data = ui_out_data (uiout);
 
   if (outstream != NULL)
-    {
-      data->original_buffer = data->buffer;
-      data->buffer = outstream;
-    }
-  else if (data->original_buffer != NULL)
-    {
-      data->buffer = data->original_buffer;
-      data->original_buffer = NULL;
-    }
+    VEC_safe_push (ui_filep, data->streams, outstream);
+  else
+    VEC_pop (ui_filep, data->streams);
 
   return 0;
 }
@@ -306,29 +310,31 @@ static void
 field_separator (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   if (data->suppress_field_separator)
     data->suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', data->buffer);
+    fputc_unfiltered (',', stream);
 }
 
 static void
 mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   field_separator (uiout);
   data->suppress_field_separator = 1;
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    fprintf_unfiltered (stream, "%s=", name);
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('{', data->buffer);
+      fputc_unfiltered ('{', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      fputc_unfiltered ('[', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -339,14 +345,15 @@ static void
 mi_close (struct ui_out *uiout, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('}', data->buffer);
+      fputc_unfiltered ('}', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      fputc_unfiltered (']', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -360,8 +367,9 @@ void
 mi_out_buffered (struct ui_out *uiout, char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  fprintf_unfiltered (data->buffer, "%s", string);
+  fprintf_unfiltered (stream, "%s", string);
 }
 
 /* Clear the buffer.  */
@@ -370,8 +378,9 @@ void
 mi_out_rewind (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_rewind (data->buffer);
+  ui_file_rewind (stream);
 }
 
 /* Dump the buffer onto the specified stream.  */
@@ -386,9 +395,10 @@ void
 mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_put (data->buffer, do_write, stream);
-  ui_file_rewind (data->buffer);
+  ui_file_put (current_stream, do_write, stream);
+  ui_file_rewind (current_stream);
 }
 
 /* Return the current MI version.  */
@@ -407,13 +417,14 @@ struct ui_out *
 mi_out_new (int mi_version)
 {
   int flags = 0;
+  struct ui_file *new_stream;
 
   mi_out_data *data = XNEW (mi_out_data);
   data->suppress_field_separator = 0;
   data->suppress_output = 0;
   data->mi_version = mi_version;
-  /* FIXME: This code should be using a ``string_file'' and not the
-     TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  data->streams = NULL;
+  new_stream = mem_fileopen ();
+  VEC_safe_push (ui_filep, data->streams, new_stream);
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
-- 
1.7.9.5

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

I'm guessing we can trigger this by using "save breakpoints" while
logging is enabled, like gdb.base/ui-redirect.exp ?
I think it'd be very good if a test to the testsuite was added.
Sounds like gdb.mi/mi-logging.exp would be a good place?

-- 
Thanks,
Pedro Alves

RE: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, July 29, 2014 5:55 PM
> To: Sendroiu Adrian-B46904; gdb-patches@sourceware.org;
> tromey@redhat.com
> Subject: Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a
> stack.
> 
> I'm guessing we can trigger this by using "save breakpoints" while
> logging is enabled, like gdb.base/ui-redirect.exp ?
> I think it'd be very good if a test to the testsuite was added.
> Sounds like gdb.mi/mi-logging.exp would be a good place?
> 
> --
> Thanks,
> Pedro Alves

This won't trigger the bug, because the logging code doesn't call ui_out_redirect if the interpreter is MI. The way I caught it was through some python script that executes commands and catches their output into a string. For example, if you have 

gdb.execute("break main", False, True)

The call sequence will be something like:
execute_command_to_string
  ui_out_redirect
  execute_command
  ...
    mi_breakpoint_created
      ui_out_redirect

Then, after executing this, the mi_uiout->data->buffer will incorrectly point to a freed ui_file structure, and any subsequent command will overwrite the pointers inside this ui_file with random data, causing a crash.

Do you have any suggestions on how to make a test case from this scenario?

Adrian

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

On 07/30/2014 09:37 AM, Adrian Sendroiu wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Tuesday, July 29, 2014 5:55 PM
>> To: Sendroiu Adrian-B46904; gdb-patches@sourceware.org;
>> tromey@redhat.com
>> Subject: Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a
>> stack.
>>
>> I'm guessing we can trigger this by using "save breakpoints" while
>> logging is enabled, like gdb.base/ui-redirect.exp ?
>> I think it'd be very good if a test to the testsuite was added.
>> Sounds like gdb.mi/mi-logging.exp would be a good place?
>>
>> --
>> Thanks,
>> Pedro Alves
> 
> This won't trigger the bug, because the logging code doesn't call ui_out_redirect if the interpreter is MI. The way I caught it was through some python script that executes commands and catches their output into a string. For example, if you have 
> 
> gdb.execute("break main", False, True)
> 
> The call sequence will be something like:
> execute_command_to_string
>   ui_out_redirect
>   execute_command
>   ...
>     mi_breakpoint_created
>       ui_out_redirect
> 
> Then, after executing this, the mi_uiout->data->buffer will incorrectly point to a freed ui_file structure, and any subsequent command will overwrite the pointers inside this ui_file with random data, causing a crash.
> 
> Do you have any suggestions on how to make a test case from this scenario?

I'm not sure what specific suggestion you're looking after.  :-)

Sound like you'd add a test that does that exactly ?

You'd either base on, or add to mi-logging.exp, and do something like:

  mi_gdb_test "python gdb.execute("break main", False, True)" ...

  mi_gdb_test <some MI command that causes a crash> ...

(and skip the test if skip_python_tests is true)


Thanks,
Pedro Alves

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

I implemented the test case. A couple of issues I encountered:

- skip_python_tests doesn't work with MI, because it searches for $gdb_prompt.
I implemented another one called mi_skip_python_tests which uses $mi_gdb_prompt.

- executing "python gdb.execute("break main", ... ) directly doesn't work,
because "python" is a cli command, and before running it the current_uiout will
be temporarily changed to cli_uiout, so the first redirection won't be done
on mi_uiout. To bypass this I implemented a small workaround: set a breakpoint,
then set the python command to be executed when the breakpoint hits.

- when running the test on a gdb without the redirection fix, the test will be
reported as UNRESOLVED, because gdb crashes.

--- 8< ---
Subject: [PATCH] mi-out: Implement mi redirection using a stack.

The current implementation doesn't support nested redirections because it only
has an "original_buffer" where it saves the current stream when a redirection is
done. To overcome this limitation, this patch reimplements it using a stack of
streams.

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* mi/mi-out.c (ui_filep): New typedef.
	(DEF_VEC_P (ui_filep)): New type.
	(struct ui_out_data): <buffer> Remove field.
	<original_buffer> Remove field.
	<streams>: New field.
	(mi_field_string): Get the output stream from the top of the
	stack of streams.
	(mi_field_fmt): Likewise.
	(mi_flush): Likewise.
	(field_separator): Likewise.
	(mi_open): Likewise.
	(mi_close): Likewise.
	(mi_out_buffered): Likewise.
	(mi_out_rewind): Likewise.
	(mi_out_put): Likewise.
	(mi_out_new): Initialize the stack of streams. Push a newly
	created stream on the stack. Remove obsolete comment.
	(mi_redirect): Reimplement using push and pop operations.

gdb/testsuite/
2014-07-30  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* gdb.mi/mi-logging.exp: Add test for nested mi redirection.
	* lib/mi-support.exp: (mi_skip_python_tests): New function.
---
 gdb/mi/mi-out.c                     |   77 ++++++++++++++++++++---------------
 gdb/testsuite/gdb.mi/mi-logging.exp |   14 +++++++
 gdb/testsuite/lib/mi-support.exp    |   37 +++++++++++++++++
 3 files changed, 95 insertions(+), 33 deletions(-)

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 6ec41e6..6731509 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -22,14 +22,17 @@
 #include "defs.h"
 #include "ui-out.h"
 #include "mi-out.h"
+#include "vec.h"
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
 
 struct ui_out_data
   {
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
-    struct ui_file *buffer;
-    struct ui_file *original_buffer;
+    VEC (ui_filep) *streams;
   };
 typedef struct ui_out_data mi_out_data;
 
@@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
 		 enum ui_align align, const char *fldname, const char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=", fldname);
-  fprintf_unfiltered (data->buffer, "\"");
+    fprintf_unfiltered (stream, "%s=", fldname);
+  fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', data->buffer);
-  fprintf_unfiltered (data->buffer, "\"");
+    fputstr_unfiltered (string, '"', stream);
+  fprintf_unfiltered (stream, "\"");
 }
 
 /* This is the only field function that does not align.  */
@@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 	      const char *format, va_list args)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
+    fprintf_unfiltered (stream, "%s=\"", fldname);
   else
-    fputs_unfiltered ("\"", data->buffer);
-  vfprintf_unfiltered (data->buffer, format, args);
-  fputs_unfiltered ("\"", data->buffer);
+    fputs_unfiltered ("\"", stream);
+  vfprintf_unfiltered (stream, format, args);
+  fputs_unfiltered ("\"", stream);
 }
 
 void
@@ -275,8 +284,9 @@ void
 mi_flush (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  gdb_flush (data->buffer);
+  gdb_flush (stream);
 }
 
 int
@@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
   mi_out_data *data = ui_out_data (uiout);
 
   if (outstream != NULL)
-    {
-      data->original_buffer = data->buffer;
-      data->buffer = outstream;
-    }
-  else if (data->original_buffer != NULL)
-    {
-      data->buffer = data->original_buffer;
-      data->original_buffer = NULL;
-    }
+    VEC_safe_push (ui_filep, data->streams, outstream);
+  else
+    VEC_pop (ui_filep, data->streams);
 
   return 0;
 }
@@ -306,29 +310,31 @@ static void
 field_separator (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   if (data->suppress_field_separator)
     data->suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', data->buffer);
+    fputc_unfiltered (',', stream);
 }
 
 static void
 mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   field_separator (uiout);
   data->suppress_field_separator = 1;
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    fprintf_unfiltered (stream, "%s=", name);
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('{', data->buffer);
+      fputc_unfiltered ('{', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      fputc_unfiltered ('[', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -339,14 +345,15 @@ static void
 mi_close (struct ui_out *uiout, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('}', data->buffer);
+      fputc_unfiltered ('}', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      fputc_unfiltered (']', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -360,8 +367,9 @@ void
 mi_out_buffered (struct ui_out *uiout, char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  fprintf_unfiltered (data->buffer, "%s", string);
+  fprintf_unfiltered (stream, "%s", string);
 }
 
 /* Clear the buffer.  */
@@ -370,8 +378,9 @@ void
 mi_out_rewind (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_rewind (data->buffer);
+  ui_file_rewind (stream);
 }
 
 /* Dump the buffer onto the specified stream.  */
@@ -386,9 +395,10 @@ void
 mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_put (data->buffer, do_write, stream);
-  ui_file_rewind (data->buffer);
+  ui_file_put (current_stream, do_write, stream);
+  ui_file_rewind (current_stream);
 }
 
 /* Return the current MI version.  */
@@ -407,13 +417,14 @@ struct ui_out *
 mi_out_new (int mi_version)
 {
   int flags = 0;
+  struct ui_file *new_stream;
 
   mi_out_data *data = XNEW (mi_out_data);
   data->suppress_field_separator = 0;
   data->suppress_output = 0;
   data->mi_version = mi_version;
-  /* FIXME: This code should be using a ``string_file'' and not the
-     TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  data->streams = NULL;
+  new_stream = mem_fileopen ();
+  VEC_safe_push (ui_filep, data->streams, new_stream);
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
index d5e4193..063ea97 100644
--- a/gdb/testsuite/gdb.mi/mi-logging.exp
+++ b/gdb/testsuite/gdb.mi/mi-logging.exp
@@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
     fail "Redirect log file contents"
 }
 
+# This triggers a nested mi_ui_out redirection, by disabling a breakpoint
+# inside a python command that has to_string = True.
+if ![mi_skip_python_tests] {
+    mi_gdb_test "-break-insert do_nothing" ".*"
+    mi_gdb_test "-break-commands 2 \"python gdb.execute('disable 2', True, True)\"" ".*"
+
+    mi_gdb_test "-exec-continue" ".*"
+
+    set s [string repeat "A" 100]
+
+    # This will crash gdb if redirection is not done properly.
+    mi_gdb_test "echo $s" ".*\\^done" "mi nested redirect"
+}
+
 mi_gdb_exit
 
 remote_file host delete $milogfile
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 204f1e8..23a5b80 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2491,3 +2491,40 @@ proc mi_make_breakpoint_table {bp_list} {
     # Assemble the final regexp.
     return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}"
 }
+
+proc mi_skip_python_tests {} {
+    global mi_gdb_prompt
+    global gdb_py_is_py3k
+    global gdb_py_is_py24
+
+    gdb_test_multiple "python print ('test')" "verify python support" {
+	-re "not supported.*$mi_gdb_prompt$"	{
+	    unsupported "Python support is disabled."
+	    return 1
+	}
+	-re "$mi_gdb_prompt$"	{}
+    }
+
+    set gdb_py_is_py24 0
+    gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
+	-re "3.*$mi_gdb_prompt$"	{
+            set gdb_py_is_py3k 1
+        }
+	-re ".*$mi_gdb_prompt$"	{
+            set gdb_py_is_py3k 0
+        }
+
+    }
+    if { $gdb_py_is_py3k == 0 } {
+        gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
+	    -re "\[45\].*$mi_gdb_prompt$" {
+                set gdb_py_is_py24 1
+            }
+	    -re ".*$mi_gdb_prompt$" {
+                set gdb_py_is_py24 0
+            }
+        }
+    }
+
+    return 0
+}
-- 
1.7.9.5

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

Awesome!  Thanks for doing this.

This is close.  Some notes on the technicalities below.

On 07/31/2014 04:08 PM, Adrian Sendroiu wrote:
> +# This triggers a nested mi_ui_out redirection, by disabling a breakpoint
> +# inside a python command that has to_string = True.
> +if ![mi_skip_python_tests] {
> +    mi_gdb_test "-break-insert do_nothing" ".*"
> +    mi_gdb_test "-break-commands 2 \"python gdb.execute('disable 2', True, True)\"" ".*"

It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a
test that adds another breakpoint before this, this test stops being
effective, silently.

> +
> +    mi_gdb_test "-exec-continue" ".*"

This should use mi_send_resuming_command/mi_expect_stop
or mi_execute_to, so that the test works when the whole MI
testsuite is run in async mode.

> +
> +    set s [string repeat "A" 100]
> +
> +    # This will crash gdb if redirection is not done properly.
> +    mi_gdb_test "echo $s" ".*\\^done" "mi nested redirect"

In addition to that, it'd be good to confirm the breakpoint did
end up disabled, which likewise confirms the breakpoint command
was set on the breakpoint we wanted.  (might not need the "echo"
if that itself already causes a crash.)

> +proc mi_skip_python_tests {} {
> +    global mi_gdb_prompt
> +    global gdb_py_is_py3k
> +    global gdb_py_is_py24
> +
> +    gdb_test_multiple "python print ('test')" "verify python support" {
> +	-re "not supported.*$mi_gdb_prompt$"	{
> +	    unsupported "Python support is disabled."
> +	    return 1
> +	}
> +	-re "$mi_gdb_prompt$"	{}
> +    }
> +
> +    set gdb_py_is_py24 0
> +    gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
> +	-re "3.*$mi_gdb_prompt$"	{
> +            set gdb_py_is_py3k 1
> +        }
> +	-re ".*$mi_gdb_prompt$"	{
> +            set gdb_py_is_py3k 0
> +        }
> +
> +    }
> +    if { $gdb_py_is_py3k == 0 } {
> +        gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
> +	    -re "\[45\].*$mi_gdb_prompt$" {
> +                set gdb_py_is_py24 1
> +            }
> +	    -re ".*$mi_gdb_prompt$" {
> +                set gdb_py_is_py24 0
> +            }
> +        }
> +    }
> +
> +    return 0
> +}

I think that we can avoid this duplication by renaming
skip_python_tests, adding it a prompt_re parameter, and
using that instead of $gdb_prompt.  Something like:

proc skip_python_tests {} {
   skip_python_tests_prompt "$gdb_prompt $"
}

proc mi_skip_python_tests {
   skip_python_tests_prompt "$mi_gdb_prompt$"
}

Did you try that?

Thanks,
Pedro Alves

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

> It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a
> test that adds another breakpoint before this, this test stops being
> effective, silently.

done

>> +    mi_gdb_test "-exec-continue" ".*"
> 
> This should use mi_send_resuming_command/mi_expect_stop
> or mi_execute_to, so that the test works when the whole MI
> testsuite is run in async mode.

done. Although I ran into another problem here, because mi_expect_stop expects a
message that looks like *stopped + prompt, while in my case it was something like
*stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler
test case: just executing two python commands nested inside one another, like
"python gdb.execute('python gdb.execute(...".


> I think that we can avoid this duplication by renaming
> skip_python_tests, adding it a prompt_re parameter, and
> using that instead of $gdb_prompt.  Something like:
> 
> proc skip_python_tests {} {
>    skip_python_tests_prompt "$gdb_prompt $"
> }
> 
> proc mi_skip_python_tests {
>    skip_python_tests_prompt "$mi_gdb_prompt$"
> }
> 
> Did you try that?

done

--- 8< ---
Subject: [PATCH] mi-out: Implement mi redirection using a stack.

The current implementation doesn't support nested redirections because it only
has an "original_buffer" where it saves the current stream when a redirection is
done. To overcome this limitation, this patch reimplements it using a stack of
streams.

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* mi/mi-out.c (ui_filep): New typedef.
	(DEF_VEC_P (ui_filep)): New type.
	(struct ui_out_data): <buffer> Remove field.
	<original_buffer> Remove field.
	<streams>: New field.
	(mi_field_string): Get the output stream from the top of the
	stack of streams.
	(mi_field_fmt): Likewise.
	(mi_flush): Likewise.
	(field_separator): Likewise.
	(mi_open): Likewise.
	(mi_close): Likewise.
	(mi_out_buffered): Likewise.
	(mi_out_rewind): Likewise.
	(mi_out_put): Likewise.
	(mi_out_new): Initialize the stack of streams. Push a newly
	created stream on the stack. Remove obsolete comment.
	(mi_redirect): Reimplement using push and pop operations.

gdb/testsuite/
2014-08-05  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* gdb.mi/mi-logging.exp: Add test for nested mi redirection.
	* lib/gdb.exp: (skip_python_tests_prompt): New function.
	(skip_python_tests): Use skip_python_tests_prompt.
	* lib/mi-support.exp: (mi_skip_python_tests): New function.
---
 gdb/mi/mi-out.c                     |   77 ++++++++++++++++++++---------------
 gdb/testsuite/gdb.mi/mi-logging.exp |   14 +++++++
 gdb/testsuite/lib/gdb.exp           |   20 +++++----
 gdb/testsuite/lib/mi-support.exp    |    7 ++++
 4 files changed, 77 insertions(+), 41 deletions(-)

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 6ec41e6..6731509 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -22,14 +22,17 @@
 #include "defs.h"
 #include "ui-out.h"
 #include "mi-out.h"
+#include "vec.h"
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
 
 struct ui_out_data
   {
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
-    struct ui_file *buffer;
-    struct ui_file *original_buffer;
+    VEC (ui_filep) *streams;
   };
 typedef struct ui_out_data mi_out_data;
 
@@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
 		 enum ui_align align, const char *fldname, const char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=", fldname);
-  fprintf_unfiltered (data->buffer, "\"");
+    fprintf_unfiltered (stream, "%s=", fldname);
+  fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', data->buffer);
-  fprintf_unfiltered (data->buffer, "\"");
+    fputstr_unfiltered (string, '"', stream);
+  fprintf_unfiltered (stream, "\"");
 }
 
 /* This is the only field function that does not align.  */
@@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 	      const char *format, va_list args)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
+    fprintf_unfiltered (stream, "%s=\"", fldname);
   else
-    fputs_unfiltered ("\"", data->buffer);
-  vfprintf_unfiltered (data->buffer, format, args);
-  fputs_unfiltered ("\"", data->buffer);
+    fputs_unfiltered ("\"", stream);
+  vfprintf_unfiltered (stream, format, args);
+  fputs_unfiltered ("\"", stream);
 }
 
 void
@@ -275,8 +284,9 @@ void
 mi_flush (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  gdb_flush (data->buffer);
+  gdb_flush (stream);
 }
 
 int
@@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
   mi_out_data *data = ui_out_data (uiout);
 
   if (outstream != NULL)
-    {
-      data->original_buffer = data->buffer;
-      data->buffer = outstream;
-    }
-  else if (data->original_buffer != NULL)
-    {
-      data->buffer = data->original_buffer;
-      data->original_buffer = NULL;
-    }
+    VEC_safe_push (ui_filep, data->streams, outstream);
+  else
+    VEC_pop (ui_filep, data->streams);
 
   return 0;
 }
@@ -306,29 +310,31 @@ static void
 field_separator (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   if (data->suppress_field_separator)
     data->suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', data->buffer);
+    fputc_unfiltered (',', stream);
 }
 
 static void
 mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   field_separator (uiout);
   data->suppress_field_separator = 1;
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    fprintf_unfiltered (stream, "%s=", name);
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('{', data->buffer);
+      fputc_unfiltered ('{', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      fputc_unfiltered ('[', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -339,14 +345,15 @@ static void
 mi_close (struct ui_out *uiout, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('}', data->buffer);
+      fputc_unfiltered ('}', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      fputc_unfiltered (']', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -360,8 +367,9 @@ void
 mi_out_buffered (struct ui_out *uiout, char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  fprintf_unfiltered (data->buffer, "%s", string);
+  fprintf_unfiltered (stream, "%s", string);
 }
 
 /* Clear the buffer.  */
@@ -370,8 +378,9 @@ void
 mi_out_rewind (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_rewind (data->buffer);
+  ui_file_rewind (stream);
 }
 
 /* Dump the buffer onto the specified stream.  */
@@ -386,9 +395,10 @@ void
 mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_put (data->buffer, do_write, stream);
-  ui_file_rewind (data->buffer);
+  ui_file_put (current_stream, do_write, stream);
+  ui_file_rewind (current_stream);
 }
 
 /* Return the current MI version.  */
@@ -407,13 +417,14 @@ struct ui_out *
 mi_out_new (int mi_version)
 {
   int flags = 0;
+  struct ui_file *new_stream;
 
   mi_out_data *data = XNEW (mi_out_data);
   data->suppress_field_separator = 0;
   data->suppress_output = 0;
   data->mi_version = mi_version;
-  /* FIXME: This code should be using a ``string_file'' and not the
-     TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  data->streams = NULL;
+  new_stream = mem_fileopen ();
+  VEC_safe_push (ui_filep, data->streams, new_stream);
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
index d5e4193..bd884d3 100644
--- a/gdb/testsuite/gdb.mi/mi-logging.exp
+++ b/gdb/testsuite/gdb.mi/mi-logging.exp
@@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
     fail "Redirect log file contents"
 }
 
+# This triggers a nested mi_ui_out redirection, by executing a python command
+# inside another python command, both of which are run with to_string = True.
+if ![mi_skip_python_tests] {
+    mi_gdb_test "-break-insert do_nothing" ".*"
+    mi_gdb_test "commands \$bpnum\npython gdb.execute('python gdb.execute(\"help\", True, True)', True, True)\nend" ".*"
+
+    mi_send_resuming_command "exec-continue" "continue"
+
+    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
+
+    # This will crash gdb if redirection is not done properly.
+    mi_gdb_test "help" ".*" "nested redirect"
+}
+
 mi_gdb_exit
 
 remote_file host delete $milogfile
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8cb98ae..8bb2d23 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1603,34 +1603,33 @@ proc skip_d_tests {} {
 
 # Return a 1 for configurations that do not support Python scripting.
 
-proc skip_python_tests {} {
-    global gdb_prompt
+proc skip_python_tests_prompt { prompt_re } {
     global gdb_py_is_py3k
     global gdb_py_is_py24
 
     gdb_test_multiple "python print ('test')" "verify python support" {
-	-re "not supported.*$gdb_prompt $"	{
+	-re "not supported.*$prompt_re"	{
 	    unsupported "Python support is disabled."
 	    return 1
 	}
-	-re "$gdb_prompt $"	{}
+	-re "$prompt_re"	{}
     }
 
     set gdb_py_is_py24 0
     gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
-	-re "3.*$gdb_prompt $"	{
+	-re "3.*$prompt_re"	{
             set gdb_py_is_py3k 1
         }
-	-re ".*$gdb_prompt $"	{
+	-re ".*$prompt_re"	{
             set gdb_py_is_py3k 0
         }
     }
     if { $gdb_py_is_py3k == 0 } {
         gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
-	    -re "\[45\].*$gdb_prompt $" {
+	    -re "\[45\].*$prompt_re" {
                 set gdb_py_is_py24 1
             }
-	    -re ".*$gdb_prompt $" {
+	    -re ".*$prompt_re" {
                 set gdb_py_is_py24 0
             }
         }
@@ -1639,6 +1638,11 @@ proc skip_python_tests {} {
     return 0
 }
 
+proc skip_python_tests { } {
+    global gdb_prompt
+    skip_python_tests_prompt "$gdb_prompt $"
+}
+
 # Return a 1 if we should skip shared library tests.
 
 proc skip_shlib_tests {} {
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 204f1e8..80a3627 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2491,3 +2491,10 @@ proc mi_make_breakpoint_table {bp_list} {
     # Assemble the final regexp.
     return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}"
 }
+
+# Return a 1 for configurations that do not support Python scripting.
+
+proc mi_skip_python_tests {} {
+    global mi_gdb_prompt
+    skip_python_tests_prompt "$mi_gdb_prompt$"
+}
-- 
1.7.9.5


 

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

ping

On 08/05/2014 04:55 PM, Adrian Sendroiu wrote:
>> It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a
>> test that adds another breakpoint before this, this test stops being
>> effective, silently.
> 
> done
> 
>>> +    mi_gdb_test "-exec-continue" ".*"
>>
>> This should use mi_send_resuming_command/mi_expect_stop
>> or mi_execute_to, so that the test works when the whole MI
>> testsuite is run in async mode.
> 
> done. Although I ran into another problem here, because mi_expect_stop expects a
> message that looks like *stopped + prompt, while in my case it was something like
> *stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler
> test case: just executing two python commands nested inside one another, like
> "python gdb.execute('python gdb.execute(...".
> 
> 
>> I think that we can avoid this duplication by renaming
>> skip_python_tests, adding it a prompt_re parameter, and
>> using that instead of $gdb_prompt.  Something like:
>>
>> proc skip_python_tests {} {
>>    skip_python_tests_prompt "$gdb_prompt $"
>> }
>>
>> proc mi_skip_python_tests {
>>    skip_python_tests_prompt "$mi_gdb_prompt$"
>> }
>>
>> Did you try that?
> 
> done
> 
> --- 8< ---
> Subject: [PATCH] mi-out: Implement mi redirection using a stack.
> 
> The current implementation doesn't support nested redirections because it only
> has an "original_buffer" where it saves the current stream when a redirection is
> done. To overcome this limitation, this patch reimplements it using a stack of
> streams.
> 
> gdb/
> 2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>
> 
> 	* mi/mi-out.c (ui_filep): New typedef.
> 	(DEF_VEC_P (ui_filep)): New type.
> 	(struct ui_out_data): <buffer> Remove field.
> 	<original_buffer> Remove field.
> 	<streams>: New field.
> 	(mi_field_string): Get the output stream from the top of the
> 	stack of streams.
> 	(mi_field_fmt): Likewise.
> 	(mi_flush): Likewise.
> 	(field_separator): Likewise.
> 	(mi_open): Likewise.
> 	(mi_close): Likewise.
> 	(mi_out_buffered): Likewise.
> 	(mi_out_rewind): Likewise.
> 	(mi_out_put): Likewise.
> 	(mi_out_new): Initialize the stack of streams. Push a newly
> 	created stream on the stack. Remove obsolete comment.
> 	(mi_redirect): Reimplement using push and pop operations.
> 
> gdb/testsuite/
> 2014-08-05  Adrian Sendroiu  <adrian.sendroiu@freescale.com>
> 
> 	* gdb.mi/mi-logging.exp: Add test for nested mi redirection.
> 	* lib/gdb.exp: (skip_python_tests_prompt): New function.
> 	(skip_python_tests): Use skip_python_tests_prompt.
> 	* lib/mi-support.exp: (mi_skip_python_tests): New function.
> ---
>  gdb/mi/mi-out.c                     |   77 ++++++++++++++++++++---------------
>  gdb/testsuite/gdb.mi/mi-logging.exp |   14 +++++++
>  gdb/testsuite/lib/gdb.exp           |   20 +++++----
>  gdb/testsuite/lib/mi-support.exp    |    7 ++++
>  4 files changed, 77 insertions(+), 41 deletions(-)
> 
> diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
> index 6ec41e6..6731509 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -22,14 +22,17 @@
>  #include "defs.h"
>  #include "ui-out.h"
>  #include "mi-out.h"
> +#include "vec.h"
> +
> +typedef struct ui_file *ui_filep;
> +DEF_VEC_P (ui_filep);
>  
>  struct ui_out_data
>    {
>      int suppress_field_separator;
>      int suppress_output;
>      int mi_version;
> -    struct ui_file *buffer;
> -    struct ui_file *original_buffer;
> +    VEC (ui_filep) *streams;
>    };
>  typedef struct ui_out_data mi_out_data;
>  
> @@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
>  		 enum ui_align align, const char *fldname, const char *string)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream;
>  
>    if (data->suppress_output)
>      return;
>  
> +  stream = VEC_last (ui_filep, data->streams);
> +
>    field_separator (uiout);
>    if (fldname)
> -    fprintf_unfiltered (data->buffer, "%s=", fldname);
> -  fprintf_unfiltered (data->buffer, "\"");
> +    fprintf_unfiltered (stream, "%s=", fldname);
> +  fprintf_unfiltered (stream, "\"");
>    if (string)
> -    fputstr_unfiltered (string, '"', data->buffer);
> -  fprintf_unfiltered (data->buffer, "\"");
> +    fputstr_unfiltered (string, '"', stream);
> +  fprintf_unfiltered (stream, "\"");
>  }
>  
>  /* This is the only field function that does not align.  */
> @@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
>  	      const char *format, va_list args)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream;
>  
>    if (data->suppress_output)
>      return;
>  
> +  stream = VEC_last (ui_filep, data->streams);
> +
>    field_separator (uiout);
>    if (fldname)
> -    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
> +    fprintf_unfiltered (stream, "%s=\"", fldname);
>    else
> -    fputs_unfiltered ("\"", data->buffer);
> -  vfprintf_unfiltered (data->buffer, format, args);
> -  fputs_unfiltered ("\"", data->buffer);
> +    fputs_unfiltered ("\"", stream);
> +  vfprintf_unfiltered (stream, format, args);
> +  fputs_unfiltered ("\"", stream);
>  }
>  
>  void
> @@ -275,8 +284,9 @@ void
>  mi_flush (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  gdb_flush (data->buffer);
> +  gdb_flush (stream);
>  }
>  
>  int
> @@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
>    mi_out_data *data = ui_out_data (uiout);
>  
>    if (outstream != NULL)
> -    {
> -      data->original_buffer = data->buffer;
> -      data->buffer = outstream;
> -    }
> -  else if (data->original_buffer != NULL)
> -    {
> -      data->buffer = data->original_buffer;
> -      data->original_buffer = NULL;
> -    }
> +    VEC_safe_push (ui_filep, data->streams, outstream);
> +  else
> +    VEC_pop (ui_filep, data->streams);
>  
>    return 0;
>  }
> @@ -306,29 +310,31 @@ static void
>  field_separator (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    if (data->suppress_field_separator)
>      data->suppress_field_separator = 0;
>    else
> -    fputc_unfiltered (',', data->buffer);
> +    fputc_unfiltered (',', stream);
>  }
>  
>  static void
>  mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    field_separator (uiout);
>    data->suppress_field_separator = 1;
>    if (name)
> -    fprintf_unfiltered (data->buffer, "%s=", name);
> +    fprintf_unfiltered (stream, "%s=", name);
>    switch (type)
>      {
>      case ui_out_type_tuple:
> -      fputc_unfiltered ('{', data->buffer);
> +      fputc_unfiltered ('{', stream);
>        break;
>      case ui_out_type_list:
> -      fputc_unfiltered ('[', data->buffer);
> +      fputc_unfiltered ('[', stream);
>        break;
>      default:
>        internal_error (__FILE__, __LINE__, _("bad switch"));
> @@ -339,14 +345,15 @@ static void
>  mi_close (struct ui_out *uiout, enum ui_out_type type)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    switch (type)
>      {
>      case ui_out_type_tuple:
> -      fputc_unfiltered ('}', data->buffer);
> +      fputc_unfiltered ('}', stream);
>        break;
>      case ui_out_type_list:
> -      fputc_unfiltered (']', data->buffer);
> +      fputc_unfiltered (']', stream);
>        break;
>      default:
>        internal_error (__FILE__, __LINE__, _("bad switch"));
> @@ -360,8 +367,9 @@ void
>  mi_out_buffered (struct ui_out *uiout, char *string)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  fprintf_unfiltered (data->buffer, "%s", string);
> +  fprintf_unfiltered (stream, "%s", string);
>  }
>  
>  /* Clear the buffer.  */
> @@ -370,8 +378,9 @@ void
>  mi_out_rewind (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  ui_file_rewind (data->buffer);
> +  ui_file_rewind (stream);
>  }
>  
>  /* Dump the buffer onto the specified stream.  */
> @@ -386,9 +395,10 @@ void
>  mi_out_put (struct ui_out *uiout, struct ui_file *stream)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
>  
> -  ui_file_put (data->buffer, do_write, stream);
> -  ui_file_rewind (data->buffer);
> +  ui_file_put (current_stream, do_write, stream);
> +  ui_file_rewind (current_stream);
>  }
>  
>  /* Return the current MI version.  */
> @@ -407,13 +417,14 @@ struct ui_out *
>  mi_out_new (int mi_version)
>  {
>    int flags = 0;
> +  struct ui_file *new_stream;
>  
>    mi_out_data *data = XNEW (mi_out_data);
>    data->suppress_field_separator = 0;
>    data->suppress_output = 0;
>    data->mi_version = mi_version;
> -  /* FIXME: This code should be using a ``string_file'' and not the
> -     TUI buffer hack. */
> -  data->buffer = mem_fileopen ();
> +  data->streams = NULL;
> +  new_stream = mem_fileopen ();
> +  VEC_safe_push (ui_filep, data->streams, new_stream);
>    return ui_out_new (&mi_ui_out_impl, data, flags);
>  }
> diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
> index d5e4193..bd884d3 100644
> --- a/gdb/testsuite/gdb.mi/mi-logging.exp
> +++ b/gdb/testsuite/gdb.mi/mi-logging.exp
> @@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
>      fail "Redirect log file contents"
>  }
>  
> +# This triggers a nested mi_ui_out redirection, by executing a python command
> +# inside another python command, both of which are run with to_string = True.
> +if ![mi_skip_python_tests] {
> +    mi_gdb_test "-break-insert do_nothing" ".*"
> +    mi_gdb_test "commands \$bpnum\npython gdb.execute('python gdb.execute(\"help\", True, True)', True, True)\nend" ".*"
> +
> +    mi_send_resuming_command "exec-continue" "continue"
> +
> +    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
> +
> +    # This will crash gdb if redirection is not done properly.
> +    mi_gdb_test "help" ".*" "nested redirect"
> +}
> +
>  mi_gdb_exit
>  
>  remote_file host delete $milogfile
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 8cb98ae..8bb2d23 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1603,34 +1603,33 @@ proc skip_d_tests {} {
>  
>  # Return a 1 for configurations that do not support Python scripting.
>  
> -proc skip_python_tests {} {
> -    global gdb_prompt
> +proc skip_python_tests_prompt { prompt_re } {
>      global gdb_py_is_py3k
>      global gdb_py_is_py24
>  
>      gdb_test_multiple "python print ('test')" "verify python support" {
> -	-re "not supported.*$gdb_prompt $"	{
> +	-re "not supported.*$prompt_re"	{
>  	    unsupported "Python support is disabled."
>  	    return 1
>  	}
> -	-re "$gdb_prompt $"	{}
> +	-re "$prompt_re"	{}
>      }
>  
>      set gdb_py_is_py24 0
>      gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
> -	-re "3.*$gdb_prompt $"	{
> +	-re "3.*$prompt_re"	{
>              set gdb_py_is_py3k 1
>          }
> -	-re ".*$gdb_prompt $"	{
> +	-re ".*$prompt_re"	{
>              set gdb_py_is_py3k 0
>          }
>      }
>      if { $gdb_py_is_py3k == 0 } {
>          gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
> -	    -re "\[45\].*$gdb_prompt $" {
> +	    -re "\[45\].*$prompt_re" {
>                  set gdb_py_is_py24 1
>              }
> -	    -re ".*$gdb_prompt $" {
> +	    -re ".*$prompt_re" {
>                  set gdb_py_is_py24 0
>              }
>          }
> @@ -1639,6 +1638,11 @@ proc skip_python_tests {} {
>      return 0
>  }
>  
> +proc skip_python_tests { } {
> +    global gdb_prompt
> +    skip_python_tests_prompt "$gdb_prompt $"
> +}
> +
>  # Return a 1 if we should skip shared library tests.
>  
>  proc skip_shlib_tests {} {
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 204f1e8..80a3627 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -2491,3 +2491,10 @@ proc mi_make_breakpoint_table {bp_list} {
>      # Assemble the final regexp.
>      return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}"
>  }
> +
> +# Return a 1 for configurations that do not support Python scripting.
> +
> +proc mi_skip_python_tests {} {
> +    global mi_gdb_prompt
> +    skip_python_tests_prompt "$mi_gdb_prompt$"
> +}
> 

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

Hi Adrian,

On 08/05/2014 02:55 PM, Adrian Sendroiu wrote:
>> It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a
>> test that adds another breakpoint before this, this test stops being
>> effective, silently.
> 
> done
> 
>>> +    mi_gdb_test "-exec-continue" ".*"
>>
>> This should use mi_send_resuming_command/mi_expect_stop
>> or mi_execute_to, so that the test works when the whole MI
>> testsuite is run in async mode.
> 
> done. Although I ran into another problem here, because mi_expect_stop expects a
> message that looks like *stopped + prompt, while in my case it was something like
> *stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler
> test case: just executing two python commands nested inside one another, like
> "python gdb.execute('python gdb.execute(...".
> 
> 
>> I think that we can avoid this duplication by renaming
>> skip_python_tests, adding it a prompt_re parameter, and
>> using that instead of $gdb_prompt.  Something like:
>>
>> proc skip_python_tests {} {
>>    skip_python_tests_prompt "$gdb_prompt $"
>> }
>>
>> proc mi_skip_python_tests {
>>    skip_python_tests_prompt "$mi_gdb_prompt$"
>> }
>>
>> Did you try that?
> 
> done
> 

Thanks.

I'm getting these with this patch:

Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-logging.exp ...
PASS: gdb.mi/mi-logging.exp: breakpoint at main
PASS: gdb.mi/mi-logging.exp: mi runto main
PASS: gdb.mi/mi-logging.exp: logging on
PASS: gdb.mi/mi-logging.exp: logged step
PASS: gdb.mi/mi-logging.exp: logged next
ERROR: Got interactive prompt.
UNRESOLVED: gdb.mi/mi-logging.exp: logging off
PASS: gdb.mi/mi-logging.exp: Log file contents
ERROR: Got interactive prompt.
UNRESOLVED: gdb.mi/mi-logging.exp: redirect logging on
ERROR: Got interactive prompt.
UNRESOLVED: gdb.mi/mi-logging.exp: redirect logging off
FAIL: gdb.mi/mi-logging.exp: Redirect log file contents
FAIL: gdb.mi/mi-logging.exp: verify python support (GDB internal error)
ERROR: Could not resync from internal error (timeout)
UNRESOLVED: gdb.mi/mi-logging.exp: check if python 3 (got interactive prompt)
ERROR: tcl error sourcing /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-logging.exp.
ERROR: can't read "gdb_py_is_py3k": no such variable
    while executing
...


gdb.log shows:

Expecting: ^(-gdb-set logging off[
]+)?(.*[
]+[(]gdb[)]
[ ]*)
-gdb-set logging off
^done~"/home/pedro/gdb/mygit/src/gdb/mi/mi-out.c:398: internal-error: VEC_ui_filep_last: Assertion `last' failed.\nA problem internal to GDB has been detected,\nfurther deb
ugging may prove unreliable.\nQuit this debugging session? "
~"(y or n) "
ERROR: Got interactive prompt.
UNRESOLVED: gdb.mi/mi-logging.exp: logging off
PASS: gdb.mi/mi-logging.exp: Log file contents
Expecting: ^(-gdb-set logging redirect on[
]+)?(.*[
]+[(]gdb[)]
[ ]*)

Any idea what went wrong ?
Thanks,
Pedro Alves

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

There's also the 1/2 patch from the series that needs to be applied
before this.

https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html

Adrian

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

On 08/05/2014 02:55 PM, Adrian Sendroiu wrote:

> There's also the 1/2 patch from the series that needs to be applied
> before this.
>
> https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html

Ah.  Silly me, completely missed that.  The test indeed passes cleanly
for me with that applied.

> +    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
> +
> +    # This will crash gdb if redirection is not done properly.
> +    mi_gdb_test "help" ".*" "nested redirect"

I think it'd be good to replace this ".*" with a stricter match, just
in case something goes wrong with undoing the redirection, but nothing
crashes.  Something like (untested):

 mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect"

This is OK with a change along those lines.

Thanks!

Pedro Alves

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Sergio Durigan Junior]

On Monday, September 08 2014, Pedro Alves wrote:

>> +    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
>> +
>> +    # This will crash gdb if redirection is not done properly.
>> +    mi_gdb_test "help" ".*" "nested redirect"
>
> I think it'd be good to replace this ".*" with a stricter match, just
> in case something goes wrong with undoing the redirection, but nothing
> crashes.  Something like (untested):
>
>  mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect"

I've been seeing some cases like this in the list (i.e., writing tests
using ".*" a lot).  Maybe this is a good thing to mention in our wiki...

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

> I think it'd be good to replace this ".*" with a stricter match, just
> in case something goes wrong with undoing the redirection, but nothing
> crashes.  Something like (untested):
> 
>  mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect"
> 
> This is OK with a change along those lines.

Ok, I changed to mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect"

Adrian

--- 8< ---
Subject: [PATCH] mi-out: Implement mi redirection using a stack.

The current implementation doesn't support nested redirections because it only
has an "original_buffer" where it saves the current stream when a redirection is
done. To overcome this limitation, this patch reimplements it using a stack of
streams.

gdb/
2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* mi/mi-out.c (ui_filep): New typedef.
	(DEF_VEC_P (ui_filep)): New type.
	(struct ui_out_data): <buffer> Remove field.
	<original_buffer> Remove field.
	<streams>: New field.
	(mi_field_string): Get the output stream from the top of the
	stack of streams.
	(mi_field_fmt): Likewise.
	(mi_flush): Likewise.
	(field_separator): Likewise.
	(mi_open): Likewise.
	(mi_close): Likewise.
	(mi_out_buffered): Likewise.
	(mi_out_rewind): Likewise.
	(mi_out_put): Likewise.
	(mi_out_new): Initialize the stack of streams. Push a newly
	created stream on the stack. Remove obsolete comment.
	(mi_redirect): Reimplement using push and pop operations.

gdb/testsuite/
2014-08-05  Adrian Sendroiu  <adrian.sendroiu@freescale.com>

	* gdb.mi/mi-logging.exp: Add test for nested mi redirection.
	* lib/gdb.exp: (skip_python_tests_prompt): New function.
	(skip_python_tests): Use skip_python_tests_prompt.
	* lib/mi-support.exp: (mi_skip_python_tests): New function.
---
 gdb/mi/mi-out.c                     | 77 +++++++++++++++++++++----------------
 gdb/testsuite/gdb.mi/mi-logging.exp | 14 +++++++
 gdb/testsuite/lib/gdb.exp           | 20 ++++++----
 gdb/testsuite/lib/mi-support.exp    |  7 ++++
 4 files changed, 77 insertions(+), 41 deletions(-)

diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index 6ec41e6..6731509 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -22,14 +22,17 @@
 #include "defs.h"
 #include "ui-out.h"
 #include "mi-out.h"
+#include "vec.h"
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
 
 struct ui_out_data
   {
     int suppress_field_separator;
     int suppress_output;
     int mi_version;
-    struct ui_file *buffer;
-    struct ui_file *original_buffer;
+    VEC (ui_filep) *streams;
   };
 typedef struct ui_out_data mi_out_data;
 
@@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
 		 enum ui_align align, const char *fldname, const char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=", fldname);
-  fprintf_unfiltered (data->buffer, "\"");
+    fprintf_unfiltered (stream, "%s=", fldname);
+  fprintf_unfiltered (stream, "\"");
   if (string)
-    fputstr_unfiltered (string, '"', data->buffer);
-  fprintf_unfiltered (data->buffer, "\"");
+    fputstr_unfiltered (string, '"', stream);
+  fprintf_unfiltered (stream, "\"");
 }
 
 /* This is the only field function that does not align.  */
@@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
 	      const char *format, va_list args)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream;
 
   if (data->suppress_output)
     return;
 
+  stream = VEC_last (ui_filep, data->streams);
+
   field_separator (uiout);
   if (fldname)
-    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
+    fprintf_unfiltered (stream, "%s=\"", fldname);
   else
-    fputs_unfiltered ("\"", data->buffer);
-  vfprintf_unfiltered (data->buffer, format, args);
-  fputs_unfiltered ("\"", data->buffer);
+    fputs_unfiltered ("\"", stream);
+  vfprintf_unfiltered (stream, format, args);
+  fputs_unfiltered ("\"", stream);
 }
 
 void
@@ -275,8 +284,9 @@ void
 mi_flush (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  gdb_flush (data->buffer);
+  gdb_flush (stream);
 }
 
 int
@@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
   mi_out_data *data = ui_out_data (uiout);
 
   if (outstream != NULL)
-    {
-      data->original_buffer = data->buffer;
-      data->buffer = outstream;
-    }
-  else if (data->original_buffer != NULL)
-    {
-      data->buffer = data->original_buffer;
-      data->original_buffer = NULL;
-    }
+    VEC_safe_push (ui_filep, data->streams, outstream);
+  else
+    VEC_pop (ui_filep, data->streams);
 
   return 0;
 }
@@ -306,29 +310,31 @@ static void
 field_separator (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   if (data->suppress_field_separator)
     data->suppress_field_separator = 0;
   else
-    fputc_unfiltered (',', data->buffer);
+    fputc_unfiltered (',', stream);
 }
 
 static void
 mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   field_separator (uiout);
   data->suppress_field_separator = 1;
   if (name)
-    fprintf_unfiltered (data->buffer, "%s=", name);
+    fprintf_unfiltered (stream, "%s=", name);
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('{', data->buffer);
+      fputc_unfiltered ('{', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered ('[', data->buffer);
+      fputc_unfiltered ('[', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -339,14 +345,15 @@ static void
 mi_close (struct ui_out *uiout, enum ui_out_type type)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
   switch (type)
     {
     case ui_out_type_tuple:
-      fputc_unfiltered ('}', data->buffer);
+      fputc_unfiltered ('}', stream);
       break;
     case ui_out_type_list:
-      fputc_unfiltered (']', data->buffer);
+      fputc_unfiltered (']', stream);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("bad switch"));
@@ -360,8 +367,9 @@ void
 mi_out_buffered (struct ui_out *uiout, char *string)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  fprintf_unfiltered (data->buffer, "%s", string);
+  fprintf_unfiltered (stream, "%s", string);
 }
 
 /* Clear the buffer.  */
@@ -370,8 +378,9 @@ void
 mi_out_rewind (struct ui_out *uiout)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_rewind (data->buffer);
+  ui_file_rewind (stream);
 }
 
 /* Dump the buffer onto the specified stream.  */
@@ -386,9 +395,10 @@ void
 mi_out_put (struct ui_out *uiout, struct ui_file *stream)
 {
   mi_out_data *data = ui_out_data (uiout);
+  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
 
-  ui_file_put (data->buffer, do_write, stream);
-  ui_file_rewind (data->buffer);
+  ui_file_put (current_stream, do_write, stream);
+  ui_file_rewind (current_stream);
 }
 
 /* Return the current MI version.  */
@@ -407,13 +417,14 @@ struct ui_out *
 mi_out_new (int mi_version)
 {
   int flags = 0;
+  struct ui_file *new_stream;
 
   mi_out_data *data = XNEW (mi_out_data);
   data->suppress_field_separator = 0;
   data->suppress_output = 0;
   data->mi_version = mi_version;
-  /* FIXME: This code should be using a ``string_file'' and not the
-     TUI buffer hack. */
-  data->buffer = mem_fileopen ();
+  data->streams = NULL;
+  new_stream = mem_fileopen ();
+  VEC_safe_push (ui_filep, data->streams, new_stream);
   return ui_out_new (&mi_ui_out_impl, data, flags);
 }
diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
index d5e4193..27ff1fe 100644
--- a/gdb/testsuite/gdb.mi/mi-logging.exp
+++ b/gdb/testsuite/gdb.mi/mi-logging.exp
@@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
     fail "Redirect log file contents"
 }
 
+# This triggers a nested mi_ui_out redirection, by executing a python command
+# inside another python command, both of which are run with to_string = True.
+if ![mi_skip_python_tests] {
+    mi_gdb_test "-break-insert do_nothing" ".*"
+    mi_gdb_test "commands \$bpnum\npython gdb.execute('python gdb.execute(\"help\", True, True)', True, True)\nend" ".*"
+
+    mi_send_resuming_command "exec-continue" "continue"
+
+    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
+
+    # This will crash gdb if redirection is not done properly.
+    mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect"
+}
+
 mi_gdb_exit
 
 remote_file host delete $milogfile
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1019ecd..4e067cc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1603,34 +1603,33 @@ proc skip_d_tests {} {
 
 # Return a 1 for configurations that do not support Python scripting.
 
-proc skip_python_tests {} {
-    global gdb_prompt
+proc skip_python_tests_prompt { prompt_re } {
     global gdb_py_is_py3k
     global gdb_py_is_py24
 
     gdb_test_multiple "python print ('test')" "verify python support" {
-	-re "not supported.*$gdb_prompt $"	{
+	-re "not supported.*$prompt_re"	{
 	    unsupported "Python support is disabled."
 	    return 1
 	}
-	-re "$gdb_prompt $"	{}
+	-re "$prompt_re"	{}
     }
 
     set gdb_py_is_py24 0
     gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
-	-re "3.*$gdb_prompt $"	{
+	-re "3.*$prompt_re"	{
             set gdb_py_is_py3k 1
         }
-	-re ".*$gdb_prompt $"	{
+	-re ".*$prompt_re"	{
             set gdb_py_is_py3k 0
         }
     }
     if { $gdb_py_is_py3k == 0 } {
         gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
-	    -re "\[45\].*$gdb_prompt $" {
+	    -re "\[45\].*$prompt_re" {
                 set gdb_py_is_py24 1
             }
-	    -re ".*$gdb_prompt $" {
+	    -re ".*$prompt_re" {
                 set gdb_py_is_py24 0
             }
         }
@@ -1639,6 +1638,11 @@ proc skip_python_tests {} {
     return 0
 }
 
+proc skip_python_tests { } {
+    global gdb_prompt
+    skip_python_tests_prompt "$gdb_prompt $"
+}
+
 # Return a 1 if we should skip shared library tests.
 
 proc skip_shlib_tests {} {
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 204f1e8..80a3627 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2491,3 +2491,10 @@ proc mi_make_breakpoint_table {bp_list} {
     # Assemble the final regexp.
     return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}"
 }
+
+# Return a 1 for configurations that do not support Python scripting.
+
+proc mi_skip_python_tests {} {
+    global mi_gdb_prompt
+    skip_python_tests_prompt "$mi_gdb_prompt$"
+}
-- 
1.8.5.5

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Adrian Sendroiu]

ping

Adrian Sendroiu <molecula2788@gmail.com> writes:

>> I think it'd be good to replace this ".*" with a stricter match, just
>> in case something goes wrong with undoing the redirection, but nothing
>> crashes.  Something like (untested):
>> 
>>  mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect"
>> 
>> This is OK with a change along those lines.
>
> Ok, I changed to mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect"
>
> Adrian
>
> --- 8< ---
> Subject: [PATCH] mi-out: Implement mi redirection using a stack.
>
> The current implementation doesn't support nested redirections because it only
> has an "original_buffer" where it saves the current stream when a redirection is
> done. To overcome this limitation, this patch reimplements it using a stack of
> streams.
>
> gdb/
> 2014-07-23  Adrian Sendroiu  <adrian.sendroiu@freescale.com>
>
> 	* mi/mi-out.c (ui_filep): New typedef.
> 	(DEF_VEC_P (ui_filep)): New type.
> 	(struct ui_out_data): <buffer> Remove field.
> 	<original_buffer> Remove field.
> 	<streams>: New field.
> 	(mi_field_string): Get the output stream from the top of the
> 	stack of streams.
> 	(mi_field_fmt): Likewise.
> 	(mi_flush): Likewise.
> 	(field_separator): Likewise.
> 	(mi_open): Likewise.
> 	(mi_close): Likewise.
> 	(mi_out_buffered): Likewise.
> 	(mi_out_rewind): Likewise.
> 	(mi_out_put): Likewise.
> 	(mi_out_new): Initialize the stack of streams. Push a newly
> 	created stream on the stack. Remove obsolete comment.
> 	(mi_redirect): Reimplement using push and pop operations.
>
> gdb/testsuite/
> 2014-08-05  Adrian Sendroiu  <adrian.sendroiu@freescale.com>
>
> 	* gdb.mi/mi-logging.exp: Add test for nested mi redirection.
> 	* lib/gdb.exp: (skip_python_tests_prompt): New function.
> 	(skip_python_tests): Use skip_python_tests_prompt.
> 	* lib/mi-support.exp: (mi_skip_python_tests): New function.
> ---
>  gdb/mi/mi-out.c                     | 77 +++++++++++++++++++++----------------
>  gdb/testsuite/gdb.mi/mi-logging.exp | 14 +++++++
>  gdb/testsuite/lib/gdb.exp           | 20 ++++++----
>  gdb/testsuite/lib/mi-support.exp    |  7 ++++
>  4 files changed, 77 insertions(+), 41 deletions(-)
>
> diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
> index 6ec41e6..6731509 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -22,14 +22,17 @@
>  #include "defs.h"
>  #include "ui-out.h"
>  #include "mi-out.h"
> +#include "vec.h"
> +
> +typedef struct ui_file *ui_filep;
> +DEF_VEC_P (ui_filep);
>  
>  struct ui_out_data
>    {
>      int suppress_field_separator;
>      int suppress_output;
>      int mi_version;
> -    struct ui_file *buffer;
> -    struct ui_file *original_buffer;
> +    VEC (ui_filep) *streams;
>    };
>  typedef struct ui_out_data mi_out_data;
>  
> @@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width,
>  		 enum ui_align align, const char *fldname, const char *string)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream;
>  
>    if (data->suppress_output)
>      return;
>  
> +  stream = VEC_last (ui_filep, data->streams);
> +
>    field_separator (uiout);
>    if (fldname)
> -    fprintf_unfiltered (data->buffer, "%s=", fldname);
> -  fprintf_unfiltered (data->buffer, "\"");
> +    fprintf_unfiltered (stream, "%s=", fldname);
> +  fprintf_unfiltered (stream, "\"");
>    if (string)
> -    fputstr_unfiltered (string, '"', data->buffer);
> -  fprintf_unfiltered (data->buffer, "\"");
> +    fputstr_unfiltered (string, '"', stream);
> +  fprintf_unfiltered (stream, "\"");
>  }
>  
>  /* This is the only field function that does not align.  */
> @@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width,
>  	      const char *format, va_list args)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream;
>  
>    if (data->suppress_output)
>      return;
>  
> +  stream = VEC_last (ui_filep, data->streams);
> +
>    field_separator (uiout);
>    if (fldname)
> -    fprintf_unfiltered (data->buffer, "%s=\"", fldname);
> +    fprintf_unfiltered (stream, "%s=\"", fldname);
>    else
> -    fputs_unfiltered ("\"", data->buffer);
> -  vfprintf_unfiltered (data->buffer, format, args);
> -  fputs_unfiltered ("\"", data->buffer);
> +    fputs_unfiltered ("\"", stream);
> +  vfprintf_unfiltered (stream, format, args);
> +  fputs_unfiltered ("\"", stream);
>  }
>  
>  void
> @@ -275,8 +284,9 @@ void
>  mi_flush (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  gdb_flush (data->buffer);
> +  gdb_flush (stream);
>  }
>  
>  int
> @@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream)
>    mi_out_data *data = ui_out_data (uiout);
>  
>    if (outstream != NULL)
> -    {
> -      data->original_buffer = data->buffer;
> -      data->buffer = outstream;
> -    }
> -  else if (data->original_buffer != NULL)
> -    {
> -      data->buffer = data->original_buffer;
> -      data->original_buffer = NULL;
> -    }
> +    VEC_safe_push (ui_filep, data->streams, outstream);
> +  else
> +    VEC_pop (ui_filep, data->streams);
>  
>    return 0;
>  }
> @@ -306,29 +310,31 @@ static void
>  field_separator (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    if (data->suppress_field_separator)
>      data->suppress_field_separator = 0;
>    else
> -    fputc_unfiltered (',', data->buffer);
> +    fputc_unfiltered (',', stream);
>  }
>  
>  static void
>  mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    field_separator (uiout);
>    data->suppress_field_separator = 1;
>    if (name)
> -    fprintf_unfiltered (data->buffer, "%s=", name);
> +    fprintf_unfiltered (stream, "%s=", name);
>    switch (type)
>      {
>      case ui_out_type_tuple:
> -      fputc_unfiltered ('{', data->buffer);
> +      fputc_unfiltered ('{', stream);
>        break;
>      case ui_out_type_list:
> -      fputc_unfiltered ('[', data->buffer);
> +      fputc_unfiltered ('[', stream);
>        break;
>      default:
>        internal_error (__FILE__, __LINE__, _("bad switch"));
> @@ -339,14 +345,15 @@ static void
>  mi_close (struct ui_out *uiout, enum ui_out_type type)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
>    switch (type)
>      {
>      case ui_out_type_tuple:
> -      fputc_unfiltered ('}', data->buffer);
> +      fputc_unfiltered ('}', stream);
>        break;
>      case ui_out_type_list:
> -      fputc_unfiltered (']', data->buffer);
> +      fputc_unfiltered (']', stream);
>        break;
>      default:
>        internal_error (__FILE__, __LINE__, _("bad switch"));
> @@ -360,8 +367,9 @@ void
>  mi_out_buffered (struct ui_out *uiout, char *string)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  fprintf_unfiltered (data->buffer, "%s", string);
> +  fprintf_unfiltered (stream, "%s", string);
>  }
>  
>  /* Clear the buffer.  */
> @@ -370,8 +378,9 @@ void
>  mi_out_rewind (struct ui_out *uiout)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *stream = VEC_last (ui_filep, data->streams);
>  
> -  ui_file_rewind (data->buffer);
> +  ui_file_rewind (stream);
>  }
>  
>  /* Dump the buffer onto the specified stream.  */
> @@ -386,9 +395,10 @@ void
>  mi_out_put (struct ui_out *uiout, struct ui_file *stream)
>  {
>    mi_out_data *data = ui_out_data (uiout);
> +  struct ui_file *current_stream = VEC_last (ui_filep, data->streams);
>  
> -  ui_file_put (data->buffer, do_write, stream);
> -  ui_file_rewind (data->buffer);
> +  ui_file_put (current_stream, do_write, stream);
> +  ui_file_rewind (current_stream);
>  }
>  
>  /* Return the current MI version.  */
> @@ -407,13 +417,14 @@ struct ui_out *
>  mi_out_new (int mi_version)
>  {
>    int flags = 0;
> +  struct ui_file *new_stream;
>  
>    mi_out_data *data = XNEW (mi_out_data);
>    data->suppress_field_separator = 0;
>    data->suppress_output = 0;
>    data->mi_version = mi_version;
> -  /* FIXME: This code should be using a ``string_file'' and not the
> -     TUI buffer hack. */
> -  data->buffer = mem_fileopen ();
> +  data->streams = NULL;
> +  new_stream = mem_fileopen ();
> +  VEC_safe_push (ui_filep, data->streams, new_stream);
>    return ui_out_new (&mi_ui_out_impl, data, flags);
>  }
> diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
> index d5e4193..27ff1fe 100644
> --- a/gdb/testsuite/gdb.mi/mi-logging.exp
> +++ b/gdb/testsuite/gdb.mi/mi-logging.exp
> @@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
>      fail "Redirect log file contents"
>  }
>  
> +# This triggers a nested mi_ui_out redirection, by executing a python command
> +# inside another python command, both of which are run with to_string = True.
> +if ![mi_skip_python_tests] {
> +    mi_gdb_test "-break-insert do_nothing" ".*"
> +    mi_gdb_test "commands \$bpnum\npython gdb.execute('python gdb.execute(\"help\", True, True)', True, True)\nend" ".*"
> +
> +    mi_send_resuming_command "exec-continue" "continue"
> +
> +    mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint"
> +
> +    # This will crash gdb if redirection is not done properly.
> +    mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect"
> +}
> +
>  mi_gdb_exit
>  
>  remote_file host delete $milogfile
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1019ecd..4e067cc 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1603,34 +1603,33 @@ proc skip_d_tests {} {
>  
>  # Return a 1 for configurations that do not support Python scripting.
>  
> -proc skip_python_tests {} {
> -    global gdb_prompt
> +proc skip_python_tests_prompt { prompt_re } {
>      global gdb_py_is_py3k
>      global gdb_py_is_py24
>  
>      gdb_test_multiple "python print ('test')" "verify python support" {
> -	-re "not supported.*$gdb_prompt $"	{
> +	-re "not supported.*$prompt_re"	{
>  	    unsupported "Python support is disabled."
>  	    return 1
>  	}
> -	-re "$gdb_prompt $"	{}
> +	-re "$prompt_re"	{}
>      }
>  
>      set gdb_py_is_py24 0
>      gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
> -	-re "3.*$gdb_prompt $"	{
> +	-re "3.*$prompt_re"	{
>              set gdb_py_is_py3k 1
>          }
> -	-re ".*$gdb_prompt $"	{
> +	-re ".*$prompt_re"	{
>              set gdb_py_is_py3k 0
>          }
>      }
>      if { $gdb_py_is_py3k == 0 } {
>          gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" {
> -	    -re "\[45\].*$gdb_prompt $" {
> +	    -re "\[45\].*$prompt_re" {
>                  set gdb_py_is_py24 1
>              }
> -	    -re ".*$gdb_prompt $" {
> +	    -re ".*$prompt_re" {
>                  set gdb_py_is_py24 0
>              }
>          }
> @@ -1639,6 +1638,11 @@ proc skip_python_tests {} {
>      return 0
>  }
>  
> +proc skip_python_tests { } {
> +    global gdb_prompt
> +    skip_python_tests_prompt "$gdb_prompt $"
> +}
> +
>  # Return a 1 if we should skip shared library tests.
>  
>  proc skip_shlib_tests {} {
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 204f1e8..80a3627 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -2491,3 +2491,10 @@ proc mi_make_breakpoint_table {bp_list} {
>      # Assemble the final regexp.
>      return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}"
>  }
> +
> +# Return a 1 for configurations that do not support Python scripting.
> +
> +proc mi_skip_python_tests {} {
> +    global mi_gdb_prompt
> +    skip_python_tests_prompt "$mi_gdb_prompt$"
> +}

Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack.

[Pedro Alves]

On 09/26/2014 10:24 AM, Adrian Sendroiu wrote:

> Adrian Sendroiu <molecula2788@gmail.com> writes:
> 
>>> I think it'd be good to replace this ".*" with a stricter match, just
>>> in case something goes wrong with undoing the redirection, but nothing
>>> crashes.  Something like (untested):
>>>
>>>  mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect"
>>>
>>> This is OK with a change along those lines.
>>
>> Ok, I changed to mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect"

OK.

Thanks,
Pedro Alves

Re: [PATCH 0/2] Reimplement redirection for MI

[Doug Evans]

On Wed, Jul 23, 2014 at 7:26 AM, Adrian Sendroiu
<adrian.sendroiu@freescale.com> wrote:
> The current implementation uses a "saved_buffer" field to save the current
> stream when doing a redirection, which will not function correctly in the case
> of nested mi_redirect calls. These patches reimplement the redirection using a
> stack of streams, in a manner similar with the one in the cli interpreter.
>
> Adrian Sendroiu (2):
>   cli/cli-logging.c: don't call ui_out_redirect for MI when disabling
>     logging
>   mi-out: Implement mi redirection using a stack.
>
>  gdb/cli/cli-logging.c |    3 +-
>  gdb/mi/mi-out.c       |   75 +++++++++++++++++++++++++++++--------------------
>  2 files changed, 46 insertions(+), 32 deletions(-)

Hi.

Filing for reference sake: This is pr 18833.
https://sourceware.org/bugzilla/show_bug.cgi?id=18833

Also for reference sake: threads for 1/2 and 2/2 are here:
https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html
https://sourceware.org/ml/gdb-patches/2014-07/msg00573.html

2/2 v2:
https://sourceware.org/ml/gdb-patches/2014-07/msg00665.html