From patchwork Mon Aug 17 22:32:34 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 8251 Received: (qmail 18554 invoked by alias); 17 Aug 2015 22:32:40 -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 18541 invoked by uid 89); 17 Aug 2015 22:32:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f201.google.com Received: from mail-pd0-f201.google.com (HELO mail-pd0-f201.google.com) (209.85.192.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 17 Aug 2015 22:32:36 +0000 Received: by pdav4 with SMTP id v4so14136196pda.0 for ; Mon, 17 Aug 2015 15:32:34 -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 :content-type; bh=bGkPhJEVsx/e7FweKWNhLpZedvwUaQKkpwMpAZB2yfk=; b=B+Nz7Mb0dJjw8KD9RuaIeWqrJ7fKuzXXLMXmHyMXOsfTHYvYjEvS0jVVMUl1iMXo1l i7M4KNUOO4wqHUMB5uhGuYUPXmHeaf24tk1wV+53DWGJjVv/IoIrHdFKiBNtZcmUvMaf Joh8XhDifxZmeL5OCVEwUpoRzTvHnh5TRBHtGiHocXWdMhq8/LokMuPCz7hZZcB6qtov 4kkXd3/R6LNz0aBV8k0OlyGMXtfe0AWGG42GREhr8TvNizc2bJkkHfBSilVauW+7z26T qns0qvf8F9zo7+QsPRZCBn8M6QzJAD3BSpbMyAcYXXb5JeA9vQuKMo8vwjAjfduwQql/ pDFg== X-Gm-Message-State: ALoCoQnK4QLIsK0I7M91nuHG+uR7YkqNS9UiWEmLIOpo/MbzwvikxBsa6l3H96K8yvc/rV1DsWdtwDCXBwQMxHM/whRhA2uiheP4CYDrsjja9m7rpDPNdt/wyNsl6KI3pSGS0jjAWncODM8lVKG5NS53d2m+xDzimuwoGGeJLCXtwdhnidV2UAk= MIME-Version: 1.0 X-Received: by 10.67.3.40 with SMTP id bt8mr3090431pad.34.1439850754546; Mon, 17 Aug 2015 15:32:34 -0700 (PDT) Message-ID: <047d7b15fcc1c172a6051d8960c4@google.com> Date: Mon, 17 Aug 2015 22:32:34 +0000 Subject: [PATCH] [PR mi/18833] Provide unlimited redirection stack for MI From: Doug Evans To: gdb-patches@sourceware.org X-IsSubscribed: yes Hi. 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. 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. 2015-08-17 Doug Evans 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. testsuite/ * 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/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..b4693f4 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 (int mi_version, mi_out_data *self, 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 (mi_version, data, stream); return ui_out_new (&mi_ui_out_impl, data, flags); } 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..2f69453 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-mi-objfile.exp @@ -0,0 +1,58 @@ +# 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 Python-based +# pretty-printing for MI. + +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$"]