|
From: Mark W. <ma...@so...> - 2023-04-13 22:10:05
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=0432ce486d61f84f6fcbeab0d812bb1f61c57561 commit 0432ce486d61f84f6fcbeab0d812bb1f61c57561 Author: Alexandra Petlanova Hajkova <aha...@re...> Date: Thu Mar 3 04:46:03 2022 -0500 vgdb: implement the extended-remote protocol Executing vgdb --multi makes vgdb talk the gdb extended-remote protocol. This means that the gdb run command is supported and vgdb will start up the program under valgrind. Which means you don't need to run gdb and valgrind from different terminals. Also vgdb keeps being connected to gdb after valgrind exits. So you can easily rerun the program with the same breakpoints in place. vgdb now implements a minimal gdbserver that just recognizes a few extended-remote protocol packets. Once it starts up valgrind it sets up noack and qsupported then it will forward packets between gdb and valgrind gdbserver. After valgrind shutsdown it resumes handling gdb packets itself. https://bugs.kde.org/show_bug.cgi?id=434057 Co-authored-by: Mark Wielaard <ma...@kl...> Diff: --- NEWS | 12 + coregrind/docs/vgdb-manpage.xml | 2 +- coregrind/m_gdbserver/m_gdbserver.c | 5 +- coregrind/m_gdbserver/remote-utils.c | 5 +- coregrind/m_gdbserver/server.c | 9 + coregrind/m_main.c | 3 + coregrind/m_options.c | 1 + coregrind/vgdb.c | 836 ++++++++++++++++++++++++++++++++++- docs/xml/manual-core-adv.xml | 61 ++- include/pub_tool_options.h | 5 + vg-in-place | 2 +- 11 files changed, 914 insertions(+), 27 deletions(-) diff --git a/NEWS b/NEWS index 4fd2baf04c..68acfff7cf 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,17 @@ AMD64/macOS 10.13 and nanoMIPS/Linux. $3 = 40 (gdb) monitor who_point_at 0x1130a0 40 +* The vgdb utility now supports extended-remote protocol when + invoked with --multi. In this mode the GDB run command is + supported. Which means you don't need to run gdb and valgrind + from different terminals. So for example to start you program + in gdb and run it under valgrind you can do: + $ gdb prog + (gdb) set remote exec-file prog + (gdb) set sysroot / + (gdb) target extended-remote | vgdb --multi + (gdb) start + * The behaviour of realloc with a size of zero can now be changed for tools that intercept malloc. Those tools are memcheck, helgrind, drd, massif and dhat. @@ -116,6 +127,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 392331 Spurious lock not held error from inside pthread_cond_timedwait 400793 pthread_rwlock_timedwrlock false positive 433873 openat2 syscall unimplemented on Linux +434057 Add stdio mode to valgrind's gdbserver 435441 valgrind fails to interpose malloc on musl 1.2.2 due to weak symbol name and no libc soname 444110 priv/guest_ppc_toIR.c:36198:31: warning: duplicated 'if' condition. 444488 Use glibc.pthread.stack_cache_size tunable diff --git a/coregrind/docs/vgdb-manpage.xml b/coregrind/docs/vgdb-manpage.xml index 7ba82f8e80..1fb7ea0411 100644 --- a/coregrind/docs/vgdb-manpage.xml +++ b/coregrind/docs/vgdb-manpage.xml @@ -29,7 +29,7 @@ <title>Description</title> <para><command>vgdb</command> ("Valgrind to GDB") is used as an -intermediary between Valgrind and GDB or a shell. It has two usage modes: +intermediary between Valgrind and GDB or a shell. It has three usage modes: </para> <xi:include href="../../docs/xml/manual-core-adv.xml" diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index e0238bd072..f8fbc5af23 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -601,8 +601,9 @@ void VG_(gdbserver_prerun_action) (ThreadId tid) { // Using VG_(clo_vgdb_error) allows the user to control if gdbserver // stops after a fork. - if (VG_(clo_vgdb_error) == 0 - || VgdbStopAtiS(VgdbStopAt_Startup, VG_(clo_vgdb_stop_at))) { + if ((VG_(clo_vgdb_error) == 0 + || (VgdbStopAtiS(VgdbStopAt_Startup, VG_(clo_vgdb_stop_at)))) + && !(VG_(clo_launched_with_multi))) { /* The below call allows gdb to attach at startup before the first guest instruction is executed. */ VG_(umsg)("(action at startup) vgdb me ... \n"); diff --git a/coregrind/m_gdbserver/remote-utils.c b/coregrind/m_gdbserver/remote-utils.c index 2ec8e6fe59..8e3614fb43 100644 --- a/coregrind/m_gdbserver/remote-utils.c +++ b/coregrind/m_gdbserver/remote-utils.c @@ -372,8 +372,9 @@ void remote_open (const HChar *name) pid); } if (VG_(clo_verbosity) > 1 - || VG_(clo_vgdb_error) < 999999999 - || VG_(clo_vgdb_stop_at) != 0) { + || ((VG_(clo_vgdb_error) < 999999999 + || VG_(clo_vgdb_stop_at) != 0) + && !(VG_(clo_launched_with_multi)))) { VG_(umsg)("\n"); VG_(umsg)( "TO DEBUG THIS PROCESS USING GDB: start GDB like this\n" diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 0f639b2743..3c2516086d 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -843,6 +843,7 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p) } } + /* Without argument, traditional remote protocol. */ if (strcmp ("qAttached", arg_own_buf) == 0) { /* tell gdb to always detach, never kill the process */ arg_own_buf[0] = '1'; @@ -850,6 +851,14 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p) return; } + /* With argument, extended-remote protocol. */ + if (strncmp ("qAttached:", arg_own_buf, strlen ("qAttached:")) == 0) { + /* We just created this process */ + arg_own_buf[0] = '0'; + arg_own_buf[1] = 0; + return; + } + if (strcmp ("qSymbol::", arg_own_buf) == 0) { /* We have no symbol to read. */ write_ok (arg_own_buf); diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 0a7d81389e..6181046d1e 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -279,6 +279,7 @@ static void usage_NORETURN ( int need_help ) " --progress-interval=<number> report progress every <number>\n" " CPU seconds [0, meaning disabled]\n" " --command-line-only=no|yes only use command line options [no]\n" +" --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]\n" "\n" " Vex options for all Valgrind tools:\n" " --vex-iropt-verbosity=<0..9> [0]\n" @@ -562,6 +563,8 @@ static void process_option (Clo_Mode mode, } else if VG_INT_CLOM (cloPD, arg, "--vgdb-poll", VG_(clo_vgdb_poll)) {} else if VG_INT_CLOM (cloPD, arg, "--vgdb-error", VG_(clo_vgdb_error)) {} + else if VG_BOOL_CLO (arg, "--launched-with-multi", + VG_(clo_launched_with_multi)) {} else if VG_USET_CLOM (cloPD, arg, "--vgdb-stop-at", "startup,exit,abexit,valgrindabexit", VG_(clo_vgdb_stop_at)) {} diff --git a/coregrind/m_options.c b/coregrind/m_options.c index 92ac3ad190..1483af2d9d 100644 --- a/coregrind/m_options.c +++ b/coregrind/m_options.c @@ -106,6 +106,7 @@ VgVgdb VG_(clo_vgdb) = Vg_VgdbYes; #endif Int VG_(clo_vgdb_poll) = 5000; Int VG_(clo_vgdb_error) = 999999999; +Bool VG_(clo_launched_with_multi) = False; UInt VG_(clo_vgdb_stop_at) = 0; const HChar *VG_(clo_vgdb_prefix) = NULL; const HChar *VG_(arg_vgdb_prefix) = NULL; diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index 9a9b90e585..1cc37a3566 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -24,6 +24,8 @@ The GNU General Public License is contained in the file COPYING. */ +/* For accept4. */ +#define _GNU_SOURCE #include "vgdb.h" #include "config.h" @@ -45,6 +47,7 @@ #include <sys/socket.h> #include <sys/stat.h> #include <sys/time.h> +#include <sys/wait.h> /* vgdb has two usages: 1. relay application between gdb and the gdbserver embedded in valgrind. @@ -70,6 +73,9 @@ int debuglevel; Bool timestamp = False; char timestamp_out[20]; static char *vgdb_prefix = NULL; +static char *valgrind_path = NULL; +static char **vargs; +static char cvargs = 0; char *timestamp_str (Bool produce) { @@ -169,7 +175,17 @@ void map_vgdbshared(char* shared_mem) { struct stat fdstat; void **s; - shared_mem_fd = open(shared_mem, O_RDWR); + int tries = 50; + int err; + + /* valgrind might still be starting up, give it 5 seconds. */ + do { + shared_mem_fd = open(shared_mem, O_RDWR | O_CLOEXEC); + err = errno; + if (shared_mem_fd == -1 && err == ENOENT && tries > 0) + usleep (100000); /* wait 0.1 seconds */ + } while (shared_mem_fd == -1 && err == ENOENT && tries-- > 0); + /* shared_mem_fd will not be closed till vgdb exits. */ if (shared_mem_fd == -1) @@ -330,7 +346,7 @@ int open_fifo(const char* name, int flags, const char* desc) { int fd; DEBUG(1, "opening %s %s\n", name, desc); - fd = open(name, flags); + fd = open(name, flags | O_CLOEXEC); if (fd == -1) XERROR(errno, "error opening %s %s\n", name, desc); @@ -440,6 +456,10 @@ static char *shared_mem; static int from_gdb = 0; /* stdin by default, changed if --port is given. */ static char *from_gdb_to_pid; /* fifo name to write gdb command to pid */ + +static int to_gdb = 1; /* stdout by default, changed if --port is given. */ +static char *to_gdb_from_pid; /* fifo name to read pid replies */ + /* Returns True in case read/write operations were done properly. Returns False in case of error. to_pid is the file descriptor to write to the process pid. */ @@ -448,6 +468,7 @@ Bool read_from_gdb_write_to_pid(int to_pid) { char buf[PBUFSIZ+1]; // +1 for trailing \0 int nrread; + Bool ret; nrread = read_buf(from_gdb, buf, "from gdb on stdin"); if (nrread <= 0) { @@ -459,11 +480,14 @@ Bool read_from_gdb_write_to_pid(int to_pid) shutting_down = True; return False; } - return write_buf(to_pid, buf, nrread, "to_pid", /* notify */ True); + ret = write_buf(to_pid, buf, nrread, "to_pid", /* notify */ True); + if (!ret) { + /* Let gdb know the packet couldn't be delivered. */ + write_buf(to_gdb, "$E01#a6", 8, "error back to gdb", False); + } + return ret; } -static int to_gdb = 1; /* stdout by default, changed if --port is given. */ -static char *to_gdb_from_pid; /* fifo name to read pid replies */ /* Returns True in case read/write operations were done properly. Returns False in case of error. from_pid is the file descriptor to read data from the process pid. */ @@ -491,7 +515,12 @@ void wait_for_gdb_connect(int in_port) { struct sockaddr_in addr; +#ifdef SOCK_CLOEXEC + int listen_gdb = socket(PF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); +#else int listen_gdb = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); +#endif + int gdb_connect; if (-1 == listen_gdb) { @@ -520,7 +549,12 @@ void wait_for_gdb_connect(int in_port) XERROR(errno, "error listen failed\n"); } +#ifdef SOCK_CLOEXEC + gdb_connect = accept4(listen_gdb, NULL, NULL, SOCK_CLOEXEC); +#else gdb_connect = accept(listen_gdb, NULL, NULL); +#endif + if (gdb_connect < 0) { XERROR(errno, "accept failed\n"); } @@ -565,6 +599,15 @@ void prepare_fifos_and_shared_mem(int pid) map_vgdbshared(shared_mem); } +static void +cleanup_fifos_and_shared_mem(void) +{ + free(from_gdb_to_pid); + free(to_gdb_from_pid); + free(shared_mem); + close(shared_mem_fd); +} + /* Convert hex digit A to a number. */ static int @@ -841,10 +884,673 @@ void close_connection(int to_pid, int from_pid) #endif } -/* Relay data between gdb and Valgrind gdbserver, till EOF or an - error is encountered. */ static -void gdb_relay(int pid) +int tohex (int nib) +{ + if (nib < 10) + return '0' + nib; + else + return 'a' + nib - 10; +} + +/* Returns an allocated hex-decoded string from the buf starting at offset + off. Will update off to where the buf has been decoded. Stops decoding + at end of buf (zero) or when seeing the delim char. */ +static +char *decode_hexstring (const char *buf, size_t prefixlen, size_t len) +{ + int buflen; + char *buf_print; + + if (len) + buflen = len; + else + buflen = strlen(buf) - prefixlen; + + buf_print = vmalloc (buflen/2 + 1); + + for (int i = 0; i < buflen; i = i + 2) { + buf_print[i/2] = ((fromhex(buf[i+prefixlen]) << 4) + + fromhex(buf[i+prefixlen+1])); + } + buf_print[buflen/2] = '\0'; + DEBUG(1, "decode_hexstring: %s\n", buf_print); + return buf_print; +} + +static Bool +write_to_gdb (const char *m, int cnt) +{ + int written = 0; + while (written < cnt) { + int res = write (to_gdb, m + written, cnt - written); + if (res < 0) { + perror ("write_to_gdb"); + return False; + } + written += res; + } + + return True; +} + +static Bool +write_checksum (const char *str) +{ + unsigned char csum = 0; + int i = 0; + while (str[i] != 0) + csum += str[i++]; + + char p[2]; + p[0] = tohex ((csum >> 4) & 0x0f); + p[1] = tohex (csum & 0x0f); + return write_to_gdb (p, 2); +} + +static Bool +write_reply(const char *reply) +{ + write_to_gdb ("$", 1); + write_to_gdb (reply, strlen (reply)); + write_to_gdb ("#", 1); + return write_checksum (reply); +} + +/* Creates a packet from a string message, called needs to free. */ +static char * +create_packet(const char *msg) +{ + unsigned char csum = 0; + int i = 1; + char *p = vmalloc (strlen (msg) + 5); /* $ + msg + # + hexhex + 0 */ + strcpy (&p[1], msg); + p[0] = '$'; + while (p[i] != 0) + csum += p[i++]; + p[i++] = '#'; + p[i++] = tohex ((csum >> 4) & 0x0f); + p[i++] = tohex (csum & 0x0f); + p[i] = '\0'; + return p; +} + +static int read_one_char (char *c) +{ + int i; + do + i = read (from_gdb, c, 1); + while (i == -1 && errno == EINTR); + + return i; +} + +static Bool +send_packet(const char *reply, int noackmode) +{ + int ret; + char c; + +send_packet_start: + if (!write_reply(reply)) + return False; + if (!noackmode) { + // Look for '+' or '-'. + // We must wait for "+" if !noackmode. + do { + ret = read_one_char(&c); + if (ret <= 0) + return False; + // And if in !noackmode if we get "-" we should resent the packet. + if (c == '-') + goto send_packet_start; + } while (c != '+'); + DEBUG(1, "sent packet to gdb got: %c\n",c); + } + return True; +} + +// Reads one packet from_gdb starting with $ into buf. +// Skipping any other characters. +// Returns the size of the packet, 0 for end of input, +// or -1 if no packet could be read. +static int receive_packet(char *buf, int noackmode) +{ + int bufcnt = 0, ret; + char c, c1, c2; + unsigned char csum = 0; + + // Look for first '$' (start of packet) or error. + receive_packet_start: + do { + ret = read_one_char(&c); + if (ret <= 0) + return ret; + } while (c != '$'); + + // Found start of packet ('$') + while (bufcnt < (PBUFSIZ+1)) { + ret = read_one_char(&c); + if (ret <= 0) + return ret; + if (c == '#') { + if ((ret = read_one_char(&c1)) <= 0 + || (ret = read_one_char(&c2)) <= 0) { + return ret; + } + c1 = fromhex(c1); + c2 = fromhex(c2); + break; + } + buf[bufcnt] = c; + csum += buf[bufcnt]; + bufcnt++; + } + + // Packet complete, add terminator. + buf[bufcnt] ='\0'; + + if (!(csum == (c1 << 4) + c2)) { + TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", + (c1 << 4) + c2, csum, buf); + if (!noackmode) + if (!write_to_gdb ("-", 1)) + return -1; + /* Try again, gdb should resend the packet. */ + bufcnt = 0; + csum = 0; + goto receive_packet_start; + } + + if (!noackmode) + if (!write_to_gdb ("+", 1)) + return -1; + return bufcnt; +} + +// Returns a pointer to the char after the next delim char. +static const char *next_delim_string (const char *buf, char delim) +{ + while (*buf) { + if (*buf++ == delim) + break; + } + return buf; +} + +// Throws away the packet name and decodes the hex string, which is placed in +// decoded_string (the caller owns this and is responsible for freeing it). +static int split_hexdecode(const char *buf, const char *string, + const char *delim, char **decoded_string) +{ + const char *next_str = next_delim_string(buf, *delim); + if (next_str) { + *decoded_string = decode_hexstring (next_str, 0, 0); + DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string); + return 1; + } else { + TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf); + return 0; + } +} + +static int count_delims(char delim, char *buf) +{ + size_t count = 0; + char *ptr = buf; + + while (*ptr) + count += *ptr++ == delim; + return count; +} + +// Determine the length of the arguments. +// This depends on the len array being initialized to -1 for each element. +// We first skip the command (e.g. vRun;arg0;arg1) +static void count_len(char delim, char *buf, size_t *len) +{ + int i = 0; + char *ptr = buf; + + // Skip the command + while (*ptr && *ptr != delim) + ptr++; + + // Delimiter counts towards the first arg0 + if (*ptr == delim) { + ptr++; + len[i]++; + } + + // For each arg0... count chars (delim counts towards next arg) + while (*ptr) { + i += *ptr++ == delim; + len[i]++; + } +} + +/* Declare here, will be used early, implementation follows later. */ +static void gdb_relay(int pid, int send_noack_mode, char *q_buf); + +/* Returns zero on success (and the pid of the valgrind process), + or the errno from the child on failure. */ +static +int fork_and_exec_valgrind (int argc, char **argv, const char *working_dir, + pid_t *pid) +{ + int err = 0; + // We will use a pipe to track what the child does, + // so we can report failure. + int pipefd[2]; + if (pipe2 (pipefd, O_CLOEXEC) == -1) { + err = errno; + perror ("pipe2 failed"); + return err; + } + + pid_t p = fork (); + if (p < 0) { + err = errno; + perror ("fork failed"); + return err; + } else if (p > 0) { + // I am the parent (vgdb), p is the pid of the child (valgrind) + // We only read from the child to see if everything is OK. + // If the pipe closes, we get zero back, which is good. + // An error reading the pipe is bad (and really shouldn't happen). + // Otherwise the child sent us an errno code about what went wrong. + close (pipefd[1]); + + while (err == 0) { + int r = read (pipefd[0], &err, sizeof (int)); + if (r == 0) // end of file, good pipe closed after execve + break; + if (r == -1) { + if (errno == EINTR) + continue; + else { + err = errno; + perror ("pipe read"); + } + } + } + + close (pipefd[0]); + if (err != 0) + return err; + else { + *pid = p; + return 0; + } + } else { + // p == 0, I am the child (will start valgrind) + // We write success to the pipe, no need to read from it. + close (pipefd[0]); + + if (working_dir != NULL && working_dir[0] != '\0') { + if (chdir (working_dir) != 0) { + err = errno; + perror("chdir"); + write (pipefd[1], &err, sizeof (int)); + _exit (-1); + } + } + + /* Try to launch valgrind. Add --vgdb-error=0 to stop immediately so we + can attach and --launched-with-multi to let valgrind know it doesn't + need to show a banner how to connect to gdb, we will do that + automagically. And add --vgdb-shadow-registers=yes to make shadow + registers available by default. Add any other valgrind arguments the + user gave with --vargs. Then the rest of the arguments to valgrind are + the program to exec plus its arguments. */ + const int extra_vargs = 3; + /* vargv[0] == "valgrind", + vargv[1..extra_vargs] == static valgrind arguments vgdb needs, + vargv[extra_vargs+1..extra_vargs+1+cvargs] == user valgrind arguments, + vargv[extra_vargs+1+cvargs..extra_vargs+1+cvargs+args] == prog + args, + vargs[arguments - 1] = NULL */ + int arguments = 1 + extra_vargs + cvargs + argc + 1; + // We combine const and non-const char[]. This is mildly annoying + // since we then need a char *const * for execvp. So we strdup the + // const char*. Not pretty :{ + char **vargv = vmalloc (arguments * sizeof (char *)); + vargv[0] = strdup ("valgrind"); + vargv[1] = strdup ("--vgdb-error=0"); + vargv[2] = strdup ("--launched-with-multi=yes"); + vargv[3] = strdup ("--vgdb-shadow-registers=yes"); + // Add --vargs + for (int i = 0; i < cvargs; i++) { + vargv[i + extra_vargs + 1] = vargs[i]; + } + // Add command and args + for (int i = 0; i < argc; i++) { + vargv[i + extra_vargs + 1 + cvargs] = argv[i]; + } + vargv[arguments - 1] = NULL; + + if (!valgrind_path) { + // TODO use execvpe (or something else if not on GNU/Linux + /* We want to make a copy of the environ on start. When we + get a QEnvironmentReset we copy that back. If we get an + EvironSet/Add/Remove we update the copy. */ + execvp ("valgrind", vargv); + } + else { + vargv[0] = valgrind_path; + execvp (vargv[0], vargv); + } + + // We really shouldn't get here... + err = errno; + /* Note we are after fork and exec failed, we cannot really call + perror or printf in this situation since they aren't async-safe. */ + // perror ("execvp valgrind"); + // printf ("execve returned??? confusing: %d\n", res); + write (pipefd[1], &err, sizeof (int)); + _exit (-1); + } + + abort (); // Impossible +} + +/* Do multi stuff. */ +static +void do_multi_mode(void) +{ + char *buf = vmalloc(PBUFSIZ+1); + char *q_buf = vmalloc(PBUFSIZ+1); //save the qSupported packet sent by gdb + //to send it to the valgrind gdbserver later + q_buf[0] = '\0'; + int noackmode = 0, pkt_size = 0, bad_unknown_packets = 0; + char *string = NULL; + char *working_dir = NULL; + DEBUG(1, "doing multi stuff...\n"); + while (1){ + /* We get zero if the pipe was closed (EOF), or -1 on error reading from + the pipe to gdb. */ + pkt_size = receive_packet(buf, noackmode); + if (pkt_size <= 0) { + DEBUG(1, "receive_packet: %d\n", pkt_size); + break; + } + + DEBUG(1, "packet recieved: '%s'\n", buf); + +#define QSUPPORTED "qSupported:" +#define STARTNOACKMODE "QStartNoAckMode" +#define QRCMD "qRcmd" // This is the monitor command in gdb +#define VRUN "vRun" +#define XFER "qXfer" +#define QATTACHED "qAttached" +#define QENVIRONMENTHEXENCODED "QEnvironmentHexEncoded" +#define QENVIRONMENTRESET "QEnvironmentReset" +#define QENVIRONMENTUNSET "QEnvironmentUnset" +#define QSETWORKINGDIR "QSetWorkingDir" +#define QTSTATUS "qTStatus" + + if (strncmp(QSUPPORTED, buf, strlen(QSUPPORTED)) == 0) { + DEBUG(1, "CASE %s\n", QSUPPORTED); + // And here is our reply. + // XXX error handling? We don't check the arguments. + char *reply; + strcpy(q_buf, buf); + // Keep this in sync with coregrind/m_gdbserver/server.c + asprintf (&reply, + "PacketSize=%x;" + "QStartNoAckMode+;" + "QPassSignals+;" + "QCatchSyscalls+;" + /* Just report support always. */ + "qXfer:auxv:read+;" + /* We'll force --vgdb-shadow-registers=yes */ + "qXfer:features:read+;" + "qXfer:exec-file:read+;" + "qXfer:siginfo:read+;" + /* Some extra's vgdb support before valgrind starts up. */ + "QEnvironmentHexEncoded+;" + "QEnvironmentReset+;" + "QEnvironmentUnset+;" + "QSetWorkingDir+", (UInt)PBUFSIZ - 1); + send_packet(reply, noackmode); + free (reply); + } + else if (strncmp(STARTNOACKMODE, buf, strlen(STARTNOACKMODE)) == 0) { + // We have to ack this one + send_packet("OK", 0); + noackmode = 1; + } + else if (buf[0] == '!') { + send_packet("OK", noackmode); + } + else if (buf[0] == '?') { + send_packet("W00", noackmode); + } + else if (strncmp("H", buf, strlen("H")) == 0) { + // Set thread packet, but we are not running yet. + send_packet("E01", noackmode); + } + else if (strncmp("vMustReplyEmpty", buf, strlen("vMustReplyEmpty")) == 0) { + send_packet ("", noackmode); + } + else if (strncmp(QRCMD, buf, strlen(QRCMD)) == 0) { + send_packet ("No running target, monitor commands not available yet.", noackmode); + + char *decoded_string = decode_hexstring (buf, strlen (QRCMD) + 1, 0); + DEBUG(1, "qRcmd decoded: %s\n", decoded_string); + free (decoded_string); + } + else if (strncmp(VRUN, buf, strlen(VRUN)) == 0) { + // vRun;filename[;argument]* + // vRun, filename and arguments are split on ';', + // no ';' at the end. + // If there are no arguments count is one (just the filename). + // Otherwise it is the number of arguments plus one (the filename). + // The filename must be there and starts after the first ';'. + // TODO: Handle vRun;[;argument]* + // https://www.sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets + // If filename is an empty string, the stub may use a default program + // (e.g. the last program run). + size_t count = count_delims(';', buf); + size_t *len = vmalloc(count * sizeof(count)); + const char *delim = ";"; + const char *next_str = next_delim_string(buf, *delim); + char **decoded_string = vmalloc(count * sizeof (char *)); + + // Count the lenghts of each substring, init to -1 to compensate for + // each substring starting with a delim char. + for (int i = 0; i < count; i++) + len[i] = -1; + count_len(';', buf, len); + if (next_str) { + DEBUG(1, "vRun: next_str %s\n", next_str); + for (int i = 0; i < count; i++) { + /* Handle the case when the arguments + * was specified to gdb's run command + * but no remote exec-file was set, + * so the first vRun argument is missing. + * For example vRun;;6c. */ + if (*next_str == *delim) { + next_str++; + /* empty string that can be freed. */ + decoded_string[i] = strdup(""); + } + else { + decoded_string[i] = decode_hexstring (next_str, 0, len[i]); + if (i < count - 1) + next_str = next_delim_string(next_str, *delim); + } + DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", decoded_string[i], next_str, i, len[i]); + } + + /* If we didn't get any arguments or the filename is an empty + string, valgrind won't know which program to run. */ + DEBUG (1, "count: %d, len[0]: %d\n", count, len[0]); + if (! count || len[0] == 0) { + free(len); + for (int i = 0; i < count; i++) + free (decoded_string[i]); + free (decoded_string); + send_packet ("E01", noackmode); + continue; + } + + /* We have collected the decoded strings so we can use them to + launch valgrind with the correct arguments... We then use the + valgrind pid to start relaying packets. */ + pid_t valgrind_pid = -1; + int res = fork_and_exec_valgrind (count, + decoded_string, + working_dir, + &valgrind_pid); + + if (res == 0) { + // Lets report we Stopped with SIGTRAP (05). + send_packet ("S05", noackmode); + prepare_fifos_and_shared_mem(valgrind_pid); + DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", from_gdb_to_pid, to_gdb_from_pid); + // gdb_rely is an endless loop till valgrind quits. + shutting_down = False; + + gdb_relay (valgrind_pid, 1, q_buf); + cleanup_fifos_and_shared_mem(); + DEBUG(1, "valgrind relay done\n"); + int status; + pid_t p = waitpid (valgrind_pid, &status, 0); + DEBUG(2, "waitpid: %d\n", (int) p); + if (p == -1) + DEBUG(1, "waitpid error %s\n", strerror (errno)); + else { + if (WIFEXITED(status)) + DEBUG(1, "valgrind exited with %d\n", + WEXITSTATUS(status)); + else if (WIFSIGNALED(status)) + DEBUG(1, "valgrind kill by signal %d\n", + WTERMSIG(status)); + else + DEBUG(1, "valgind unexpectedly stopped or continued"); + } + } else { + send_packet ("E01", noackmode); + DEBUG(1, "OOPS! couldn't launch valgrind %s\n", + strerror (res)); + } + + free(len); + for (int i = 0; i < count; i++) + free (decoded_string[i]); + free (decoded_string); + } else { + free(len); + send_packet ("E01", noackmode); + DEBUG(1, "vRun decoding error: no next_string!\n"); + continue; + } + } else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) { + send_packet ("1", noackmode); + DEBUG(1, "qAttached sent: '1'\n"); + const char *next_str = next_delim_string(buf, ':'); + if (next_str) { + char *decoded_string = decode_hexstring (next_str, 0, 0); + DEBUG(1, "qAttached decoded: %s, next_str %s\n", decoded_string, next_str); + free (decoded_string); + } else { + DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf); + continue; + } + } /* Reset the state of environment variables in the remote target before starting + the inferior. In this context, reset means unsetting all environment variables + that were previously set by the user (i.e., were not initially present in the environment). */ + else if (strncmp(QENVIRONMENTRESET, buf, + strlen(QENVIRONMENTRESET)) == 0) { + send_packet ("OK", noackmode); + // TODO clear all environment strings. We're not using + // environment strings now. But we should. + } else if (strncmp(QENVIRONMENTHEXENCODED, buf, + strlen(QENVIRONMENTHEXENCODED)) == 0) { + send_packet ("OK", noackmode); + if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string)) + break; + // TODO Collect all environment strings and add them to environ + // before launcing valgrind. + free (string); + string = NULL; + } else if (strncmp(QENVIRONMENTUNSET, buf, + strlen(QENVIRONMENTUNSET)) == 0) { + send_packet ("OK", noackmode); + if (!split_hexdecode(buf, QENVIRONMENTUNSET, ":", &string)) + break; + // TODO Remove this environment string from the collection. + free (string); + string = NULL; + } else if (strncmp(QSETWORKINGDIR, buf, + strlen(QSETWORKINGDIR)) == 0) { + // Silly, but we can only reply OK, even if the working directory is + // bad. Errors will be reported when we try to execute the actual + // process. + send_packet ("OK", noackmode); + // Free any previously set working_dir + free (working_dir); + working_dir = NULL; + if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { + continue; // We cannot report the error to gdb... + } + DEBUG(1, "set working dir to: %s\n", working_dir); + } else if (strncmp(XFER, buf, strlen(XFER)) == 0) { + char *buf_dup = strdup(buf); + DEBUG(1, "strdup: buf_dup %s\n", buf_dup); + if (buf_dup) { + const char *delim = ":"; + size_t count = count_delims(delim[0], buf); + if (count < 4) { + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + char *decoded_string = decode_hexstring (buf_dup, 0, 0); + DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); + free (decoded_string); + } + free (buf_dup); + } else { + DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf); + free (buf_dup); + continue; + } + // Whether we could decode it or not, we cannot handle it now. We + // need valgrind gdbserver to properly reply. So error out here. + send_packet ("E00", noackmode); + } else if (strncmp(QTSTATUS, buf, strlen(QTSTATUS)) == 0) { + // We don't support trace experiments + DEBUG(1, "Got QTSTATUS\n"); + send_packet ("", noackmode); + } else if (strcmp("qfThreadInfo", buf) == 0) { + DEBUG(1, "Got qfThreadInfo\n"); + /* There are no threads yet, reply 'l' end of list. */ + send_packet ("l", noackmode); + } else if (buf[0] != '\0') { + // We didn't understand. + DEBUG(1, "Unknown packet received: '%s'\n", buf); + bad_unknown_packets++; + if (bad_unknown_packets > 10) { + DEBUG(1, "Too many bad/unknown packets received\n"); + break; + } + send_packet ("", noackmode); + } + } + DEBUG(1, "done doing multi stuff...\n"); + free(working_dir); + free(buf); + free(q_buf); + + shutting_down = True; + close (to_gdb); + close (from_gdb); +} + +/* Relay data between gdb and Valgrind gdbserver, till EOF or an error is + encountered. q_buf is the qSupported packet received from gdb. */ +static +void gdb_relay(int pid, int send_noack_mode, char *q_buf) { int from_pid = -1; /* fd to read from pid */ int to_pid = -1; /* fd to write to pid */ @@ -863,6 +1569,16 @@ void gdb_relay(int pid) sigusr1_fd = to_pid; /* allow simulating user typing control-c */ + Bool waiting_for_noack_mode = False; + Bool waiting_for_qsupported = False; + if(send_noack_mode) { + DEBUG(1, "gdb_relay: to_pid %d, from_pid: %d\n", to_pid, from_pid); + write_buf(to_pid, "$QStartNoAckMode#b0", 19, + "write start no ack mode", + /* notify */ True); + waiting_for_noack_mode = True; + } + while (1) { ConnectionKind ck; int ret; @@ -903,11 +1619,46 @@ void gdb_relay(int pid) if (pollfds[ck].revents & POLLIN) { switch (ck) { case FROM_GDB: + if (waiting_for_noack_mode || waiting_for_qsupported) + break; /* Don't add any messages while vgdb is talking. */ if (!read_from_gdb_write_to_pid(to_pid)) shutting_down = True; break; case FROM_PID: - if (!read_from_pid_write_to_gdb(from_pid)) + // First handle any messages from vgdb + if (waiting_for_noack_mode) { + char buf[PBUFSIZ+1]; // +1 for trailing \0 + size_t buflen; + buflen = getpkt(buf, from_pid, to_pid); + if (buflen != 2 || strcmp(buf, "OK") != 0) { + if (buflen != 2) + ERROR(0, "no ack mode: unexpected buflen %d, buf %s\n", + buflen, buf); + else + ERROR(0, "no ack mode: unexpected packet %s\n", buf); + } + waiting_for_noack_mode = False; + + /* Propagate qSupported to valgrind, we already replied. */ + if (q_buf != NULL && q_buf[0] != '\0') { + char *pkt = create_packet (q_buf); + write_buf(to_pid, pkt, strlen(pkt), + "write qSupported", /* notify */ True); + free(pkt); + waiting_for_qsupported = True; + } + } else if (waiting_for_qsupported) { + char buf[PBUFSIZ+1]; // +1 for trailing \0 + size_t buflen; + buflen = getpkt(buf, from_pid, to_pid); + /* Should we sanity check the result? */ + if (buflen > 0) { + waiting_for_qsupported = False; + } else { + ERROR(0, "Unexpected getpkt for qSupported reply: %d\n", + buflen); + } + } else if (!read_from_pid_write_to_gdb(from_pid)) shutting_down = True; break; default: XERROR(0, "unexpected POLLIN on %s\n", @@ -1106,7 +1857,7 @@ void report_pid(int pid, Bool on_stdout) TSFPRINTF(out, "use --pid=%d for ", pid); sprintf(cmdline_file, "/proc/%d/cmdline", pid); - fd = open(cmdline_file, O_RDONLY); + fd = open(cmdline_file, O_RDONLY | O_CLOEXEC); if (fd == -1) { DEBUG(1, "error opening cmdline file %s %s\n", cmdline_file, strerror(errno)); @@ -1148,6 +1899,7 @@ void usage(void) " [--wait=<number>] [--max-invoke-ms=<number>]\n" " [--port=<portnr>\n" " [--cmd-time-out=<number>] [-l] [-T] [-D] [-d]\n" +" [--multi]\n" " \n" " --pid arg must be given if multiple Valgrind gdbservers are found.\n" " --vgdb-prefix arg must be given to both Valgrind and vgdb utility\n" @@ -1161,6 +1913,9 @@ void usage(void) " --port instructs vgdb to listen for gdb on the specified port nr.\n" " --cmd-time-out (default 99999999) tells vgdb to exit if the found Valgrind\n" " gdbserver has not processed a command after number seconds\n" +" --multi start in extended-remote mode, wait for gdb to tell us what to run\n" +" --valgrind, pass the path to valgrind to use. If not specified, the system valgrind will be launched.\n" +" --vargs everything that follows is an argument for valgrind." " -l arg tells to show the list of running Valgrind gdbserver and then exit.\n" " -T arg tells to add timestamps to vgdb information messages.\n" " -D arg tells to show shared mem status and then exit.\n" @@ -1371,6 +2126,7 @@ static void parse_options(int argc, char** argv, Bool *p_show_shared_mem, Bool *p_show_list, + Bool *p_multi_mode, int *p_arg_pid, int *p_check_trials, int *p_port, @@ -1379,6 +2135,7 @@ void parse_options(int argc, char** argv, { Bool show_shared_mem = False; Bool show_list = False; + Bool multi_mode = False; int arg_pid = -1; int check_trials = 1; int last_command = -1; @@ -1397,6 +2154,8 @@ void parse_options(int argc, char** argv, show_shared_mem = True; } else if (is_opt(argv[i], "-l")) { show_list = True; + } else if (is_opt(argv[i], "--multi")) { + multi_mode = True; } else if (is_opt(argv[i], "-T")) { timestamp = True; } else if (is_opt(argv[i], "--pid=")) { @@ -1431,7 +2190,28 @@ void parse_options(int argc, char** argv, arg_errors++; } } else if (is_opt(argv[i], "--vgdb-prefix=")) { - vgdb_prefix = argv[i] + 14; + vgdb_prefix = strdup (argv[i] + 14); + } else if (is_opt(argv[i], "--valgrind=")) { + char *path = argv[i] + 11; + /* Compute the absolute path. */ + valgrind_path = realpath(path, NULL); + if (!valgrind_path) { + TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", path, strerror (errno)); + exit(1); + } + DEBUG(2, "valgrind's real path: %s\n", valgrind_path); + } else if (is_opt(argv[i], "--vargs")) { + // Everything that follows now is an argument for valgrind + // No other options (or commands) can follow + // argc - i is the number of left over arguments + // allocate enough space, but all args in it. + cvargs = argc - i - 1; + vargs = vmalloc (cvargs * sizeof(vargs)); + i++; + for (int j = 0; i < argc; i++) { + vargs[j] = argv[i]; + j++; + } } else if (is_opt(argv[i], "-c")) { last_command++; commands[last_command] = vmalloc(1); @@ -1464,6 +2244,15 @@ void parse_options(int argc, char** argv, if (vgdb_prefix == NULL) vgdb_prefix = vgdb_prefix_default(); + if (multi_mode + && (show_shared_mem + || show_list + || last_command != -1)) { + arg_errors++; + TSFPRINTF(stderr, + "Cannot use -D, -l or COMMANDs when using --multi mode\n"); + } + if (isatty(0) && !show_shared_mem && !show_list @@ -1507,6 +2296,7 @@ void parse_options(int argc, char** argv, *p_show_shared_mem = show_shared_mem; *p_show_list = show_list; + *p_multi_mode = multi_mode; *p_arg_pid = arg_pid; *p_check_trials = check_trials; *p_port = int_port; @@ -1520,6 +2310,7 @@ int main(int argc, char** argv) Bool show_shared_mem; Bool show_list; + Bool multi_mode; int arg_pid; int check_trials; int in_port; @@ -1529,6 +2320,7 @@ int main(int argc, char** argv) parse_options(argc, argv, &show_shared_mem, &show_list, + &multi_mode, &arg_pid, &check_trials, &in_port, @@ -1542,9 +2334,13 @@ int main(int argc, char** argv) if (max_invoke_ms > 0 || last_command == -1) install_handlers(); - pid = search_arg_pid(arg_pid, check_trials, show_list); + if (!multi_mode) { + pid = search_arg_pid(arg_pid, check_trials, show_list); - prepare_fifos_and_shared_mem(pid); + prepare_fifos_and_shared_mem(pid); + } else { + pid = 0; + } if (in_port > 0) wait_for_gdb_connect(in_port); @@ -1561,16 +2357,18 @@ int main(int argc, char** argv) exit(0); } - if (last_command >= 0) { + if (multi_mode) { + do_multi_mode (); + } else if (last_command >= 0) { standalone_send_commands(pid, last_command, commands); } else { - gdb_relay(pid); + gdb_relay(pid, 0, NULL); } - - free(from_gdb_to_pid); - free(to_gdb_from_pid); - free(shared_mem); + free(vgdb_prefix); + free(valgrind_path); + if (!multi_mode) + cleanup_fifos_and_shared_mem(); for (i = 0; i <= last_command; i++) free(commands[i]); diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml index 1609ee2a95..bb695d2d3d 100644 --- a/docs/xml/manual-core-adv.xml +++ b/docs/xml/manual-core-adv.xml @@ -482,6 +482,29 @@ Loaded symbols for /lib/ld-linux.so.2 (gdb) ]]></programlisting> +<para>If you want to use the <option>--multi</option> mode which makes vgdb start in extended-remote mode, set the following in GDB: +<screen><![CDATA[ +# gdb prog +(gdb) set remote exec-file prog +(gdb) set sysroot / +(gdb) target extended-remote | vgdb --multi --vargs -q +(gdb) start +Temporary breakpoint 1 at 0x24e0 +Starting program: prog +relaying data between gdb and process 2999348 + +Temporary breakpoint 1, 0x000000000010a4a0 in main () +(gdb) +]]></screen> +</para> + +<para>Note that in <option>--multi</option> mode you don't have to +start valgrind separately. vgdb will start valgrind for +you. vgdb <option>--multi</option> mode is experimental and currently +has some limitations like not being able to see program stdin and +stdout. Also you have to explicitly set the remote exec-file and +sysroot to tell GDB the "remote" and local files are the same.</para> + <para>Once GDB is connected to the Valgrind gdbserver, it can be used in the same way as if you were debugging the program natively:</para> <itemizedlist> @@ -1254,7 +1277,7 @@ $5 = 36 <para> vgdb ("Valgrind to GDB") is a small program that is used as an intermediary between Valgrind and GDB or a shell. -Therefore, it has two usage modes: +It has three usage modes: </para> <!-- start of xi:include in the manpage --> <orderedlist id="vgdb.desc.modes"> @@ -1275,6 +1298,19 @@ Therefore, it has two usage modes: </para> </listitem> + <listitem id="manual-core-adv.vgdb-multi" xreflabel="vgdb multi"> + <para>In the <option>--multi</option> mode, Vgdb uses the extended + remote protocol to communicate with Gdb. This allows you to view + output from both valgrind and GDB in the GDB session. This is + accomplished via the "target extended-remote | vgdb --multi". In + this mode you no longer need to start valgrind yourself. vgdb will + start up valgrind when gdb tells it to run a new program. For this + usage, the vgdb OPTIONS(s) can also include <option>--valgrind</option> + and <option>--vargs</option> to describe how valgrind should be + started. + </para> + </listitem> + </orderedlist> <!-- end of xi:include in the manpage --> @@ -1369,7 +1405,28 @@ gdb prog where targetip is the ip address or hostname of the target computer. </para></listitem> </varlistentry> - + + <varlistentry> + <term><option>--vgdb-multi</option></term> + <listitem><para>Makes vgdb start in extended-remote mode and to wait + for gdb to tell us what to run.</para></listitem> + </varlistentry> + + <varlistentry> + <term><option>--valgrind</option></term> + <listitem><para>The path to valgrind to use, in extended-remote + mode. If not specified, the system valgrind will be + launched.</para></listitem> + </varlistentry> + + <varlistentry> + <term><option>--vargs</option></term> + <listitem><para>Options to run valgrind with, in extended-remote + mode. For example <option>-q</option>. Everything + following <option>--vargs</option> will be provided as arguments + to valgrind as is. </para></listitem> + </varlistentry> + <varlistentry> <term><option>-c</option></term> <listitem><para>To give more than one command to a diff --git a/include/pub_tool_options.h b/include/pub_tool_options.h index 39c4137349..0f7c11bafb 100644 --- a/include/pub_tool_options.h +++ b/include/pub_tool_options.h @@ -332,6 +332,11 @@ extern Bool VG_(clo_stats); Note that this value can be changed dynamically. */ extern Int VG_(clo_vgdb_error); +/* Set by vgdb in --multi mode when launching valgrind. This suppresses + the "TO DEBUG" banner because vgdb will take care of attaching in that + case. */ +extern Bool VG_(clo_launched_with_multi); + /* If user has provided the --vgdb-prefix command line option, VG_(arg_vgdb_prefix) points at the provided argument (including the '--vgdb-prefix=' string). diff --git a/vg-in-place b/vg-in-place index b364c10dba..052fd36b7e 100755 --- a/vg-in-place +++ b/vg-in-place @@ -28,5 +28,5 @@ vgbasedir=`dirname $scriptname` # 'inner' builds. VALGRIND_LIB="$vgbasedir/.in_place" \ VALGRIND_LIB_INNER="$vgbasedir/.in_place" \ - "$vgbasedir/coregrind/valgrind" "$@" + exec "$vgbasedir/coregrind/valgrind" "$@" |
|
From: Philippe W. <phi...@sk...> - 2023-04-15 17:58:27
|
On Thu, 2023-04-13 at 22:09 +0000, Mark Wielaard wrote: > +* The vgdb utility now supports extended-remote protocol when > + invoked with --multi. In this mode the GDB run command is > + supported. Which means you don't need to run gdb and valgrind > + from different terminals. So for example to start you program > + in gdb and run it under valgrind you can do: > + $ gdb prog > + (gdb) set remote exec-file prog > + (gdb) set sysroot / > + (gdb) target extended-remote | vgdb --multi > + (gdb) start Thanks to Mark and Alexandra for this work (and sorry for the lack of earlier feedback about this). I did some tests: philippe@md:gdbserver_tests$ gdb sleepers GNU gdb (GDB) 14.0.50.20230402-git Copyright (C) 2023 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Loaded DUEL.py 0.9.6, high level data exploration language Reading symbols from sleepers... (gdb) set remote exec-file sleepers (gdb) set sysroot / (gdb) target extended-remote | vgdb --multi Remote debugging using | vgdb --multi (gdb) start Temporary breakpoint 1 at 0x16fc: file sleepers.c, line 138. Starting program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers valgrind: sleepers: command not found syscall failed: No such file or directory error opening /tmp/vgdb-pipe-shared-mem-vgdb-52790-by-philippe-on-md shared memory file Remote communication error. Target disconnected.: Connection reset by peer. (gdb) The problem is solved by giving an absolute path for the remote exec-file: (gdb) set remote exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers So, it looks like gdb knows the absolute path of the program to launch, but does not pass it to valgrind. It might be possible to have the full path given to vgdb ? The vgdb --help output is missing the --valgrind and the --vargs in the OPTIONS summary: OPTIONS are [--pid=<number>] [--vgdb-prefix=<prefix>] [--wait=<number>] [--max-invoke-ms=<number>] [--port=<portnr> [--cmd-time-out=<number>] [-l] [-T] [-D] [-d] [--multi] The vgdb --help is missing \n after the --vargs description: --vargs everything that follows is an argument for valgrind. -l arg tells to show the list of running Valgrind gdbserver and then exit. For --valgrind, pass the path to valgrind to use. If not specified, the system valgrind will be launched. Wouldn't it better (if possible) to by default launch the valgrind found at the same place as where vgdb is found ? Finally, once giving an absolute remote exec-file and an absolute path to the valgrind to launch, I cannot have it working: gdb sleepers GNU gdb (GDB) 14.0.50.20230402-git Copyright (C) 2023 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Loaded DUEL.py 0.9.6, high level data exploration language Reading symbols from sleepers... (gdb) set remote exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers (gdb) set sysroot / (gdb) target extended-remote | vgdb --multi --valgrind=/home/philippe/valgrind/git/trunk_untouched/Inst/bin/valgrind Remote debugging using | vgdb --multi --valgrind=/home/philippe/valgrind/git/trunk_untouched/Inst/bin/valgrind (gdb) start Temporary breakpoint 1 at 0x16fc: file sleepers.c, line 138. Starting program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers ==100738== Memcheck, a memory error detector ==100738== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==100738== Using Valgrind-3.21.0.RC1 and LibVEX; rerun with -h for copyright info ==100738== Command: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers ==100738== relaying data between gdb and process 100738 syscall failed: Resource temporarily unavailable error reading static buf readchar syscall failed: Resource temporarily unavailable readchar no ack mode: unexpected buflen -1, buf Unknown remote qXfer reply: 1 (gdb) quit Not too sure what is going wrong/what I am doing wrong ... (tested with 53834800424510b177c59d097c3a51a66bbaf659) Thanks Philippe |
|
From: Paul F. <pj...@wa...> - 2023-04-15 19:43:30
|
On 15-04-23 19:58, Philippe Waroquiers via Valgrind-developers wrote:
> Not too sure what is going wrong/what I am doing wrong ...
> (tested with 53834800424510b177c59d097c3a51a66bbaf659)
At first I tried running from the build dir, but that was complicated so
I switched to using it from my home dir install.
I also use the following functions in my .gdbinit file (hopefully this
will become easier eventually). It works, but doesn't allow you to
specify your own --vargs
define set-program-name
set logging file tmp.gdb
set logging overwrite on
set logging redirect on
set logging enabled on
python print(f"set $programname =
\"{gdb.current_progspace().filename}\"")
set logging enabled off
set logging redirect off
set logging overwrite off
source tmp.gdb
end
define start_vg
set-program-name
eval "set remote exec-file %s", $programname
show remote exec-file
set sysroot /
target extended-remote | vgdb --multi --vargs -q
start
end
A+
Paul
|
|
From: Philippe W. <phi...@sk...> - 2023-04-15 20:27:53
|
Some additional comments below ... Thanks Philippe On Thu, 2023-04-13 at 22:09 +0000, Mark Wielaard wrote: > https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=0432ce486d61f84f6fcbeab0d812bb1f61c57561 > > commit 0432ce486d61f84f6fcbeab0d812bb1f61c57561 > Author: Alexandra Petlanova Hajkova <aha...@re...> > Date: Thu Mar 3 04:46:03 2022 -0500 > > vgdb: implement the extended-remote protocol > > > > Executing vgdb --multi makes vgdb talk the gdb extended-remote > protocol. This means that the gdb run command is supported and > vgdb will start up the program under valgrind. Which means you > don't need to run gdb and valgrind from different terminals. > Also vgdb keeps being connected to gdb after valgrind exits. So > you can easily rerun the program with the same breakpoints in > place. > > > > vgdb now implements a minimal gdbserver that just recognizes > a few extended-remote protocol packets. Once it starts up valgrind > it sets up noack and qsupported then it will forward packets > between gdb and valgrind gdbserver. After valgrind shutsdown it > resumes handling gdb packets itself. > > > > https://bugs.kde.org/show_bug.cgi?id=434057 > > > > Co-authored-by: Mark Wielaard <ma...@kl...> valgrind --help does not describe the 'multi' related option, but --help-debug describes it. That was a little bit surprising to me. As this is not a real user option, it however does not fully fit in the section: uncommon user options for all Valgrind tools: > > diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c > index 9a9b90e585..1cc37a3566 100644 > --- a/coregrind/vgdb.c > +++ b/coregrind/vgdb.c > > /* vgdb has two usages: Maybe this should describe here 3 usages ? > @@ -169,7 +175,17 @@ void map_vgdbshared(char* shared_mem) > { > struct stat fdstat; > void **s; > - shared_mem_fd = open(shared_mem, O_RDWR); > + int tries = 50; > + int err; > + > + /* valgrind might still be starting up, give it 5 seconds. */ Maybe the --wait arg (if provided) could override this hard-coded value ? > +/* Returns an allocated hex-decoded string from the buf starting at offset > + off. Will update off to where the buf has been decoded. Stops decoding > + at end of buf (zero) or when seeing the delim char. */ The comment seems not aligned with the below function (e.g. there is no off arg). > +static > +char *decode_hexstring (const char *buf, size_t prefixlen, size_t len) > +{ > + int buflen; > + char *buf_print; > + > + if (len) > + buflen = len; > + else > + buflen = strlen(buf) - prefixlen; > + > + buf_print = vmalloc (buflen/2 + 1); > + > + for (int i = 0; i < buflen; i = i + 2) { > + buf_print[i/2] = ((fromhex(buf[i+prefixlen]) << 4) > + + fromhex(buf[i+prefixlen+1])); > + } > + buf_print[buflen/2] = '\0'; > + DEBUG(1, "decode_hexstring: %s\n", buf_print); > + return buf_print; > +} > + > > > > + > +/* Creates a packet from a string message, called needs to free. */ caller needs to free ? > +static char * > +create_packet(const char *msg) > In some functions from here onwards, there are a bunch of indentation with 4 spaces while the rest of vgdb.c is 3. > + > +static int read_one_char (char *c) > +{ > + int i; > + do > + i = read (from_gdb, c, 1); > + while (i == -1 && errno == EINTR); > + > + return i; > +} > + > > + > +// Throws away the packet name and decodes the hex string, which is placed in What is the packet name ? > +// decoded_string (the caller owns this and is responsible for freeing it). > +static int split_hexdecode(const char *buf, const char *string, > + const char *delim, char **decoded_string) > +{ > + const char *next_str = next_delim_string(buf, *delim); > + if (next_str) { > + *decoded_string = decode_hexstring (next_str, 0, 0); > + DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string); > + return 1; > + } else { > + TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf); > + return 0; > + } > +} > + > +static int count_delims(char delim, char *buf) This should return a size_t > +{ > + size_t count = 0; > + char *ptr = buf; > + > + while (*ptr) > + count += *ptr++ == delim; > + return count; > +} > + > > > +/* Do multi stuff. */ > +static > +void do_multi_mode(void) > +{ > + char *buf = vmalloc(PBUFSIZ+1); > + char *q_buf = vmalloc(PBUFSIZ+1); //save the qSupported packet sent by gdb > + //to send it to the valgrind gdbserver later > + q_buf[0] = '\0'; > + int noackmode = 0, pkt_size = 0, bad_unknown_packets = 0; > + char *string = NULL; > + char *working_dir = NULL; > + DEBUG(1, "doing multi stuff...\n"); > + while (1){ > + /* We get zero if the pipe was closed (EOF), or -1 on error reading from > + the pipe to gdb. */ > + pkt_size = receive_packet(buf, noackmode); > + if (pkt_size <= 0) { > + DEBUG(1, "receive_packet: %d\n", pkt_size); > + break; > + } > + > + DEBUG(1, "packet recieved: '%s'\n", buf); typo: received > + > +#define QSUPPORTED "qSupported:" > +#define STARTNOACKMODE "QStartNoAckMode" > +#define QRCMD "qRcmd" // This is the monitor command in gdb > +#define VRUN "vRun" > +#define XFER "qXfer" > +#define QATTACHED "qAttached" > +#define QENVIRONMENTHEXENCODED "QEnvironmentHexEncoded" > +#define QENVIRONMENTRESET "QEnvironmentReset" > +#define QENVIRONMENTUNSET "QEnvironmentUnset" > +#define QSETWORKINGDIR "QSetWorkingDir" > +#define QTSTATUS "qTStatus" > + > + if (strncmp(QSUPPORTED, buf, strlen(QSUPPORTED)) == 0) { > + DEBUG(1, "CASE %s\n", QSUPPORTED); > + // And here is our reply. > + // XXX error handling? We don't check the arguments. > + char *reply; > + strcpy(q_buf, buf); > + // Keep this in sync with coregrind/m_gdbserver/server.c Worth adding a note in server.c to also point at this place ? > + asprintf (&reply, > + "PacketSize=%x;" > + "QStartNoAckMode+;" > + "QPassSignals+;" > + "QCatchSyscalls+;" > + /* Just report support always. */ > + "qXfer:auxv:read+;" > + /* We'll force --vgdb-shadow-registers=yes */ > + "qXfer:features:read+;" > + "qXfer:exec-file:read+;" > + "qXfer:siginfo:read+;" > + /* Some extra's vgdb support before valgrind starts up. */ > + "QEnvironmentHexEncoded+;" > + "QEnvironmentReset+;" > + "QEnvironmentUnset+;" > + "QSetWorkingDir+", (UInt)PBUFSIZ - 1); > + send_packet(reply, noackmode); > + free (reply); > + } > + else if (strncmp(STARTNOACKMODE, buf, strlen(STARTNOACKMODE)) == 0) { > + // We have to ack this one > + send_packet("OK", 0); > + noackmode = 1; > + } > + else if (buf[0] == '!') { > + send_packet("OK", noackmode); > + } > + else if (buf[0] == '?') { > + send_packet("W00", noackmode); > + } > + else if (strncmp("H", buf, strlen("H")) == 0) { > + // Set thread packet, but we are not running yet. > + send_packet("E01", noackmode); > + } > + else if (strncmp("vMustReplyEmpty", buf, strlen("vMustReplyEmpty")) == 0) { > + send_packet ("", noackmode); > + } > + else if (strncmp(QRCMD, buf, strlen(QRCMD)) == 0) { > + send_packet ("No running target, monitor commands not available yet.", noackmode); > + > + char *decoded_string = decode_hexstring (buf, strlen (QRCMD) + 1, 0); > + DEBUG(1, "qRcmd decoded: %s\n", decoded_string); > + free (decoded_string); > + } > + else if (strncmp(VRUN, buf, strlen(VRUN)) == 0) { > + // vRun;filename[;argument]* > + // vRun, filename and arguments are split on ';', > + // no ';' at the end. > + // If there are no arguments count is one (just the filename). > + // Otherwise it is the number of arguments plus one (the filename). > + // The filename must be there and starts after the first ';'. > + // TODO: Handle vRun;[;argument]* > + // https://www.sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets > + // If filename is an empty string, the stub may use a default program > + // (e.g. the last program run). > + size_t count = count_delims(';', buf); > + size_t *len = vmalloc(count * sizeof(count)); > + const char *delim = ";"; > + const char *next_str = next_delim_string(buf, *delim); > + char **decoded_string = vmalloc(count * sizeof (char *)); > + > + // Count the lenghts of each substring, init to -1 to compensate for typo: lenghts > + // each substring starting with a delim char. > + for (int i = 0; i < count; i++) > + len[i] = -1; > + count_len(';', buf, len); > + if (next_str) { > + DEBUG(1, "vRun: next_str %s\n", next_str); > + for (int i = 0; i < count; i++) { > + /* Handle the case when the arguments > + * was specified to gdb's run command > + * but no remote exec-file was set, > + * so the first vRun argument is missing. > + * For example vRun;;6c. */ > + if (*next_str == *delim) { > + next_str++; > + /* empty string that can be freed. */ > + decoded_string[i] = strdup(""); > + } > + else { > + decoded_string[i] = decode_hexstring (next_str, 0, len[i]); > + if (i < count - 1) > + next_str = next_delim_string(next_str, *delim); > + } > + DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", decoded_string[i], next_str, i, len[i]); Line too long. > + } > + > + /* If we didn't get any arguments or the filename is an empty > + string, valgrind won't know which program to run. */ > + DEBUG (1, "count: %d, len[0]: %d\n", count, len[0]); > + if (! count || len[0] == 0) { > + free(len); > + for (int i = 0; i < count; i++) > + free (decoded_string[i]); > + free (decoded_string); > + send_packet ("E01", noackmode); > + continue; > + } > + > + /* We have collected the decoded strings so we can use them to > + launch valgrind with the correct arguments... We then use the > + valgrind pid to start relaying packets. */ > + pid_t valgrind_pid = -1; > + int res = fork_and_exec_valgrind (count, > + decoded_string, > + working_dir, > + &valgrind_pid); > + > + if (res == 0) { > + // Lets report we Stopped with SIGTRAP (05). > + send_packet ("S05", noackmode); > + prepare_fifos_and_shared_mem(valgrind_pid); > + DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", from_gdb_to_pid, to_gdb_from_pid); line too long > + // gdb_rely is an endless loop till valgrind quits. typo: gdb_rely > + shutting_down = False; > + > + gdb_relay (valgrind_pid, 1, q_buf); > + cleanup_fifos_and_shared_mem(); > + DEBUG(1, "valgrind relay done\n"); > + int status; > + pid_t p = waitpid (valgrind_pid, &status, 0); > + DEBUG(2, "waitpid: %d\n", (int) p); > + if (p == -1) > + DEBUG(1, "waitpid error %s\n", strerror (errno)); > + else { > + if (WIFEXITED(status)) > + DEBUG(1, "valgrind exited with %d\n", > + WEXITSTATUS(status)); > + else if (WIFSIGNALED(status)) > + DEBUG(1, "valgrind kill by signal %d\n", > + WTERMSIG(status)); > + else > + DEBUG(1, "valgind unexpectedly stopped or continued"); typo: valgind > + } > + } else { > + send_packet ("E01", noackmode); > + DEBUG(1, "OOPS! couldn't launch valgrind %s\n", > + strerror (res)); > + } > + > + free(len); > + for (int i = 0; i < count; i++) > + free (decoded_string[i]); > + free (decoded_string); > + } else { > + free(len); > + send_packet ("E01", noackmode); missing indentation > + DEBUG(1, "vRun decoding error: no next_string!\n"); > + continue; > + } > + } else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) { > + send_packet ("1", noackmode); > + DEBUG(1, "qAttached sent: '1'\n"); > + const char *next_str = next_delim_string(buf, ':'); > + if (next_str) { > + char *decoded_string = decode_hexstring (next_str, 0, 0); > + DEBUG(1, "qAttached decoded: %s, next_str %s\n", decoded_string, next_str); > + free (decoded_string); > + } else { > + DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf); > + continue; > + } > + } /* Reset the state of environment variables in the remote target before starting > + the inferior. In this context, reset means unsetting all environment variables > + that were previously set by the user (i.e., were not initially present in the environment). */ line too long Below we have indentation with 5 chars at many places. > + else if (strncmp(QENVIRONMENTRESET, buf, > + strlen(QENVIRONMENTRESET)) == 0) { > + send_packet ("OK", noackmode); > + // TODO clear all environment strings. We're not using > + // environment strings now. But we should. > + } else if (strncmp(QENVIRONMENTHEXENCODED, buf, > + strlen(QENVIRONMENTHEXENCODED)) == 0) { > + send_packet ("OK", noackmode); > + if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string)) > + break; > + // TODO Collect all environment strings and add them to environ > + // before launcing valgrind. typo: launcing > + free (string); > + string = NULL; > + } else if (strncmp(QENVIRONMENTUNSET, buf, > + strlen(QENVIRONMENTUNSET)) == 0) { > + send_packet ("OK", noackmode); > + if (!split_hexdecode(buf, QENVIRONMENTUNSET, ":", &string)) > + break; > + // TODO Remove this environment string from the collection. > + free (string); > + string = NULL; > + } else if (strncmp(QSETWORKINGDIR, buf, > + strlen(QSETWORKINGDIR)) == 0) { > + // Silly, but we can only reply OK, even if the working directory is > + // bad. Errors will be reported when we try to execute the actual > + // process. > + send_packet ("OK", noackmode); > + // Free any previously set working_dir > + free (working_dir); > + working_dir = NULL; > + if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { > + continue; // We cannot report the error to gdb... > + } > + DEBUG(1, "set working dir to: %s\n", working_dir); > + } else if (strncmp(XFER, buf, strlen(XFER)) == 0) { > + char *buf_dup = strdup(buf); > + DEBUG(1, "strdup: buf_dup %s\n", buf_dup); > + if (buf_dup) { > + const char *delim = ":"; > + size_t count = count_delims(delim[0], buf); > + if (count < 4) { > + strsep(&buf_dup, delim); > + strsep(&buf_dup, delim); > + strsep(&buf_dup, delim); > + char *decoded_string = decode_hexstring (buf_dup, 0, 0); > + DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); > + free (decoded_string); > + } > + free (buf_dup); > + } else { > + DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf); > + free (buf_dup); > + continue; > + } > + // Whether we could decode it or not, we cannot handle it now. We > + // need valgrind gdbserver to properly reply. So error out here. > + send_packet ("E00", noackmode); > + } else if (strncmp(QTSTATUS, buf, strlen(QTSTATUS)) == 0) { > + // We don't support trace experiments > + DEBUG(1, "Got QTSTATUS\n"); > + send_packet ("", noackmode); > + } else if (strcmp("qfThreadInfo", buf) == 0) { > + DEBUG(1, "Got qfThreadInfo\n"); > + /* There are no threads yet, reply 'l' end of list. */ > + send_packet ("l", noackmode); > + } else if (buf[0] != '\0') { > + // We didn't understand. > + DEBUG(1, "Unknown packet received: '%s'\n", buf); > + bad_unknown_packets++; > + if (bad_unknown_packets > 10) { > + DEBUG(1, "Too many bad/unknown packets received\n"); > + break; > + } > + send_packet ("", noackmode); > + } > + } > + DEBUG(1, "done doing multi stuff...\n"); > + free(working_dir); > + free(buf); > + free(q_buf); > + > + shutting_down = True; > + close (to_gdb); > + close (from_gdb); > +} > + > +/* Relay data between gdb and Valgrind gdbserver, till EOF or an error is > + encountered. q_buf is the qSupported packet received from gdb. */ > +static > +void gdb_relay(int pid, int send_noack_mode, char *q_buf) > { > int from_pid = -1; /* fd to read from pid */ > int to_pid = -1; /* fd to write to pid */ > @@ -863,6 +1569,16 @@ void gdb_relay(int pid) ... > @@ -1379,6 +2135,7 @@ void parse_options(int argc, char** argv, > TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", path, strerror (errno)); Line too long. ... > // argc - i is the number of left over arguments > // allocate enough space, but all args in it. Not clear what is meant by 'but all args in it.' > @@ -1275,6 +1298,19 @@ Therefore, it has two usage modes: > </para> > </listitem> > > + <listitem id="manual-core-adv.vgdb-multi" xreflabel="vgdb multi"> > + <para>In the <option>--multi</option> mode, Vgdb uses the extended Elsewhere in the doc, we use vgdb, not Vgdb. > + remote protocol to communicate with Gdb. This allows you to view And we use GDB, not Gdb. > + output from both valgrind and GDB in the GDB session. This is > + accomplished via the "target extended-remote | vgdb --multi". In > + this mode you no longer need to start valgrind yourself. vgdb will > + start up valgrind when gdb tells it to run a new program. For this > + usage, the vgdb OPTIONS(s) can also include <option>--valgrind</option> > + and <option>--vargs</option> to describe how valgrind should be > + started. > + </para> > + </listitem> |
|
From: Mark W. <ma...@kl...> - 2023-04-16 00:28:21
|
Hi Philippe, Thanks for testing things out. On Sat, Apr 15, 2023 at 07:58:16PM +0200, Philippe Waroquiers wrote: > I did some tests: > > philippe@md:gdbserver_tests$ gdb sleepers > GNU gdb (GDB) 14.0.50.20230402-git > Copyright (C) 2023 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > Type "show copying" and "show warranty" for details. > This GDB was configured as "x86_64-pc-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <https://www.gnu.org/software/gdb/bugs/>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > > For help, type "help". > Type "apropos word" to search for commands related to "word"... > Loaded DUEL.py 0.9.6, high level data exploration language > Reading symbols from sleepers... > (gdb) set remote exec-file sleepers > (gdb) set sysroot / > (gdb) target extended-remote | vgdb --multi > Remote debugging using | vgdb --multi > (gdb) start > Temporary breakpoint 1 at 0x16fc: file sleepers.c, line 138. > Starting program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers > valgrind: sleepers: command not found > syscall failed: No such file or directory > error opening /tmp/vgdb-pipe-shared-mem-vgdb-52790-by-philippe-on-md shared memory file > Remote communication error. Target disconnected.: Connection reset by peer. > (gdb) > > > > The problem is solved by giving an absolute path for the remote exec-file: > (gdb) set remote exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers It doesn't need to be an absolute path, it can also be a relative path like: set remote exec-file ./sleepers Note that this is similar to how valgrind normally resolves executables. $ valgrind sleepers valgrind: sleepers: command not found $ valgrind ./sleepers ==210626== Memcheck, a memory error detector ==210626== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==210626== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info ==210626== Command: ./sleepers ==210626== loops/sleep_ms/burn/threads_spec/affinity: 15 1000 0 BSBSBSBS 0 Brussels ready to sleep and/or burn London ready to sleep and/or burn Petaouchnok ready to sleep and/or burn main ready to sleep and/or burn Brussels finished to sleep and/or burn London finished to sleep and/or burn main finished to sleep and/or burn Petaouchnok finished to sleep and/or burn [...] > So, it looks like gdb knows the absolute path of the program to launch, but does not pass > it to valgrind. It might be possible to have the full path given to vgdb ? Yes that would be nice. Tom Tromey suggested we create a python plugin to do some of the things we are currently requiring the user to set by hand. Paul made a gdb alias that does some of it. I hope we will add a new target valgrind to gdb itself that will do that and that does the stdin/stdout redirecting (that is a current issue with target extended-remote | ... it will "eat" the stdin/out of the child process). > The vgdb --help output is missing the --valgrind and the --vargs in the OPTIONS summary: > OPTIONS are [--pid=<number>] [--vgdb-prefix=<prefix>] > [--wait=<number>] [--max-invoke-ms=<number>] > [--port=<portnr> > [--cmd-time-out=<number>] [-l] [-T] [-D] [-d] > [--multi] > > The vgdb --help is missing \n after the --vargs description: > --vargs everything that follows is an argument for valgrind. -l arg tells to show the list of running Valgrind gdbserver and then exit. Oops. Fixed. > For > --valgrind, pass the path to valgrind to use. If not specified, the system valgrind will be launched. > Wouldn't it better (if possible) to by default launch the valgrind found at the same place as where vgdb is found ? Yes that would be nice, I'll look into it. But I think that normally if vgdb is on the PATH then so will valgrind. > Finally, once giving an absolute remote exec-file and an absolute path to the valgrind to launch, > I cannot have it working: > gdb sleepers > GNU gdb (GDB) 14.0.50.20230402-git > Copyright (C) 2023 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > Type "show copying" and "show warranty" for details. > This GDB was configured as "x86_64-pc-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <https://www.gnu.org/software/gdb/bugs/>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > > For help, type "help". > Type "apropos word" to search for commands related to "word"... > Loaded DUEL.py 0.9.6, high level data exploration language > Reading symbols from sleepers... > (gdb) set remote exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers > (gdb) set sysroot / > (gdb) target extended-remote | vgdb --multi --valgrind=/home/philippe/valgrind/git/trunk_untouched/Inst/bin/valgrind > Remote debugging using | vgdb --multi --valgrind=/home/philippe/valgrind/git/trunk_untouched/Inst/bin/valgrind > (gdb) start > Temporary breakpoint 1 at 0x16fc: file sleepers.c, line 138. > Starting program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers > ==100738== Memcheck, a memory error detector > ==100738== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. > ==100738== Using Valgrind-3.21.0.RC1 and LibVEX; rerun with -h for copyright info > ==100738== Command: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers > ==100738== > relaying data between gdb and process 100738 > syscall failed: Resource temporarily unavailable > error reading static buf readchar > syscall failed: Resource temporarily unavailable > readchar > no ack mode: unexpected buflen -1, buf > Unknown remote qXfer reply: 1 > (gdb) quit > > Not too sure what is going wrong/what I am doing wrong ... Nothing. There is something about sleepers that causes this. It also happens for me. I'll try to debug it. But it works when you give vgdb -d -d debug options... Cheers, Mark |
|
From: Philippe W. <phi...@sk...> - 2023-04-16 12:36:01
|
On Sun, 2023-04-16 at 02:28 +0200, Mark Wielaard wrote: > > The problem is solved by giving an absolute path for the remote exec-file: > > (gdb) set remote exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers > > It doesn't need to be an absolute path, it can also be a relative path > like: set remote exec-file ./sleepers > > Note that this is similar to how valgrind normally resolves > executables. Effectively. I was confused as GDB does not use the PATH. E.G.: philippe@md:gdbserver_tests$ valgrind -q sleepers valgrind: sleepers: command not found philippe@md:gdbserver_tests$ gdb -q sleepers Loaded DUEL.py 0.9.6, high level data exploration language Reading symbols from sleepers... (gdb) start Temporary breakpoint 1 at 0x16fc: file sleepers.c, line 138. So, it looks like in multi mode, I put my brain in "GDB mode". where absolute (or relative path) is not needed :). > > > > Not too sure what is going wrong/what I am doing wrong ... > > Nothing. There is something about sleepers that causes this. It also > happens for me. I'll try to debug it. But it works when you give vgdb > -d -d debug options... Humph, race condition/timing related bugs are hard to debug :(. Thanks Philippe |
|
From: Mark W. <ma...@kl...> - 2023-04-19 15:41:45
|
Hi Philippe,
On Sun, 2023-04-16 at 14:35 +0200, Philippe Waroquiers wrote:
> > > Not too sure what is going wrong/what I am doing wrong ...
> >
> > Nothing. There is something about sleepers that causes this. It also
> > happens for me. I'll try to debug it. But it works when you give vgdb
> > -d -d debug options...
> Humph, race condition/timing related bugs are hard to debug :(.
Finally found where this happens (sometimes):
diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index 7ed9a8b2e..2373bf335 100644
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -398,7 +398,9 @@ int read_buf(int fd, char* buf, const char* desc)
{
int nrread;
DEBUG(2, "reading %s\n", desc);
- nrread = read(fd, buf, PBUFSIZ);
+ do {
+ nrread = read(fd, buf, PBUFSIZ);
+ } while (nrread == -1 && errno == EAGAIN);
if (nrread == -1) {
ERROR(errno, "error reading %s\n", desc);
return -1;
This means the pipe isn't actually ready to be read from. Which really
shouldn't happen because we do a poll on the fd to make sure we get an
POLLIN event before starting to read from it.
I'll check in the above if I cannot find a more elegant solution.
Cheers,
Mark
|
|
From: Alexandra H. <aha...@re...> - 2023-04-20 12:17:47
|
---
coregrind/m_gdbserver/server.c | 2 +-
coregrind/m_main.c | 4 +-
coregrind/vgdb.c | 275 ++++++++++++-----------
docs/xml/manual-core-adv.xml | 5 +-
none/tests/cmdline2.stdout.exp | 1 -
none/tests/cmdline2.stdout.exp-non-linux | 1 -
6 files changed, 146 insertions(+), 142 deletions(-)
diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c
index 3c2516086..83825408a 100644
--- a/coregrind/m_gdbserver/server.c
+++ b/coregrind/m_gdbserver/server.c
@@ -1105,7 +1105,7 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p)
return;
}
- /* Protocol features query. */
+ /* Protocol features query. Keep this in sync with coregind/vgdb.c. */
if (strncmp ("qSupported", arg_own_buf, 10) == 0
&& (arg_own_buf[10] == ':' || arg_own_buf[10] == '\0')) {
VG_(sprintf) (arg_own_buf, "PacketSize=%x", (UInt)PBUFSIZ - 1);
diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 6181046d1..e19796327 100644
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -279,8 +279,6 @@ static void usage_NORETURN ( int need_help )
" --progress-interval=<number> report progress every <number>\n"
" CPU seconds [0, meaning disabled]\n"
" --command-line-only=no|yes only use command line options [no]\n"
-" --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]\n"
-"\n"
" Vex options for all Valgrind tools:\n"
" --vex-iropt-verbosity=<0..9> [0]\n"
" --vex-iropt-level=<0..2> [2]\n"
@@ -563,6 +561,8 @@ static void process_option (Clo_Mode mode,
}
else if VG_INT_CLOM (cloPD, arg, "--vgdb-poll", VG_(clo_vgdb_poll)) {}
else if VG_INT_CLOM (cloPD, arg, "--vgdb-error", VG_(clo_vgdb_error)) {}
+ /* --launched-with-multi is an internal option used by vgdb to suppress
+ some output that valgrind normally shows when using --vgdb-error. */
else if VG_BOOL_CLO (arg, "--launched-with-multi",
VG_(clo_launched_with_multi)) {}
else if VG_USET_CLOM (cloPD, arg, "--vgdb-stop-at",
diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index ca673e368..a3c5f9d88 100644
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -900,8 +900,7 @@ int tohex (int nib)
return 'a' + nib - 10;
}
-/* Returns an allocated hex-decoded string from the buf starting at offset
- off. Will update off to where the buf has been decoded. Stops decoding
+/* Returns an allocated hex-decoded string from the buf. Stops decoding
at end of buf (zero) or when seeing the delim char. */
static
char *decode_hexstring (const char *buf, size_t prefixlen, size_t len)
@@ -947,7 +946,7 @@ write_checksum (const char *str)
unsigned char csum = 0;
int i = 0;
while (str[i] != 0)
- csum += str[i++];
+ csum += str[i++];
char p[2];
p[0] = tohex ((csum >> 4) & 0x0f);
@@ -964,7 +963,7 @@ write_reply(const char *reply)
return write_checksum (reply);
}
-/* Creates a packet from a string message, called needs to free. */
+/* Creates a packet from a string message, caller needs to free. */
static char *
create_packet(const char *msg)
{
@@ -995,24 +994,24 @@ static int read_one_char (char *c)
static Bool
send_packet(const char *reply, int noackmode)
{
- int ret;
- char c;
+ int ret;
+ char c;
send_packet_start:
if (!write_reply(reply))
- return False;
- if (!noackmode) {
- // Look for '+' or '-'.
- // We must wait for "+" if !noackmode.
- do {
- ret = read_one_char(&c);
- if (ret <= 0)
- return False;
- // And if in !noackmode if we get "-" we should resent the packet.
- if (c == '-')
- goto send_packet_start;
- } while (c != '+');
- DEBUG(1, "sent packet to gdb got: %c\n",c);
+ return False;
+ if (!noackmode) {
+ // Look for '+' or '-'.
+ // We must wait for "+" if !noackmode.
+ do {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return False;
+ // And if in !noackmode if we get "-" we should resent the packet.
+ if (c == '-')
+ goto send_packet_start;
+ } while (c != '+');
+ DEBUG(1, "sent packet to gdb got: %c\n",c);
}
return True;
}
@@ -1023,92 +1022,96 @@ send_packet_start:
// or -1 if no packet could be read.
static int receive_packet(char *buf, int noackmode)
{
- int bufcnt = 0, ret;
- char c, c1, c2;
- unsigned char csum = 0;
-
- // Look for first '$' (start of packet) or error.
- receive_packet_start:
- do {
- ret = read_one_char(&c);
- if (ret <= 0)
- return ret;
- } while (c != '$');
+ int bufcnt = 0, ret;
+ char c, c1, c2;
+ unsigned char csum = 0;
- // Found start of packet ('$')
- while (bufcnt < (PBUFSIZ+1)) {
- ret = read_one_char(&c);
- if (ret <= 0)
- return ret;
- if (c == '#') {
- if ((ret = read_one_char(&c1)) <= 0
- || (ret = read_one_char(&c2)) <= 0) {
- return ret;
- }
- c1 = fromhex(c1);
- c2 = fromhex(c2);
- break;
+ // Look for first '$' (start of packet) or error.
+receive_packet_start:
+ do {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return ret;
+ } while (c != '$');
+
+ // Found start of packet ('$')
+ while (bufcnt < (PBUFSIZ+1)) {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return ret;
+ if (c == '#') {
+ if ((ret = read_one_char(&c1)) <= 0
+ || (ret = read_one_char(&c2)) <= 0) {
+ return ret;
}
- buf[bufcnt] = c;
- csum += buf[bufcnt];
- bufcnt++;
- }
+ c1 = fromhex(c1);
+ c2 = fromhex(c2);
+ break;
+ }
+ buf[bufcnt] = c;
+ csum += buf[bufcnt];
+ bufcnt++;
+ }
- // Packet complete, add terminator.
- buf[bufcnt] ='\0';
-
- if (!(csum == (c1 << 4) + c2)) {
- TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
- (c1 << 4) + c2, csum, buf);
- if (!noackmode)
- if (!write_to_gdb ("-", 1))
- return -1;
- /* Try again, gdb should resend the packet. */
- bufcnt = 0;
- csum = 0;
- goto receive_packet_start;
- }
+ // Packet complete, add terminator.
+ buf[bufcnt] ='\0';
- if (!noackmode)
- if (!write_to_gdb ("+", 1))
- return -1;
- return bufcnt;
+ if (!(csum == (c1 << 4) + c2)) {
+ TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
+ (c1 << 4) + c2, csum, buf);
+ if (!noackmode)
+ if (!write_to_gdb ("-", 1))
+ return -1;
+ /* Try again, gdb should resend the packet. */
+ bufcnt = 0;
+ csum = 0;
+ goto receive_packet_start;
+ }
+
+ if (!noackmode)
+ if (!write_to_gdb ("+", 1))
+ return -1;
+ return bufcnt;
}
// Returns a pointer to the char after the next delim char.
static const char *next_delim_string (const char *buf, char delim)
{
- while (*buf) {
- if (*buf++ == delim)
- break;
- }
- return buf;
+ while (*buf) {
+ if (*buf++ == delim)
+ break;
+ }
+ return buf;
}
-// Throws away the packet name and decodes the hex string, which is placed in
-// decoded_string (the caller owns this and is responsible for freeing it).
+/* buf starts with the packet name followed by the delimiter, for example
+ * vRun;2f62696e2f6c73, ";" is the delimiter here, or
+ * qXfer:features:read:target.xml:0,1000, where the delimiter is ":".
+ * The packet name is thrown away and the hex string is decoded and
+ * is placed in decoded_string (the caller owns this and is responsible
+ * for freeing it). */
static int split_hexdecode(const char *buf, const char *string,
const char *delim, char **decoded_string)
{
- const char *next_str = next_delim_string(buf, *delim);
- if (next_str) {
- *decoded_string = decode_hexstring (next_str, 0, 0);
- DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string);
- return 1;
- } else {
- TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf);
- return 0;
- }
+ const char *next_str = next_delim_string(buf, *delim);
+ if (next_str) {
+ *decoded_string = decode_hexstring (next_str, 0, 0);
+ DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string);
+ return 1;
+ } else {
+ TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf);
+ return 0;
+ }
}
-static int count_delims(char delim, char *buf)
+static size_t count_delims(char delim, char *buf)
{
- size_t count = 0;
- char *ptr = buf;
+ size_t count = 0;
+ char *ptr = buf;
- while (*ptr)
- count += *ptr++ == delim;
- return count;
+ while (*ptr)
+ count += *ptr++ == delim;
+ return count;
}
// Determine the length of the arguments.
@@ -1298,7 +1301,7 @@ void do_multi_mode(void)
break;
}
- DEBUG(1, "packet recieved: '%s'\n", buf);
+ DEBUG(1, "packet received: '%s'\n", buf);
#define QSUPPORTED "qSupported:"
#define STARTNOACKMODE "QStartNoAckMode"
@@ -1403,7 +1406,8 @@ void do_multi_mode(void)
if (i < count - 1)
next_str = next_delim_string(next_str, *delim);
}
- DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", decoded_string[i], next_str, i, len[i]);
+ DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n",
+ decoded_string[i], next_str, i, len[i]);
}
/* If we didn't get any arguments or the filename is an empty
@@ -1431,8 +1435,9 @@ void do_multi_mode(void)
// Lets report we Stopped with SIGTRAP (05).
send_packet ("S05", noackmode);
prepare_fifos_and_shared_mem(valgrind_pid);
- DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", from_gdb_to_pid, to_gdb_from_pid);
- // gdb_rely is an endless loop till valgrind quits.
+ DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n",
+ from_gdb_to_pid, to_gdb_from_pid);
+ // gdb_relay is an endless loop till valgrind quits.
shutting_down = False;
gdb_relay (valgrind_pid, 1, q_buf);
@@ -1451,7 +1456,7 @@ void do_multi_mode(void)
DEBUG(1, "valgrind kill by signal %d\n",
WTERMSIG(status));
else
- DEBUG(1, "valgind unexpectedly stopped or continued");
+ DEBUG(1, "valgrind unexpectedly stopped or continued");
}
} else {
send_packet ("E01", noackmode);
@@ -1461,17 +1466,17 @@ void do_multi_mode(void)
free(len);
for (int i = 0; i < count; i++)
- free (decoded_string[i]);
- free (decoded_string);
- } else {
- free(len);
- send_packet ("E01", noackmode);
- DEBUG(1, "vRun decoding error: no next_string!\n");
- continue;
- }
+ free (decoded_string[i]);
+ free (decoded_string);
+ } else {
+ free(len);
+ send_packet ("E01", noackmode);
+ DEBUG(1, "vRun decoding error: no next_string!\n");
+ continue;
+ }
} else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) {
- send_packet ("1", noackmode);
- DEBUG(1, "qAttached sent: '1'\n");
+ send_packet ("1", noackmode);
+ DEBUG(1, "qAttached sent: '1'\n");
const char *next_str = next_delim_string(buf, ':');
if (next_str) {
char *decoded_string = decode_hexstring (next_str, 0, 0);
@@ -1481,9 +1486,10 @@ void do_multi_mode(void)
DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf);
continue;
}
- } /* Reset the state of environment variables in the remote target before starting
- the inferior. In this context, reset means unsetting all environment variables
- that were previously set by the user (i.e., were not initially present in the environment). */
+ } /* Reset the state of environment variables in the remote target
+ before starting the inferior. In this context, reset means
+ unsetting all environment variables that were previously set
+ by the user (i.e., were not initially present in the environment). */
else if (strncmp(QENVIRONMENTRESET, buf,
strlen(QENVIRONMENTRESET)) == 0) {
send_packet ("OK", noackmode);
@@ -1495,7 +1501,7 @@ void do_multi_mode(void)
if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string))
break;
// TODO Collect all environment strings and add them to environ
- // before launcing valgrind.
+ // before launching valgrind.
free (string);
string = NULL;
} else if (strncmp(QENVIRONMENTUNSET, buf,
@@ -1507,33 +1513,33 @@ void do_multi_mode(void)
free (string);
string = NULL;
} else if (strncmp(QSETWORKINGDIR, buf,
- strlen(QSETWORKINGDIR)) == 0) {
- // Silly, but we can only reply OK, even if the working directory is
- // bad. Errors will be reported when we try to execute the actual
- // process.
- send_packet ("OK", noackmode);
- // Free any previously set working_dir
- free (working_dir);
- working_dir = NULL;
- if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) {
- continue; // We cannot report the error to gdb...
- }
- DEBUG(1, "set working dir to: %s\n", working_dir);
+ strlen(QSETWORKINGDIR)) == 0) {
+ // Silly, but we can only reply OK, even if the working directory is
+ // bad. Errors will be reported when we try to execute the actual
+ // process.
+ send_packet ("OK", noackmode);
+ // Free any previously set working_dir
+ free (working_dir);
+ working_dir = NULL;
+ if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) {
+ continue; // We cannot report the error to gdb...
+ }
+ DEBUG(1, "set working dir to: %s\n", working_dir);
} else if (strncmp(XFER, buf, strlen(XFER)) == 0) {
- char *buf_dup = strdup(buf);
- DEBUG(1, "strdup: buf_dup %s\n", buf_dup);
- if (buf_dup) {
- const char *delim = ":";
- size_t count = count_delims(delim[0], buf);
- if (count < 4) {
- strsep(&buf_dup, delim);
- strsep(&buf_dup, delim);
- strsep(&buf_dup, delim);
- char *decoded_string = decode_hexstring (buf_dup, 0, 0);
- DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup);
- free (decoded_string);
- }
- free (buf_dup);
+ char *buf_dup = strdup(buf);
+ DEBUG(1, "strdup: buf_dup %s\n", buf_dup);
+ if (buf_dup) {
+ const char *delim = ":";
+ size_t count = count_delims(delim[0], buf);
+ if (count < 4) {
+ strsep(&buf_dup, delim);
+ strsep(&buf_dup, delim);
+ strsep(&buf_dup, delim);
+ char *decoded_string = decode_hexstring (buf_dup, 0, 0);
+ DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup);
+ free (decoded_string);
+ }
+ free (buf_dup);
} else {
DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf);
free (buf_dup);
@@ -2220,7 +2226,8 @@ void parse_options(int argc, char** argv,
/* Compute the absolute path. */
valgrind_path = realpath(path, NULL);
if (!valgrind_path) {
- TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", path, strerror (errno));
+ TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n",
+ path, strerror (errno));
exit(1);
}
DEBUG(2, "valgrind's real path: %s\n", valgrind_path);
@@ -2228,7 +2235,7 @@ void parse_options(int argc, char** argv,
// Everything that follows now is an argument for valgrind
// No other options (or commands) can follow
// argc - i is the number of left over arguments
- // allocate enough space, but all args in it.
+ // allocate enough space, put all args in it.
cvargs = argc - i - 1;
vargs = vmalloc (cvargs * sizeof(vargs));
i++;
diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml
index bb695d2d3..ff8c8124a 100644
--- a/docs/xml/manual-core-adv.xml
+++ b/docs/xml/manual-core-adv.xml
@@ -1299,8 +1299,8 @@ It has three usage modes:
</listitem>
<listitem id="manual-core-adv.vgdb-multi" xreflabel="vgdb multi">
- <para>In the <option>--multi</option> mode, Vgdb uses the extended
- remote protocol to communicate with Gdb. This allows you to view
+ <para>In the <option>--multi</option> mode, vgdb uses the extended
+ remote protocol to communicate with GDB. This allows you to view
output from both valgrind and GDB in the GDB session. This is
accomplished via the "target extended-remote | vgdb --multi". In
this mode you no longer need to start valgrind yourself. vgdb will
@@ -2271,5 +2271,4 @@ almost 300 different wrappers.</para>
-
</chapter>
diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp
index 10485a3b4..241d33afa 100644
--- a/none/tests/cmdline2.stdout.exp
+++ b/none/tests/cmdline2.stdout.exp
@@ -190,7 +190,6 @@ usage: valgrind [options] prog-and-args
--progress-interval=<number> report progress every <number>
CPU seconds [0, meaning disabled]
--command-line-only=no|yes only use command line options [no]
- --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]
Vex options for all Valgrind tools:
--vex-iropt-verbosity=<0..9> [0]
diff --git a/none/tests/cmdline2.stdout.exp-non-linux b/none/tests/cmdline2.stdout.exp-non-linux
index 6e08284ac..63af17bf7 100644
--- a/none/tests/cmdline2.stdout.exp-non-linux
+++ b/none/tests/cmdline2.stdout.exp-non-linux
@@ -188,7 +188,6 @@ usage: valgrind [options] prog-and-args
--progress-interval=<number> report progress every <number>
CPU seconds [0, meaning disabled]
--command-line-only=no|yes only use command line options [no]
- --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]
Vex options for all Valgrind tools:
--vex-iropt-verbosity=<0..9> [0]
--
2.39.2
|
|
From: Mark W. <ma...@kl...> - 2023-04-20 13:02:11
|
Hi Sasha,
Looks like this was in response to Philippe's review. Thanks.
I added one more comment at the top describing the three usages of
vgdb. And fixed up a few places where tabs were used for indentation
(we are not very consistent in that either, after the release we'll
look into adopting something like clang-format so you don't have to do
all this by hand). And a missing newline in coregrind/m_main.c to make
none/tests/cmdline2 pass.
Pushed with the following commit message to show what was changed:
commit a32e26dd072a82aacafcdd22bd7c94c7b4d2afcc
Author: Alexandra Hájková <aha...@re...>
Date: Thu Apr 20 14:17:29 2023 +0200
vgdb --multi: fix various typos, indentation and such
Remove --launched-with-multi from --help-debug output since it is not
a real user option. Do add a comment in m_main.c explaining the
internal usage.
Add a top-level comment describing the three usages of vgdb.
Fix comment description of decode_hexstring, create_packet,
split_hexdecode.
Consistently use 3 space indention in send_packet and receive_packet
and next_delim_string and split_hexdecode, count_delims,
do_multi_mode.
Fix return type of count_delims to size_t.
Add a note in coregrind/m_gdbserver/server.c to sync qSupported
replies with coregrind/vgdb.c.
Use vgdb (all lowercase) and GDB (all caps) consistently in the
manual.
Thanks,
Mark
|
|
From: Mark W. <ma...@kl...> - 2023-04-20 13:19:35
|
Hi Sasha,
On Thu, 2023-04-20 at 15:01 +0200, Mark Wielaard wrote:
> Looks like this was in response to Philippe's review. Thanks.
> I added one more comment at the top describing the three usages of
> vgdb. And fixed up a few places where tabs were used for indentation
> (we are not very consistent in that either, after the release we'll
> look into adopting something like clang-format so you don't have to do
> all this by hand). And a missing newline in coregrind/m_main.c to make
> none/tests/cmdline2 pass.
>
> Pushed with the following commit message to show what was changed:
And then I messed up and didn't include those small fixups I described,
sorry. Pushed a followup commit:
Author: Mark Wielaard <ma...@kl...>
Date: Thu Apr 20 15:04:03 2023 +0200
vgdb --multi: fix various typos, indentation and such (followup)
commit 56ccb1e36c4722b56e3e602b986bc45025cb685d missed a few small
fixlets:
- one more comment at the top describing the three usages of vgdb.
- fixed up a few places where tabs were used for indentation (we are
not very consistent in that either, after the release we'll look
into adopting something like clang-format so you don't have to do
all this by hand).
- Add a missing newline in coregrind/m_main.c to make
none/tests/cmdline2 pass.
Sorry for messing this up. One day git and I will become friends...
Cheers,
Mark
|
|
From: Alexandra H. <aha...@re...> - 2023-04-25 15:29:08
|
when waiting for valgrind to start.
---
coregrind/vgdb.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index 21e67c65b..8ec424077 100644
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -178,14 +178,20 @@ void add_written(int nrw)
static int shared_mem_fd = -1;
static
-void map_vgdbshared(char* shared_mem)
+void map_vgdbshared(char* shared_mem, int check_trials)
{
struct stat fdstat;
void **s;
int tries = 50;
int err;
- /* valgrind might still be starting up, give it 5 seconds. */
+ /* valgrind might still be starting up, give it 5 seconds by
+ * default, or check_trails seconds if it is set by --wait
+ * to more than a second. */
+ if (check_trials > 1) {
+ DEBUG(1, "check_trials %d\n", check_trials);
+ tries = check_trials * 10;
+ }
do {
shared_mem_fd = open(shared_mem, O_RDWR | O_CLOEXEC);
err = errno;
@@ -581,7 +587,7 @@ void wait_for_gdb_connect(int in_port)
/* prepares the FIFOs filenames, map the shared memory. */
static
-void prepare_fifos_and_shared_mem(int pid)
+void prepare_fifos_and_shared_mem(int pid, int check_trials)
{
const HChar *user, *host;
unsigned len;
@@ -610,7 +616,7 @@ void prepare_fifos_and_shared_mem(int pid)
DEBUG(1, "vgdb: using %s %s %s\n",
from_gdb_to_pid, to_gdb_from_pid, shared_mem);
- map_vgdbshared(shared_mem);
+ map_vgdbshared(shared_mem, check_trials);
}
static void
@@ -1303,7 +1309,7 @@ int fork_and_exec_valgrind (int argc, char **argv, const char *working_dir,
/* Do multi stuff. */
static
-void do_multi_mode(void)
+void do_multi_mode(int check_trials)
{
char *buf = vmalloc(PBUFSIZ+1);
char *q_buf = vmalloc(PBUFSIZ+1); //save the qSupported packet sent by gdb
@@ -1458,7 +1464,7 @@ void do_multi_mode(void)
if (res == 0) {
// Lets report we Stopped with SIGTRAP (05).
send_packet ("S05", noackmode);
- prepare_fifos_and_shared_mem(valgrind_pid);
+ prepare_fifos_and_shared_mem(valgrind_pid, check_trials);
DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n",
from_gdb_to_pid, to_gdb_from_pid);
// gdb_relay is an endless loop till valgrind quits.
@@ -2392,7 +2398,8 @@ int main(int argc, char** argv)
if (!multi_mode) {
pid = search_arg_pid(arg_pid, check_trials, show_list);
- prepare_fifos_and_shared_mem(pid);
+ /* We pass 1 for check_trails here, because search_arg_pid already waited. */
+ prepare_fifos_and_shared_mem(pid, 1);
} else {
pid = 0;
}
@@ -2413,7 +2420,9 @@ int main(int argc, char** argv)
}
if (multi_mode) {
- do_multi_mode ();
+ /* check_trails is the --wait argument in seconds, defaulting to 1
+ * if not given. */
+ do_multi_mode (check_trials);
} else if (last_command >= 0) {
standalone_send_commands(pid, last_command, commands);
} else {
--
2.39.2
|
|
From: Paul F. <pj...@wa...> - 2023-04-26 11:41:23
|
On 25-04-23 17:28, Alexandra Hájková wrote: > when waiting for valgrind to start. Hi Alexandra Would you like this to go in before 3.21 ships? A+ Paul |
|
From: Mark W. <ma...@kl...> - 2023-04-26 19:37:31
|
Hi, On Wed, 2023-04-26 at 13:41 +0200, Paul Floyd wrote: > On 25-04-23 17:28, Alexandra Hájková wrote: > > when waiting for valgrind to start. > > Would you like this to go in before 3.21 ships? This would be nice to get in before the release because it makes the vgdb --wait argument work correctly for --multi mode. I double checked the patch and it looks good to me. So pushed. Thanks, Mark |