From: Mark Wielaard Date: Thu, 20 Apr 2023 13:04:03 +0000 (+0200) Subject: vgdb --multi: fix various typos, indentation and such (followup) X-Git-Tag: VALGRIND_3_21_0~38 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9fcac92ab371d210dc54f49a80146be9d0b0433a;p=thirdparty%2Fvalgrind.git 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. --- diff --git a/coregrind/m_main.c b/coregrind/m_main.c index e19796327d..a857e5afeb 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -278,7 +278,7 @@ static void usage_NORETURN ( int need_help ) " --sym-offsets=yes|no show syms in form 'name+offset'? [no]\n" " --progress-interval= report progress every \n" " CPU seconds [0, meaning disabled]\n" -" --command-line-only=no|yes only use command line options [no]\n" +" --command-line-only=no|yes only use command line options [no]\n\n" " Vex options for all Valgrind tools:\n" " --vex-iropt-verbosity=<0..9> [0]\n" " --vex-iropt-level=<0..2> [2]\n" diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index a3c5f9d88f..6e5e819563 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -49,9 +49,10 @@ #include #include -/* vgdb has two usages: +/* vgdb has three usages: 1. relay application between gdb and the gdbserver embedded in valgrind. 2. standalone to send monitor commands to a running valgrind-ified process + 3. multi mode where vgdb uses the GDB extended remote protocol. It is made of a main program which reads arguments. If no arguments are given or only --pid and --vgdb-prefix, then usage 1 is @@ -67,6 +68,12 @@ As a standalone utility, vgdb builds command packets to write to valgrind, sends it and reads the reply. The same two threads are used to write/read. Once all the commands are sent and their replies received, vgdb will exit. + + When --multi is given vgdb communicates with GDB through the extended remote + protocol and will launch valgrind whenever GDB sends the vRun packet, after + which it will function in the first mode, relaying packets between GDB and + the gdbserver embedded in valgrind till that valgrind quits. vgdb will stay + connected to GDB. */ int debuglevel; @@ -710,7 +717,7 @@ getpkt(char *buf, int fromfd, int ackfd) c2 = fromhex(readchar (fromfd)); if (csum == (c1 << 4) + c2) - break; + break; TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", (c1 << 4) + c2, csum, buf); @@ -1003,14 +1010,14 @@ send_packet_start: 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 != '+'); + 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; @@ -1036,36 +1043,36 @@ receive_packet_start: // 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++; + 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; + 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) @@ -1315,68 +1322,68 @@ void do_multi_mode(void) #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); + 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) { + 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). + // 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 = ";"; @@ -1386,186 +1393,186 @@ void do_multi_mode(void) // 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; + 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_relay 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, "valgrind 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++) + 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_relay 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, "valgrind 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); + free (decoded_string); } else { - free(len); - send_packet ("E01", noackmode); - DEBUG(1, "vRun decoding error: no next_string!\n"); - continue; + 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) { + } 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 { + } 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 launching 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); + } + } /* 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 launching 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; + 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); - } + } 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);