From 6e3bb2af58a42d57f0fa1f47ee5ecda287123c65 Mon Sep 17 00:00:00 2001 From: Philippe Waroquiers Date: Wed, 4 Sep 2013 21:42:43 +0000 Subject: [PATCH] Fix 324514 gdbserver monitor cmd output behaviour consistency + allow user to put a "marker" msg in process log output * v.info n_errs_found accepts optional msg, added in the output of the monitor command. * use VG_(printf) rather than VG_(gdb_printf) when output of command should be redirected according to v.set gdb_output|log_output|mixed_output * also avoid calling gdb_printf in output sink processing to output zero bytes, as gdb_printf expects to have a null terminated string, which is not ensured when 0 bytes have to be output. * some minor reformatting (replace char* xxx by char *xxx). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13532 --- NEWS | 9 ++++ coregrind/m_gdbserver/server.c | 63 ++++++++++++++++++-------- coregrind/m_libcprint.c | 4 +- docs/xml/manual-core-adv.xml | 12 +++-- gdbserver_tests/mcbreak.stderrB.exp | 6 ++- gdbserver_tests/mcbreak.stdinB.gdb | 4 ++ gdbserver_tests/mchelp.stdoutB.exp | 4 +- gdbserver_tests/mssnapshot.stderrB.exp | 2 +- include/pub_tool_gdbserver.h | 13 ++++++ memcheck/mc_main.c | 24 +++++----- 10 files changed, 102 insertions(+), 39 deletions(-) diff --git a/NEWS b/NEWS index d5770a564d..d0ef4e2ef0 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,11 @@ Release 3.9.0 (?? ?????? 201?) - Addition of GDB server monitor command 'v.info open_fds' that gives the list of open file descriptors and additional details. + - Optional message in the 'v.info n_errs_found' monitor command (e.g. + 'v.info n_errs_found test 1234 finished'), allowing to have + a comment string in the process output, separating errors of different + tests (or test phases). + - Addition of GDB server monitor command 'v.info execontext' that shows information about the stack traces recorded by Valgrind. This can be used to analyse one possible cause of Valgrind high @@ -449,6 +454,10 @@ FIXED r?? 322851 0bXXX binary literal syntax is not standard FIXED 2736 +324514 gdbserver monitor cmd output behaviour consistency + allow user + to put a "marker" msg in process log output + FIXED 13532 + 207815 Adds some of the drm ioctls to syswrap-linux.c FIXED 13486 diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 5e79fd017f..46abb7cfa6 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -122,6 +122,31 @@ void kill_request (const char *msg) VG_(exit) (0); } +// s is a NULL terminated string made of O or more words (separated by spaces). +// Returns a pointer to the Nth word in s. +// If Nth word does not exist, return a pointer to the last (0) byte of s. +static +const char *wordn (const char *s, int n) +{ + int word_seen = 0; + Bool searching_word = True; + + while (*s) { + if (*s == ' ') + searching_word = True; + else { + if (searching_word) { + searching_word = False; + word_seen++; + if (word_seen == n) + return s; + } + } + s++; + } + return s; +} + /* handle_gdb_valgrind_command handles the provided mon string command. If command is recognised, return 1 else return 0. Note that in case of ambiguous command, 1 is returned. @@ -130,13 +155,13 @@ void kill_request (const char *msg) 'v.set *_output' is handled. */ static -int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) +int handle_gdb_valgrind_command (char *mon, OutputSink *sink_wanted_at_return) { UWord ret = 0; char s[strlen(mon)+1]; /* copy for strtok_r */ - char* wcmd; - HChar* ssaveptr; - const char* endptr; + char *wcmd; + HChar *ssaveptr; + const char *endptr; int kwdid; int int_value; @@ -175,7 +200,7 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) " v.wait [] : sleep (default 0) then continue\n" " v.info all_errors : show all errors found so far\n" " v.info last_error : show last error found\n" -" v.info n_errs_found : show the nr of errors found so far\n" +" v.info n_errs_found [msg] : show the nr of errors found so far and the given msg\n" " v.info open_fds : show open file descriptors (only if --track-fds=yes)\n" " v.kill : kill the Valgrind process\n" " v.set gdb_output : set valgrind output to gdb\n" @@ -222,15 +247,15 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) if (*endptr != '\0') { VG_(gdb_printf) ("missing or malformed integer value\n"); } else if (kwdid == 0) { - VG_(gdb_printf) ("vgdb-error value changed from %d to %d\n", + VG_(printf) ("vgdb-error value changed from %d to %d\n", VG_(dyn_vgdb_error), int_value); VG_(dyn_vgdb_error) = int_value; } else if (kwdid == 1) { - VG_(gdb_printf) ("debuglog value changed from %d to %d\n", + VG_(printf) ("debuglog value changed from %d to %d\n", VG_(debugLog_getLevel)(), int_value); VG_(debugLog_startup) (int_value, "gdbsrv"); } else if (kwdid == 2) { - VG_(gdb_printf) + VG_(printf) ("merge-recursive-frames value changed from %d to %d\n", VG_(clo_merge_recursive_frames), int_value); VG_(clo_merge_recursive_frames) = int_value; @@ -273,10 +298,11 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) VG_(show_all_errors)(/* verbosity */ 2, /* xml */ False); break; case 1: // n_errs_found - VG_(gdb_printf) ("n_errs_found %d n_errs_shown %d (vgdb-error %d)\n", - VG_(get_n_errs_found) (), - VG_(get_n_errs_shown) (), - VG_(dyn_vgdb_error)); + VG_(printf) ("n_errs_found %d n_errs_shown %d (vgdb-error %d) %s\n", + VG_(get_n_errs_found) (), + VG_(get_n_errs_shown) (), + VG_(dyn_vgdb_error), + wordn (mon, 3)); break; case 2: // last_error VG_(show_last_error)(); @@ -328,10 +354,10 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) wcmd = strtok_r (NULL, " ", &ssaveptr); if (wcmd != NULL) { int_value = strtol (wcmd, NULL, 10); - VG_(gdb_printf) ("gdbserver: continuing in %d ms ...\n", int_value); + VG_(printf) ("gdbserver: continuing in %d ms ...\n", int_value); VG_(poll)(NULL, 0, int_value); } - VG_(gdb_printf) ("gdbserver: continuing after wait ...\n"); + VG_(printf) ("gdbserver: continuing after wait ...\n"); ret = 1; break; case 4: /* v.kill */ @@ -410,7 +436,7 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) Note that in case of ambiguous command, 1 is returned. */ static -int handle_gdb_monitor_command (char* mon) +int handle_gdb_monitor_command (char *mon) { UWord ret = 0; UWord tool_ret = 0; @@ -450,6 +476,8 @@ int handle_gdb_monitor_command (char* mon) VG_(dyn_vgdb_error) = save_dyn_vgdb_error; } + VG_(message_flush) (); + /* restore or set the desired output */ VG_(log_output_sink).fd = sink_wanted_at_return.fd; if (ret | tool_ret) @@ -496,7 +524,7 @@ void handle_set (char *arg_own_buf, int *new_packet_len_p) arg_own_buf[0] = 0; } -Bool VG_(client_monitor_command) (HChar* cmd) +Bool VG_(client_monitor_command) (HChar *cmd) { const Bool connected = remote_connected(); const int saved_command_output_to_log = command_output_to_log; @@ -535,9 +563,6 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p) cmd[cmdlen] = '\0'; if (handle_gdb_monitor_command (cmd)) { - /* In case the command is from a standalone vgdb, - connection will be closed soon => flush the output. */ - VG_(message_flush) (); write_ok (arg_own_buf); return; } else { diff --git a/coregrind/m_libcprint.c b/coregrind/m_libcprint.c index d4e3bee967..8c887e3fdd 100644 --- a/coregrind/m_libcprint.c +++ b/coregrind/m_libcprint.c @@ -74,7 +74,9 @@ void send_bytes_to_logging_sink ( OutputSink* sink, const HChar* msg, Int nbytes any more output. */ if (sink->fd >= 0) VG_(write)( sink->fd, msg, nbytes ); - else if (sink->fd == -2) + else if (sink->fd == -2 && nbytes > 0) + /* send to gdb the provided data, which must be + a null terminated string with len >= 1 */ VG_(gdb_printf)("%s", msg); } } diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml index 23b88fe524..025ea70cad 100644 --- a/docs/xml/manual-core-adv.xml +++ b/docs/xml/manual-core-adv.xml @@ -657,7 +657,7 @@ ambiguous, the Valgrind gdbserver will report the list of commands (or argument values) that can match: - v.info n_errs_found shows the number of + v.info n_errs_found [msg] shows the number of errors found so far, the nr of errors shown so far and the current - value of the argument. + value of the argument. The optional + msg (one or more words) is appended. + Typically, this can be used to insert markers in a process output + file between several tests executed in sequence by a process + started only once. This allows to associate the errors reported + by Valgrind with the specific test that produced these errors. + diff --git a/gdbserver_tests/mcbreak.stderrB.exp b/gdbserver_tests/mcbreak.stderrB.exp index 3bf12a0ff2..65281d2c06 100644 --- a/gdbserver_tests/mcbreak.stderrB.exp +++ b/gdbserver_tests/mcbreak.stderrB.exp @@ -1,7 +1,11 @@ relaying data between gdb and process .... vgdb-error value changed from 0 to 999999 vgdb-error value changed from 999999 to 0 -n_errs_found 1 n_errs_shown 1 (vgdb-error 0) +n_errs_found 1 n_errs_shown 1 (vgdb-error 0) +n_errs_found 1 n_errs_shown 1 (vgdb-error 0) a +n_errs_found 1 n_errs_shown 1 (vgdb-error 0) b +n_errs_found 1 n_errs_shown 1 (vgdb-error 0) c d +n_errs_found 1 n_errs_shown 1 (vgdb-error 0) eeeeeee fffffff ggggggg vgdb-error value changed from 0 to 0 monitor command request to kill this process Remote connection closed diff --git a/gdbserver_tests/mcbreak.stdinB.gdb b/gdbserver_tests/mcbreak.stdinB.gdb index 4bd8c93f78..4f932e6afe 100644 --- a/gdbserver_tests/mcbreak.stdinB.gdb +++ b/gdbserver_tests/mcbreak.stdinB.gdb @@ -58,6 +58,10 @@ finish delete continue monitor v.info n_errs_found +monitor v.info n_errs_found a +monitor v.info n_errs_found b +monitor v.info n_errs_found c d +monitor v.info n_errs_found eeeeeee fffffff ggggggg # inferior call "in the middle" of an instruction is not working at least # on all platforms, so comment the below. # print whoami("after error: inferior call pushed from mcbreak.stdinB.gdb") diff --git a/gdbserver_tests/mchelp.stdoutB.exp b/gdbserver_tests/mchelp.stdoutB.exp index ab267d3447..0179b53a41 100644 --- a/gdbserver_tests/mchelp.stdoutB.exp +++ b/gdbserver_tests/mchelp.stdoutB.exp @@ -3,7 +3,7 @@ general valgrind monitor commands: v.wait [] : sleep (default 0) then continue v.info all_errors : show all errors found so far v.info last_error : show last error found - v.info n_errs_found : show the nr of errors found so far + v.info n_errs_found [msg] : show the nr of errors found so far and the given msg v.info open_fds : show open file descriptors (only if --track-fds=yes) v.kill : kill the Valgrind process v.set gdb_output : set valgrind output to gdb @@ -45,7 +45,7 @@ general valgrind monitor commands: v.wait [] : sleep (default 0) then continue v.info all_errors : show all errors found so far v.info last_error : show last error found - v.info n_errs_found : show the nr of errors found so far + v.info n_errs_found [msg] : show the nr of errors found so far and the given msg v.info open_fds : show open file descriptors (only if --track-fds=yes) v.kill : kill the Valgrind process v.set gdb_output : set valgrind output to gdb diff --git a/gdbserver_tests/mssnapshot.stderrB.exp b/gdbserver_tests/mssnapshot.stderrB.exp index 551865f324..80ccc54b39 100644 --- a/gdbserver_tests/mssnapshot.stderrB.exp +++ b/gdbserver_tests/mssnapshot.stderrB.exp @@ -5,7 +5,7 @@ general valgrind monitor commands: v.wait [] : sleep (default 0) then continue v.info all_errors : show all errors found so far v.info last_error : show last error found - v.info n_errs_found : show the nr of errors found so far + v.info n_errs_found [msg] : show the nr of errors found so far and the given msg v.info open_fds : show open file descriptors (only if --track-fds=yes) v.kill : kill the Valgrind process v.set gdb_output : set valgrind output to gdb diff --git a/include/pub_tool_gdbserver.h b/include/pub_tool_gdbserver.h index 03be811c42..6627a2f32c 100644 --- a/include/pub_tool_gdbserver.h +++ b/include/pub_tool_gdbserver.h @@ -130,6 +130,19 @@ extern void VG_(needs_watchpoint) ( // can be used during the processing of the VG_USERREQ__GDB_MONITOR_COMMAND // tool client request to output information to gdb or vgdb. +// The output of VG_(gdb_printf) is not subject to 'output control' +// by the user: e.g. the monitor command 'v.set log_output' has no effect. +// The output of VG_(gdb_printf) is given to gdb/vgdb. The only case +// in which this output is not given to gdb/vgdb is when the connection +// with gdb/vgdb has been lost : in such a case, output is written +// to the valgrind log output. +// To produce some output which is subject to user output control via +// monitor command v.set gdb_output or mixed output, use VG_(printf) +// or VG_(umsg) or similar. +// Typically, VG_(gdb_printf) has to be used when there is no point +// having this output in the output log of Valgrind. Examples +// is the monitor help output, or if vgdb is used to implement +// 'tool control scripts' such as callgrind_control. extern UInt VG_(gdb_printf) ( const HChar *format, ... ) PRINTF_CHECK(1, 2); /* Utility functions to (e.g.) parse gdb monitor commands. diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index bbc2138940..00c9993601 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -5369,21 +5369,21 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) False /* is client request */ ); /* we are before the first character on next line, print a \n. */ if ((i % 32) == 0 && i != 0) - VG_(gdb_printf) ("\n"); + VG_(printf) ("\n"); /* we are before the next block of 4 starts, print a space. */ else if ((i % 4) == 0 && i != 0) - VG_(gdb_printf) (" "); + VG_(printf) (" "); if (res == 1) { - VG_(gdb_printf) ("%02x", vbits); + VG_(printf) ("%02x", vbits); } else { tl_assert(3 == res); unaddressable++; - VG_(gdb_printf) ("__"); + VG_(printf) ("__"); } } - VG_(gdb_printf) ("\n"); + VG_(printf) ("\n"); if (unaddressable) { - VG_(gdb_printf) + VG_(printf) ("Address %p len %ld has %d bytes unaddressable\n", (void *)address, szB, unaddressable); } @@ -5517,17 +5517,17 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) case -1: break; case 0: if (is_mem_addressable ( address, szB, &bad_addr )) - VG_(gdb_printf) ("Address %p len %ld addressable\n", + VG_(printf) ("Address %p len %ld addressable\n", (void *)address, szB); else - VG_(gdb_printf) + VG_(printf) ("Address %p len %ld not addressable:\nbad address %p\n", (void *)address, szB, (void *) bad_addr); MC_(pp_describe_addr) (address); break; case 1: res = is_mem_defined ( address, szB, &bad_addr, &otag ); if (MC_AddrErr == res) - VG_(gdb_printf) + VG_(printf) ("Address %p len %ld not addressable:\nbad address %p\n", (void *)address, szB, (void *) bad_addr); else if (MC_ValueErr == res) { @@ -5543,7 +5543,7 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) src = ""; break; default: tl_assert(0); } - VG_(gdb_printf) + VG_(printf) ("Address %p len %ld not defined:\n" "Uninitialised value at %p%s\n", (void *)address, szB, (void *) bad_addr, src); @@ -5554,8 +5554,8 @@ static Bool handle_gdb_monitor_command (ThreadId tid, HChar *req) } } else - VG_(gdb_printf) ("Address %p len %ld defined\n", - (void *)address, szB); + VG_(printf) ("Address %p len %ld defined\n", + (void *)address, szB); MC_(pp_describe_addr) (address); break; default: tl_assert(0); -- 2.47.2