[PATCH] [PR breakpoints/18275] Introduce dprintf-flush-function for call-style dprintfs
Simon Marchi
simon.marchi@ericsson.com
Tue Sep 22 15:44:00 GMT 2015
On 15-09-09 03:32 PM, Simon Marchi wrote:
> A little bit of context: a colleague of mine likes debugging the JVM and
> wants to use dynamic printfs to do so. Unfortunately, openjdk sets the
> standard output as fully buffered, as opposed to line buffered by
> default. This means that when using simple dprintfs such as:
>
> (gdb) dprintf file.c:1234, "I'm here\n"
>
> the output won't show up on the terminal. He can always stop the
> program and manually call fflush from gdb. Not very practical for
> debugging.
>
> He could also call setlinebuf(stdout) manually. However, this can't be
> done before the inferior is started, and it's possible for the inferior
> to change it back. He could also write a wrapper to printf that calls
> fflush after printing. In both cases you need some quite good insight
> to guess why your dprintfs are not showing up. I'd rather have something
> that "just works".
>
> The parameter dprintf-flush-function is meant to refer to a function
> called after the dprintf-function was called, in order to do the
> flushing. By default, the function is set to fflush and is passed
> stdout, which makes sense since the default value of the dprintf-function
> (printf) prints on stdout. If dprintf-channel is set, it passes that
> value instead to the function.
>
> A test is included. It doesn't do much, it just verifies that the
> specified flush function indeed gets called. I also updated dprintf.exp
> to test the error message when dprintf-function is set to an empty
> value.
>
> gdb/ChangeLog:
>
> PR breakpoints/18275
> * breakpoint.c (update_dprintf_command_list): Add call to the
> flush function to the command list, factor out code.
> (_initialize_breakpoint): Register set/show
> dprintf-flush-function.
> (dprintf_flush_function): New global.
> (get_dprintf_call_print_line): New function.
> (get_dprintf_call_flush_line): New function.
> * NEWS: Mention set/show dprintf-flush-function.
>
> gdb/testsuite/ChangeLog:
>
> PR breakpoints/18275
> * gdb.base/dprintf-flush.exp: New file.
> * gdb.base/dprintf-flush.c: New file.
> * gdb.base/dprintf.exp: Test error message when dprintf-function
> is empty.
>
> gdb/doc/ChangeLog:
>
> PR breakpoints/18275
> * gdb.texinfo (Dynamic Printf): Document set
> dprintf-flush-function.
>
> Change-Id: Ia47bdd7ba1e684dcbde2eddfb1d05d1a2df03316
> ---
> gdb/NEWS | 4 ++
> gdb/breakpoint.c | 113 +++++++++++++++++++++++++++----
> gdb/doc/gdb.texinfo | 11 +++
> gdb/testsuite/gdb.base/dprintf-flush.c | 58 ++++++++++++++++
> gdb/testsuite/gdb.base/dprintf-flush.exp | 56 +++++++++++++++
> gdb/testsuite/gdb.base/dprintf.exp | 7 ++
> 6 files changed, 237 insertions(+), 12 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/dprintf-flush.c
> create mode 100644 gdb/testsuite/gdb.base/dprintf-flush.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 0cf51e1..b65d669 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -36,6 +36,10 @@ set remote multiprocess-extensions-packet
> show remote multiprocess-extensions-packet
> Set/show the use of the remote protocol multiprocess extensions.
>
> +set dprintf-flush-function
> +show dprintf-flush-function
> + Set/show the function to use for flushing after a "call" dynamic printf.
> +
> * The "disassemble" command accepts a new modifier: /s.
> It prints mixed source+disassembly like /m with two differences:
> - disassembled instructions are now printed in program order, and
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 5067222..1dc7dd5 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -345,6 +345,11 @@ static const char *dprintf_style = dprintf_style_gdb;
>
> static char *dprintf_function = "";
>
> +/* The function to use for flushing output after a dynamic printf when
> + dprintf-style is "call". It works the same way as dprintf_function. */
> +
> +static char *dprintf_flush_function = "";
> +
> /* The channel to use for dynamic printf if the preferred style is to
> call into the inferior; if a nonempty string, it will be passed to
> the call as the first argument, with the format string as the
> @@ -9024,6 +9029,76 @@ bp_loc_is_permanent (struct bp_location *loc)
> return retval;
> }
>
> +/* Return an allocated string with the command line for printing a
> + "dprintf-style call" dprintf. */
> +
> +static char *
> +get_dprintf_call_print_line (char *dprintf_function,
> + char *dprintf_channel,
> + char *dprintf_args)
> +{
> + char *arg, *printf_line;
> +
> + gdb_assert (strcmp (dprintf_style, dprintf_style_call) == 0);
> + gdb_assert (dprintf_args != NULL);
> +
> + if (dprintf_function == NULL || strlen (dprintf_function) == 0)
> + {
> + error (_("No function supplied for dprintf call"));
> + }
> +
> + if (dprintf_channel != NULL && strlen (dprintf_channel) > 0)
> + {
> + printf_line = xstrprintf ("call (void) %s (%s,%s)",
> + dprintf_function,
> + dprintf_channel,
> + dprintf_args);
> + }
> + else
> + {
> + printf_line = xstrprintf ("call (void) %s (%s)",
> + dprintf_function,
> + dprintf_args);
> + }
> +
> + return printf_line;
> +}
> +
> +/* Return an allocated string with the command line for flushing the output
> + after a "dprintf-style call" dprintf. This function returns NULL if
> + dprintf_flush_function is NULL or empty. */
> +
> +static char *
> +get_dprintf_call_flush_line (char *dprintf_flush_function,
> + char *dprintf_channel)
> +{
> + char *arg, *flush_line;
> +
> + gdb_assert (strcmp (dprintf_style, dprintf_style_call) == 0);
> +
> + if (dprintf_flush_function == NULL || strlen (dprintf_flush_function) == 0)
> + {
> + return NULL;
> + }
> +
> + if (dprintf_channel != NULL && strlen (dprintf_channel) > 0)
> + {
> + arg = dprintf_channel;
> + }
> + else
> + {
> + /* On a 64-bits platform (e.g. x86-64), if we only have minimal
> + symbols for stdout, calling fflush (stdout) won't work.
> + stdout will be treated as a 4-bytes integer and only 4 bytes will
> + be passed to fflush, when in reality it's an 8-bytes pointer.
> + Taking the address, casting to void** and dereferencing allows to
> + make sure that we go read and pass the full pointer. */
> + arg = "*((void **) &stdout)";
> + }
> +
> + return xstrprintf ("call (void) %s (%s)", dprintf_flush_function, arg);
> +}
> +
> /* Build a command list for the dprintf corresponding to the current
> settings of the dprintf style options. */
>
> @@ -9032,6 +9107,7 @@ update_dprintf_command_list (struct breakpoint *b)
> {
> char *dprintf_args = b->extra_string;
> char *printf_line = NULL;
> + char *flush_line = NULL;
>
> if (!dprintf_args)
> return;
> @@ -9051,18 +9127,10 @@ update_dprintf_command_list (struct breakpoint *b)
> printf_line = xstrprintf ("printf %s", dprintf_args);
> else if (strcmp (dprintf_style, dprintf_style_call) == 0)
> {
> - if (!dprintf_function)
> - error (_("No function supplied for dprintf call"));
> -
> - if (dprintf_channel && strlen (dprintf_channel) > 0)
> - printf_line = xstrprintf ("call (void) %s (%s,%s)",
> - dprintf_function,
> - dprintf_channel,
> - dprintf_args);
> - else
> - printf_line = xstrprintf ("call (void) %s (%s)",
> - dprintf_function,
> - dprintf_args);
> + printf_line = get_dprintf_call_print_line (dprintf_function,
> + dprintf_channel, dprintf_args);
> + flush_line = get_dprintf_call_flush_line (dprintf_flush_function,
> + dprintf_channel);
> }
> else if (strcmp (dprintf_style, dprintf_style_agent) == 0)
> {
> @@ -9089,6 +9157,19 @@ update_dprintf_command_list (struct breakpoint *b)
> printf_cmd_line->next = NULL;
> printf_cmd_line->line = printf_line;
>
> + if (flush_line)
> + {
> + struct command_line *flush_cmd_line = XNEW (struct command_line);
> +
> + flush_cmd_line->control_type = simple_control;
> + flush_cmd_line->body_count = 0;
> + flush_cmd_line->body_list = NULL;
> + flush_cmd_line->next = NULL;
> + flush_cmd_line->line = flush_line;
> +
> + printf_cmd_line->next = flush_cmd_line;
> + }
> +
> breakpoint_set_commands (b, printf_cmd_line);
> }
> }
> @@ -16440,6 +16521,14 @@ Show the function to use for dynamic printf"), NULL,
> update_dprintf_commands, NULL,
> &setlist, &showlist);
>
> + dprintf_flush_function = xstrdup ("fflush");
> + add_setshow_string_cmd ("dprintf-flush-function", class_support,
> + &dprintf_flush_function, _("\
> +Set the function to use for flushing after a \"call\" dynamic printf"), _("\
> +Show the function to use for flushing after a \"call\" dynamic printf"), NULL,
> + update_dprintf_commands, NULL,
> + &setlist, &showlist);
> +
> dprintf_channel = xstrdup ("");
> add_setshow_string_cmd ("dprintf-channel", class_support,
> &dprintf_channel, _("\
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index cd0abad..2a8eca5 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4886,6 +4886,17 @@ default its value is @code{printf}. You may set it to any expression.
> that @value{GDBN} can evaluate to a function, as per the @code{call}
> command.
>
> +@item set dprintf-flush-function @var{function}
> +Set the function to call after @code{dprintf-function} if the dprintf
> +style is @code{call}. This function can be used to force the flushing
> +of the channel used by the dprintf mechanism, so that debugging
> +statements are not buffered. Setting @code{dprintf-flush-function} to
> +an empty value disables the feature. By default its value is @code{fflush}.
> +
> +The @code{dprintf-flush-function} must accept a single argument. If
> +@code{dprintf-channel} is set, its value is passed as the argument.
> +Otherwise, @code{stdout} is used.
> +
> @item set dprintf-channel @var{channel}
> Set a ``channel'' for dprintf. If set to a non-empty value,
> @value{GDBN} will evaluate it as an expression and pass the result as
> diff --git a/gdb/testsuite/gdb.base/dprintf-flush.c b/gdb/testsuite/gdb.base/dprintf-flush.c
> new file mode 100644
> index 0000000..409b412
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dprintf-flush.c
> @@ -0,0 +1,58 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + 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 <http://www.gnu.org/licenses/>. */
> +
> +#include <stdio.h>
> +
> +static int myflush_called = 0;
> +static FILE *myflush_arg = NULL;
> +
> +static void myflush (FILE *f)
> +{
> + myflush_called++;
> + myflush_arg = f;
> + fflush (f);
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> + volatile int a = 0;
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + {
> + a++; /* dprintf here */
> + a++; /* breakpoint here */
> + }
> +
> + return a;
> +}
> +
> +#include <stdlib.h>
> +/* Make sure function 'malloc' is linked into program. One some bare-metal
> + port, if we don't use 'malloc', it will not be linked in program. 'malloc'
> + is needed, otherwise we'll see such error message
> +
> + evaluation of this expression requires the program to have a function
> + "malloc". */
> +void
> +bar (void)
> +{
> + void *p = malloc (16);
> +
> + free (p);
> +}
> diff --git a/gdb/testsuite/gdb.base/dprintf-flush.exp b/gdb/testsuite/gdb.base/dprintf-flush.exp
> new file mode 100644
> index 0000000..40b41a0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dprintf-flush.exp
> @@ -0,0 +1,56 @@
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> + return -1
> +}
> +
> +if ![runto main] {
> + return -1
> +}
> +
> +set dp_location [gdb_get_line_number "dprintf here"]
> +set break_location [gdb_get_line_number "breakpoint here"]
> +gdb_test "dprintf $dp_location, \"Hello\\n\"" "Dprintf $decimal at $hex: file .*$srcfile, line $dp_location\." "set dprintf"
> +gdb_test "break $break_location"
> +
> +# Test with the default dprintf-channel (stdout).
> +gdb_test_no_output "set dprintf-style call"
> +gdb_test_no_output "set dprintf-flush-function myflush"
> +
> +gdb_continue_to_breakpoint "breakpoint here"
> +
> +gdb_test "print myflush_called" ".* = 1" "myflush was called 1"
> +gdb_test "print *((void**)&stdout) == myflush_arg" ".* = 1" "myflush's default arg is stdout"
> +
> +# Test with a custom dprintf-channel.
> +gdb_test_no_output "set dprintf-channel *((void**)&stderr)"
> +gdb_test_no_output "set dprintf-function fprintf"
> +
> +gdb_continue_to_breakpoint "breakpoint here"
> +
> +gdb_test "print myflush_called" ".* = 2" "myflush was called 2"
> +gdb_test "print *((void**)&stderr) == myflush_arg" ".* = 1" "myflush's default arg is stderr"
> +
> +# Test with an empty dprintf-flush-function (at least that gdb doesn't crash)
> +gdb_test_no_output "set dprintf-flush-function" "set empty dprintf-flush-function"
> +
> +gdb_continue_to_breakpoint "breakpoint here"
> +
> +# These values shouldn't have changed since the previous test
> +gdb_test "print myflush_called" ".* = 2" "myflush was called 2"
> +gdb_test "print *((void**)&stderr) == myflush_arg" ".* = 1" "myflush's default arg is stderr"
> diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
> index 23905e4..a036ba2 100644
> --- a/gdb/testsuite/gdb.base/dprintf.exp
> +++ b/gdb/testsuite/gdb.base/dprintf.exp
> @@ -123,6 +123,13 @@ proc test_call {} {
>
> test_dprintf "At foo entry.*arg=1235, g=2222\r\n" "2nd dprintf"
> }
> +
> + with_test_prefix "no print function" {
> + restart
> +
> + gdb_test_no_output "set dprintf-style call" "set dprintf style to call"
> + gdb_test "set dprintf-function" "No function supplied for dprintf call"
> + }
> }
>
> # The "call" style depends on having I/O functions available.
Ping.
More information about the Gdb-patches
mailing list