You can subscribe to this list here.
| 2002 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
(1) |
Oct
(122) |
Nov
(152) |
Dec
(69) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2003 |
Jan
(6) |
Feb
(25) |
Mar
(73) |
Apr
(82) |
May
(24) |
Jun
(25) |
Jul
(10) |
Aug
(11) |
Sep
(10) |
Oct
(54) |
Nov
(203) |
Dec
(182) |
| 2004 |
Jan
(307) |
Feb
(305) |
Mar
(430) |
Apr
(312) |
May
(187) |
Jun
(342) |
Jul
(487) |
Aug
(637) |
Sep
(336) |
Oct
(373) |
Nov
(441) |
Dec
(210) |
| 2005 |
Jan
(385) |
Feb
(480) |
Mar
(636) |
Apr
(544) |
May
(679) |
Jun
(625) |
Jul
(810) |
Aug
(838) |
Sep
(634) |
Oct
(521) |
Nov
(965) |
Dec
(543) |
| 2006 |
Jan
(494) |
Feb
(431) |
Mar
(546) |
Apr
(411) |
May
(406) |
Jun
(322) |
Jul
(256) |
Aug
(401) |
Sep
(345) |
Oct
(542) |
Nov
(308) |
Dec
(481) |
| 2007 |
Jan
(427) |
Feb
(326) |
Mar
(367) |
Apr
(255) |
May
(244) |
Jun
(204) |
Jul
(223) |
Aug
(231) |
Sep
(354) |
Oct
(374) |
Nov
(497) |
Dec
(362) |
| 2008 |
Jan
(322) |
Feb
(482) |
Mar
(658) |
Apr
(422) |
May
(476) |
Jun
(396) |
Jul
(455) |
Aug
(267) |
Sep
(280) |
Oct
(253) |
Nov
(232) |
Dec
(304) |
| 2009 |
Jan
(486) |
Feb
(470) |
Mar
(458) |
Apr
(423) |
May
(696) |
Jun
(461) |
Jul
(551) |
Aug
(575) |
Sep
(134) |
Oct
(110) |
Nov
(157) |
Dec
(102) |
| 2010 |
Jan
(226) |
Feb
(86) |
Mar
(147) |
Apr
(117) |
May
(107) |
Jun
(203) |
Jul
(193) |
Aug
(238) |
Sep
(300) |
Oct
(246) |
Nov
(23) |
Dec
(75) |
| 2011 |
Jan
(133) |
Feb
(195) |
Mar
(315) |
Apr
(200) |
May
(267) |
Jun
(293) |
Jul
(353) |
Aug
(237) |
Sep
(278) |
Oct
(611) |
Nov
(274) |
Dec
(260) |
| 2012 |
Jan
(303) |
Feb
(391) |
Mar
(417) |
Apr
(441) |
May
(488) |
Jun
(655) |
Jul
(590) |
Aug
(610) |
Sep
(526) |
Oct
(478) |
Nov
(359) |
Dec
(372) |
| 2013 |
Jan
(467) |
Feb
(226) |
Mar
(391) |
Apr
(281) |
May
(299) |
Jun
(252) |
Jul
(311) |
Aug
(352) |
Sep
(481) |
Oct
(571) |
Nov
(222) |
Dec
(231) |
| 2014 |
Jan
(185) |
Feb
(329) |
Mar
(245) |
Apr
(238) |
May
(281) |
Jun
(399) |
Jul
(382) |
Aug
(500) |
Sep
(579) |
Oct
(435) |
Nov
(487) |
Dec
(256) |
| 2015 |
Jan
(338) |
Feb
(357) |
Mar
(330) |
Apr
(294) |
May
(191) |
Jun
(108) |
Jul
(142) |
Aug
(261) |
Sep
(190) |
Oct
(54) |
Nov
(83) |
Dec
(22) |
| 2016 |
Jan
(49) |
Feb
(89) |
Mar
(33) |
Apr
(50) |
May
(27) |
Jun
(34) |
Jul
(53) |
Aug
(53) |
Sep
(98) |
Oct
(206) |
Nov
(93) |
Dec
(53) |
| 2017 |
Jan
(65) |
Feb
(82) |
Mar
(102) |
Apr
(86) |
May
(187) |
Jun
(67) |
Jul
(23) |
Aug
(93) |
Sep
(65) |
Oct
(45) |
Nov
(35) |
Dec
(17) |
| 2018 |
Jan
(26) |
Feb
(35) |
Mar
(38) |
Apr
(32) |
May
(8) |
Jun
(43) |
Jul
(27) |
Aug
(30) |
Sep
(43) |
Oct
(42) |
Nov
(38) |
Dec
(67) |
| 2019 |
Jan
(32) |
Feb
(37) |
Mar
(53) |
Apr
(64) |
May
(49) |
Jun
(18) |
Jul
(14) |
Aug
(53) |
Sep
(25) |
Oct
(30) |
Nov
(49) |
Dec
(31) |
| 2020 |
Jan
(87) |
Feb
(45) |
Mar
(37) |
Apr
(51) |
May
(99) |
Jun
(36) |
Jul
(11) |
Aug
(14) |
Sep
(20) |
Oct
(24) |
Nov
(40) |
Dec
(23) |
| 2021 |
Jan
(14) |
Feb
(53) |
Mar
(85) |
Apr
(15) |
May
(19) |
Jun
(3) |
Jul
(14) |
Aug
(1) |
Sep
(57) |
Oct
(73) |
Nov
(56) |
Dec
(22) |
| 2022 |
Jan
(3) |
Feb
(22) |
Mar
(6) |
Apr
(55) |
May
(46) |
Jun
(39) |
Jul
(15) |
Aug
(9) |
Sep
(11) |
Oct
(34) |
Nov
(20) |
Dec
(36) |
| 2023 |
Jan
(79) |
Feb
(41) |
Mar
(99) |
Apr
(169) |
May
(48) |
Jun
(16) |
Jul
(16) |
Aug
(57) |
Sep
(19) |
Oct
|
Nov
|
Dec
|
|
From: Mark W. <ma...@so...> - 2023-04-16 00:01:06
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=03d9229f0bdb28cf35e3bfc98010952594b091cd commit 03d9229f0bdb28cf35e3bfc98010952594b091cd Author: Mark Wielaard <ma...@kl...> Date: Sun Apr 16 01:55:48 2023 +0200 Fixup vgdb --help message The --valgrind and the --vargs were missingin the OPTIONS summary. A \n was missing after the --vargs description. Diff: --- coregrind/vgdb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index 1cc37a3566..c4a7042984 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -1899,7 +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" +" [--multi] [--valgrind=<valgrind-exe>] [--vargs ...]\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" @@ -1915,7 +1915,7 @@ void usage(void) " 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." +" --vargs everything that follows is an argument for valgrind.\n" " -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" |
|
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: 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 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 11:29:04
|
Hi Here's a list of the items that, time permitting, I'd like to get into 3.21. Plus any recently-released FreeBSD 13.2 fixes. inconsistent RDTSCP support on x86_64 https://bugs.kde.org/show_bug.cgi?id=374596 I was recently contacted by the patch author for this one. It's a bit tricky to test as it requires ancient hardware or some sort of virtualization. That or a good knowledge of all possible hwcaps combinations. Likely false positive "uninitialised value(s)" for __wmemchr_avx2 and __wmemcmp_avx2_movbe https://bugs.kde.org/show_bug.cgi?id=397083 Looks fairly straightforward, I just need to find a machine less old than mine to test it on. Enhancement: add a client request to DHAT to mark memory to be histogrammed https://bugs.kde.org/show_bug.cgi?id=464103 Again fairly simple and straightforward. Add mismatched detection to C++ 17 aligned new/delete https://bugs.kde.org/show_bug.cgi?id=433859 Add validation to C++17 aligned new/delete alignment size https://bugs.kde.org/show_bug.cgi?id=433857 Add mismatched detection to C++ 14 sized delete https://bugs.kde.org/show_bug.cgi?id=467441 aligned_alloc problems, part 2 https://bugs.kde.org/show_bug.cgi?id=466105 There's a lot of stuff there (patch in the first bugzi item). All the C++ stuff is nice to have (and mostly also covered by ASan) but I don't think that that kind of error happens frequently in the wild. memalign and aligned alloc are more likely to be useful for detecting implementation defined behaviour. A+ Paul |
|
From: Mark W. <ma...@kl...> - 2023-04-15 02:07:05
|
An RC1 tarball for 3.21.0 is now available at https://sourceware.org/pub/valgrind/valgrind-3.21.0.RC1.tar.bz2 (md5sum = a3c7eeff47262cecdf5f1d68b38710b7) (sha1sum = 46fc5898415001e045abc1b4e2909a41144ed9c4) https://sourceware.org/pub/valgrind/valgrind-3.21.0.RC1.tar.bz2.asc Please give it a try in configurations that are important for you and report any problems you have, either on this mailing list, or (preferably) via our bug tracker at https://bugs.kde.org/enter_bug.cgi?product=valgrind There are still some patches being reviewed and a RC2 will appear end of next week. If nothing critical emerges after that, a final release will happen on Friday 28 April. |
|
From: Nicholas N. <n.n...@gm...> - 2023-04-15 01:50:36
|
On Sat, 15 Apr 2023 at 11:00, Mark Wielaard <ma...@kl...> wrote: > > When you add the doc changes could you also add a NEWS entry? > Yes, it's on my todo list. > Note that under --branch-sim=no|yes [no] cachegrind/docs/cg-manual.xml > says: > > <para>Enables or disables collection of branch instruction and > misprediction counts. By default this is disabled as it > slows Cachegrind down by approximately 25%. Note that you > cannot specify <option>--cache-sim=no</option> > and <option>--branch-sim=no</option> > together, as that would leave Cachegrind with no > information to collect.</para> > > Which seems untrue now that both --cache-sim= and --branch-sim= > default to no. > True, I will update that. Nick |
|
From: Mark W. <ma...@so...> - 2023-04-15 01:49:45
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=53834800424510b177c59d097c3a51a66bbaf659 commit 53834800424510b177c59d097c3a51a66bbaf659 Author: Mark Wielaard <ma...@kl...> Date: Sat Apr 15 03:49:15 2023 +0200 Set version to 3.21.0-RC1 Diff: --- NEWS | 2 +- configure.ac | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index e8225fdd40..a8ab817e17 100644 --- a/NEWS +++ b/NEWS @@ -157,7 +157,7 @@ To see details of a given bug, visit https://bugs.kde.org/show_bug.cgi?id=XXXXXX where XXXXXX is the bug number as listed above. -(3.21.0.RC1: ?? Apr 2023) +(3.21.0.RC1: 14 Apr 2023) Release 3.20.0 (24 Oct 2022) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/configure.ac b/configure.ac index 80bdbb417c..66437a8000 100755 --- a/configure.ac +++ b/configure.ac @@ -17,8 +17,8 @@ m4_define([v_major_ver], [3]) m4_define([v_minor_ver], [21]) m4_define([v_micro_ver], [0]) -m4_define([v_suffix_ver], [GIT]) -m4_define([v_rel_date], ["?? Apr 2023"]) +m4_define([v_suffix_ver], [RC1]) +m4_define([v_rel_date], ["14 Apr 2023"]) m4_define([v_version], m4_if(v_suffix_ver, [], [v_major_ver.v_minor_ver.v_micro_ver], |
|
From: Mark W. <ma...@kl...> - 2023-04-15 01:01:02
|
Hi Nick,
On Wed, Apr 12, 2023 at 03:48:15AM +0000, Nicholas Nethercote wrote:
> Make `--cache-sim=no` the default for Cachegrind.
>
> Also, don't print cache simulation details in the `desc:` line when the
> cache simulation is disabled.
>
> Docs changes are yet to come.
When you add the doc changes could you also add a NEWS entry?
Note that under --branch-sim=no|yes [no] cachegrind/docs/cg-manual.xml
says:
<para>Enables or disables collection of branch instruction and
misprediction counts. By default this is disabled as it
slows Cachegrind down by approximately 25%. Note that you
cannot specify <option>--cache-sim=no</option>
and <option>--branch-sim=no</option>
together, as that would leave Cachegrind with no
information to collect.</para>
Which seems untrue now that both --cache-sim= and --branch-sim=
default to no.
Thanks,
Mark
|
|
From: Mark W. <ma...@so...> - 2023-04-14 23:04:45
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=68cf3b5dbfecb96c618c371359000daaaf4293b5 commit 68cf3b5dbfecb96c618c371359000daaaf4293b5 Author: Mark Wielaard <ma...@kl...> Date: Sat Apr 15 01:04:23 2023 +0200 Add bug 467036 Add time cost statistics for Regtest to NEWS Diff: --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 6d93d2603e..e8225fdd40 100644 --- a/NEWS +++ b/NEWS @@ -146,6 +146,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 464969 D language demangling 465435 m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed. 466104 aligned_alloc problems, part 1 +467036 Add time cost statistics for Regtest 467482 Build failure on aarch64 Alpine 467714 fdleak_* and rlimit tests fail when parent process has more than 64 descriptors opened |
|
From: Mark W. <ma...@so...> - 2023-04-14 23:02:09
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=f7ddfc7cfd750978f405d7e2be8f76412fd1653d commit f7ddfc7cfd750978f405d7e2be8f76412fd1653d Author: Mark Wielaard <ma...@kl...> Date: Sat Apr 15 00:59:26 2023 +0200 Regtest: add time cost statistics Add running time of each (sub) directory in seconds https://bugs.kde.org/show_bug.cgi?id=467036 Contributed-by: Jojo R <rj...@li...> Diff: --- tests/vg_regtest.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/vg_regtest.in b/tests/vg_regtest.in index 0fe63411a1..7152765ed7 100755 --- a/tests/vg_regtest.in +++ b/tests/vg_regtest.in @@ -641,6 +641,7 @@ sub test_one_dir($$) my @fs = glob "*"; my $found_tests = (0 != (grep { $_ =~ /\.vgtest$/ } @fs)); + my $tests_start_time = time; if ($found_tests) { print "-- Running tests in $full_dir $dashes\n"; } @@ -652,7 +653,11 @@ sub test_one_dir($$) } } if ($found_tests) { - print "-- Finished tests in $full_dir $dashes\n"; + my $tests_cost_time = time - $tests_start_time; + my $end_time = "(in $tests_cost_time sec)"; + my $end_dashes = "-" x (50 - (length $full_dir) + - (length $end_time) - 1); + print "-- Finished tests in $full_dir $end_time $end_dashes\n"; } chdir(".."); |
|
From: Mark W. <ma...@so...> - 2023-04-14 22:15:16
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=16be0ca4ba53154642bd45e6aa60ffba57369a0c commit 16be0ca4ba53154642bd45e6aa60ffba57369a0c Author: Mark Wielaard <ma...@kl...> Date: Sat Apr 15 00:13:57 2023 +0200 tests fdleak.h close all open file descriptors > 2 Use sysconf (_SC_OPEN_MAX) to find the upper limit. Or use 1024 if that fails. https://bugs.kde.org/show_bug.cgi?id=467714 Diff: --- NEWS | 2 ++ none/tests/fdleak.h | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 13efee7d4a..6d93d2603e 100644 --- a/NEWS +++ b/NEWS @@ -147,6 +147,8 @@ are not entered into bugzilla tend to get forgotten about or ignored. 465435 m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed. 466104 aligned_alloc problems, part 1 467482 Build failure on aarch64 Alpine +467714 fdleak_* and rlimit tests fail when parent process has more than + 64 descriptors opened 467839 Gdbserver: Improve compatibility of library directory name n-i-bz FreeBSD rfork syscall fail with EINVAL or ENOSYS rather than VG_(unimplemented) diff --git a/none/tests/fdleak.h b/none/tests/fdleak.h index 66fd84e96f..c94dbde319 100644 --- a/none/tests/fdleak.h +++ b/none/tests/fdleak.h @@ -2,6 +2,7 @@ #define _FDLEAK_H_ #include <stdlib.h> +#include <unistd.h> #include <stdio.h> #define DO(op) \ @@ -23,6 +24,15 @@ * - For Ubuntu 8.04, see also * https://bugs.launchpad.net/ubuntu/+source/seahorse/+bug/235184 */ -#define CLOSE_INHERITED_FDS { int i; for (i = 3; i < 64; i++) close(i); } +static inline void close_inherited () { + int i; int max_fds = sysconf (_SC_OPEN_MAX); + if (max_fds < 0) + max_fds = 1024; /* Fallback if sysconf fails, returns -1. */ + + /* Only leave 0 (stdin), 1 (stdout) and 2 (stderr) open. */ + for (i = 3; i < max_fds; i++) + close(i); +} +#define CLOSE_INHERITED_FDS close_inherited () #endif /* _FDLEAK_H_ */ |
|
From: Mark W. <ma...@so...> - 2023-04-14 22:08:58
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=fb6fbe4e8446ec7dd4a01662dbefc48f9349160b commit fb6fbe4e8446ec7dd4a01662dbefc48f9349160b Author: Mark Wielaard <ma...@kl...> Date: Sat Apr 15 00:00:36 2023 +0200 gdbserver_tests: Improve compatibility of library directory name Some linux os make softlink from customized directory like lib64xxx into standard system lib64 directory. https://bugs.kde.org/show_bug.cgi?id=467839 Contributed-by: JojoR <rj...@gm...> Diff: --- NEWS | 1 + gdbserver_tests/filter_gdb.in | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index b8d5e333d7..13efee7d4a 100644 --- a/NEWS +++ b/NEWS @@ -147,6 +147,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 465435 m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed. 466104 aligned_alloc problems, part 1 467482 Build failure on aarch64 Alpine +467839 Gdbserver: Improve compatibility of library directory name n-i-bz FreeBSD rfork syscall fail with EINVAL or ENOSYS rather than VG_(unimplemented) To see details of a given bug, visit diff --git a/gdbserver_tests/filter_gdb.in b/gdbserver_tests/filter_gdb.in index 28f807d09e..16186dfe2c 100755 --- a/gdbserver_tests/filter_gdb.in +++ b/gdbserver_tests/filter_gdb.in @@ -147,10 +147,10 @@ s/in _select ()/in syscall .../ # (on 32 bits, we have an int_80, on 64 bits, directly select) s/in \(.__\)\{0,1\}select () from \/.*$/in syscall .../ -/^ from \/lib\/libc.so.*$/d -/^ from \/lib64\/libc.so.*$/d -/^ from \/lib64\/.*\/libc.so.*$/d -/^ from \/lib64\/.*\/libc-.*.so/d +/^ from \/lib.*\/libc.so.*$/d +/^ from \/lib64.*\/libc.so.*$/d +/^ from \/lib64.*\/.*\/libc.so.*$/d +/^ from \/lib64.*\/.*\/libc-.*.so/d s/ from \/lib\/libc\.so.*// # and yet another (gdb 7.0 way) to get a system call |
|
From: Mark W. <ma...@so...> - 2023-04-14 21:10:30
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=d387566dd7dbb2ebe6fea5bfb4b2353bd5ab86ba commit d387566dd7dbb2ebe6fea5bfb4b2353bd5ab86ba Author: Earl Chew <ear...@ya...> Date: Fri Apr 14 22:56:16 2023 +0200 Support Linux syscall 434 pidfd_open Diff: --- NEWS | 1 + coregrind/m_syswrap/priv_syswrap-linux.h | 3 +++ coregrind/m_syswrap/syswrap-amd64-linux.c | 1 + coregrind/m_syswrap/syswrap-arm-linux.c | 1 + coregrind/m_syswrap/syswrap-arm64-linux.c | 1 + coregrind/m_syswrap/syswrap-linux.c | 16 ++++++++++++++++ coregrind/m_syswrap/syswrap-mips32-linux.c | 1 + coregrind/m_syswrap/syswrap-mips64-linux.c | 1 + coregrind/m_syswrap/syswrap-nanomips-linux.c | 1 + coregrind/m_syswrap/syswrap-ppc32-linux.c | 1 + coregrind/m_syswrap/syswrap-ppc64-linux.c | 1 + coregrind/m_syswrap/syswrap-s390x-linux.c | 1 + coregrind/m_syswrap/syswrap-x86-linux.c | 1 + include/vki/vki-scnums-shared-linux.h | 1 + 14 files changed, 31 insertions(+) diff --git a/NEWS b/NEWS index 68acfff7cf..b8d5e333d7 100644 --- a/NEWS +++ b/NEWS @@ -140,6 +140,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 462830 WARNING: unhandled amd64-freebsd syscall: 474 463027 broken check for MPX instruction support in assembler 464476 Firefox fails to start under Valgrind +464609 Valgrind memcheck should support Linux pidfd_open 464680 Show issues caused by memory policies like selinux deny_execmem 464859 Build failures with GCC-13 (drd tsan_unittest) 464969 D language demangling diff --git a/coregrind/m_syswrap/priv_syswrap-linux.h b/coregrind/m_syswrap/priv_syswrap-linux.h index 4f85069476..a73b6247e7 100644 --- a/coregrind/m_syswrap/priv_syswrap-linux.h +++ b/coregrind/m_syswrap/priv_syswrap-linux.h @@ -320,6 +320,9 @@ DECL_TEMPLATE(linux, sys_io_uring_setup); DECL_TEMPLATE(linux, sys_io_uring_enter); DECL_TEMPLATE(linux, sys_io_uring_register); +// Linux-specific (new in Linux 5.3) +DECL_TEMPLATE(linux, sys_pidfd_open); + // Linux-specific (new in Linux 5.9) DECL_TEMPLATE(linux, sys_close_range); DECL_TEMPLATE(linux, sys_openat2); diff --git a/coregrind/m_syswrap/syswrap-amd64-linux.c b/coregrind/m_syswrap/syswrap-amd64-linux.c index 9054199857..1aeebd274b 100644 --- a/coregrind/m_syswrap/syswrap-amd64-linux.c +++ b/coregrind/m_syswrap/syswrap-amd64-linux.c @@ -876,6 +876,7 @@ static SyscallTableEntry syscall_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 LINXY(__NR_openat2, sys_openat2), // 437 diff --git a/coregrind/m_syswrap/syswrap-arm-linux.c b/coregrind/m_syswrap/syswrap-arm-linux.c index d583cef0c7..8b1a8fe702 100644 --- a/coregrind/m_syswrap/syswrap-arm-linux.c +++ b/coregrind/m_syswrap/syswrap-arm-linux.c @@ -1052,6 +1052,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-arm64-linux.c b/coregrind/m_syswrap/syswrap-arm64-linux.c index 3ed71e143b..9532360007 100644 --- a/coregrind/m_syswrap/syswrap-arm64-linux.c +++ b/coregrind/m_syswrap/syswrap-arm64-linux.c @@ -831,6 +831,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index 34d4cf1f55..26f1fbee3c 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -13605,6 +13605,22 @@ POST(sys_openat2) } } +PRE(sys_pidfd_open) +{ + PRINT("sys_pidfd_open ( %ld, %lu )", SARG1, ARG2); +} + +POST(sys_pidfd_open) +{ + if (!ML_(fd_allowed)(RES, "pidfd", tid, True)) { + VG_(close)(RES); + SET_STATUS_Failure( VKI_EMFILE ); + } else { + if (VG_(clo_track_fds)) + ML_(record_fd_open_nameless) (tid, RES); + } +} + #undef PRE #undef POST diff --git a/coregrind/m_syswrap/syswrap-mips32-linux.c b/coregrind/m_syswrap/syswrap-mips32-linux.c index f556e063f0..de27998b3f 100644 --- a/coregrind/m_syswrap/syswrap-mips32-linux.c +++ b/coregrind/m_syswrap/syswrap-mips32-linux.c @@ -1136,6 +1136,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-mips64-linux.c b/coregrind/m_syswrap/syswrap-mips64-linux.c index 41a5404c55..67e7c2c2f6 100644 --- a/coregrind/m_syswrap/syswrap-mips64-linux.c +++ b/coregrind/m_syswrap/syswrap-mips64-linux.c @@ -815,6 +815,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY (__NR_io_uring_setup, sys_io_uring_setup), LINXY (__NR_io_uring_enter, sys_io_uring_enter), LINXY (__NR_io_uring_register, sys_io_uring_register), + LINXY (__NR_pidfd_open, sys_pidfd_open), GENX_ (__NR_clone3, sys_ni_syscall), LINXY (__NR_close_range, sys_close_range), LINX_ (__NR_faccessat2, sys_faccessat2), diff --git a/coregrind/m_syswrap/syswrap-nanomips-linux.c b/coregrind/m_syswrap/syswrap-nanomips-linux.c index f9d4b19f4a..9c535c68ea 100644 --- a/coregrind/m_syswrap/syswrap-nanomips-linux.c +++ b/coregrind/m_syswrap/syswrap-nanomips-linux.c @@ -824,6 +824,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY (__NR_io_uring_setup, sys_io_uring_setup), LINXY (__NR_io_uring_enter, sys_io_uring_enter), LINXY (__NR_io_uring_register, sys_io_uring_register), + LINXY (__NR_pidfd_open, sys_pidfd_open), GENX_ (__NR_clone3, sys_ni_syscall), LINXY (__NR_close_range, sys_close_range), LINX_ (__NR_faccessat2, sys_faccessat2), diff --git a/coregrind/m_syswrap/syswrap-ppc32-linux.c b/coregrind/m_syswrap/syswrap-ppc32-linux.c index 637b2504e1..12c0730271 100644 --- a/coregrind/m_syswrap/syswrap-ppc32-linux.c +++ b/coregrind/m_syswrap/syswrap-ppc32-linux.c @@ -1056,6 +1056,7 @@ static SyscallTableEntry syscall_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-ppc64-linux.c b/coregrind/m_syswrap/syswrap-ppc64-linux.c index 93956d3cc2..3c33d1267e 100644 --- a/coregrind/m_syswrap/syswrap-ppc64-linux.c +++ b/coregrind/m_syswrap/syswrap-ppc64-linux.c @@ -1025,6 +1025,7 @@ static SyscallTableEntry syscall_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-s390x-linux.c b/coregrind/m_syswrap/syswrap-s390x-linux.c index 73f9684c46..a377cb7315 100644 --- a/coregrind/m_syswrap/syswrap-s390x-linux.c +++ b/coregrind/m_syswrap/syswrap-s390x-linux.c @@ -866,6 +866,7 @@ static SyscallTableEntry syscall_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register), // 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 diff --git a/coregrind/m_syswrap/syswrap-x86-linux.c b/coregrind/m_syswrap/syswrap-x86-linux.c index 5968d49dbf..a9ba15dfe6 100644 --- a/coregrind/m_syswrap/syswrap-x86-linux.c +++ b/coregrind/m_syswrap/syswrap-x86-linux.c @@ -1647,6 +1647,7 @@ static SyscallTableEntry syscall_table[] = { LINXY(__NR_io_uring_enter, sys_io_uring_enter), // 426 LINXY(__NR_io_uring_register, sys_io_uring_register),// 427 + LINXY(__NR_pidfd_open, sys_pidfd_open), // 434 GENX_(__NR_clone3, sys_ni_syscall), // 435 LINXY(__NR_close_range, sys_close_range), // 436 LINXY(__NR_openat2, sys_openat2), // 437 diff --git a/include/vki/vki-scnums-shared-linux.h b/include/vki/vki-scnums-shared-linux.h index 048460d0b2..d90cdd3124 100644 --- a/include/vki/vki-scnums-shared-linux.h +++ b/include/vki/vki-scnums-shared-linux.h @@ -39,6 +39,7 @@ #define __NR_fsmount 432 #define __NR_fspick 433 +#define __NR_pidfd_open 434 #define __NR_clone3 435 #define __NR_close_range 436 #define __NR_openat2 437 |
|
From: Paul F. <pa...@so...> - 2023-04-14 06:16:55
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1e42338863964f22cdf3d4614c3b501c22d26390 commit 1e42338863964f22cdf3d4614c3b501c22d26390 Author: Paul Floyd <pj...@wa...> Date: Fri Apr 14 08:15:40 2023 +0200 regtest: add new multi option to cmdline2 help output expecteds Diff: --- none/tests/cmdline2.stdout.exp | 1 + none/tests/cmdline2.stdout.exp-non-linux | 1 + 2 files changed, 2 insertions(+) diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index 241d33afa5..10485a3b40 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -190,6 +190,7 @@ 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 63af17bf74..6e08284acd 100644 --- a/none/tests/cmdline2.stdout.exp-non-linux +++ b/none/tests/cmdline2.stdout.exp-non-linux @@ -188,6 +188,7 @@ 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] |
|
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: Paul F. <pa...@so...> - 2023-04-13 20:49:48
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=0ae17c117edbad1785c148693a31268b3bec4565 commit 0ae17c117edbad1785c148693a31268b3bec4565 Author: Paul Floyd <pj...@wa...> Date: Thu Apr 13 22:45:46 2023 +0200 FreeBSD: auxv changes for FreeBSD 13.2 Diff: --- configure.ac | 13 ++++++++++--- coregrind/m_initimg/initimg-freebsd.c | 2 +- none/tests/freebsd/auxv.c | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index a8d9b95138..80bdbb417c 100755 --- a/configure.ac +++ b/configure.ac @@ -398,8 +398,10 @@ case "${host_os}" in freebsd_12_2=1220 AC_DEFINE([FREEBSD_13_0], 1300, [FREEBSD_VERS value for FreeBSD 13.0]) freebsd_13_0=1300 - AC_DEFINE([FREEBSD_13_1], 1310, [FREEBSD_VERS value for FreeBSD 13.1+]) + AC_DEFINE([FREEBSD_13_1], 1310, [FREEBSD_VERS value for FreeBSD 13.1]) freebsd_13_1=1310 + AC_DEFINE([FREEBSD_13_2], 1320, [FREEBSD_VERS value for FreeBSD 13.2]) + freebsd_13_2=1320 AC_DEFINE([FREEBSD_14], 1400, [FREEBSD_VERS value for FreeBSD 14.x]) freebsd_14=1400 @@ -438,11 +440,16 @@ case "${host_os}" in AC_DEFINE([FREEBSD_VERS], FREEBSD_13_0, [FreeBSD version]) freebsd_vers=$freebsd_13_0 ;; - *) - AC_MSG_RESULT([FreeBSD 13.1+ (${kernel})]) + 13.1-*) + AC_MSG_RESULT([FreeBSD 13.1 (${kernel})]) AC_DEFINE([FREEBSD_VERS], FREEBSD_13_1, [FreeBSD version]) freebsd_vers=$freebsd_13_1 ;; + 13.2-*) + AC_MSG_RESULT([FreeBSD 13.2 (${kernel})]) + AC_DEFINE([FREEBSD_VERS], FREEBSD_13_2, [FreeBSD version]) + freebsd_vers=$freebsd_13_2 + ;; esac ;; 14.*) diff --git a/coregrind/m_initimg/initimg-freebsd.c b/coregrind/m_initimg/initimg-freebsd.c index 53a9aca873..ba01279ebb 100644 --- a/coregrind/m_initimg/initimg-freebsd.c +++ b/coregrind/m_initimg/initimg-freebsd.c @@ -742,7 +742,7 @@ static Addr setup_client_stack(void* init_sp, // case AT_KPRELOAD: #endif -#if (FREEBSD_VERS >= FREEBSD_14) +#if (FREEBSD_VERS >= FREEBSD_13_2) case VKI_AT_USRSTACKBASE: auxv->u.a_val = VG_(get_usrstack)(); break; diff --git a/none/tests/freebsd/auxv.c b/none/tests/freebsd/auxv.c index 6425fc3043..f2e1299045 100644 --- a/none/tests/freebsd/auxv.c +++ b/none/tests/freebsd/auxv.c @@ -54,7 +54,7 @@ Elf_AuxStr aux_map[AT_COUNT] = { {"AT_KPRELOAD", 34}, // {"AT_COUNT", 35}, #endif -#if (FREEBSD_VERS >= FREEBSD_14) +#if (FREEBSD_VERS >= FREEBSD_13_2) {"AT_USRSTACKBASE", 35}, {"AT_USRSTACKLIM", 36}, // {"AT_COUNT", 37}, |
|
From: Paul F. <pa...@so...> - 2023-04-13 20:15:03
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=b66f25877c3908191972348f01b9fb2c7a9cb6c5 commit b66f25877c3908191972348f01b9fb2c7a9cb6c5 Author: Paul Floyd <pj...@wa...> Date: Thu Apr 13 22:10:56 2023 +0200 FreeBSD: Helgrind suppression for std::__1::__thread_local_data on FreeBSD 13.2 Diff: --- freebsd-helgrind.supp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/freebsd-helgrind.supp b/freebsd-helgrind.supp index 71bfcb6abc..32af0a7626 100644 --- a/freebsd-helgrind.supp +++ b/freebsd-helgrind.supp @@ -158,7 +158,12 @@ fun:_ZL11* } { - HELGRIND-CXX-TLS + HELGRIND-CXX-TLS1 Helgrind:Race fun:_ZNSt3__121__thread_specific_ptrINS_15__thread_structEE11set_pointerEPS1_ } +{ + HELGRIND-CXX-TLS2 + Helgrind:Race + fun:_ZNSt3__119__thread_local_dataEv +} |
|
From: Paul F. <pa...@so...> - 2023-04-13 20:08:38
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=ec1049b5d1160a8b58fe19ceea1f184e50a6a7d3 commit ec1049b5d1160a8b58fe19ceea1f184e50a6a7d3 Author: Paul Floyd <pj...@wa...> Date: Thu Apr 13 22:06:34 2023 +0200 regtest: update filter_stanza so that sized_aligned_new_delete_args works on 32bit platforms Diff: --- memcheck/tests/filter_stanza | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/memcheck/tests/filter_stanza b/memcheck/tests/filter_stanza index 9152ff1090..ecd88b7c0b 100755 --- a/memcheck/tests/filter_stanza +++ b/memcheck/tests/filter_stanza @@ -1,5 +1,5 @@ #! /bin/sh -./filter_stderr "$@" | +./filter_size_t "$@" | awk -f filter_stanza.awk |
|
From: Floyd, P. <pj...@wa...> - 2023-04-12 08:03:45
|
On 11/04/2023 09:57, Mark Wielaard wrote: > Hi, > > Working towards a new release (3.21.0 currently planned for April 28) > there is a bit more automation to show pre-releases and documentation: > > https://snapshots.sourceware.org/valgrind/trunk/ > > Every 15 minutes a buildbot will check for new commits and create a > dist, html manual pages and documentation downloads from latest git > trunk (currently the git master branch, after the release we'll switch > to using the main branch). Hi Mark I'll look into using that. I maintain two versions of Valgrind ports, one "official" one based on released versions and one "-devel" version. The latter uses GH at the moment, but I think it'd be nicer to use these sourceware snapshots. A+ Paul |
|
From: Nicholas N. <nj...@so...> - 2023-04-12 03:48:29
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=57dbcacfdbff0d4c12dcd52ff56f159631499dc6 commit 57dbcacfdbff0d4c12dcd52ff56f159631499dc6 Author: Nicholas Nethercote <n.n...@gm...> Date: Tue Apr 11 16:42:21 2023 +1000 Make `--cache-sim=no` the default for Cachegrind. Also, don't print cache simulation details in the `desc:` line when the cache simulation is disabled. Docs changes are yet to come. Diff: --- cachegrind/cg_main.c | 20 +++++++++++--------- cachegrind/tests/ann-diff1.stderr.exp | 14 -------------- cachegrind/tests/ann-diff2.stderr.exp | 14 -------------- cachegrind/tests/ann-merge1.stderr.exp | 14 -------------- cachegrind/tests/ann1a.stderr.exp | 14 -------------- cachegrind/tests/ann1b.stderr.exp | 14 -------------- cachegrind/tests/ann2.stderr.exp | 14 -------------- cachegrind/tests/chdir.stderr.exp | 14 -------------- cachegrind/tests/dlclose.stderr.exp | 14 -------------- cachegrind/tests/notpower2.vgtest | 2 +- cachegrind/tests/wrap5.stderr.exp | 14 -------------- cachegrind/tests/x86/fpu-28-108.stderr.exp | 14 -------------- 12 files changed, 12 insertions(+), 150 deletions(-) diff --git a/cachegrind/cg_main.c b/cachegrind/cg_main.c index 927f3fb328..c4e111aa30 100644 --- a/cachegrind/cg_main.c +++ b/cachegrind/cg_main.c @@ -57,7 +57,7 @@ /*--- Options ---*/ /*------------------------------------------------------------*/ -static Bool clo_cache_sim = True; /* do cache simulation? */ +static Bool clo_cache_sim = False; /* do cache simulation? */ static Bool clo_branch_sim = False; /* do branch simulation? */ static const HChar* clo_cachegrind_out_file = "cachegrind.out.%p"; @@ -1391,21 +1391,23 @@ static void fprint_CC_table_and_calc_totals(void) if (fp == NULL) { // If the file can't be opened for whatever reason (conflict // between multiple cachegrinded processes?), give up now. - VG_(umsg)("error: can't open cache simulation output file '%s'\n", + VG_(umsg)("error: can't open output data file '%s'\n", cachegrind_out_file ); - VG_(umsg)(" ... so simulation results will be missing.\n"); + VG_(umsg)(" ... so detailed results will be missing.\n"); VG_(free)(cachegrind_out_file); return; } else { VG_(free)(cachegrind_out_file); } - // "desc:" lines (giving I1/D1/LL cache configuration). The spaces after - // the 2nd colon makes cg_annotate's output look nicer. - VG_(fprintf)(fp, "desc: I1 cache: %s\n" - "desc: D1 cache: %s\n" - "desc: LL cache: %s\n", - I1.desc_line, D1.desc_line, LL.desc_line); + if (clo_cache_sim) { + // "desc:" lines (giving I1/D1/LL cache configuration). The spaces after + // the 2nd colon makes cg_annotate's output look nicer. + VG_(fprintf)(fp, "desc: I1 cache: %s\n" + "desc: D1 cache: %s\n" + "desc: LL cache: %s\n", + I1.desc_line, D1.desc_line, LL.desc_line); + } // "cmd:" line VG_(fprintf)(fp, "cmd: %s", VG_(args_the_exename)); diff --git a/cachegrind/tests/ann-diff1.stderr.exp b/cachegrind/tests/ann-diff1.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann-diff1.stderr.exp +++ b/cachegrind/tests/ann-diff1.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/ann-diff2.stderr.exp b/cachegrind/tests/ann-diff2.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann-diff2.stderr.exp +++ b/cachegrind/tests/ann-diff2.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/ann-merge1.stderr.exp b/cachegrind/tests/ann-merge1.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann-merge1.stderr.exp +++ b/cachegrind/tests/ann-merge1.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/ann1a.stderr.exp b/cachegrind/tests/ann1a.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann1a.stderr.exp +++ b/cachegrind/tests/ann1a.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/ann1b.stderr.exp b/cachegrind/tests/ann1b.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann1b.stderr.exp +++ b/cachegrind/tests/ann1b.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/ann2.stderr.exp b/cachegrind/tests/ann2.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/ann2.stderr.exp +++ b/cachegrind/tests/ann2.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/chdir.stderr.exp b/cachegrind/tests/chdir.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/chdir.stderr.exp +++ b/cachegrind/tests/chdir.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/dlclose.stderr.exp b/cachegrind/tests/dlclose.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/dlclose.stderr.exp +++ b/cachegrind/tests/dlclose.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/notpower2.vgtest b/cachegrind/tests/notpower2.vgtest index 21caffe94e..bf05bb41c7 100644 --- a/cachegrind/tests/notpower2.vgtest +++ b/cachegrind/tests/notpower2.vgtest @@ -1,3 +1,3 @@ prog: ../../tests/true -vgopts: --I1=32768,8,64 --D1=24576,6,64 --LL=3145728,12,64 +vgopts: --cache-sim=yes --I1=32768,8,64 --D1=24576,6,64 --LL=3145728,12,64 cleanup: rm cachegrind.out.* diff --git a/cachegrind/tests/wrap5.stderr.exp b/cachegrind/tests/wrap5.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/wrap5.stderr.exp +++ b/cachegrind/tests/wrap5.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: diff --git a/cachegrind/tests/x86/fpu-28-108.stderr.exp b/cachegrind/tests/x86/fpu-28-108.stderr.exp index e8084c12c3..ec68407b27 100644 --- a/cachegrind/tests/x86/fpu-28-108.stderr.exp +++ b/cachegrind/tests/x86/fpu-28-108.stderr.exp @@ -1,17 +1,3 @@ I refs: -I1 misses: -LLi misses: -I1 miss rate: -LLi miss rate: - -D refs: -D1 misses: -LLd misses: -D1 miss rate: -LLd miss rate: - -LL refs: -LL misses: -LL miss rate: |
|
From: Mark W. <ma...@kl...> - 2023-04-11 07:58:06
|
Hi, Working towards a new release (3.21.0 currently planned for April 28) there is a bit more automation to show pre-releases and documentation: https://snapshots.sourceware.org/valgrind/trunk/ Every 15 minutes a buildbot will check for new commits and create a dist, html manual pages and documentation downloads from latest git trunk (currently the git master branch, after the release we'll switch to using the main branch). Be careful, these aren't official releases, it is as if getting a random git checkout, but hopefully it is useful to see what is coming and have the latest (draft) documentation for new features for the next release. If you try these out please explicitly mention which snapshot you used in any bug reports. Cheers, Mark |
|
From: Nicholas N. <nj...@so...> - 2023-04-10 23:59:15
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=8765b3358faae875684c6f3b48e3af103162fe36 commit 8765b3358faae875684c6f3b48e3af103162fe36 Author: Nicholas Nethercote <n.n...@gm...> Date: Wed Mar 29 09:55:51 2023 +1100 Overhaul `cg_annotate` output. Most notable, the "Function summary" section, which printed one CC for each `file:function` combination, has been replaced by two sections, "File:function summary" and "Function:file summary". These new sections both feature "deep CCs", which have an "outer CC" for the file (or function), and one or more "inner CCs" for the paired functions (or files). Here is a file:function example, which helps show which files have a lot of events, even if those events are spread across a lot of functions. ``` > 12,427,830 (5.4%, 26.3%) /home/njn/moz/gecko-dev/js/src/ds/LifoAlloc.h: 6,107,862 (2.7%) js::frontend::ParseNodeVerifier::visit(js::frontend::ParseNode*) 3,685,203 (1.6%) js::detail::BumpChunk::setBump(unsigned char*) 1,640,591 (0.7%) js::LifoAlloc::alloc(unsigned long) 711,008 (0.3%) js::detail::BumpChunk::assertInvariants() ``` And here is a function:file example, which shows how heavy inlining can result in a machine code function being derived from source code from multiple files: ``` > 1,343,736 (0.6%, 35.6%) js::gc::TenuredCell::isMarkedGray() const: 651,108 (0.3%) /home/njn/moz/gecko-dev/js/src/d64/dist/include/js/HeapAPI.h 292,672 (0.1%) /home/njn/moz/gecko-dev/js/src/gc/Cell.h 254,854 (0.1%) /home/njn/moz/gecko-dev/js/src/gc/Heap.h ``` Previously these patterns were very hard to find, and it was easy to overlook a hot piece of code because its counts were spread across multiple non-adjacent entries. I have already found these changes very useful for profiling Rust code. Also, cumulative percentages on the outer CCs (e.g. the 26.3% and 35.6% in the example) tell you what fraction of all events are covered by the entries so far, something I've wanted for a long time. Some other, related changes: - Column event headers are now padded with `_`, e.g. `Ir__________`. This makes the column/event mapping clearer. - The "Cachegrind profile" section is now called "Metadata", which is shorter and clearer. - A few minor test tweaks, beyond those required for the output changes. - I converted some doc comments to normal comments. Not standard Python, but nicer to read, and there are no public APIs here. - Roughly 2x speedups to `cg_annotate` and smaller improvements for `cg_diff` and `cg_merge`, due to the following. - Change the `Cc` class to a type alias for `list[int]`, to avoid the class overhead (sigh). - Process event count lines in a single split, instead of a regex match + split. - Add the `add_cc_to_ccs` function, which does multiple CC additions in a single function call. - Better handling of dicts while reading input, minimizing lookups. - Pre-computing the missing CC string for each CcPrinter, instead of regenerating it each time. Diff: --- auxprogs/pylintrc | 8 +- cachegrind/cg_annotate.in | 583 +++++++++++++++++++++++------------ cachegrind/cg_diff.in | 113 +++---- cachegrind/cg_merge.in | 103 +++---- cachegrind/tests/ann-diff1.post.exp | 21 +- cachegrind/tests/ann-diff2.post.exp | 28 +- cachegrind/tests/ann-merge1.post.exp | 77 +++-- cachegrind/tests/ann-merge1a.cgout | 14 +- cachegrind/tests/ann-merge1b.cgout | 10 +- cachegrind/tests/ann1a.post.exp | 67 +++- cachegrind/tests/ann1b.post.exp | 48 ++- cachegrind/tests/ann2.cgout | 25 +- cachegrind/tests/ann2.post.exp | 181 ++++++----- 13 files changed, 772 insertions(+), 506 deletions(-) diff --git a/auxprogs/pylintrc b/auxprogs/pylintrc index 8f51d2d686..2d26cf00d2 100644 --- a/auxprogs/pylintrc +++ b/auxprogs/pylintrc @@ -12,10 +12,12 @@ [MESSAGES CONTROL] disable= - # We don't care about having docstrings for all functions/classes. + # We don't care about having docstrings everywhere. missing-class-docstring, missing-function-docstring, - # We don't care about large functions, sometimes it's necessary. - too-many-branches, too-many-locals, too-many-statements, + missing-module-docstring, + # We don't care about these, sometimes they are necessary. + too-many-arguments, too-many-branches, too-many-lines, too-many-locals, + too-many-statements, # Zero or one public methods in a class is fine. too-few-public-methods, diff --git a/cachegrind/cg_annotate.in b/cachegrind/cg_annotate.in index 5dad969f6a..0b68e094c3 100755 --- a/cachegrind/cg_annotate.in +++ b/cachegrind/cg_annotate.in @@ -26,15 +26,12 @@ # # The GNU General Public License is contained in the file COPYING. -""" -This script reads Cachegrind output files and produces human-readable reports. -""" - +# This script reads Cachegrind output files and produces human-readable output. +# # Use `make pyann` to "build" this script with `auxprogs/pybuild.rs` every time # it is changed. This runs the formatters, type-checkers, and linters on # `cg_annotate.in` and then generates `cg_annotate`. - from __future__ import annotations import os @@ -42,16 +39,12 @@ import re import sys from argparse import ArgumentParser, BooleanOptionalAction, Namespace from collections import defaultdict -from typing import DefaultDict, NewType, NoReturn, TextIO +from typing import DefaultDict, NoReturn, TextIO +# A typed wrapper for parsed args. class Args(Namespace): - """ - A typed wrapper for parsed args. - - None of these fields are modified after arg parsing finishes. - """ - + # None of these fields are modified after arg parsing finishes. show: list[str] sort: list[str] threshold: float # a percentage @@ -72,16 +65,14 @@ class Args(Namespace): return f raise ValueError + # Add a bool argument that defaults to true. + # + # Supports these forms: `--foo`, `--no-foo`, `--foo=yes`, `--foo=no`. + # The latter two were the forms supported by the old Perl version of + # `cg_annotate`, and are now deprecated. def add_bool_argument( p: ArgumentParser, new_name: str, old_name: str, help_: str ) -> None: - """ - Add a bool argument that defaults to true. - - Supports these forms: `--foo`, `--no-foo`, `--foo=yes`, `--foo=no`. - The latter two were the forms supported by the old Perl version of - `cg_annotate`, and are now deprecated. - """ new_flag = "--" + new_name old_flag = "--" + old_name dest = new_name.replace("-", "_") @@ -110,28 +101,25 @@ class Args(Namespace): p = ArgumentParser(description="Process a Cachegrind output file.") p.add_argument("--version", action="version", version="%(prog)s-@VERSION@") - p.add_argument( "--show", type=comma_separated_list, metavar="A,B,C", help="only show figures for events A,B,C (default: all events)", ) - p.add_argument( "--sort", type=comma_separated_list, metavar="A,B,C", help="sort functions by events A,B,C (default: event column order)", ) - p.add_argument( "--threshold", type=threshold, default=0.1, metavar="N:[0,20]", - help="only show functions with more than N%% of primary sort event " - "counts (default: %(default)s)", + help="only show file:function/function:file pairs with more than " + "N%% of primary sort event counts (default: %(default)s)", ) add_bool_argument( p, @@ -182,6 +170,9 @@ class Events: # The event names. events: list[str] + # Equal to `len(self.events)`. + num_events: int + # The order in which we must traverse events for --show. Can be shorter # than `events`. show_events: list[str] @@ -226,10 +217,10 @@ class Events: self.sort_indices = [event_indices[event] for event in self.sort_events] - def mk_cc(self, text: str) -> Cc: - """Raises a `ValueError` exception on syntax error.""" + # Raises a `ValueError` exception on syntax error. + def mk_cc(self, str_counts: list[str]) -> Cc: # This is slightly faster than a list comprehension. - counts = list(map(int, text.split())) + counts = list(map(int, str_counts)) if len(counts) == self.num_events: pass @@ -239,48 +230,81 @@ class Events: else: raise ValueError - return Cc(counts) + return counts def mk_empty_cc(self) -> Cc: # This is much faster than a list comprehension. - return Cc([0] * self.num_events) + return [0] * self.num_events + + def mk_empty_dcc(self) -> Dcc: + return Dcc(self.mk_empty_cc(), defaultdict(self.mk_empty_cc)) + + +# A "cost centre", which is a dumb container for counts. Always the same length +# as `Events.events`, but it doesn't even know event names. `Events.mk_cc` and +# `Events.mk_empty_cc` are used for construction. +# +# This used to be a class with a single field `counts: list[int]`, but this +# type is very hot and just using a type alias is much faster. +Cc = list[int] + +# Add the counts in `a_cc` to `b_cc`. +def add_cc_to_cc(a_cc: Cc, b_cc: Cc) -> None: + for i, a_count in enumerate(a_cc): + b_cc[i] += a_count + + +# Unrolled version of `add_cc_to_cc`, for speed. +def add_cc_to_ccs( + a_cc: Cc, b_cc1: Cc, b_cc2: Cc, b_cc3: Cc, b_cc4: Cc, b_cc5: Cc +) -> None: + for i, a_count in enumerate(a_cc): + b_cc1[i] += a_count + b_cc2[i] += a_count + b_cc3[i] += a_count + b_cc4[i] += a_count + b_cc5[i] += a_count -class Cc: - """ - This is a dumb container for counts. +# Update `min_cc` and `max_cc` with `self`. +def update_cc_extremes(self: Cc, min_cc: Cc, max_cc: Cc) -> None: + for i, count in enumerate(self): + if count > max_cc[i]: + max_cc[i] = count + elif count < min_cc[i]: + min_cc[i] = count - It doesn't know anything about events, i.e. what each count means. It can - do basic operations like `__iadd__` and `__eq__`, and anything more must be - done elsewhere. `Events.mk_cc` and `Events.mk_empty_cc` are used for - construction. - """ - # Always the same length as `Events.events`. - counts: list[int] +# A deep cost centre with a dict for the inner names and CCs. +class Dcc: + outer_cc: Cc + inner_dict_name_cc: DictNameCc - def __init__(self, counts: list[int]) -> None: - self.counts = counts + def __init__(self, outer_cc: Cc, inner_dict_name_cc: DictNameCc) -> None: + self.outer_cc = outer_cc + self.inner_dict_name_cc = inner_dict_name_cc - def __repr__(self) -> str: - return str(self.counts) - def __eq__(self, other: object) -> bool: - if not isinstance(other, Cc): - return NotImplemented - return self.counts == other.counts +# A deep cost centre with a list for the inner names and CCs. Used during +# filtering and sorting. +class Lcc: + outer_cc: Cc + inner_list_name_cc: ListNameCc - def __iadd__(self, other: Cc) -> Cc: - for i, other_count in enumerate(other.counts): - self.counts[i] += other_count - return self + def __init__(self, outer_cc: Cc, inner_list_name_cc: ListNameCc) -> None: + self.outer_cc = outer_cc + self.inner_list_name_cc = inner_list_name_cc -# A paired filename and function name. -Flfn = NewType("Flfn", tuple[str, str]) +# Per-file/function CCs. The list version is used during filtering and sorting. +DictNameCc = DefaultDict[str, Cc] +ListNameCc = list[tuple[str, Cc]] -# Per-function CCs. -DictFlfnCc = DefaultDict[Flfn, Cc] +# Per-file/function DCCs. The outer names are filenames and the inner names are +# function names, or vice versa. The list version is used during filtering and +# sorting. +DictNameDcc = DefaultDict[str, Dcc] +ListNameLcc = list[tuple[str, Lcc]] # Per-line CCs, organised by filename and line number. DictLineCc = DefaultDict[int, Cc] @@ -292,7 +316,15 @@ def die(msg: str) -> NoReturn: sys.exit(1) -def read_cgout_file() -> tuple[str, str, Events, DictFlfnCc, DictFlDictLineCc, Cc]: +def read_cgout_file() -> tuple[ + str, + str, + Events, + DictNameDcc, + DictNameDcc, + DictFlDictLineCc, + Cc, +]: # The file format is described in Cachegrind's manual. try: cgout_file = open(args.cgout_filename[0], "r", encoding="utf-8") @@ -334,52 +366,67 @@ def read_cgout_file() -> tuple[str, str, Events, DictFlfnCc, DictFlDictLineCc, C def mk_empty_dict_line_cc() -> DictLineCc: return defaultdict(events.mk_empty_cc) - curr_fl = "" - curr_flfn = Flfn(("", "")) + # The current filename and function name. + fl = "" + fn = "" # Different places where we accumulate CC data. - dict_flfn_cc: DictFlfnCc = defaultdict(events.mk_empty_cc) + dict_fl_dcc: DictNameDcc = defaultdict(events.mk_empty_dcc) + dict_fn_dcc: DictNameDcc = defaultdict(events.mk_empty_dcc) dict_fl_dict_line_cc: DictFlDictLineCc = defaultdict(mk_empty_dict_line_cc) summary_cc = None - # Compile the one hot regex. - count_pat = re.compile(r"(\d+)\s+(.*)") + # These are refs into the dicts above, used to avoid repeated lookups. + # They are all overwritten before first use. + fl_dcc = events.mk_empty_dcc() + fn_dcc = events.mk_empty_dcc() + fl_dcc_inner_fn_cc = events.mk_empty_cc() + fn_dcc_inner_fl_cc = events.mk_empty_cc() + dict_line_cc = mk_empty_dict_line_cc() # Line matching is done in order of pattern frequency, for speed. - while True: - line = readline() - - if m := count_pat.match(line): - line_num = int(m.group(1)) + while line := readline(): + if line[0].isdigit(): + split_line = line.split() try: - cc = events.mk_cc(m.group(2)) + line_num = int(split_line[0]) + cc = events.mk_cc(split_line[1:]) except ValueError: parse_die("malformed or too many event counts") - # Record this CC at the function level. - flfn_cc = dict_flfn_cc[curr_flfn] - flfn_cc += cc - - # Record this CC at the file/line level. - line_cc = dict_fl_dict_line_cc[curr_fl][line_num] - line_cc += cc + # Record this CC at the file:function level, the function:file + # level, and the file/line level. + add_cc_to_ccs( + cc, + fl_dcc.outer_cc, + fn_dcc.outer_cc, + fl_dcc_inner_fn_cc, + fn_dcc_inner_fl_cc, + dict_line_cc[line_num], + ) elif line.startswith("fn="): - curr_flfn = Flfn((curr_fl, line[3:-1])) + fn = line[3:-1] + # `fl_dcc` is unchanged. + fn_dcc = dict_fn_dcc[fn] + fl_dcc_inner_fn_cc = fl_dcc.inner_dict_name_cc[fn] + fn_dcc_inner_fl_cc = fn_dcc.inner_dict_name_cc[fl] elif line.startswith("fl="): - curr_fl = line[3:-1] + fl = line[3:-1] # A `fn=` line should follow, overwriting the function name. - curr_flfn = Flfn((curr_fl, "<unspecified>")) + fn = "<unspecified>" + fl_dcc = dict_fl_dcc[fl] + fn_dcc = dict_fn_dcc[fn] + fl_dcc_inner_fn_cc = fl_dcc.inner_dict_name_cc[fn] + fn_dcc_inner_fl_cc = fn_dcc.inner_dict_name_cc[fl] + dict_line_cc = dict_fl_dict_line_cc[fl] elif m := re.match(r"summary:\s+(.*)", line): try: - summary_cc = events.mk_cc(m.group(1)) + summary_cc = events.mk_cc(m.group(1).split()) except ValueError: - parse_die("too many event counts") - - elif line == "": - break # EOF + parse_die("malformed or too many event counts") elif line == "\n" or line.startswith("#"): # Skip empty lines and comment lines. @@ -392,10 +439,10 @@ def read_cgout_file() -> tuple[str, str, Events, DictFlfnCc, DictFlDictLineCc, C if not summary_cc: parse_die("missing `summary:` line, aborting") - # Check summary is correct. + # Check summary is correct. (Only using the outer CCs.) total_cc = events.mk_empty_cc() - for flfn_cc in dict_flfn_cc.values(): - total_cc += flfn_cc + for dcc in dict_fl_dcc.values(): + add_cc_to_cc(dcc.outer_cc, total_cc) if summary_cc != total_cc: msg = ( "`summary:` line doesn't match computed total\n" @@ -404,7 +451,32 @@ def read_cgout_file() -> tuple[str, str, Events, DictFlfnCc, DictFlDictLineCc, C ) parse_die(msg) - return (desc, cmd, events, dict_flfn_cc, dict_fl_dict_line_cc, summary_cc) + return ( + desc, + cmd, + events, + dict_fl_dcc, + dict_fn_dcc, + dict_fl_dict_line_cc, + summary_cc, + ) + + +# The width of a column, in three parts. +class Width: + # Width of the widest commified event count. + count: int + + # Width of the widest first percentage, of the form ` (n.n%)` or ` (n.n%,`. + perc1: int + + # Width of the widest second percentage, of the form ` n.n%)`. + perc2: int + + def __init__(self, count: int, perc1: int, perc2: int) -> None: + self.count = count + self.perc1 = perc1 + self.perc2 = perc2 class CcPrinter: @@ -414,91 +486,165 @@ class CcPrinter: # Note: every `CcPrinter` gets the same summary CC. summary_cc: Cc - # The width of each event count column. (This column is also used for event - # names.) For simplicity, its length matches `events.events`, even though - # not all events are necessarily shown. - count_widths: list[int] + # String to print before the event names. + events_prefix: str + + # The widths of each event column. For simplicity, its length matches + # `events.events`, even though not all events are necessarily shown. + widths: list[Width] - # The width of each percentage column. Zero if --show-percs is disabled. - # Its length matches `count_widths`. - perc_widths: list[int] + # Text of a missing CC, which can be computed in advance. + missing_cc_str: str - def __init__(self, events: Events, ccs: list[Cc], summary_cc: Cc) -> None: + # Must call `init_ccs` or `init_list_name_lcc` after this. + def __init__(self, events: Events, summary_cc: Cc) -> None: self.events = events self.summary_cc = summary_cc + # Other fields initialized in `init_*`. - # Find min and max value for each event. One of them will be the - # widest value. - min_cc = events.mk_empty_cc() - max_cc = events.mk_empty_cc() + def init_ccs(self, ccs: list[Cc]) -> None: + self.events_prefix = "" + + # Find min and max count for each event. One of them will be the widest + # value. + min_cc = self.events.mk_empty_cc() + max_cc = self.events.mk_empty_cc() for cc in ccs: - for i, _ in enumerate(events.events): - count = cc.counts[i] - if count > max_cc.counts[i]: - max_cc.counts[i] = count - elif count < min_cc.counts[i]: - min_cc.counts[i] = count - - # Find maximum width for each column. - self.count_widths = [0] * events.num_events - self.perc_widths = [0] * events.num_events - for i, event in enumerate(events.events): - # Get count and perc widths of the min and max CCs. - (min_count, min_perc) = self.count_and_perc(min_cc, i) - (max_count, max_perc) = self.count_and_perc(max_cc, i) - - # The event name goes in the count column. - self.count_widths[i] = max(len(min_count), len(max_count), len(event)) - self.perc_widths[i] = max(len(min_perc), len(max_perc)) + update_cc_extremes(cc, min_cc, max_cc) + + self.init_widths(min_cc, max_cc, None, None) + + def init_list_name_lcc(self, list_name_lcc: ListNameLcc) -> None: + self.events_prefix = " " + + cumul_cc = self.events.mk_empty_cc() + + # Find min and max value for each event. One of them will be the widest + # value. Likewise for the cumulative counts. + min_cc = self.events.mk_empty_cc() + max_cc = self.events.mk_empty_cc() + min_cumul_cc = self.events.mk_empty_cc() + max_cumul_cc = self.events.mk_empty_cc() + for _, lcc in list_name_lcc: + # Consider both outer and inner CCs for `count` and `perc1`. + update_cc_extremes(lcc.outer_cc, min_cc, max_cc) + for _, inner_cc in lcc.inner_list_name_cc: + update_cc_extremes(inner_cc, min_cc, max_cc) + + # Consider only outer CCs for `perc2`. + add_cc_to_cc(lcc.outer_cc, cumul_cc) + update_cc_extremes(cumul_cc, min_cumul_cc, max_cumul_cc) + + self.init_widths(min_cc, max_cc, min_cumul_cc, max_cumul_cc) + + def init_widths( + self, min_cc1: Cc, max_cc1: Cc, min_cc2: Cc | None, max_cc2: Cc | None + ) -> None: + self.widths = [Width(0, 0, 0)] * self.events.num_events + for i in range(len(self.events.events)): + # Get count and percs widths of the min and max CCs. + (min_count, min_perc1, min_perc2) = self.count_and_percs_strs( + min_cc1, min_cc2, i + ) + (max_count, max_perc1, max_perc2) = self.count_and_percs_strs( + max_cc1, max_cc2, i + ) + self.widths[i] = Width( + max(len(min_count), len(max_count)), + max(len(min_perc1), len(max_perc1)), + max(len(min_perc2), len(max_perc2)), + ) - def print_events(self, suffix: str) -> None: + self.missing_cc_str = "" for i in self.events.show_indices: - # The event name goes in the count column. - event = self.events.events[i] - nwidth = self.count_widths[i] - pwidth = self.perc_widths[i] - empty_perc = "" - print(f"{event:<{nwidth}}{empty_perc:>{pwidth}} ", end="") - - print(suffix) - - def print_count_and_perc(self, i: int, count: str, perc: str) -> None: - nwidth = self.count_widths[i] - pwidth = self.perc_widths[i] - print(f"{count:>{nwidth}}{perc:>{pwidth}} ", end="") - - def count_and_perc(self, cc: Cc, i: int) -> tuple[str, str]: - count = f"{cc.counts[i]:,d}" # commify + self.missing_cc_str += self.count_and_percs_str(i, ".", "", "") + + # Get the count and perc string for `cc1[i]` and the perc string for + # `cc2[i]`. (Unless `cc2` is `None`, in which case `perc2` will be "".) + def count_and_percs_strs( + self, cc1: Cc, cc2: Cc | None, i: int + ) -> tuple[str, str, str]: + count = f"{cc1[i]:,d}" # commify if args.show_percs: - if cc.counts[i] == 0: - # Don't show percentages for "0" entries, it's just clutter. - perc = "" + summary_count = self.summary_cc[i] + if cc2 is None: + # A plain or inner CC, with a single percentage. + if cc1[i] == 0: + # Don't show percentages for "0" entries, it's just clutter. + perc1 = "" + elif summary_count == 0: + # Avoid dividing by zero. + perc1 = " (n/a)" + else: + perc1 = f" ({cc1[i] * 100 / summary_count:.1f}%)" + perc2 = "" else: - summary_count = self.summary_cc.counts[i] + # An outer CC, with two percentages. if summary_count == 0: - perc = " (n/a)" + # Avoid dividing by zero. + perc1 = " (n/a," + perc2 = " n/a)" else: - p = cc.counts[i] * 100 / summary_count - perc = f" ({p:.1f}%)" + perc1 = f" ({cc1[i] * 100 / summary_count:.1f}%," + perc2 = f" {cc2[i] * 100 / summary_count:.1f}%)" else: - perc = "" + perc1 = "" + perc2 = "" + + return (count, perc1, perc2) - return (count, perc) + def count_and_percs_str(self, i: int, count: str, perc1: str, perc2: str) -> str: + event_w = len(self.events.events[i]) + count_w = self.widths[i].count + perc1_w = self.widths[i].perc1 + perc2_w = self.widths[i].perc2 + pre_w = max(0, event_w - count_w - perc1_w - perc2_w) + return f"{'':>{pre_w}}{count:>{count_w}}{perc1:>{perc1_w}}{perc2:>{perc2_w}} " - def print_cc(self, cc: Cc, suffix: str) -> None: + def print_events(self, suffix: str) -> None: + print(self.events_prefix, end="") for i in self.events.show_indices: - (count, perc) = self.count_and_perc(cc, i) - self.print_count_and_perc(i, count, perc) + event = self.events.events[i] + event_w = len(event) + count_w = self.widths[i].count + perc1_w = self.widths[i].perc1 + perc2_w = self.widths[i].perc2 + print(f"{event:_<{max(event_w, count_w + perc1_w + perc2_w)}} ", end="") - print("", suffix) + print(suffix) - def print_missing_cc(self, suffix: str) -> None: - # Don't show percentages for "." entries, it's just clutter. + def print_lcc(self, lcc: Lcc, outer_name: str, cumul_cc: Cc) -> None: + print("> ", end="") + if ( + len(lcc.inner_list_name_cc) == 1 + and lcc.outer_cc == lcc.inner_list_name_cc[0][1] + ): + # There is only one inner CC, it met the threshold, and it is equal + # to the outer CC. Print the inner CC and outer CC in a single + # line, because they are the same. + inner_name = lcc.inner_list_name_cc[0][0] + self.print_cc(lcc.outer_cc, cumul_cc, f"{outer_name}:{inner_name}") + else: + # There are multiple inner CCs, and at least one met the threshold. + # Print the outer CC and then the inner CCs, indented. + self.print_cc(lcc.outer_cc, cumul_cc, f"{outer_name}:") + for inner_name, inner_cc in lcc.inner_list_name_cc: + print(" ", end="") + self.print_cc(inner_cc, None, f" {inner_name}") + print() + + # If `cc2` is `None`, it's a vanilla CC or inner CC. Otherwise, it's an + # outer CC. + def print_cc(self, cc: Cc, cc2: Cc | None, suffix: str) -> None: for i in self.events.show_indices: - self.print_count_and_perc(i, ".", "") + (count, perc1, perc2) = self.count_and_percs_strs(cc, cc2, i) + print(self.count_and_percs_str(i, count, perc1, perc2), end="") print("", suffix) + def print_missing_cc(self, suffix: str) -> None: + print(self.missing_cc_str, suffix) + # Used in various places in the output. def print_fancy(text: str) -> None: @@ -508,8 +654,8 @@ def print_fancy(text: str) -> None: print(fancy) -def print_cachegrind_profile(desc: str, cmd: str, events: Events) -> None: - print_fancy("Cachegrind profile") +def print_metadata(desc: str, cmd: str, events: Events) -> None: + print_fancy("Metadata") print(desc, end="") print("Command: ", cmd) print("Data file: ", args.cgout_filename[0]) @@ -530,53 +676,79 @@ def print_cachegrind_profile(desc: str, cmd: str, events: Events) -> None: def print_summary(events: Events, summary_cc: Cc) -> None: - printer = CcPrinter(events, [summary_cc], summary_cc) + printer = CcPrinter(events, summary_cc) + printer.init_ccs([summary_cc]) print_fancy("Summary") printer.print_events("") print() - printer.print_cc(summary_cc, "PROGRAM TOTALS") + printer.print_cc(summary_cc, None, "PROGRAM TOTALS") print() -def print_function_summary( - events: Events, dict_flfn_cc: DictFlfnCc, summary_cc: Cc +def print_name_summary( + kind: str, events: Events, dict_name_dcc: DictNameDcc, summary_cc: Cc ) -> set[str]: - # Only the first threshold percentage is actually used. + # The primary sort event is used for the threshold. threshold_index = events.sort_indices[0] # Convert the threshold from a percentage to an event count. - threshold = args.threshold * abs(summary_cc.counts[threshold_index]) / 100 - - def meets_threshold(flfn_and_cc: tuple[Flfn, Cc]) -> bool: - cc = flfn_and_cc[1] - return abs(cc.counts[threshold_index]) >= threshold - - # Create a list with the counts in sort order, so that left-to-right list - # comparison does the right thing. Plus the `Flfn` at the end for - # deterministic output when all the event counts are identical in two CCs. - def key(flfn_and_cc: tuple[Flfn, Cc]) -> tuple[list[int], Flfn]: - cc = flfn_and_cc[1] - return ([abs(cc.counts[i]) for i in events.sort_indices], flfn_and_cc[0]) - - # Filter out functions for which the primary sort event count is below the - # threshold, and sort the remainder. - filtered_flfns_and_ccs = filter(meets_threshold, dict_flfn_cc.items()) - sorted_flfns_and_ccs = sorted(filtered_flfns_and_ccs, key=key, reverse=True) - sorted_ccs = list(map(lambda flfn_and_cc: flfn_and_cc[1], sorted_flfns_and_ccs)) - - printer = CcPrinter(events, sorted_ccs, summary_cc) - print_fancy("Function summary") - printer.print_events(" file:function") - print() + threshold = args.threshold * abs(summary_cc[threshold_index]) / 100 + + def meets_threshold(name_and_cc: tuple[str, Cc]) -> bool: + cc = name_and_cc[1] + return abs(cc[threshold_index]) >= threshold + + # Create a list with the outer CC counts in sort order, so that + # left-to-right list comparison does the right thing. Plus the outer name + # at the end for deterministic output when all the event counts are + # identical in two CCs. + def key_name_and_lcc(name_and_lcc: tuple[str, Lcc]) -> tuple[list[int], str]: + (outer_name, lcc) = name_and_lcc + return ( + [abs(lcc.outer_cc[i]) for i in events.sort_indices], + outer_name, + ) + + # Similar to `key_name_and_lcc`. + def key_name_and_cc(name_and_cc: tuple[str, Cc]) -> tuple[list[int], str]: + (name, cc) = name_and_cc + return ([abs(cc[i]) for i in events.sort_indices], name) + + # This is a `filter_map` operation, which Python doesn't directly support. + list_name_lcc: ListNameLcc = [] + for outer_name, dcc in dict_name_dcc.items(): + # Filter out inner CCs for which the primary sort event count is below the + # threshold, and sort the remainder. + inner_list_name_cc = sorted( + filter(meets_threshold, dcc.inner_dict_name_cc.items()), + key=key_name_and_cc, + reverse=True, + ) + + # If no inner CCs meet the threshold, ignore the entire DCC, even if + # the outer CC meets the threshold. + if len(inner_list_name_cc) == 0: + continue - # Print per-function counts. - for flfn, flfn_cc in sorted_flfns_and_ccs: - printer.print_cc(flfn_cc, f"{flfn[0]}:{flfn[1]}") + list_name_lcc.append((outer_name, Lcc(dcc.outer_cc, inner_list_name_cc))) + list_name_lcc = sorted(list_name_lcc, key=key_name_and_lcc, reverse=True) + + printer = CcPrinter(events, summary_cc) + printer.init_list_name_lcc(list_name_lcc) + print_fancy(kind + " summary") + printer.print_events(" " + kind.lower()) print() - # Files containing a function that met the threshold. - return set(flfn_and_cc[0][0] for flfn_and_cc in sorted_flfns_and_ccs) + # Print LCCs. + threshold_names = set([]) + cumul_cc = events.mk_empty_cc() + for name, lcc in list_name_lcc: + add_cc_to_cc(lcc.outer_cc, cumul_cc) + printer.print_lcc(lcc, name, cumul_cc) + threshold_names.add(name) + + return threshold_names class AnnotatedCcs: @@ -647,7 +819,8 @@ def print_annotated_src_file( if os.stat(src_file.name).st_mtime_ns > os.stat(args.cgout_filename[0]).st_mtime_ns: warn_src_file_is_newer(src_file.name, args.cgout_filename[0]) - printer = CcPrinter(events, list(dict_line_cc.values()), summary_cc) + printer = CcPrinter(events, summary_cc) + printer.init_ccs(list(dict_line_cc.values())) # The starting fancy has already been printed by the caller. printer.print_events("") print() @@ -658,8 +831,8 @@ def print_annotated_src_file( line0_cc = dict_line_cc.pop(0, None) if line0_cc: suffix = "<unknown (line 0)>" - printer.print_cc(line0_cc, suffix) - annotated_ccs.line_nums_unknown_cc += line0_cc + printer.print_cc(line0_cc, None, suffix) + add_cc_to_cc(line0_cc, annotated_ccs.line_nums_unknown_cc) print() # Find interesting line ranges: all lines with a CC, and all lines within @@ -698,8 +871,10 @@ def print_annotated_src_file( if not src_line: return # EOF if line_nums and line_num == line_nums[0]: - printer.print_cc(dict_line_cc[line_num], src_line[:-1]) - annotated_ccs.line_nums_known_cc += dict_line_cc[line_num] + printer.print_cc(dict_line_cc[line_num], None, src_line[:-1]) + add_cc_to_cc( + dict_line_cc[line_num], annotated_ccs.line_nums_known_cc + ) del line_nums[0] else: printer.print_missing_cc(src_line[:-1]) @@ -715,8 +890,10 @@ def print_annotated_src_file( if line_nums: print() for line_num in line_nums: - printer.print_cc(dict_line_cc[line_num], f"<bogus line {line_num}>") - annotated_ccs.line_nums_known_cc += dict_line_cc[line_num] + printer.print_cc( + dict_line_cc[line_num], None, f"<bogus line {line_num}>" + ) + add_cc_to_cc(dict_line_cc[line_num], annotated_ccs.line_nums_known_cc) print() warn_bogus_lines(src_file.name) @@ -736,7 +913,7 @@ def print_annotated_src_files( def add_dict_line_cc_to_cc(dict_line_cc: DictLineCc | None, accum_cc: Cc) -> None: if dict_line_cc: for line_cc in dict_line_cc.values(): - accum_cc += line_cc + add_cc_to_cc(line_cc, accum_cc) # Exclude the unknown ("???") file, which is unannotatable. ann_src_filenames.discard("???") @@ -771,7 +948,6 @@ def print_annotated_src_files( annotated_ccs, summary_cc, ) - readable = True break except OSError: @@ -799,15 +975,16 @@ def print_annotation_summary( summary_cc: Cc, ) -> None: # Show how many events were covered by annotated lines above. - printer = CcPrinter(events, annotated_ccs.ccs(), summary_cc) + printer = CcPrinter(events, summary_cc) + printer.init_ccs(annotated_ccs.ccs()) print_fancy("Annotation summary") printer.print_events("") print() total_cc = events.mk_empty_cc() for (cc, label) in zip(annotated_ccs.ccs(), AnnotatedCcs.labels): - printer.print_cc(cc, label) - total_cc += cc + printer.print_cc(cc, None, label) + add_cc_to_cc(cc, total_cc) print() @@ -826,19 +1003,19 @@ def main() -> None: desc, cmd, events, - dict_flfn_cc, + dict_fl_dcc, + dict_fn_dcc, dict_fl_dict_line_cc, summary_cc, ) = read_cgout_file() # Each of the following calls prints a section of the output. - - print_cachegrind_profile(desc, cmd, events) - + print_metadata(desc, cmd, events) print_summary(events, summary_cc) - - ann_src_filenames = print_function_summary(events, dict_flfn_cc, summary_cc) - + ann_src_filenames = print_name_summary( + "File:function", events, dict_fl_dcc, summary_cc + ) + print_name_summary("Function:file", events, dict_fn_dcc, summary_cc) if args.annotate: annotated_ccs = print_annotated_src_files( events, ann_src_filenames, dict_fl_dict_line_cc, summary_cc diff --git a/cachegrind/cg_diff.in b/cachegrind/cg_diff.in index bae0c7abe4..38910f31b1 100755 --- a/cachegrind/cg_diff.in +++ b/cachegrind/cg_diff.in @@ -26,10 +26,8 @@ # # The GNU General Public License is contained in the file COPYING. -""" -This script diffs Cachegrind output files. -""" - +# This script diffs Cachegrind output files. +# # Use `make pydiff` to "build" this script every time it is changed. This runs # the formatters, type-checkers, and linters on `cg_diff.in` and then generates # `cg_diff`. @@ -47,13 +45,9 @@ from typing import Callable, DefaultDict, NewType, NoReturn SearchAndReplace = Callable[[str], str] +# A typed wrapper for parsed args. class Args(Namespace): - """ - A typed wrapper for parsed args. - - None of these fields are modified after arg parsing finishes. - """ - + # None of these fields are modified after arg parsing finishes. mod_filename: SearchAndReplace mod_funcname: SearchAndReplace cgout_filename1: str @@ -146,10 +140,10 @@ class Events: self.events = text.split() self.num_events = len(self.events) - def mk_cc(self, text: str) -> Cc: - """Raises a `ValueError` exception on syntax error.""" + # Raises a `ValueError` exception on syntax error. + def mk_cc(self, str_counts: list[str]) -> Cc: # This is slightly faster than a list comprehension. - counts = list(map(int, text.split())) + counts = list(map(int, str_counts)) if len(counts) == self.num_events: pass @@ -159,46 +153,31 @@ class Events: else: raise ValueError - return Cc(counts) + return counts def mk_empty_cc(self) -> Cc: # This is much faster than a list comprehension. - return Cc([0] * self.num_events) - - -class Cc: - """ - This is a dumb container for counts. - - It doesn't know anything about events, i.e. what each count means. It can - do basic operations like `__iadd__` and `__eq__`, and anything more must be - done elsewhere. `Events.mk_cc` and `Events.mk_empty_cc` are used for - construction. - """ + return [0] * self.num_events - # Always the same length as `Events.events`. - counts: list[int] - def __init__(self, counts: list[int]) -> None: - self.counts = counts - - def __repr__(self) -> str: - return str(self.counts) +# A "cost centre", which is a dumb container for counts. Always the same length +# as `Events.events`, but it doesn't even know event names. `Events.mk_cc` and +# `Events.mk_empty_cc` are used for construction. +# +# This used to be a class with a single field `counts: list[int]`, but this +# type is very hot and just using a type alias is much faster. +Cc = list[int] - def __eq__(self, other: object) -> bool: - if not isinstance(other, Cc): - return NotImplemented - return self.counts == other.counts +# Add the counts in `a_cc` to `b_cc`. +def add_cc_to_cc(a_cc: Cc, b_cc: Cc) -> None: + for i, a_count in enumerate(a_cc): + b_cc[i] += a_count - def __iadd__(self, other: Cc) -> Cc: - for i, other_count in enumerate(other.counts): - self.counts[i] += other_count - return self - def __isub__(self, other: Cc) -> Cc: - for i, other_count in enumerate(other.counts): - self.counts[i] -= other_count - return self +# Subtract the counts in `a_cc` from `b_cc`. +def sub_cc_from_cc(a_cc: Cc, b_cc: Cc) -> None: + for i, a_count in enumerate(a_cc): + b_cc[i] -= a_count # A paired filename and function name. @@ -252,33 +231,28 @@ def read_cgout_file(cgout_filename: str) -> tuple[str, Events, DictFlfnCc, Cc]: else: parse_die("missing an `events:` line") - curr_fl = "" - curr_flfn = Flfn(("", "")) + fl = "" + flfn = Flfn(("", "")) # Different places where we accumulate CC data. dict_flfn_cc: DictFlfnCc = defaultdict(events.mk_empty_cc) summary_cc = None - # Compile the one hot regex. - count_pat = re.compile(r"(\d+)\s+(.*)") - # Line matching is done in order of pattern frequency, for speed. - while True: - line = readline() - - if m := count_pat.match(line): - # The line_num isn't used. + while line := readline(): + if line[0].isdigit(): + split_line = line.split() try: - cc = events.mk_cc(m.group(2)) + # The line_num isn't used. + cc = events.mk_cc(split_line[1:]) except ValueError: parse_die("malformed or too many event counts") # Record this CC at the function level. - flfn_cc = dict_flfn_cc[curr_flfn] - flfn_cc += cc + add_cc_to_cc(cc, dict_flfn_cc[flfn]) elif line.startswith("fn="): - curr_flfn = Flfn((curr_fl, args.mod_funcname(line[3:-1]))) + flfn = Flfn((fl, args.mod_funcname(line[3:-1]))) elif line.startswith("fl="): # A longstanding bug: the use of `--mod-filename` makes it @@ -287,18 +261,15 @@ def read_cgout_file(cgout_filename: str) -> tuple[str, Events, DictFlfnCc, Cc]: # diffs anyway. It just means we get "This file was unreadable" # for modified filenames rather than a single "<unknown (line # 0)>" CC. - curr_fl = args.mod_filename(line[3:-1]) + fl = args.mod_filename(line[3:-1]) # A `fn=` line should follow, overwriting the "???". - curr_flfn = Flfn((curr_fl, "???")) + flfn = Flfn((fl, "???")) elif m := re.match(r"summary:\s+(.*)", line): try: - summary_cc = events.mk_cc(m.group(1)) + summary_cc = events.mk_cc(m.group(1).split()) except ValueError: - parse_die("too many event counts") - - elif line == "": - break # EOF + parse_die("malformed or too many event counts") elif line == "\n" or line.startswith("#"): # Skip empty lines and comment lines. @@ -314,7 +285,7 @@ def read_cgout_file(cgout_filename: str) -> tuple[str, Events, DictFlfnCc, Cc]: # Check summary is correct. total_cc = events.mk_empty_cc() for flfn_cc in dict_flfn_cc.values(): - total_cc += flfn_cc + add_cc_to_cc(flfn_cc, total_cc) if summary_cc != total_cc: msg = ( "`summary:` line doesn't match computed total\n" @@ -339,8 +310,8 @@ def main() -> None: # Subtract file 1's CCs from file 2's CCs, at the Flfn level. for flfn, flfn_cc1 in dict_flfn_cc1.items(): flfn_cc2 = dict_flfn_cc2[flfn] - flfn_cc2 -= flfn_cc1 - summary_cc2 -= summary_cc1 + sub_cc_from_cc(flfn_cc1, flfn_cc2) + sub_cc_from_cc(summary_cc1, summary_cc2) print(f"desc: Files compared: {filename1}; {filename2}") print(f"cmd: {cmd1}; {cmd2}") @@ -356,9 +327,9 @@ def main() -> None: # move around. print(f"fl={flfn[0]}") print(f"fn={flfn[1]}") - print("0", *flfn_cc2.counts, sep=" ") + print("0", *flfn_cc2, sep=" ") - print("summary:", *summary_cc2.counts, sep=" ") + print("summary:", *summary_cc2, sep=" ") if __name__ == "__main__": diff --git a/cachegrind/cg_merge.in b/cachegrind/cg_merge.in index fca73439eb..8304e8b279 100755 --- a/cachegrind/cg_merge.in +++ b/cachegrind/cg_merge.in @@ -26,10 +26,8 @@ # # The GNU General Public License is contained in the file COPYING. -""" -This script diffs Cachegrind output files. -""" - +# This script merges Cachegrind output files. +# # Use `make pymerge` to "build" this script every time it is changed. This runs # the formatters, type-checkers, and linters on `cg_merge.in` and then # generates `cg_merge`. @@ -45,13 +43,9 @@ from collections import defaultdict from typing import DefaultDict, NoReturn, TextIO +# A typed wrapper for parsed args. class Args(Namespace): - """ - A typed wrapper for parsed args. - - None of these fields are modified after arg parsing finishes. - """ - + # None of these fields are modified after arg parsing finishes. output: str cgout_filename: list[str] @@ -92,10 +86,10 @@ class Events: self.events = text.split() self.num_events = len(self.events) - def mk_cc(self, text: str) -> Cc: - """Raises a `ValueError` exception on syntax error.""" + # Raises a `ValueError` exception on syntax error. + def mk_cc(self, str_counts: list[str]) -> Cc: # This is slightly faster than a list comprehension. - counts = list(map(int, text.split())) + counts = list(map(int, str_counts)) if len(counts) == self.num_events: pass @@ -105,41 +99,26 @@ class Events: else: raise ValueError - return Cc(counts) + return counts def mk_empty_cc(self) -> Cc: # This is much faster than a list comprehension. - return Cc([0] * self.num_events) - - -class Cc: - """ - This is a dumb container for counts. - - It doesn't know anything about events, i.e. what each count means. It can - do basic operations like `__iadd__` and `__eq__`, and anything more must be - done elsewhere. `Events.mk_cc` and `Events.mk_empty_cc` are used for - construction. - """ + return [0] * self.num_events - # Always the same length as `Events.events`. - counts: list[int] - def __init__(self, counts: list[int]) -> None: - self.counts = counts - - def __repr__(self) -> str: - return str(self.counts) +# A "cost centre", which is a dumb container for counts. Always the same length +# as `Events.events`, but it doesn't even know event names. `Events.mk_cc` and +# `Events.mk_empty_cc` are used for construction. +# +# This used to be a class with a single field `counts: list[int]`, but this +# type is very hot and just using a type alias is much faster. +Cc = list[int] - def __eq__(self, other: object) -> bool: - if not isinstance(other, Cc): - return NotImplemented - return self.counts == other.counts - def __iadd__(self, other: Cc) -> Cc: - for i, other_count in enumerate(other.counts): - self.counts[i] += other_count - return self +# Add the counts in `a_cc` to `b_cc`. +def add_cc_to_cc(a_cc: Cc, b_cc: Cc) -> None: + for i, a_count in enumerate(a_cc): + b_cc[i] += a_count # Per-line CCs, organised by filename, function name, and line number. @@ -205,8 +184,8 @@ def read_cgout_file( summary_cc_present = False - curr_fl = "" - curr_fn = "" + fl = "" + fn = "" # The `cumul_*` values are passed in by reference and are modified by # this function. But they can't be properly initialized until the @@ -217,43 +196,35 @@ def read_cgout_file( cumul_dict_fl_dict_fn_dict_line_cc.default_factory = ( mk_empty_dict_fn_dict_line_cc ) - cumul_summary_cc.counts = events.mk_empty_cc().counts - - # Compile the one hot regex. - count_pat = re.compile(r"(\d+)\s+(.*)") + cumul_summary_cc.extend(events.mk_empty_cc()) # Line matching is done in order of pattern frequency, for speed. - while True: - line = readline() - - if m := count_pat.match(line): - line_num = int(m.group(1)) + while line := readline(): + if line[0].isdigit(): + split_line = line.split() try: - cc = events.mk_cc(m.group(2)) + line_num = int(split_line[0]) + cc = events.mk_cc(split_line[1:]) except ValueError: parse_die("malformed or too many event counts") # Record this CC at the file/func/line level. - line_cc = cumul_dict_fl_dict_fn_dict_line_cc[curr_fl][curr_fn][line_num] - line_cc += cc + add_cc_to_cc(cc, cumul_dict_fl_dict_fn_dict_line_cc[fl][fn][line_num]) elif line.startswith("fn="): - curr_fn = line[3:-1] + fn = line[3:-1] elif line.startswith("fl="): - curr_fl = line[3:-1] + fl = line[3:-1] # A `fn=` line should follow, overwriting the "???". - curr_fn = "???" + fn = "???" elif m := re.match(r"summary:\s+(.*)", line): summary_cc_present = True try: - cumul_summary_cc += events.mk_cc(m.group(1)) + add_cc_to_cc(events.mk_cc(m.group(1).split()), cumul_summary_cc) except ValueError: - parse_die("too many event counts") - - elif line == "": - break # EOF + parse_die("malformed or too many event counts") elif line == "\n" or line.startswith("#"): # Skip empty lines and comment lines. @@ -283,7 +254,7 @@ def main() -> None: # Different places where we accumulate CC data. Initialized to invalid # states prior to the number of events being known. cumul_dict_fl_dict_fn_dict_line_cc: DictFlDictFnDictLineCc = defaultdict(None) - cumul_summary_cc: Cc = Cc([]) + cumul_summary_cc: Cc = [] for n, filename in enumerate(args.cgout_filename): is_first_file = n == 0 @@ -321,9 +292,9 @@ def main() -> None: for fn, dict_line_cc in dict_fn_dict_line_cc.items(): print(f"fn={fn}", file=f) for line, cc in dict_line_cc.items(): - print(line, *cc.counts, file=f) + print(line, *cc, file=f) - print("summary:", *cumul_summary_cc.counts, sep=" ", file=f) + print("summary:", *cumul_summary_cc, sep=" ", file=f) if args.output: try: diff --git a/cachegrind/tests/ann-diff1.post.exp b/cachegrind/tests/ann-diff1.post.exp index cbb7f8448c..4e13f0c089 100644 --- a/cachegrind/tests/ann-diff1.post.exp +++ b/cachegrind/tests/ann-diff1.post.exp @@ -1,5 +1,5 @@ -------------------------------------------------------------------------------- --- Cachegrind profile +-- Metadata -------------------------------------------------------------------------------- Files compared: ann1.cgout; ann1b.cgout Command: ./a.out; ./a.out @@ -14,28 +14,35 @@ Annotation: on -------------------------------------------------------------------------------- -- Summary -------------------------------------------------------------------------------- -Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw +Ir________________ I1mr ILmr Dr_________________ D1mr DLmr Dw D1mw DLmw 5,000,000 (100.0%) 0 0 -2,000,000 (100.0%) 0 0 0 0 0 PROGRAM TOTALS -------------------------------------------------------------------------------- --- Function summary +-- File:function summary -------------------------------------------------------------------------------- -Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw file:function + Ir________________________ I1mr________ ILmr________ Dr_________________________ D1mr________ DLmr________ Dw__________ D1mw________ DLmw________ file:function -5,000,000 (100.0%) 0 0 -2,000,000 (100.0%) 0 0 0 0 0 a.c:MAIN +> 5,000,000 (100.0%, 100.0%) 0 (n/a, n/a) 0 (n/a, n/a) -2,000,000 (100.0%, 100.0%) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) a.c:MAIN + +-------------------------------------------------------------------------------- +-- Function:file summary +-------------------------------------------------------------------------------- + Ir________________________ I1mr________ ILmr________ Dr_________________________ D1mr________ DLmr________ Dw__________ D1mw________ DLmw________ function:file + +> 5,000,000 (100.0%, 100.0%) 0 (n/a, n/a) 0 (n/a, n/a) -2,000,000 (100.0%, 100.0%) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) 0 (n/a, n/a) MAIN:a.c -------------------------------------------------------------------------------- -- Annotated source file: a.c -------------------------------------------------------------------------------- -Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw +Ir________________ I1mr ILmr Dr_________________ D1mr DLmr Dw D1mw DLmw 5,000,000 (100.0%) 0 0 -2,000,000 (100.0%) 0 0 0 0 0 <unknown (line 0)> -------------------------------------------------------------------------------- -- Annotation summary -------------------------------------------------------------------------------- -Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw +Ir________________ I1mr ILmr Dr_________________ D1mr DLmr Dw D1mw DLmw 0 0 0 0 0 0 0 0 0 annotated: files known & above threshold & readable, line numbers known 5,000,000 (100.0%) 0 0 -2,000,000 (100.0%) 0 0 0 0 0 annotated: files known & above threshold & readable, line numbers unknown diff --git a/cachegrind/tests/ann-diff2.post.exp b/cachegrind/tests/ann-diff2.post.exp index 4c1c46d7bb..bcf09ea9ca 100644 --- a/cachegrind/tests/ann-diff2.post.exp +++ b/cachegrind/tests/ann-diff2.post.exp @@ -1,5 +1,5 @@ -------------------------------------------------------------------------------- --- Cachegrind profile +-- Metadata -------------------------------------------------------------------------------- Files compared: ann-diff2a.cgout; ann-diff2b.cgout Command: cmd1; cmd2 @@ -14,18 +14,30 @@ Annotation: on -------------------------------------------------------------------------------- -- Summary -------------------------------------------------------------------------------- -One Two +One___________ Two___________ 2,100 (100.0%) 1,900 (100.0%) PROGRAM TOTALS -------------------------------------------------------------------------------- --- Function summary +-- File:function summary -------------------------------------------------------------------------------- -One Two file:function + One___________________ Two___________________ file:function -1,000 (47.6%) 1,000 (52.6%) aux/ann-diff2-basic.rs:groffN -1,000 (47.6%) 1,000 (52.6%) aux/ann-diff2-basic.rs:fN_ffN_fooN_F4_g5 - 100 (4.8%) -100 (-5.3%) aux/ann-diff2-basic.rs:basic1 +> 2,100 (100.0%, 100.0%) 1,900 (100.0%, 100.0%) aux/ann-diff2-basic.rs: + 1,000 (47.6%) 1,000 (52.6%) groffN + 1,000 (47.6%) 1,000 (52.6%) fN_ffN_fooN_F4_g5 + 100 (4.8%) -100 (-5.3%) basic1 + +-------------------------------------------------------------------------------- +-- Function:file summary +-------------------------------------------------------------------------------- + One__________________ Two__________________ function:file + +> 1,000 (47.6%, 47.6%) 1,000 (52.6%, 52.6%) groffN:aux/ann-diff2-basic.rs + +> 1,000 (47.6%, 95.2%) 1,000 (52.6%, 105.3%) fN_ffN_fooN_F4_g5:aux/ann-diff2-basic.rs + +> 100 (4.8%, 100.0%) -100 (-5.3%, 100.0%) basic1:aux/ann-diff2-basic.rs -------------------------------------------------------------------------------- -- Annotated source file: aux/ann-diff2-basic.rs @@ -35,7 +47,7 @@ This file was unreadable -------------------------------------------------------------------------------- -- Annotation summary -------------------------------------------------------------------------------- -One Two +One___________ Two___________ 0 0 annotated: files known & above threshold & readable, line numbers known 0 0 annotated: files known & above threshold & readable, line numbers unknown diff --git a/cachegrind/tests/ann-merge1.post.exp b/cachegrind/tests/ann-merge1.post.exp index 1d133d8d7d..f12f1c235d 100644 --- a/cachegrind/tests/ann-merge1.post.exp +++ b/cachegrind/tests/ann-merge1.post.exp @@ -1,13 +1,13 @@ -------------------------------------------------------------------------------- --- Cachegrind profile +-- Metadata -------------------------------------------------------------------------------- Description 1a Description 1b Command: Command 1 Data file: ann-merge1c.cgout -Events recorded: A B C -Events shown: A B C -Event sort order: A B C +Events recorded: A +Events shown: A +Event sort order: A Threshold: 0.1 Include dirs: Annotation: on @@ -15,51 +15,66 @@ Annotation: on -------------------------------------------------------------------------------- -- Summary -------------------------------------------------------------------------------- -A B C +A__________ -86 (100.0%) 113 (100.0%) 145 (100.0%) PROGRAM TOTALS +86 (100.0%) PROGRAM TOTALS -------------------------------------------------------------------------------- --- Function summary +-- File:function summary -------------------------------------------------------------------------------- -A B C file:function + A_________________ file:function -40 (46.5%) 80 (70.8%) 120 (82.8%) ann-merge-x.rs:x1 -20 (23.3%) 10 (8.8%) 5 (3.4%) ann-merge-x.rs:x3 -16 (18.6%) 18 (15.9%) 20 (13.8%) ann-merge-y.rs:y1 -10 (11.6%) 5 (4.4%) 0 ann-merge-x.rs:x2 +> 70 (81.4%, 81.4%) ann-merge-x.rs: + 40 (46.5%) x1 + 20 (23.3%) x3 + 10 (11.6%) x2 + +> 16 (18.6%, 100.0%) ann-merge-y.rs:y1 + +-------------------------------------------------------------------------------- +-- Function:file summary +-------------------------------------------------------------------------------- + A_________________ function:file + +> 40 (46.5%, 46.5%) x1:ann-merge-x.rs + +> 20 (23.3%, 69.8%) x3:ann-merge-x.rs + +> 16 (18.6%, 88.4%) y1:ann-merge-y.rs + +> 10 (11.6%, 100.0%) x2:ann-merge-x.rs -------------------------------------------------------------------------------- -- Annotated source file: ann-merge-x.rs -------------------------------------------------------------------------------- -A B C +A_________ -20 (23.3%) 40 (35.4%) 60 (41.4%) one -10 (11.6%) 20 (17.7%) 30 (20.7%) two -10 (11.6... [truncated message content] |
|
From: Paul F. <pa...@so...> - 2023-04-10 08:34:34
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=ab6d3928a5bf6163470e4ea649b68178f8875b94 commit ab6d3928a5bf6163470e4ea649b68178f8875b94 Author: Paul Floyd <pj...@wa...> Date: Mon Apr 10 10:28:58 2023 +0200 regtest: warning cleanup All for clang and mostly Apple clang There are still numerous deprecated warnings on macOS 10.13 (sem* functions, syscall, sbrk, i386, PIEi, OSSpinLocki, swapcontext, getcontext) Diff: --- configure.ac | 4 ++++ massif/tests/Makefile.am | 2 +- memcheck/tests/Makefile.am | 8 +++++--- memcheck/tests/memalign_args.c | 4 ++-- memcheck/tests/realloc_size_zero.c | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index ea6d3991a8..a8d9b95138 100755 --- a/configure.ac +++ b/configure.ac @@ -2572,6 +2572,10 @@ AC_GCC_WARNING_SUBST_NO([stringop-truncation], [FLAG_W_NO_STRINGOP_TRUNCATION]) AC_GCC_WARNING_SUBST_NO([format-overflow], [FLAG_W_NO_FORMAT_OVERFLOW]) AC_GCC_WARNING_SUBST_NO([use-after-free], [FLAG_W_NO_USE_AFTER_FREE]) AC_GCC_WARNING_SUBST_NO([free-nonheap-object], [FLAG_W_NO_FREE_NONHEAP_OBJECT]) +AC_GCC_WARNING_SUBST_NO([fortify-source], [FLAG_W_NO_FORTIFY_SOURCE]) +AC_GCC_WARNING_SUBST_NO([builtin-memcpy-chk-size], [FLAG_W_NO_BUILTIN_MEMCPY_CHK_SIZE]) +AC_GCC_WARNING_SUBST_NO([incompatible-pointer-types-discards-qualifiers], [FLAG_W_NO_INCOMPATIBLE_POINTER_TYPES_DISCARDS_QUALIFIERS]) +AC_GCC_WARNING_SUBST_NO([suspicious-bzero], [FLAG_W_NO_SUSPICIOUS_BZERO]) AC_GCC_WARNING_SUBST_NO_VAL([alloc-size-larger-than], [1677216], [FLAG_W_NO_ALLOC_SIZE_LARGER_THAN]) diff --git a/massif/tests/Makefile.am b/massif/tests/Makefile.am index 5b3d1938f4..84c9b1273a 100644 --- a/massif/tests/Makefile.am +++ b/massif/tests/Makefile.am @@ -104,4 +104,4 @@ insig_CFLAGS = $(AM_CFLAGS) -Wno-unused-result long_names_CFLAGS = $(AM_CFLAGS) -Wno-unused-result one_CFLAGS = $(AM_CFLAGS) -Wno-unused-result thresholds_CFLAGS = $(AM_CFLAGS) -Wno-unused-result -realloc_CFLAGS = $(AM_CFLAGS) -Wno-free-nonheap-object +realloc_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_FREE_NONHEAP_OBJECT@ diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 7bfedb3058..b4d80f837a 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -645,12 +645,13 @@ origin6_fp_CFLAGS = $(AM_CFLAGS) -O # Don't allow GCC to inline memcpy() and strcpy(), # because then we can't intercept it overlap_CFLAGS = $(AM_CFLAGS) -fno-builtin-memcpy -fno-builtin-strcpy \ - -Wno-fortify-source @FLAG_W_NO_STRINGOP_OVERFLOW@ + @FLAG_W_NO_FORTIFY_SOURCE@ @FLAG_W_NO_STRINGOP_OVERFLOW@ @FLAG_W_NO_BUILTIN_MEMCPY_CHK_SIZE@ partial_load_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_USE_AFTER_FREE@ reach_thread_register_CFLAGS = $(AM_CFLAGS) -O2 reach_thread_register_LDADD = -lpthread +realloc_size_zero_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_INCOMPATIBLE_POINTER_TYPES_DISCARDS_QUALIFIERS@ realloc_size_zero_mismatch_SOURCES = realloc_size_zero_mismatch.cpp resvn_stack_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_UNINITIALIZED@ @@ -668,8 +669,9 @@ endif str_tester_CFLAGS = $(AM_CFLAGS) -Wno-shadow @FLAG_W_NO_STRINGOP_OVERFLOW@ \ @FLAG_W_NO_STRINGOP_TRUNCATION@ \ - -Wno-fortify-source -Wno-suspicious-bzero \ - @FLAG_W_NO_MEMSET_TRANSPOSED_ARGS@ @FLAG_W_NO_STRINGOP_OVERREAD@ + @FLAG_W_NO_FORTIFY_SOURCE@ @FLAG_W_NO_SUSPICIOUS_BZERO@ \ + @FLAG_W_NO_MEMSET_TRANSPOSED_ARGS@ @FLAG_W_NO_STRINGOP_OVERREAD@ \ + @FLAG_W_NO_BUILTIN_MEMCPY_CHK_SIZE@ supp_CFLAGS = $(AM_CFLAGS) @FLAG_W_NO_UNINITIALIZED@ diff --git a/memcheck/tests/memalign_args.c b/memcheck/tests/memalign_args.c index 9c774d61ee..de765bcc2b 100644 --- a/memcheck/tests/memalign_args.c +++ b/memcheck/tests/memalign_args.c @@ -12,7 +12,7 @@ int main(void) size_t align = 64U; char *mem; char *p; - int res; + (void)VALGRIND_MAKE_MEM_UNDEFINED(&size, sizeof(size)); (void)VALGRIND_MAKE_MEM_UNDEFINED(&align, sizeof(align)); #if !defined(VGO_darwin) @@ -20,7 +20,7 @@ int main(void) free(p); #endif - res = posix_memalign((void **)&mem,align,size); + (void)posix_memalign((void **)&mem,align,size); free(mem); #if !defined(VGO_darwin) diff --git a/memcheck/tests/realloc_size_zero.c b/memcheck/tests/realloc_size_zero.c index afe2a76680..c9d8e74777 100644 --- a/memcheck/tests/realloc_size_zero.c +++ b/memcheck/tests/realloc_size_zero.c @@ -4,7 +4,7 @@ int main(void) { - int i; + char* p = malloc(1024); p[0] = '\0'; errno = 0; |
|
From: Nicholas N. <nj...@so...> - 2023-04-05 23:30:10
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=81c7be88b255f51c76304abb79c06e1b5c594d0f commit 81c7be88b255f51c76304abb79c06e1b5c594d0f Author: Nicholas Nethercote <n.n...@gm...> Date: Thu Apr 6 09:25:15 2023 +1000 Improve `pylintrc`. - Move it to `auxprogs/`, alongside `pybuild.sh`. - Disable the annoying design lints, instead of just modifying the values (which often requires modifying them again later). Diff: --- auxprogs/pybuild.sh | 7 ++++--- auxprogs/pylintrc | 28 +++++++++++++++++++++++++ cachegrind/Makefile.am | 6 +++--- cachegrind/cg_annotate.in | 4 ++-- cachegrind/pylintrc | 53 ----------------------------------------------- 5 files changed, 37 insertions(+), 61 deletions(-) diff --git a/auxprogs/pybuild.sh b/auxprogs/pybuild.sh index 432a768c51..371189a68f 100755 --- a/auxprogs/pybuild.sh +++ b/auxprogs/pybuild.sh @@ -44,8 +44,9 @@ set -e ver=3.9 pyver=py39 -infile=$1 -outfile=$2 +auxprogs=$1 +infile=$2 +outfile=$3 if [ -z "$outfile" ] ; then exit 1 fi @@ -80,7 +81,7 @@ ruff check --target-version $pyver $infile echo echo "== pylint ==" -pylint --py-version $ver $infile +pylint --rcfile=$auxprogs/pylintrc --py-version $ver $infile echo "== config.status ==" make $outfile diff --git a/auxprogs/pylintrc b/auxprogs/pylintrc new file mode 100644 index 0000000000..8f51d2d686 --- /dev/null +++ b/auxprogs/pylintrc @@ -0,0 +1,28 @@ +# How to create this file. +# - Generate with `pylint --generate-rcfile > pylintrc`. Do this in a directory +# that doesn't already contain a `pylintrc` file, because the output is +# affected by any existing `pylintrc` file. +# - Then modify entries resulting in unreasonable warnings. +# - If a lint is never interesting, add it to the `disable=` list with an +# explanatory comment. +# - If a lint is interesting but needs modification, comment out the original +# value, add a new value along with an explanatory comment. +# - Remove all non-modified entries. + +[MESSAGES CONTROL] + +disable= + # We don't care about having docstrings for all functions/classes. + missing-class-docstring, missing-function-docstring, + # We don't care about large functions, sometimes it's necessary. + too-many-branches, too-many-locals, too-many-statements, + # Zero or one public methods in a class is fine. + too-few-public-methods, + +[BASIC] + +# Good variable names regexes, separated by a comma. If names match any regex, +# they will always be accepted +#good-names-rgxs= +# We allow any lower-case variable name of length 1 or 2. +good-names-rgxs=\b[a-z]\b,\b[a-z][a-z0-9]\b diff --git a/cachegrind/Makefile.am b/cachegrind/Makefile.am index cbaa90522b..0e0ef12b4d 100644 --- a/cachegrind/Makefile.am +++ b/cachegrind/Makefile.am @@ -76,14 +76,14 @@ endif # "Build" `cg_annotate`. The `+` avoids warnings about the jobserver. pyann: - +../auxprogs/pybuild.sh cg_annotate.in cg_annotate + +../auxprogs/pybuild.sh ../auxprogs cg_annotate.in cg_annotate # "Build" `cg_diff`. The `+` avoids warnings about the jobserver. pydiff: - +../auxprogs/pybuild.sh cg_diff.in cg_diff + +../auxprogs/pybuild.sh ../auxprogs cg_diff.in cg_diff # "Build" `cg_merge`. The `+` avoids warnings about the jobserver. pymerge: - +../auxprogs/pybuild.sh cg_merge.in cg_merge + +../auxprogs/pybuild.sh ../auxprogs cg_merge.in cg_merge .PHONY: pyann pydiff pymerge diff --git a/cachegrind/cg_annotate.in b/cachegrind/cg_annotate.in index dcc60de2e6..5dad969f6a 100755 --- a/cachegrind/cg_annotate.in +++ b/cachegrind/cg_annotate.in @@ -73,7 +73,7 @@ class Args(Namespace): raise ValueError def add_bool_argument( - p: ArgumentParser, new_name: str, old_name: str, help: str + p: ArgumentParser, new_name: str, old_name: str, help_: str ) -> None: """ Add a bool argument that defaults to true. @@ -92,7 +92,7 @@ class Args(Namespace): new_flag, default=True, action=BooleanOptionalAction, - help=help, + help=help_, ) p.add_argument( f"{old_flag}=yes", diff --git a/cachegrind/pylintrc b/cachegrind/pylintrc deleted file mode 100644 index 4b2cfb34af..0000000000 --- a/cachegrind/pylintrc +++ /dev/null @@ -1,53 +0,0 @@ -# How to create this file. -# - Generate with `pylint --generate-rcfile > pylintrc`. -# - Then modify entries resulting in unreasonable warnings. Comment out the -# original value, and add another comment line explaining the modification. -# - Remove all non-modified entries. - -[BASIC] - -# Minimum line length for functions/classes that require docstrings, shorter -# ones are exempt. -#docstring-min-length=-1 -# We don't care about having docstrings for all functions/classes. -docstring-min-length=1000 - -# Good variable names regexes, separated by a comma. If names match any regex, -# they will always be accepted -#good-names-rgxs= -# We allow any lower-case variable name of length 1 or 2. -good-names-rgxs=\b[a-z]\b,\b[a-z][a-z0-9]\b - -[VARIABLES] - -# List of names allowed to shadow builtins -#allowed-redefined-builtins= -# We use `help` reasonably as an argument. -allowed-redefined-builtins=help - -[DESIGN] - -# Maximum number of arguments for function / method. -#max-args=5 -# We have some large functions. -max-args=7 - -# Maximum number of branch for function / method body. -#max-branches=12 -# We have some large functions. -max-branches=25 - -# Maximum number of locals for function / method body. -#max-locals=15 -# We have some large functions. -max-locals=25 - -# Maximum number of statements in function / method body. -#max-statements=50 -# We have some large functions. -max-statements=65 - -# Minimum number of public methods for a class (see R0903). -#min-public-methods=2 -# We have some useful classes with little more than `__init__`. -min-public-methods=0 |