diff mbox

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

Message ID 53E0E251.7040803@freescale.com
State New
Headers show

Commit Message

Adrian Sendroiu Aug. 5, 2014, 1:55 p.m. UTC
> 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(-)

Comments

Adrian Sendroiu Aug. 28, 2014, 11:33 a.m. UTC | #1
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$"
> +}
>
Pedro Alves Sept. 5, 2014, 4:06 p.m. UTC | #2
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
Adrian Sendroiu Sept. 7, 2014, 3:40 p.m. UTC | #3
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
Pedro Alves Sept. 8, 2014, 1:19 p.m. UTC | #4
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
Sergio Durigan Junior Sept. 8, 2014, 6:59 p.m. UTC | #5
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...
diff mbox

Patch

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$"
+}