[PR,breakpoints/18275] Introduce dprintf-flush-function for call-style dprintfs
Commit Message
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
Comments
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.
On 09/09/2015 08: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.
This probably ends up making dprintf-call about 2x slower, as now you
we do twice the infcalls. But I can't really argue against making it
Just Work OOTB.
Just a few comments below.
>
> 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)
Write (*dprintf_function == '\0') for empty string tests.
> + {
> + error (_("No function supplied for dprintf call"));
> + }
Unnecessary braces.
> +
> + if (dprintf_channel != NULL && strlen (dprintf_channel) > 0)
As above.
> + {
> + 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)";
Urgh. Note this won't work everywhere -- you can't portably take the address of
stdout, which according to the C standard is a macro. I believe this won't
work on mingw, for example. If we do this, it should probably end up being
a gdbarch variable/method.
> + }
> +
> + 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)
if (flush_line != NULL)
> + {
> + 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)
Formatting.
> +{
> + 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>
Includes at the top.
> +/* Make sure function 'malloc' is linked into program. One some bare-metal
"On"
> + port, if we don't use 'malloc', it will not be linked in program. 'malloc'
"ports".
> + is needed, otherwise we'll see such error message
> +
> + evaluation of this expression requires the program to have a function
> + "malloc". */
I can't seem to parse this sentence.
> +void
> +bar (void)
> +{
> + void *p = malloc (16);
> +
> + free (p);
> +}
Thanks,
Pedro Alves
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Tue, 22 Sep 2015 11:44:42 -0400
>
> > 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.
The documentation parts are OK.
Thanks.
Thanks for the comments.
> Urgh. Note this won't work everywhere -- you can't portably take the address of
> stdout, which according to the C standard is a macro. I believe this won't
> work on mingw, for example. If we do this, it should probably end up being
> a gdbarch variable/method.
Ok thanks for the note. I'll make sure to test this on Windows/mingw.
@@ -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
@@ -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, _("\
@@ -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
new file mode 100644
@@ -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);
+}
new file mode 100644
@@ -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"
@@ -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.