From patchwork Tue Aug 18 21:04:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 8278 Received: (qmail 51919 invoked by alias); 18 Aug 2015 21:04:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 51909 invoked by uid 89); 18 Aug 2015 21:04:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f202.google.com Received: from mail-pd0-f202.google.com (HELO mail-pd0-f202.google.com) (209.85.192.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 18 Aug 2015 21:04:19 +0000 Received: by pdbfa8 with SMTP id fa8so16814392pdb.1 for ; Tue, 18 Aug 2015 14:04:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc :content-type; bh=TVgiWU0HoOoRbc30RL3eXoR/cEB+7CEzInVo2hDwuG0=; b=ElEsiLBkLL3dfGO3aT7Kbh+E5Z+yFf/JldgmYdQ/4fUgXOAjcOdg0w8I2NuClU/VOT 0E2tbDrnbpc2jFHCeIdY0n2+41rc70WR/lLqUnxUAjjTfQlCSIhGB7v6MRf2DPkZVsZr 7w/9jpfqrSFbUmJhZTVdlUiUfdvsDGloukVvh1EWyijXaxA+J9DdU1LQpAdSt2V3ilW5 ARqsq2tFEymYxuSODK+prgnakBAlUNJq8eOIgB2xV+iduDwnLHoo5FA2uspRlJQaRaQI ARpNCaPTcBRVDvN6OQFyUuRxoSOdTMLTFETBwSZWxpFqQwngzePrDuSxEq9YRS3ZSMWV JrNQ== X-Gm-Message-State: ALoCoQnOosTwKXHYGAZg6SXZH7UB9NWQZXFXKS6pBlbvCXdAge0hsIjShwk8dc2D3mJ+jfDAZJcV MIME-Version: 1.0 X-Received: by 10.66.164.225 with SMTP id yt1mr7775470pab.32.1439931857915; Tue, 18 Aug 2015 14:04:17 -0700 (PDT) Message-ID: <047d7b86c3b0e4b459051d9c42da@google.com> Date: Tue, 18 Aug 2015 21:04:17 +0000 Subject: Re: [PATCH] [PR mi/18833] Provide unlimited redirection stack for MI From: Doug Evans To: Pedro Alves Cc: gdb-patches@sourceware.org X-IsSubscribed: yes Pedro Alves writes: > On 08/17/2015 11:32 PM, Doug Evans wrote: > > > I wrote this patch up before noticing that someone had already done so. > > https://sourceware.org/ml/gdb-patches/2015-08/msg00438.html > > > > Adrian's email bounces, so not sure what to do with the attribution. > > Adrian's changes were done under Freescale's copyright assignment > at the time. We should simply use the email address he had then, > just like we don't go over older ChangeLog entries and change > email addresses when we change jobs. (tbc, I don't know if he > changed jobs). > > > > > The problem here is that if we invoke something from python > > with to_string=True with MI as the current interpreter, > > then that'll set up one level of redirection. > > Then if processing that python requires a second redirection, > > e.g. mi_command_param_changed, > > then MI's stack size of two will discard the original entry, > > and then when the python completes MI's uiout will be left pointing > > to freed memory from the redirection. > > > > The fix is to just copy what CLI does: use a VEC to record the stack. > > > > Adrian never completed his testcase. > > If someone wants to complete it and add it great. > > My testcase is sufficient to trigger the crash > > and is how I ran into this bug. > > I did copy over Pedro's suggestion for skip_python_tests_prompt. > > Thanks. It looks good to me. A couple minor comments below. > > > +/* Constructor for an `mi_out_data' object. */ > > + > > +static void > > +mi_out_data_ctor (int mi_version, mi_out_data *self, struct ui_file > > *stream) > > I find it a (tiny) bit surprising the 'self' isn't the first parameter. > > > diff --git a/gdb/testsuite/gdb.python/py-mi-objfile.exp > > b/gdb/testsuite/gdb.python/py-mi-objfile.exp > > new file mode 100644 > > index 0000000..2f69453 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.python/py-mi-objfile.exp > > @@ -0,0 +1,58 @@ > > > +# This file is part of the GDB testsuite. It tests Python-based > > +# pretty-printing for MI. > > + > > This comment looks stale. Thanks. Committed with the suggested changes. commit 4d6cceb4e40a057dbe4d9ad94b0641d5f4725c09 Author: Doug Evans Date: Tue Aug 18 14:02:03 2015 -0700 PR mi/18833 gdb.execute ("set param value", to_string=True) will crash gdb if using MI gdb/ChangeLog: * cli/cli-logging.c (pop_output_files): Don't restore redirection if MI-like. * mi/mi-out.c: #include "vec.h". (ui_filep): New type. (DEV_VEC_P (ui_filep)): New type. (struct ui_out_data) : Delete. (struct ui_out_data) : New member. (mi_ui_out_impl): Add data_destroy field. (mi_field_string, mi_field_fmt): Update. (mi_flush, mi_redirect, field_separator): Update. (mi_open, mi_close): Update. (mi_out_buffered, mi_out_rewind, mi_out_put): Update. (mi_out_data_ctor, mi_out_data_dtor): New functions. (mi_out_new): Call mi_out_data_ctor. testsuite/gdb/ChangeLog: * lib/gdb.exp (skip_python_tests_prompt): Renamed from skip_python_tests. New arg prompt_regexp. (skip_python_tests): New function. * lib/mi-support.exp (mi_skip_python_tests): New function. * gdb.python/py-mi-objfile-gdb.py: New file. * gdb.python/py-mi-objfile.c: New file. * gdb.python/py-mi-objfile.exp: New file. +} diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4af881e..ef8e493 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,22 @@ +2015-08-18 Doug Evans + Adrian Sendroiu + + PR mi/18833 + * cli/cli-logging.c (pop_output_files): Don't restore redirection + if MI-like. + * mi/mi-out.c: #include "vec.h". + (ui_filep): New type. + (DEV_VEC_P (ui_filep)): New type. + (struct ui_out_data) : Delete. + (struct ui_out_data) : New member. + (mi_ui_out_impl): Add data_destroy field. + (mi_field_string, mi_field_fmt): Update. + (mi_flush, mi_redirect, field_separator): Update. + (mi_open, mi_close): Update. + (mi_out_buffered, mi_out_rewind, mi_out_put): Update. + (mi_out_data_ctor, mi_out_data_dtor): New functions. + (mi_out_new): Call mi_out_data_ctor. + 2015-08-18 Sandra Loosemore * remote.c (strprefix): New. diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index 7d340c1..c282260 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -177,7 +177,9 @@ pop_output_files (void) saved_output.targ = NULL; saved_output.targerr = NULL; - ui_out_redirect (current_uiout, NULL); + /* Stay consistent with handle_redirections. */ + if (!ui_out_is_mi_like_p (current_uiout)) + ui_out_redirect (current_uiout, NULL); } /* This is a helper for the `set logging' command. */ diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index fa8282f..ef0a2df 100644 --- a/gdb/mi/mi-out.c +++ b/gdb/mi/mi-out.c @@ -22,19 +22,23 @@ #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; /* These are the MI output functions */ +static void mi_out_data_dtor (struct ui_out *ui_out); static void mi_table_begin (struct ui_out *uiout, int nbrofcols, int nr_rows, const char *tblid); static void mi_table_body (struct ui_out *uiout); @@ -85,7 +89,7 @@ static const struct ui_out_impl mi_ui_out_impl = mi_wrap_hint, mi_flush, mi_redirect, - 0, + mi_out_data_dtor, 1, /* Needs MI hacks. */ }; @@ -215,17 +219,19 @@ 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,19 @@ 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 +283,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 +294,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 +309,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 +344,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 +366,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 +377,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. */ @@ -380,9 +388,10 @@ void mi_out_put (struct ui_out *uiout, struct ui_file *stream) { mi_out_data *data = ui_out_data (uiout); + struct ui_file *outstream = VEC_last (ui_filep, data->streams); - ui_file_put (data->buffer, ui_file_write_for_put, stream); - ui_file_rewind (data->buffer); + ui_file_put (outstream, ui_file_write_for_put, stream); + ui_file_rewind (outstream); } /* Return the current MI version. */ @@ -395,19 +404,41 @@ mi_version (struct ui_out *uiout) return data->mi_version; } +/* Constructor for an `mi_out_data' object. */ + +static void +mi_out_data_ctor (mi_out_data *self, int mi_version, struct ui_file *stream) +{ + gdb_assert (stream != NULL); + + self->streams = NULL; + VEC_safe_push (ui_filep, self->streams, stream); + + self->suppress_field_separator = 0; + self->suppress_output = 0; + self->mi_version = mi_version; +} + +/* The destructor. */ + +static void +mi_out_data_dtor (struct ui_out *ui_out) +{ + mi_out_data *data = ui_out_data (ui_out); + + VEC_free (ui_filep, data->streams); + xfree (data); +} + /* Initialize private members at startup. */ struct ui_out * mi_out_new (int mi_version) { int flags = 0; - 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 (); + struct ui_file *stream = mem_fileopen (); + + mi_out_data_ctor (data, mi_version, stream); return ui_out_new (&mi_ui_out_impl, data, flags); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 9a7cf88..eac803a 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,14 @@ +2015-08-18 Doug Evans + Adrian Sendroiu + + * lib/gdb.exp (skip_python_tests_prompt): Renamed from + skip_python_tests. New arg prompt_regexp. + (skip_python_tests): New function. + * lib/mi-support.exp (mi_skip_python_tests): New function. + * gdb.python/py-mi-objfile-gdb.py: New file. + * gdb.python/py-mi-objfile.c: New file. + * gdb.python/py-mi-objfile.exp: New file. + 2015-08-17 Keith Seitz * gdb.linespec/explicit.exp: Move strace test from here ... diff --git a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py new file mode 100644 index 0000000..6e41773 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py @@ -0,0 +1,27 @@ +# Copyright (C) 2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file is part of the GDB testsuite. + +import gdb + +# PR 18833 +# We want to have two levels of redirection while MI is current_uiout. +# This will create one for to_string=True and then another for the +# parameter change notification. +gdb.execute("set width 101", to_string=True) +# And finally a command that will use the original MI stream, which in a +# buggy gdb will use just-freed data. +gdb.execute("list") diff --git a/gdb/testsuite/gdb.python/py-mi-objfile.c b/gdb/testsuite/gdb.python/py-mi-objfile.c new file mode 100644 index 0000000..b92fefb --- /dev/null +++ b/gdb/testsuite/gdb.python/py-mi-objfile.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int +main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.python/py-mi-objfile.exp b/gdb/testsuite/gdb.python/py-mi-objfile.exp new file mode 100644 index 0000000..5424cc1 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-mi-objfile.exp @@ -0,0 +1,57 @@ +# Copyright (C) 2008-2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file is part of the GDB testsuite. It tests exercises PR 18833. + +load_lib mi-support.exp +set MIFLAGS "-i=mi2" + +gdb_exit +if [mi_gdb_start] { + continue +} + +standard_testfile +set pyfile ${testfile}-gdb.py + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested ${testfile}.exp + return -1 +} + +if { [mi_skip_python_tests] } { continue } + +# Make the -gdb.py script available to gdb, it is automagically loaded by gdb. +# Care is taken to put it in the same directory as the binary so that +# gdb will find it. +set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${pyfile}] + +mi_delete_breakpoints +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_test "set auto-load safe-path ${remote_python_file}" \ + {.*\^done} \ + "set safe-path" + +if [is_remote host] { + set filename ${testfile} + remote_download host ${binfile} ${filename} +} else { + set filename ${binfile} +} + +# PR 18833. This will cause an unpatched gdb to crash. +mi_gdb_test "-file-exec-and-symbols ${filename}" \ + ".*\\^done,line=.*${srcfile}\"" \ + "file-exec-and-symbols operation" diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index c5b7a06..56cde7a 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -1734,35 +1734,35 @@ proc skip_d_tests {} { } # Return a 1 for configurations that do not support Python scripting. +# PROMPT_REGEXP is the expected prompt. -proc skip_python_tests {} { - global gdb_prompt +proc skip_python_tests_prompt { prompt_regexp } { 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_regexp" { unsupported "Python support is disabled." return 1 } - -re "$gdb_prompt $" {} + -re "$prompt_regexp" {} } 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_regexp" { set gdb_py_is_py3k 1 } - -re ".*$gdb_prompt $" { + -re ".*$prompt_regexp" { 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_regexp" { set gdb_py_is_py24 1 } - -re ".*$gdb_prompt $" { + -re ".*$prompt_regexp" { set gdb_py_is_py24 0 } } @@ -1771,6 +1771,15 @@ proc skip_python_tests {} { return 0 } +# Return a 1 for configurations that do not support Python scripting. +# Note: This also sets various globals that specify which version of Python +# is in use. See skip_python_tests_prompt. + +proc skip_python_tests {} { + global gdb_prompt + return [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 0d17ecb..dd6c41a 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -2501,3 +2501,12 @@ 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. +# Note: This also sets various globals that specify which version of Python +# is in use. See skip_python_tests_prompt. + +proc mi_skip_python_tests {} { + global mi_gdb_prompt + return [skip_python_tests_prompt "$mi_gdb_prompt$"]