From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93027 invoked by alias); 22 Sep 2015 15:44:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 93017 invoked by uid 89); 22 Sep 2015 15:44:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 22 Sep 2015 15:44:46 +0000 Received: from EUSAAHC007.ericsson.se (Unknown_Domain [147.117.188.93]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 4A.8E.26730.03C01065; Tue, 22 Sep 2015 10:07:12 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.95) with Microsoft SMTP Server id 14.3.248.2; Tue, 22 Sep 2015 11:44:43 -0400 Subject: Re: [PATCH] [PR breakpoints/18275] Introduce dprintf-flush-function for call-style dprintfs To: gdb-patches References: <1441827156-8518-1-git-send-email-simon.marchi@ericsson.com> From: Simon Marchi Message-ID: <5601776A.3030903@ericsson.com> Date: Tue, 22 Sep 2015 15:44:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <1441827156-8518-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00527.txt.bz2 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 . */ > + > +#include > + > +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 > +/* 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 . > + > +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.