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
(32) |
Oct
|
Nov
|
Dec
|
|
From: Mark W. <ma...@so...> - 2023-04-20 13:18:30
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=9fcac92ab371d210dc54f49a80146be9d0b0433a commit 9fcac92ab371d210dc54f49a80146be9d0b0433a Author: Mark Wielaard <ma...@kl...> Date: Thu Apr 20 15:04:03 2023 +0200 vgdb --multi: fix various typos, indentation and such (followup) commit 56ccb1e36c4722b56e3e602b986bc45025cb685d missed a few small fixlets: - one more comment at the top describing the three usages of vgdb. - fixed up a few places where tabs were used for indentation (we are not very consistent in that either, after the release we'll look into adopting something like clang-format so you don't have to do all this by hand). - Add a missing newline in coregrind/m_main.c to make none/tests/cmdline2 pass. Diff: --- coregrind/m_main.c | 2 +- coregrind/vgdb.c | 515 +++++++++++++++++++++++++++-------------------------- 2 files changed, 262 insertions(+), 255 deletions(-) diff --git a/coregrind/m_main.c b/coregrind/m_main.c index e19796327d..a857e5afeb 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -278,7 +278,7 @@ static void usage_NORETURN ( int need_help ) " --sym-offsets=yes|no show syms in form 'name+offset'? [no]\n" " --progress-interval=<number> report progress every <number>\n" " CPU seconds [0, meaning disabled]\n" -" --command-line-only=no|yes only use command line options [no]\n" +" --command-line-only=no|yes only use command line options [no]\n\n" " Vex options for all Valgrind tools:\n" " --vex-iropt-verbosity=<0..9> [0]\n" " --vex-iropt-level=<0..2> [2]\n" diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index a3c5f9d88f..6e5e819563 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -49,9 +49,10 @@ #include <sys/time.h> #include <sys/wait.h> -/* vgdb has two usages: +/* vgdb has three usages: 1. relay application between gdb and the gdbserver embedded in valgrind. 2. standalone to send monitor commands to a running valgrind-ified process + 3. multi mode where vgdb uses the GDB extended remote protocol. It is made of a main program which reads arguments. If no arguments are given or only --pid and --vgdb-prefix, then usage 1 is @@ -67,6 +68,12 @@ As a standalone utility, vgdb builds command packets to write to valgrind, sends it and reads the reply. The same two threads are used to write/read. Once all the commands are sent and their replies received, vgdb will exit. + + When --multi is given vgdb communicates with GDB through the extended remote + protocol and will launch valgrind whenever GDB sends the vRun packet, after + which it will function in the first mode, relaying packets between GDB and + the gdbserver embedded in valgrind till that valgrind quits. vgdb will stay + connected to GDB. */ int debuglevel; @@ -710,7 +717,7 @@ getpkt(char *buf, int fromfd, int ackfd) c2 = fromhex(readchar (fromfd)); if (csum == (c1 << 4) + c2) - break; + break; TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", (c1 << 4) + c2, csum, buf); @@ -1003,14 +1010,14 @@ send_packet_start: if (!noackmode) { // Look for '+' or '-'. // We must wait for "+" if !noackmode. - do { - ret = read_one_char(&c); - if (ret <= 0) - return False; - // And if in !noackmode if we get "-" we should resent the packet. - if (c == '-') - goto send_packet_start; - } while (c != '+'); + do { + ret = read_one_char(&c); + if (ret <= 0) + return False; + // And if in !noackmode if we get "-" we should resent the packet. + if (c == '-') + goto send_packet_start; + } while (c != '+'); DEBUG(1, "sent packet to gdb got: %c\n",c); } return True; @@ -1036,36 +1043,36 @@ receive_packet_start: // Found start of packet ('$') while (bufcnt < (PBUFSIZ+1)) { - ret = read_one_char(&c); - if (ret <= 0) - return ret; - if (c == '#') { - if ((ret = read_one_char(&c1)) <= 0 - || (ret = read_one_char(&c2)) <= 0) { - return ret; - } - c1 = fromhex(c1); - c2 = fromhex(c2); - break; - } - buf[bufcnt] = c; - csum += buf[bufcnt]; - bufcnt++; + ret = read_one_char(&c); + if (ret <= 0) + return ret; + if (c == '#') { + if ((ret = read_one_char(&c1)) <= 0 + || (ret = read_one_char(&c2)) <= 0) { + return ret; + } + c1 = fromhex(c1); + c2 = fromhex(c2); + break; + } + buf[bufcnt] = c; + csum += buf[bufcnt]; + bufcnt++; } // Packet complete, add terminator. buf[bufcnt] ='\0'; if (!(csum == (c1 << 4) + c2)) { - TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", - (c1 << 4) + c2, csum, buf); - if (!noackmode) - if (!write_to_gdb ("-", 1)) - return -1; - /* Try again, gdb should resend the packet. */ - bufcnt = 0; - csum = 0; - goto receive_packet_start; + TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", + (c1 << 4) + c2, csum, buf); + if (!noackmode) + if (!write_to_gdb ("-", 1)) + return -1; + /* Try again, gdb should resend the packet. */ + bufcnt = 0; + csum = 0; + goto receive_packet_start; } if (!noackmode) @@ -1315,68 +1322,68 @@ void do_multi_mode(void) #define QSETWORKINGDIR "QSetWorkingDir" #define QTSTATUS "qTStatus" - if (strncmp(QSUPPORTED, buf, strlen(QSUPPORTED)) == 0) { - DEBUG(1, "CASE %s\n", QSUPPORTED); - // And here is our reply. - // XXX error handling? We don't check the arguments. - char *reply; - strcpy(q_buf, buf); - // Keep this in sync with coregrind/m_gdbserver/server.c - asprintf (&reply, - "PacketSize=%x;" - "QStartNoAckMode+;" - "QPassSignals+;" - "QCatchSyscalls+;" - /* Just report support always. */ - "qXfer:auxv:read+;" - /* We'll force --vgdb-shadow-registers=yes */ - "qXfer:features:read+;" - "qXfer:exec-file:read+;" - "qXfer:siginfo:read+;" - /* Some extra's vgdb support before valgrind starts up. */ - "QEnvironmentHexEncoded+;" - "QEnvironmentReset+;" - "QEnvironmentUnset+;" - "QSetWorkingDir+", (UInt)PBUFSIZ - 1); - send_packet(reply, noackmode); - free (reply); - } - else if (strncmp(STARTNOACKMODE, buf, strlen(STARTNOACKMODE)) == 0) { - // We have to ack this one - send_packet("OK", 0); - noackmode = 1; - } - else if (buf[0] == '!') { - send_packet("OK", noackmode); - } - else if (buf[0] == '?') { - send_packet("W00", noackmode); - } - else if (strncmp("H", buf, strlen("H")) == 0) { - // Set thread packet, but we are not running yet. - send_packet("E01", noackmode); - } - else if (strncmp("vMustReplyEmpty", buf, strlen("vMustReplyEmpty")) == 0) { - send_packet ("", noackmode); - } - else if (strncmp(QRCMD, buf, strlen(QRCMD)) == 0) { - send_packet ("No running target, monitor commands not available yet.", noackmode); + if (strncmp(QSUPPORTED, buf, strlen(QSUPPORTED)) == 0) { + DEBUG(1, "CASE %s\n", QSUPPORTED); + // And here is our reply. + // XXX error handling? We don't check the arguments. + char *reply; + strcpy(q_buf, buf); + // Keep this in sync with coregrind/m_gdbserver/server.c + asprintf (&reply, + "PacketSize=%x;" + "QStartNoAckMode+;" + "QPassSignals+;" + "QCatchSyscalls+;" + /* Just report support always. */ + "qXfer:auxv:read+;" + /* We'll force --vgdb-shadow-registers=yes */ + "qXfer:features:read+;" + "qXfer:exec-file:read+;" + "qXfer:siginfo:read+;" + /* Some extra's vgdb support before valgrind starts up. */ + "QEnvironmentHexEncoded+;" + "QEnvironmentReset+;" + "QEnvironmentUnset+;" + "QSetWorkingDir+", (UInt)PBUFSIZ - 1); + send_packet(reply, noackmode); + free (reply); + } + else if (strncmp(STARTNOACKMODE, buf, strlen(STARTNOACKMODE)) == 0) { + // We have to ack this one + send_packet("OK", 0); + noackmode = 1; + } + else if (buf[0] == '!') { + send_packet("OK", noackmode); + } + else if (buf[0] == '?') { + send_packet("W00", noackmode); + } + else if (strncmp("H", buf, strlen("H")) == 0) { + // Set thread packet, but we are not running yet. + send_packet("E01", noackmode); + } + else if (strncmp("vMustReplyEmpty", buf, strlen("vMustReplyEmpty")) == 0) { + send_packet ("", noackmode); + } + else if (strncmp(QRCMD, buf, strlen(QRCMD)) == 0) { + send_packet ("No running target, monitor commands not available yet.", noackmode); - char *decoded_string = decode_hexstring (buf, strlen (QRCMD) + 1, 0); - DEBUG(1, "qRcmd decoded: %s\n", decoded_string); - free (decoded_string); - } - else if (strncmp(VRUN, buf, strlen(VRUN)) == 0) { + char *decoded_string = decode_hexstring (buf, strlen (QRCMD) + 1, 0); + DEBUG(1, "qRcmd decoded: %s\n", decoded_string); + free (decoded_string); + } + else if (strncmp(VRUN, buf, strlen(VRUN)) == 0) { // vRun;filename[;argument]* // vRun, filename and arguments are split on ';', // no ';' at the end. // If there are no arguments count is one (just the filename). // Otherwise it is the number of arguments plus one (the filename). // The filename must be there and starts after the first ';'. - // TODO: Handle vRun;[;argument]* - // https://www.sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets - // If filename is an empty string, the stub may use a default program - // (e.g. the last program run). + // TODO: Handle vRun;[;argument]* + // https://www.sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets + // If filename is an empty string, the stub may use a default program + // (e.g. the last program run). size_t count = count_delims(';', buf); size_t *len = vmalloc(count * sizeof(count)); const char *delim = ";"; @@ -1386,186 +1393,186 @@ void do_multi_mode(void) // Count the lenghts of each substring, init to -1 to compensate for // each substring starting with a delim char. for (int i = 0; i < count; i++) - len[i] = -1; + len[i] = -1; count_len(';', buf, len); if (next_str) { - DEBUG(1, "vRun: next_str %s\n", next_str); - for (int i = 0; i < count; i++) { - /* Handle the case when the arguments - * was specified to gdb's run command - * but no remote exec-file was set, - * so the first vRun argument is missing. - * For example vRun;;6c. */ - if (*next_str == *delim) { - next_str++; - /* empty string that can be freed. */ - decoded_string[i] = strdup(""); - } - else { - decoded_string[i] = decode_hexstring (next_str, 0, len[i]); - if (i < count - 1) - next_str = next_delim_string(next_str, *delim); - } - DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", - decoded_string[i], next_str, i, len[i]); - } - - /* If we didn't get any arguments or the filename is an empty - string, valgrind won't know which program to run. */ - DEBUG (1, "count: %d, len[0]: %d\n", count, len[0]); - if (! count || len[0] == 0) { - free(len); - for (int i = 0; i < count; i++) - free (decoded_string[i]); - free (decoded_string); - send_packet ("E01", noackmode); - continue; - } - - /* We have collected the decoded strings so we can use them to - launch valgrind with the correct arguments... We then use the - valgrind pid to start relaying packets. */ - pid_t valgrind_pid = -1; - int res = fork_and_exec_valgrind (count, - decoded_string, - working_dir, - &valgrind_pid); - - if (res == 0) { - // Lets report we Stopped with SIGTRAP (05). - send_packet ("S05", noackmode); - prepare_fifos_and_shared_mem(valgrind_pid); - DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", - from_gdb_to_pid, to_gdb_from_pid); - // gdb_relay is an endless loop till valgrind quits. - shutting_down = False; - - gdb_relay (valgrind_pid, 1, q_buf); - cleanup_fifos_and_shared_mem(); - DEBUG(1, "valgrind relay done\n"); - int status; - pid_t p = waitpid (valgrind_pid, &status, 0); - DEBUG(2, "waitpid: %d\n", (int) p); - if (p == -1) - DEBUG(1, "waitpid error %s\n", strerror (errno)); - else { - if (WIFEXITED(status)) - DEBUG(1, "valgrind exited with %d\n", - WEXITSTATUS(status)); - else if (WIFSIGNALED(status)) - DEBUG(1, "valgrind kill by signal %d\n", - WTERMSIG(status)); - else - DEBUG(1, "valgrind unexpectedly stopped or continued"); - } - } else { - send_packet ("E01", noackmode); - DEBUG(1, "OOPS! couldn't launch valgrind %s\n", - strerror (res)); - } - - free(len); - for (int i = 0; i < count; i++) + DEBUG(1, "vRun: next_str %s\n", next_str); + for (int i = 0; i < count; i++) { + /* Handle the case when the arguments + * was specified to gdb's run command + * but no remote exec-file was set, + * so the first vRun argument is missing. + * For example vRun;;6c. */ + if (*next_str == *delim) { + next_str++; + /* empty string that can be freed. */ + decoded_string[i] = strdup(""); + } + else { + decoded_string[i] = decode_hexstring (next_str, 0, len[i]); + if (i < count - 1) + next_str = next_delim_string(next_str, *delim); + } + DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", + decoded_string[i], next_str, i, len[i]); + } + + /* If we didn't get any arguments or the filename is an empty + string, valgrind won't know which program to run. */ + DEBUG (1, "count: %d, len[0]: %d\n", count, len[0]); + if (! count || len[0] == 0) { + free(len); + for (int i = 0; i < count; i++) + free (decoded_string[i]); + free (decoded_string); + send_packet ("E01", noackmode); + continue; + } + + /* We have collected the decoded strings so we can use them to + launch valgrind with the correct arguments... We then use the + valgrind pid to start relaying packets. */ + pid_t valgrind_pid = -1; + int res = fork_and_exec_valgrind (count, + decoded_string, + working_dir, + &valgrind_pid); + + if (res == 0) { + // Lets report we Stopped with SIGTRAP (05). + send_packet ("S05", noackmode); + prepare_fifos_and_shared_mem(valgrind_pid); + DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", + from_gdb_to_pid, to_gdb_from_pid); + // gdb_relay is an endless loop till valgrind quits. + shutting_down = False; + + gdb_relay (valgrind_pid, 1, q_buf); + cleanup_fifos_and_shared_mem(); + DEBUG(1, "valgrind relay done\n"); + int status; + pid_t p = waitpid (valgrind_pid, &status, 0); + DEBUG(2, "waitpid: %d\n", (int) p); + if (p == -1) + DEBUG(1, "waitpid error %s\n", strerror (errno)); + else { + if (WIFEXITED(status)) + DEBUG(1, "valgrind exited with %d\n", + WEXITSTATUS(status)); + else if (WIFSIGNALED(status)) + DEBUG(1, "valgrind kill by signal %d\n", + WTERMSIG(status)); + else + DEBUG(1, "valgrind unexpectedly stopped or continued"); + } + } else { + send_packet ("E01", noackmode); + DEBUG(1, "OOPS! couldn't launch valgrind %s\n", + strerror (res)); + } + + free(len); + for (int i = 0; i < count; i++) free (decoded_string[i]); - free (decoded_string); + free (decoded_string); } else { - free(len); - send_packet ("E01", noackmode); - DEBUG(1, "vRun decoding error: no next_string!\n"); - continue; + free(len); + send_packet ("E01", noackmode); + DEBUG(1, "vRun decoding error: no next_string!\n"); + continue; } - } else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) { - send_packet ("1", noackmode); - DEBUG(1, "qAttached sent: '1'\n"); - const char *next_str = next_delim_string(buf, ':'); - if (next_str) { + } else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) { + send_packet ("1", noackmode); + DEBUG(1, "qAttached sent: '1'\n"); + const char *next_str = next_delim_string(buf, ':'); + if (next_str) { char *decoded_string = decode_hexstring (next_str, 0, 0); DEBUG(1, "qAttached decoded: %s, next_str %s\n", decoded_string, next_str); free (decoded_string); - } else { + } else { DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf); continue; - } - } /* Reset the state of environment variables in the remote target - before starting the inferior. In this context, reset means - unsetting all environment variables that were previously set - by the user (i.e., were not initially present in the environment). */ - else if (strncmp(QENVIRONMENTRESET, buf, - strlen(QENVIRONMENTRESET)) == 0) { - send_packet ("OK", noackmode); - // TODO clear all environment strings. We're not using - // environment strings now. But we should. - } else if (strncmp(QENVIRONMENTHEXENCODED, buf, - strlen(QENVIRONMENTHEXENCODED)) == 0) { - send_packet ("OK", noackmode); - if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string)) - break; - // TODO Collect all environment strings and add them to environ - // before launching valgrind. - free (string); - string = NULL; - } else if (strncmp(QENVIRONMENTUNSET, buf, - strlen(QENVIRONMENTUNSET)) == 0) { - send_packet ("OK", noackmode); - if (!split_hexdecode(buf, QENVIRONMENTUNSET, ":", &string)) - break; - // TODO Remove this environment string from the collection. - free (string); - string = NULL; - } else if (strncmp(QSETWORKINGDIR, buf, - strlen(QSETWORKINGDIR)) == 0) { - // Silly, but we can only reply OK, even if the working directory is - // bad. Errors will be reported when we try to execute the actual - // process. - send_packet ("OK", noackmode); - // Free any previously set working_dir - free (working_dir); - working_dir = NULL; - if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { - continue; // We cannot report the error to gdb... - } - DEBUG(1, "set working dir to: %s\n", working_dir); - } else if (strncmp(XFER, buf, strlen(XFER)) == 0) { - char *buf_dup = strdup(buf); - DEBUG(1, "strdup: buf_dup %s\n", buf_dup); - if (buf_dup) { - const char *delim = ":"; - size_t count = count_delims(delim[0], buf); - if (count < 4) { - strsep(&buf_dup, delim); - strsep(&buf_dup, delim); - strsep(&buf_dup, delim); - char *decoded_string = decode_hexstring (buf_dup, 0, 0); - DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); - free (decoded_string); - } - free (buf_dup); + } + } /* Reset the state of environment variables in the remote target + before starting the inferior. In this context, reset means + unsetting all environment variables that were previously set + by the user (i.e., were not initially present in the environment). */ + else if (strncmp(QENVIRONMENTRESET, buf, + strlen(QENVIRONMENTRESET)) == 0) { + send_packet ("OK", noackmode); + // TODO clear all environment strings. We're not using + // environment strings now. But we should. + } else if (strncmp(QENVIRONMENTHEXENCODED, buf, + strlen(QENVIRONMENTHEXENCODED)) == 0) { + send_packet ("OK", noackmode); + if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string)) + break; + // TODO Collect all environment strings and add them to environ + // before launching valgrind. + free (string); + string = NULL; + } else if (strncmp(QENVIRONMENTUNSET, buf, + strlen(QENVIRONMENTUNSET)) == 0) { + send_packet ("OK", noackmode); + if (!split_hexdecode(buf, QENVIRONMENTUNSET, ":", &string)) + break; + // TODO Remove this environment string from the collection. + free (string); + string = NULL; + } else if (strncmp(QSETWORKINGDIR, buf, + strlen(QSETWORKINGDIR)) == 0) { + // Silly, but we can only reply OK, even if the working directory is + // bad. Errors will be reported when we try to execute the actual + // process. + send_packet ("OK", noackmode); + // Free any previously set working_dir + free (working_dir); + working_dir = NULL; + if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { + continue; // We cannot report the error to gdb... + } + DEBUG(1, "set working dir to: %s\n", working_dir); + } else if (strncmp(XFER, buf, strlen(XFER)) == 0) { + char *buf_dup = strdup(buf); + DEBUG(1, "strdup: buf_dup %s\n", buf_dup); + if (buf_dup) { + const char *delim = ":"; + size_t count = count_delims(delim[0], buf); + if (count < 4) { + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + char *decoded_string = decode_hexstring (buf_dup, 0, 0); + DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); + free (decoded_string); + } + free (buf_dup); } else { - DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf); - free (buf_dup); - continue; + DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf); + free (buf_dup); + continue; } // Whether we could decode it or not, we cannot handle it now. We // need valgrind gdbserver to properly reply. So error out here. send_packet ("E00", noackmode); - } else if (strncmp(QTSTATUS, buf, strlen(QTSTATUS)) == 0) { - // We don't support trace experiments - DEBUG(1, "Got QTSTATUS\n"); - send_packet ("", noackmode); - } else if (strcmp("qfThreadInfo", buf) == 0) { - DEBUG(1, "Got qfThreadInfo\n"); - /* There are no threads yet, reply 'l' end of list. */ - send_packet ("l", noackmode); - } else if (buf[0] != '\0') { - // We didn't understand. - DEBUG(1, "Unknown packet received: '%s'\n", buf); - bad_unknown_packets++; - if (bad_unknown_packets > 10) { - DEBUG(1, "Too many bad/unknown packets received\n"); - break; - } - send_packet ("", noackmode); - } + } else if (strncmp(QTSTATUS, buf, strlen(QTSTATUS)) == 0) { + // We don't support trace experiments + DEBUG(1, "Got QTSTATUS\n"); + send_packet ("", noackmode); + } else if (strcmp("qfThreadInfo", buf) == 0) { + DEBUG(1, "Got qfThreadInfo\n"); + /* There are no threads yet, reply 'l' end of list. */ + send_packet ("l", noackmode); + } else if (buf[0] != '\0') { + // We didn't understand. + DEBUG(1, "Unknown packet received: '%s'\n", buf); + bad_unknown_packets++; + if (bad_unknown_packets > 10) { + DEBUG(1, "Too many bad/unknown packets received\n"); + break; + } + send_packet ("", noackmode); + } } DEBUG(1, "done doing multi stuff...\n"); free(working_dir); |
|
From: Mark W. <ma...@kl...> - 2023-04-20 13:02:11
|
Hi Sasha,
Looks like this was in response to Philippe's review. Thanks.
I added one more comment at the top describing the three usages of
vgdb. And fixed up a few places where tabs were used for indentation
(we are not very consistent in that either, after the release we'll
look into adopting something like clang-format so you don't have to do
all this by hand). And a missing newline in coregrind/m_main.c to make
none/tests/cmdline2 pass.
Pushed with the following commit message to show what was changed:
commit a32e26dd072a82aacafcdd22bd7c94c7b4d2afcc
Author: Alexandra Hájková <aha...@re...>
Date: Thu Apr 20 14:17:29 2023 +0200
vgdb --multi: fix various typos, indentation and such
Remove --launched-with-multi from --help-debug output since it is not
a real user option. Do add a comment in m_main.c explaining the
internal usage.
Add a top-level comment describing the three usages of vgdb.
Fix comment description of decode_hexstring, create_packet,
split_hexdecode.
Consistently use 3 space indention in send_packet and receive_packet
and next_delim_string and split_hexdecode, count_delims,
do_multi_mode.
Fix return type of count_delims to size_t.
Add a note in coregrind/m_gdbserver/server.c to sync qSupported
replies with coregrind/vgdb.c.
Use vgdb (all lowercase) and GDB (all caps) consistently in the
manual.
Thanks,
Mark
|
|
From: Mark W. <ma...@so...> - 2023-04-20 13:00:25
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=56ccb1e36c4722b56e3e602b986bc45025cb685d commit 56ccb1e36c4722b56e3e602b986bc45025cb685d Author: Alexandra Hájková <aha...@re...> Date: Thu Apr 20 14:17:29 2023 +0200 vgdb --multi: fix various typos, indentation and such Remove --launched-with-multi from --help-debug output since it is not a real user option. Do add a comment in m_main.c explaining the internal usage. Add a top-level comment describing the three usages of vgdb. Fix comment description of decode_hexstring, create_packet, split_hexdecode. Consistently use 3 space indention in send_packet and receive_packet and next_delim_string and split_hexdecode, count_delims, do_multi_mode. Fix return type of count_delims to size_t. Add a note in coregrind/m_gdbserver/server.c to sync qSupported replies with coregrind/vgdb.c. Use vgdb (all lowercase) and GDB (all caps) consistently in the manual. Diff: --- coregrind/m_gdbserver/server.c | 2 +- coregrind/m_main.c | 4 +- coregrind/vgdb.c | 275 ++++++++++++++++--------------- docs/xml/manual-core-adv.xml | 5 +- none/tests/cmdline2.stdout.exp | 1 - none/tests/cmdline2.stdout.exp-non-linux | 1 - 6 files changed, 146 insertions(+), 142 deletions(-) diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 3c2516086d..83825408ae 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -1105,7 +1105,7 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p) return; } - /* Protocol features query. */ + /* Protocol features query. Keep this in sync with coregind/vgdb.c. */ if (strncmp ("qSupported", arg_own_buf, 10) == 0 && (arg_own_buf[10] == ':' || arg_own_buf[10] == '\0')) { VG_(sprintf) (arg_own_buf, "PacketSize=%x", (UInt)PBUFSIZ - 1); diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 6181046d1e..e19796327d 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -279,8 +279,6 @@ static void usage_NORETURN ( int need_help ) " --progress-interval=<number> report progress every <number>\n" " CPU seconds [0, meaning disabled]\n" " --command-line-only=no|yes only use command line options [no]\n" -" --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]\n" -"\n" " Vex options for all Valgrind tools:\n" " --vex-iropt-verbosity=<0..9> [0]\n" " --vex-iropt-level=<0..2> [2]\n" @@ -563,6 +561,8 @@ static void process_option (Clo_Mode mode, } else if VG_INT_CLOM (cloPD, arg, "--vgdb-poll", VG_(clo_vgdb_poll)) {} else if VG_INT_CLOM (cloPD, arg, "--vgdb-error", VG_(clo_vgdb_error)) {} + /* --launched-with-multi is an internal option used by vgdb to suppress + some output that valgrind normally shows when using --vgdb-error. */ else if VG_BOOL_CLO (arg, "--launched-with-multi", VG_(clo_launched_with_multi)) {} else if VG_USET_CLOM (cloPD, arg, "--vgdb-stop-at", diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index ca673e368d..a3c5f9d88f 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -900,8 +900,7 @@ int tohex (int nib) return 'a' + nib - 10; } -/* Returns an allocated hex-decoded string from the buf starting at offset - off. Will update off to where the buf has been decoded. Stops decoding +/* Returns an allocated hex-decoded string from the buf. Stops decoding at end of buf (zero) or when seeing the delim char. */ static char *decode_hexstring (const char *buf, size_t prefixlen, size_t len) @@ -947,7 +946,7 @@ write_checksum (const char *str) unsigned char csum = 0; int i = 0; while (str[i] != 0) - csum += str[i++]; + csum += str[i++]; char p[2]; p[0] = tohex ((csum >> 4) & 0x0f); @@ -964,7 +963,7 @@ write_reply(const char *reply) return write_checksum (reply); } -/* Creates a packet from a string message, called needs to free. */ +/* Creates a packet from a string message, caller needs to free. */ static char * create_packet(const char *msg) { @@ -995,24 +994,24 @@ static int read_one_char (char *c) static Bool send_packet(const char *reply, int noackmode) { - int ret; - char c; + int ret; + char c; send_packet_start: if (!write_reply(reply)) - return False; - if (!noackmode) { - // Look for '+' or '-'. - // We must wait for "+" if !noackmode. - do { - ret = read_one_char(&c); - if (ret <= 0) - return False; - // And if in !noackmode if we get "-" we should resent the packet. - if (c == '-') - goto send_packet_start; - } while (c != '+'); - DEBUG(1, "sent packet to gdb got: %c\n",c); + return False; + if (!noackmode) { + // Look for '+' or '-'. + // We must wait for "+" if !noackmode. + do { + ret = read_one_char(&c); + if (ret <= 0) + return False; + // And if in !noackmode if we get "-" we should resent the packet. + if (c == '-') + goto send_packet_start; + } while (c != '+'); + DEBUG(1, "sent packet to gdb got: %c\n",c); } return True; } @@ -1023,92 +1022,96 @@ send_packet_start: // or -1 if no packet could be read. static int receive_packet(char *buf, int noackmode) { - int bufcnt = 0, ret; - char c, c1, c2; - unsigned char csum = 0; - - // Look for first '$' (start of packet) or error. - receive_packet_start: - do { - ret = read_one_char(&c); - if (ret <= 0) - return ret; - } while (c != '$'); + int bufcnt = 0, ret; + char c, c1, c2; + unsigned char csum = 0; - // Found start of packet ('$') - while (bufcnt < (PBUFSIZ+1)) { - ret = read_one_char(&c); - if (ret <= 0) - return ret; - if (c == '#') { - if ((ret = read_one_char(&c1)) <= 0 - || (ret = read_one_char(&c2)) <= 0) { - return ret; - } - c1 = fromhex(c1); - c2 = fromhex(c2); - break; + // Look for first '$' (start of packet) or error. +receive_packet_start: + do { + ret = read_one_char(&c); + if (ret <= 0) + return ret; + } while (c != '$'); + + // Found start of packet ('$') + while (bufcnt < (PBUFSIZ+1)) { + ret = read_one_char(&c); + if (ret <= 0) + return ret; + if (c == '#') { + if ((ret = read_one_char(&c1)) <= 0 + || (ret = read_one_char(&c2)) <= 0) { + return ret; } - buf[bufcnt] = c; - csum += buf[bufcnt]; - bufcnt++; - } + c1 = fromhex(c1); + c2 = fromhex(c2); + break; + } + buf[bufcnt] = c; + csum += buf[bufcnt]; + bufcnt++; + } - // Packet complete, add terminator. - buf[bufcnt] ='\0'; - - if (!(csum == (c1 << 4) + c2)) { - TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", - (c1 << 4) + c2, csum, buf); - if (!noackmode) - if (!write_to_gdb ("-", 1)) - return -1; - /* Try again, gdb should resend the packet. */ - bufcnt = 0; - csum = 0; - goto receive_packet_start; - } + // Packet complete, add terminator. + buf[bufcnt] ='\0'; - if (!noackmode) - if (!write_to_gdb ("+", 1)) - return -1; - return bufcnt; + if (!(csum == (c1 << 4) + c2)) { + TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", + (c1 << 4) + c2, csum, buf); + if (!noackmode) + if (!write_to_gdb ("-", 1)) + return -1; + /* Try again, gdb should resend the packet. */ + bufcnt = 0; + csum = 0; + goto receive_packet_start; + } + + if (!noackmode) + if (!write_to_gdb ("+", 1)) + return -1; + return bufcnt; } // Returns a pointer to the char after the next delim char. static const char *next_delim_string (const char *buf, char delim) { - while (*buf) { - if (*buf++ == delim) - break; - } - return buf; + while (*buf) { + if (*buf++ == delim) + break; + } + return buf; } -// Throws away the packet name and decodes the hex string, which is placed in -// decoded_string (the caller owns this and is responsible for freeing it). +/* buf starts with the packet name followed by the delimiter, for example + * vRun;2f62696e2f6c73, ";" is the delimiter here, or + * qXfer:features:read:target.xml:0,1000, where the delimiter is ":". + * The packet name is thrown away and the hex string is decoded and + * is placed in decoded_string (the caller owns this and is responsible + * for freeing it). */ static int split_hexdecode(const char *buf, const char *string, const char *delim, char **decoded_string) { - const char *next_str = next_delim_string(buf, *delim); - if (next_str) { - *decoded_string = decode_hexstring (next_str, 0, 0); - DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string); - return 1; - } else { - TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf); - return 0; - } + const char *next_str = next_delim_string(buf, *delim); + if (next_str) { + *decoded_string = decode_hexstring (next_str, 0, 0); + DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string); + return 1; + } else { + TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf); + return 0; + } } -static int count_delims(char delim, char *buf) +static size_t count_delims(char delim, char *buf) { - size_t count = 0; - char *ptr = buf; + size_t count = 0; + char *ptr = buf; - while (*ptr) - count += *ptr++ == delim; - return count; + while (*ptr) + count += *ptr++ == delim; + return count; } // Determine the length of the arguments. @@ -1298,7 +1301,7 @@ void do_multi_mode(void) break; } - DEBUG(1, "packet recieved: '%s'\n", buf); + DEBUG(1, "packet received: '%s'\n", buf); #define QSUPPORTED "qSupported:" #define STARTNOACKMODE "QStartNoAckMode" @@ -1403,7 +1406,8 @@ void do_multi_mode(void) if (i < count - 1) next_str = next_delim_string(next_str, *delim); } - DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", decoded_string[i], next_str, i, len[i]); + DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", + decoded_string[i], next_str, i, len[i]); } /* If we didn't get any arguments or the filename is an empty @@ -1431,8 +1435,9 @@ void do_multi_mode(void) // Lets report we Stopped with SIGTRAP (05). send_packet ("S05", noackmode); prepare_fifos_and_shared_mem(valgrind_pid); - DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", from_gdb_to_pid, to_gdb_from_pid); - // gdb_rely is an endless loop till valgrind quits. + DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", + from_gdb_to_pid, to_gdb_from_pid); + // gdb_relay is an endless loop till valgrind quits. shutting_down = False; gdb_relay (valgrind_pid, 1, q_buf); @@ -1451,7 +1456,7 @@ void do_multi_mode(void) DEBUG(1, "valgrind kill by signal %d\n", WTERMSIG(status)); else - DEBUG(1, "valgind unexpectedly stopped or continued"); + DEBUG(1, "valgrind unexpectedly stopped or continued"); } } else { send_packet ("E01", noackmode); @@ -1461,17 +1466,17 @@ void do_multi_mode(void) free(len); for (int i = 0; i < count; i++) - free (decoded_string[i]); - free (decoded_string); - } else { - free(len); - send_packet ("E01", noackmode); - DEBUG(1, "vRun decoding error: no next_string!\n"); - continue; - } + free (decoded_string[i]); + free (decoded_string); + } else { + free(len); + send_packet ("E01", noackmode); + DEBUG(1, "vRun decoding error: no next_string!\n"); + continue; + } } else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) { - send_packet ("1", noackmode); - DEBUG(1, "qAttached sent: '1'\n"); + send_packet ("1", noackmode); + DEBUG(1, "qAttached sent: '1'\n"); const char *next_str = next_delim_string(buf, ':'); if (next_str) { char *decoded_string = decode_hexstring (next_str, 0, 0); @@ -1481,9 +1486,10 @@ void do_multi_mode(void) DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf); continue; } - } /* Reset the state of environment variables in the remote target before starting - the inferior. In this context, reset means unsetting all environment variables - that were previously set by the user (i.e., were not initially present in the environment). */ + } /* Reset the state of environment variables in the remote target + before starting the inferior. In this context, reset means + unsetting all environment variables that were previously set + by the user (i.e., were not initially present in the environment). */ else if (strncmp(QENVIRONMENTRESET, buf, strlen(QENVIRONMENTRESET)) == 0) { send_packet ("OK", noackmode); @@ -1495,7 +1501,7 @@ void do_multi_mode(void) if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string)) break; // TODO Collect all environment strings and add them to environ - // before launcing valgrind. + // before launching valgrind. free (string); string = NULL; } else if (strncmp(QENVIRONMENTUNSET, buf, @@ -1507,33 +1513,33 @@ void do_multi_mode(void) free (string); string = NULL; } else if (strncmp(QSETWORKINGDIR, buf, - strlen(QSETWORKINGDIR)) == 0) { - // Silly, but we can only reply OK, even if the working directory is - // bad. Errors will be reported when we try to execute the actual - // process. - send_packet ("OK", noackmode); - // Free any previously set working_dir - free (working_dir); - working_dir = NULL; - if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { - continue; // We cannot report the error to gdb... - } - DEBUG(1, "set working dir to: %s\n", working_dir); + strlen(QSETWORKINGDIR)) == 0) { + // Silly, but we can only reply OK, even if the working directory is + // bad. Errors will be reported when we try to execute the actual + // process. + send_packet ("OK", noackmode); + // Free any previously set working_dir + free (working_dir); + working_dir = NULL; + if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) { + continue; // We cannot report the error to gdb... + } + DEBUG(1, "set working dir to: %s\n", working_dir); } else if (strncmp(XFER, buf, strlen(XFER)) == 0) { - char *buf_dup = strdup(buf); - DEBUG(1, "strdup: buf_dup %s\n", buf_dup); - if (buf_dup) { - const char *delim = ":"; - size_t count = count_delims(delim[0], buf); - if (count < 4) { - strsep(&buf_dup, delim); - strsep(&buf_dup, delim); - strsep(&buf_dup, delim); - char *decoded_string = decode_hexstring (buf_dup, 0, 0); - DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); - free (decoded_string); - } - free (buf_dup); + char *buf_dup = strdup(buf); + DEBUG(1, "strdup: buf_dup %s\n", buf_dup); + if (buf_dup) { + const char *delim = ":"; + size_t count = count_delims(delim[0], buf); + if (count < 4) { + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + strsep(&buf_dup, delim); + char *decoded_string = decode_hexstring (buf_dup, 0, 0); + DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup); + free (decoded_string); + } + free (buf_dup); } else { DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf); free (buf_dup); @@ -2220,7 +2226,8 @@ void parse_options(int argc, char** argv, /* Compute the absolute path. */ valgrind_path = realpath(path, NULL); if (!valgrind_path) { - TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", path, strerror (errno)); + TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", + path, strerror (errno)); exit(1); } DEBUG(2, "valgrind's real path: %s\n", valgrind_path); @@ -2228,7 +2235,7 @@ void parse_options(int argc, char** argv, // Everything that follows now is an argument for valgrind // No other options (or commands) can follow // argc - i is the number of left over arguments - // allocate enough space, but all args in it. + // allocate enough space, put all args in it. cvargs = argc - i - 1; vargs = vmalloc (cvargs * sizeof(vargs)); i++; diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml index bb695d2d3d..ff8c8124af 100644 --- a/docs/xml/manual-core-adv.xml +++ b/docs/xml/manual-core-adv.xml @@ -1299,8 +1299,8 @@ It has three usage modes: </listitem> <listitem id="manual-core-adv.vgdb-multi" xreflabel="vgdb multi"> - <para>In the <option>--multi</option> mode, Vgdb uses the extended - remote protocol to communicate with Gdb. This allows you to view + <para>In the <option>--multi</option> mode, vgdb uses the extended + remote protocol to communicate with GDB. This allows you to view output from both valgrind and GDB in the GDB session. This is accomplished via the "target extended-remote | vgdb --multi". In this mode you no longer need to start valgrind yourself. vgdb will @@ -2271,5 +2271,4 @@ almost 300 different wrappers.</para> - </chapter> diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index 10485a3b40..241d33afa5 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -190,7 +190,6 @@ usage: valgrind [options] prog-and-args --progress-interval=<number> report progress every <number> CPU seconds [0, meaning disabled] --command-line-only=no|yes only use command line options [no] - --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no] Vex options for all Valgrind tools: --vex-iropt-verbosity=<0..9> [0] diff --git a/none/tests/cmdline2.stdout.exp-non-linux b/none/tests/cmdline2.stdout.exp-non-linux index 6e08284acd..63af17bf74 100644 --- a/none/tests/cmdline2.stdout.exp-non-linux +++ b/none/tests/cmdline2.stdout.exp-non-linux @@ -188,7 +188,6 @@ usage: valgrind [options] prog-and-args --progress-interval=<number> report progress every <number> CPU seconds [0, meaning disabled] --command-line-only=no|yes only use command line options [no] - --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no] Vex options for all Valgrind tools: --vex-iropt-verbosity=<0..9> [0] |
|
From: Alexandra H. <aha...@re...> - 2023-04-20 12:17:47
|
---
coregrind/m_gdbserver/server.c | 2 +-
coregrind/m_main.c | 4 +-
coregrind/vgdb.c | 275 ++++++++++++-----------
docs/xml/manual-core-adv.xml | 5 +-
none/tests/cmdline2.stdout.exp | 1 -
none/tests/cmdline2.stdout.exp-non-linux | 1 -
6 files changed, 146 insertions(+), 142 deletions(-)
diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c
index 3c2516086..83825408a 100644
--- a/coregrind/m_gdbserver/server.c
+++ b/coregrind/m_gdbserver/server.c
@@ -1105,7 +1105,7 @@ void handle_query (char *arg_own_buf, int *new_packet_len_p)
return;
}
- /* Protocol features query. */
+ /* Protocol features query. Keep this in sync with coregind/vgdb.c. */
if (strncmp ("qSupported", arg_own_buf, 10) == 0
&& (arg_own_buf[10] == ':' || arg_own_buf[10] == '\0')) {
VG_(sprintf) (arg_own_buf, "PacketSize=%x", (UInt)PBUFSIZ - 1);
diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 6181046d1..e19796327 100644
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -279,8 +279,6 @@ static void usage_NORETURN ( int need_help )
" --progress-interval=<number> report progress every <number>\n"
" CPU seconds [0, meaning disabled]\n"
" --command-line-only=no|yes only use command line options [no]\n"
-" --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]\n"
-"\n"
" Vex options for all Valgrind tools:\n"
" --vex-iropt-verbosity=<0..9> [0]\n"
" --vex-iropt-level=<0..2> [2]\n"
@@ -563,6 +561,8 @@ static void process_option (Clo_Mode mode,
}
else if VG_INT_CLOM (cloPD, arg, "--vgdb-poll", VG_(clo_vgdb_poll)) {}
else if VG_INT_CLOM (cloPD, arg, "--vgdb-error", VG_(clo_vgdb_error)) {}
+ /* --launched-with-multi is an internal option used by vgdb to suppress
+ some output that valgrind normally shows when using --vgdb-error. */
else if VG_BOOL_CLO (arg, "--launched-with-multi",
VG_(clo_launched_with_multi)) {}
else if VG_USET_CLOM (cloPD, arg, "--vgdb-stop-at",
diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index ca673e368..a3c5f9d88 100644
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -900,8 +900,7 @@ int tohex (int nib)
return 'a' + nib - 10;
}
-/* Returns an allocated hex-decoded string from the buf starting at offset
- off. Will update off to where the buf has been decoded. Stops decoding
+/* Returns an allocated hex-decoded string from the buf. Stops decoding
at end of buf (zero) or when seeing the delim char. */
static
char *decode_hexstring (const char *buf, size_t prefixlen, size_t len)
@@ -947,7 +946,7 @@ write_checksum (const char *str)
unsigned char csum = 0;
int i = 0;
while (str[i] != 0)
- csum += str[i++];
+ csum += str[i++];
char p[2];
p[0] = tohex ((csum >> 4) & 0x0f);
@@ -964,7 +963,7 @@ write_reply(const char *reply)
return write_checksum (reply);
}
-/* Creates a packet from a string message, called needs to free. */
+/* Creates a packet from a string message, caller needs to free. */
static char *
create_packet(const char *msg)
{
@@ -995,24 +994,24 @@ static int read_one_char (char *c)
static Bool
send_packet(const char *reply, int noackmode)
{
- int ret;
- char c;
+ int ret;
+ char c;
send_packet_start:
if (!write_reply(reply))
- return False;
- if (!noackmode) {
- // Look for '+' or '-'.
- // We must wait for "+" if !noackmode.
- do {
- ret = read_one_char(&c);
- if (ret <= 0)
- return False;
- // And if in !noackmode if we get "-" we should resent the packet.
- if (c == '-')
- goto send_packet_start;
- } while (c != '+');
- DEBUG(1, "sent packet to gdb got: %c\n",c);
+ return False;
+ if (!noackmode) {
+ // Look for '+' or '-'.
+ // We must wait for "+" if !noackmode.
+ do {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return False;
+ // And if in !noackmode if we get "-" we should resent the packet.
+ if (c == '-')
+ goto send_packet_start;
+ } while (c != '+');
+ DEBUG(1, "sent packet to gdb got: %c\n",c);
}
return True;
}
@@ -1023,92 +1022,96 @@ send_packet_start:
// or -1 if no packet could be read.
static int receive_packet(char *buf, int noackmode)
{
- int bufcnt = 0, ret;
- char c, c1, c2;
- unsigned char csum = 0;
-
- // Look for first '$' (start of packet) or error.
- receive_packet_start:
- do {
- ret = read_one_char(&c);
- if (ret <= 0)
- return ret;
- } while (c != '$');
+ int bufcnt = 0, ret;
+ char c, c1, c2;
+ unsigned char csum = 0;
- // Found start of packet ('$')
- while (bufcnt < (PBUFSIZ+1)) {
- ret = read_one_char(&c);
- if (ret <= 0)
- return ret;
- if (c == '#') {
- if ((ret = read_one_char(&c1)) <= 0
- || (ret = read_one_char(&c2)) <= 0) {
- return ret;
- }
- c1 = fromhex(c1);
- c2 = fromhex(c2);
- break;
+ // Look for first '$' (start of packet) or error.
+receive_packet_start:
+ do {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return ret;
+ } while (c != '$');
+
+ // Found start of packet ('$')
+ while (bufcnt < (PBUFSIZ+1)) {
+ ret = read_one_char(&c);
+ if (ret <= 0)
+ return ret;
+ if (c == '#') {
+ if ((ret = read_one_char(&c1)) <= 0
+ || (ret = read_one_char(&c2)) <= 0) {
+ return ret;
}
- buf[bufcnt] = c;
- csum += buf[bufcnt];
- bufcnt++;
- }
+ c1 = fromhex(c1);
+ c2 = fromhex(c2);
+ break;
+ }
+ buf[bufcnt] = c;
+ csum += buf[bufcnt];
+ bufcnt++;
+ }
- // Packet complete, add terminator.
- buf[bufcnt] ='\0';
-
- if (!(csum == (c1 << 4) + c2)) {
- TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
- (c1 << 4) + c2, csum, buf);
- if (!noackmode)
- if (!write_to_gdb ("-", 1))
- return -1;
- /* Try again, gdb should resend the packet. */
- bufcnt = 0;
- csum = 0;
- goto receive_packet_start;
- }
+ // Packet complete, add terminator.
+ buf[bufcnt] ='\0';
- if (!noackmode)
- if (!write_to_gdb ("+", 1))
- return -1;
- return bufcnt;
+ if (!(csum == (c1 << 4) + c2)) {
+ TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
+ (c1 << 4) + c2, csum, buf);
+ if (!noackmode)
+ if (!write_to_gdb ("-", 1))
+ return -1;
+ /* Try again, gdb should resend the packet. */
+ bufcnt = 0;
+ csum = 0;
+ goto receive_packet_start;
+ }
+
+ if (!noackmode)
+ if (!write_to_gdb ("+", 1))
+ return -1;
+ return bufcnt;
}
// Returns a pointer to the char after the next delim char.
static const char *next_delim_string (const char *buf, char delim)
{
- while (*buf) {
- if (*buf++ == delim)
- break;
- }
- return buf;
+ while (*buf) {
+ if (*buf++ == delim)
+ break;
+ }
+ return buf;
}
-// Throws away the packet name and decodes the hex string, which is placed in
-// decoded_string (the caller owns this and is responsible for freeing it).
+/* buf starts with the packet name followed by the delimiter, for example
+ * vRun;2f62696e2f6c73, ";" is the delimiter here, or
+ * qXfer:features:read:target.xml:0,1000, where the delimiter is ":".
+ * The packet name is thrown away and the hex string is decoded and
+ * is placed in decoded_string (the caller owns this and is responsible
+ * for freeing it). */
static int split_hexdecode(const char *buf, const char *string,
const char *delim, char **decoded_string)
{
- const char *next_str = next_delim_string(buf, *delim);
- if (next_str) {
- *decoded_string = decode_hexstring (next_str, 0, 0);
- DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string);
- return 1;
- } else {
- TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf);
- return 0;
- }
+ const char *next_str = next_delim_string(buf, *delim);
+ if (next_str) {
+ *decoded_string = decode_hexstring (next_str, 0, 0);
+ DEBUG(1, "split_hexdecode decoded %s\n", *decoded_string);
+ return 1;
+ } else {
+ TSFPRINTF(stderr, "%s decoding error: finding the hex string in %s failed!\n", string, buf);
+ return 0;
+ }
}
-static int count_delims(char delim, char *buf)
+static size_t count_delims(char delim, char *buf)
{
- size_t count = 0;
- char *ptr = buf;
+ size_t count = 0;
+ char *ptr = buf;
- while (*ptr)
- count += *ptr++ == delim;
- return count;
+ while (*ptr)
+ count += *ptr++ == delim;
+ return count;
}
// Determine the length of the arguments.
@@ -1298,7 +1301,7 @@ void do_multi_mode(void)
break;
}
- DEBUG(1, "packet recieved: '%s'\n", buf);
+ DEBUG(1, "packet received: '%s'\n", buf);
#define QSUPPORTED "qSupported:"
#define STARTNOACKMODE "QStartNoAckMode"
@@ -1403,7 +1406,8 @@ void do_multi_mode(void)
if (i < count - 1)
next_str = next_delim_string(next_str, *delim);
}
- DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n", decoded_string[i], next_str, i, len[i]);
+ DEBUG(1, "vRun decoded: %s, next_str %s, len[%d] %d\n",
+ decoded_string[i], next_str, i, len[i]);
}
/* If we didn't get any arguments or the filename is an empty
@@ -1431,8 +1435,9 @@ void do_multi_mode(void)
// Lets report we Stopped with SIGTRAP (05).
send_packet ("S05", noackmode);
prepare_fifos_and_shared_mem(valgrind_pid);
- DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n", from_gdb_to_pid, to_gdb_from_pid);
- // gdb_rely is an endless loop till valgrind quits.
+ DEBUG(1, "from_gdb_to_pid %s, to_gdb_from_pid %s\n",
+ from_gdb_to_pid, to_gdb_from_pid);
+ // gdb_relay is an endless loop till valgrind quits.
shutting_down = False;
gdb_relay (valgrind_pid, 1, q_buf);
@@ -1451,7 +1456,7 @@ void do_multi_mode(void)
DEBUG(1, "valgrind kill by signal %d\n",
WTERMSIG(status));
else
- DEBUG(1, "valgind unexpectedly stopped or continued");
+ DEBUG(1, "valgrind unexpectedly stopped or continued");
}
} else {
send_packet ("E01", noackmode);
@@ -1461,17 +1466,17 @@ void do_multi_mode(void)
free(len);
for (int i = 0; i < count; i++)
- free (decoded_string[i]);
- free (decoded_string);
- } else {
- free(len);
- send_packet ("E01", noackmode);
- DEBUG(1, "vRun decoding error: no next_string!\n");
- continue;
- }
+ free (decoded_string[i]);
+ free (decoded_string);
+ } else {
+ free(len);
+ send_packet ("E01", noackmode);
+ DEBUG(1, "vRun decoding error: no next_string!\n");
+ continue;
+ }
} else if (strncmp(QATTACHED, buf, strlen(QATTACHED)) == 0) {
- send_packet ("1", noackmode);
- DEBUG(1, "qAttached sent: '1'\n");
+ send_packet ("1", noackmode);
+ DEBUG(1, "qAttached sent: '1'\n");
const char *next_str = next_delim_string(buf, ':');
if (next_str) {
char *decoded_string = decode_hexstring (next_str, 0, 0);
@@ -1481,9 +1486,10 @@ void do_multi_mode(void)
DEBUG(1, "qAttached decoding error: strdup of %s failed!\n", buf);
continue;
}
- } /* Reset the state of environment variables in the remote target before starting
- the inferior. In this context, reset means unsetting all environment variables
- that were previously set by the user (i.e., were not initially present in the environment). */
+ } /* Reset the state of environment variables in the remote target
+ before starting the inferior. In this context, reset means
+ unsetting all environment variables that were previously set
+ by the user (i.e., were not initially present in the environment). */
else if (strncmp(QENVIRONMENTRESET, buf,
strlen(QENVIRONMENTRESET)) == 0) {
send_packet ("OK", noackmode);
@@ -1495,7 +1501,7 @@ void do_multi_mode(void)
if (!split_hexdecode(buf, QENVIRONMENTHEXENCODED, ":", &string))
break;
// TODO Collect all environment strings and add them to environ
- // before launcing valgrind.
+ // before launching valgrind.
free (string);
string = NULL;
} else if (strncmp(QENVIRONMENTUNSET, buf,
@@ -1507,33 +1513,33 @@ void do_multi_mode(void)
free (string);
string = NULL;
} else if (strncmp(QSETWORKINGDIR, buf,
- strlen(QSETWORKINGDIR)) == 0) {
- // Silly, but we can only reply OK, even if the working directory is
- // bad. Errors will be reported when we try to execute the actual
- // process.
- send_packet ("OK", noackmode);
- // Free any previously set working_dir
- free (working_dir);
- working_dir = NULL;
- if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) {
- continue; // We cannot report the error to gdb...
- }
- DEBUG(1, "set working dir to: %s\n", working_dir);
+ strlen(QSETWORKINGDIR)) == 0) {
+ // Silly, but we can only reply OK, even if the working directory is
+ // bad. Errors will be reported when we try to execute the actual
+ // process.
+ send_packet ("OK", noackmode);
+ // Free any previously set working_dir
+ free (working_dir);
+ working_dir = NULL;
+ if (!split_hexdecode(buf, QSETWORKINGDIR, ":", &working_dir)) {
+ continue; // We cannot report the error to gdb...
+ }
+ DEBUG(1, "set working dir to: %s\n", working_dir);
} else if (strncmp(XFER, buf, strlen(XFER)) == 0) {
- char *buf_dup = strdup(buf);
- DEBUG(1, "strdup: buf_dup %s\n", buf_dup);
- if (buf_dup) {
- const char *delim = ":";
- size_t count = count_delims(delim[0], buf);
- if (count < 4) {
- strsep(&buf_dup, delim);
- strsep(&buf_dup, delim);
- strsep(&buf_dup, delim);
- char *decoded_string = decode_hexstring (buf_dup, 0, 0);
- DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup);
- free (decoded_string);
- }
- free (buf_dup);
+ char *buf_dup = strdup(buf);
+ DEBUG(1, "strdup: buf_dup %s\n", buf_dup);
+ if (buf_dup) {
+ const char *delim = ":";
+ size_t count = count_delims(delim[0], buf);
+ if (count < 4) {
+ strsep(&buf_dup, delim);
+ strsep(&buf_dup, delim);
+ strsep(&buf_dup, delim);
+ char *decoded_string = decode_hexstring (buf_dup, 0, 0);
+ DEBUG(1, "qXfer decoded: %s, buf_dup %s\n", decoded_string, buf_dup);
+ free (decoded_string);
+ }
+ free (buf_dup);
} else {
DEBUG(1, "qXfer decoding error: strdup of %s failed!\n", buf);
free (buf_dup);
@@ -2220,7 +2226,8 @@ void parse_options(int argc, char** argv,
/* Compute the absolute path. */
valgrind_path = realpath(path, NULL);
if (!valgrind_path) {
- TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n", path, strerror (errno));
+ TSFPRINTF(stderr, "%s is not a correct path. %s, exiting.\n",
+ path, strerror (errno));
exit(1);
}
DEBUG(2, "valgrind's real path: %s\n", valgrind_path);
@@ -2228,7 +2235,7 @@ void parse_options(int argc, char** argv,
// Everything that follows now is an argument for valgrind
// No other options (or commands) can follow
// argc - i is the number of left over arguments
- // allocate enough space, but all args in it.
+ // allocate enough space, put all args in it.
cvargs = argc - i - 1;
vargs = vmalloc (cvargs * sizeof(vargs));
i++;
diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml
index bb695d2d3..ff8c8124a 100644
--- a/docs/xml/manual-core-adv.xml
+++ b/docs/xml/manual-core-adv.xml
@@ -1299,8 +1299,8 @@ It has three usage modes:
</listitem>
<listitem id="manual-core-adv.vgdb-multi" xreflabel="vgdb multi">
- <para>In the <option>--multi</option> mode, Vgdb uses the extended
- remote protocol to communicate with Gdb. This allows you to view
+ <para>In the <option>--multi</option> mode, vgdb uses the extended
+ remote protocol to communicate with GDB. This allows you to view
output from both valgrind and GDB in the GDB session. This is
accomplished via the "target extended-remote | vgdb --multi". In
this mode you no longer need to start valgrind yourself. vgdb will
@@ -2271,5 +2271,4 @@ almost 300 different wrappers.</para>
-
</chapter>
diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp
index 10485a3b4..241d33afa 100644
--- a/none/tests/cmdline2.stdout.exp
+++ b/none/tests/cmdline2.stdout.exp
@@ -190,7 +190,6 @@ usage: valgrind [options] prog-and-args
--progress-interval=<number> report progress every <number>
CPU seconds [0, meaning disabled]
--command-line-only=no|yes only use command line options [no]
- --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]
Vex options for all Valgrind tools:
--vex-iropt-verbosity=<0..9> [0]
diff --git a/none/tests/cmdline2.stdout.exp-non-linux b/none/tests/cmdline2.stdout.exp-non-linux
index 6e08284ac..63af17bf7 100644
--- a/none/tests/cmdline2.stdout.exp-non-linux
+++ b/none/tests/cmdline2.stdout.exp-non-linux
@@ -188,7 +188,6 @@ usage: valgrind [options] prog-and-args
--progress-interval=<number> report progress every <number>
CPU seconds [0, meaning disabled]
--command-line-only=no|yes only use command line options [no]
- --launched-with-multi=no|yes valgrind launched in vgdb multi mode [no]
Vex options for all Valgrind tools:
--vex-iropt-verbosity=<0..9> [0]
--
2.39.2
|
|
From: Mark W. <ma...@so...> - 2023-04-20 11:02:18
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=0ead4c39f0268420f24df64629d80bd45614fe04 commit 0ead4c39f0268420f24df64629d80bd45614fe04 Author: Mark Wielaard <ma...@kl...> Date: Thu Apr 20 12:59:02 2023 +0200 vgdb: Handle EAGAIN in read_buf The file descriptor is on non-blocking mode and read_buf should only be called when poll gave us an POLLIN event signaling the file descriptor is ready for reading from. Still sometimes we do get an occasional EAGAIN. Just do as told in that case and try to read again. Also fix an ERROR errno in getpkt. This has never been observed, but not getting the actual errno if the write fails in that case would be really confusing. Diff: --- coregrind/vgdb.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index 7ed9a8b2e9..ca673e368d 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -398,7 +398,14 @@ int read_buf(int fd, char* buf, const char* desc) { int nrread; DEBUG(2, "reading %s\n", desc); - nrread = read(fd, buf, PBUFSIZ); + /* The file descriptor is on non-blocking mode and read_buf should only + be called when poll gave us an POLLIN event signaling the file + descriptor is ready for reading from. Still sometimes we do get an + occasional EAGAIN. Just do as told in that case and try to read + again. */ + do { + nrread = read(fd, buf, PBUFSIZ); + } while (nrread == -1 && errno == EAGAIN); if (nrread == -1) { ERROR(errno, "error reading %s\n", desc); return -1; @@ -708,7 +715,7 @@ getpkt(char *buf, int fromfd, int ackfd) TSFPRINTF(stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", (c1 << 4) + c2, csum, buf); if (write(ackfd, "-", 1) != 1) - ERROR(0, "error when writing - (nack)\n"); + ERROR(errno, "error when writing - (nack)\n"); else add_written(1); } |
|
From: Mark W. <ma...@so...> - 2023-04-20 10:47:29
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=6effd73e9071efde14cf55c6b04c1217cc4c8515 commit 6effd73e9071efde14cf55c6b04c1217cc4c8515 Author: Mark Wielaard <ma...@kl...> Date: Wed Apr 19 15:53:53 2023 +0200 gdbserver_tests/hginfo.vgtest: Use --ignore-thread-creation=yes The testcase might notice an extra lock created by pthread_create. https://bugs.kde.org/show_bug.cgi?id=444487 Diff: --- NEWS | 1 + gdbserver_tests/hginfo.vgtest | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a8fed85bf4..203b422784 100644 --- a/NEWS +++ b/NEWS @@ -133,6 +133,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 435441 valgrind fails to interpose malloc on musl 1.2.2 due to weak symbol name and no libc soname 439685 compiler warning in callgrind/main.c 444110 priv/guest_ppc_toIR.c:36198:31: warning: duplicated 'if' condition. +444487 hginfo test detects an extra lock inside data symbol "_rtld_local" 444488 Use glibc.pthread.stack_cache_size tunable 444568 drd/tests/pth_barrier_thr_cr fails on Fedora 38 445743 "The impossible happened: mutex is locked simultaneously by two threads" diff --git a/gdbserver_tests/hginfo.vgtest b/gdbserver_tests/hginfo.vgtest index 5d00e1e214..0ea8ab4b34 100644 --- a/gdbserver_tests/hginfo.vgtest +++ b/gdbserver_tests/hginfo.vgtest @@ -1,7 +1,7 @@ # test helgrind monitor command # test 'v.info location' monitor command prog: ../helgrind/tests/hg01_all_ok -vgopts: --tool=helgrind --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-hginfo -q +vgopts: --tool=helgrind --ignore-thread-creation=yes --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-hginfo -q prereq: test -e gdb.eval stdout_filter: filter_make_empty stderr_filter: filter_stderr |
|
From: Mark W. <ma...@so...> - 2023-04-19 22:50:57
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=d270b7b15bafd7eb555994483556e3c22400bf47 commit d270b7b15bafd7eb555994483556e3c22400bf47 Author: Mark Wielaard <ma...@kl...> Date: Thu Apr 20 00:42:40 2023 +0200 Bug 439685 compiler warning in callgrind/main.c main.c: In function 'vgCallgrind_post_syscalltime': main.c:1779:25: warning: '*((void *)&ts_now+8)' may be used uninitialized in this function [-Wmaybe-uninitialized] struct vki_timespec ts_now; main.c:1779:25: warning: 'ts_now' may be used uninitialized in this function [-Wmaybe-uninitialized] In function collect_time the conditional expression in the switch statement has type int (after integral promotions). GCC assumes that it may have values other than the ones listed in the enumerated type it was promoted from. In that case the memory pointed to by its 1st argument remains unintialised. Later on vki_timespec_diff will read the contents of ts_now undoditionally. Hence the warning. Using the default case for the tl_assert () removes the warning and makes the code more robust should another enumerator ever be added to Collect_Systime. Contributed-by: Florian Krohm <fl...@ei...> Diff: --- NEWS | 1 + callgrind/main.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 1b9bdb869d..a8fed85bf4 100644 --- a/NEWS +++ b/NEWS @@ -131,6 +131,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 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 +439685 compiler warning in callgrind/main.c 444110 priv/guest_ppc_toIR.c:36198:31: warning: duplicated 'if' condition. 444488 Use glibc.pthread.stack_cache_size tunable 444568 drd/tests/pth_barrier_thr_cr fails on Fedora 38 diff --git a/callgrind/main.c b/callgrind/main.c index 4970d5a4d7..0c2467a90f 100644 --- a/callgrind/main.c +++ b/callgrind/main.c @@ -1711,7 +1711,7 @@ static void collect_time (struct vki_timespec *systime, struct vki_timespec *syscputime) { switch (CLG_(clo).collect_systime) { - case systime_no: tl_assert (0); + default: tl_assert (0); case systime_msec: { UInt ms_timer = VG_(read_millisecond_timer)(); systime->tv_sec = ms_timer / 1000; |
|
From: Mark W. <ma...@so...> - 2023-04-19 21:55:55
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=b8a9da078b21be2d110adbd563e1c07fc3f8d74b commit b8a9da078b21be2d110adbd563e1c07fc3f8d74b Author: Igor Nunes <igo...@gm...> Date: Fri Mar 20 17:12:13 2020 +0000 Enable getcpu on arm 32 https://bugs.kde.org/show_bug.cgi?id=419054 Diff: --- NEWS | 1 + coregrind/m_syswrap/syswrap-arm-linux.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 4b18ad9d0a..1b9bdb869d 100644 --- a/NEWS +++ b/NEWS @@ -127,6 +127,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 374596 inconsistent RDTSCP support on x86_64 392331 Spurious lock not held error from inside pthread_cond_timedwait 400793 pthread_rwlock_timedwrlock false positive +419054 Unhandled syscall getcpu on arm32 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 diff --git a/coregrind/m_syswrap/syswrap-arm-linux.c b/coregrind/m_syswrap/syswrap-arm-linux.c index 8b1a8fe702..bca5095893 100644 --- a/coregrind/m_syswrap/syswrap-arm-linux.c +++ b/coregrind/m_syswrap/syswrap-arm-linux.c @@ -957,7 +957,6 @@ static SyscallTableEntry syscall_main_table[] = { // LINX_(__NR_tee, sys_ni_syscall), // 315 // LINX_(__NR_vmsplice, sys_ni_syscall), // 316 LINXY(__NR_move_pages, sys_move_pages), // 317 -// LINX_(__NR_getcpu, sys_ni_syscall), // 318 LINX_(__NR_utimensat, sys_utimensat), // 320 LINXY(__NR_signalfd, sys_signalfd), // 321 @@ -981,6 +980,7 @@ static SyscallTableEntry syscall_main_table[] = { LINXY(__NR_pselect6, sys_pselect6), // 335 LINXY(__NR_ppoll, sys_ppoll), // 336 + LINXY(__NR_getcpu, sys_getcpu), // 345 LINXY(__NR_epoll_pwait, sys_epoll_pwait), // 346 LINX_(__NR_fallocate, sys_fallocate), // 352 |
|
From: Carl L. <ca...@so...> - 2023-04-19 18:44:33
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=19c9e2418cf7aed6a1ce9f17ff3a2adcb877390c commit 19c9e2418cf7aed6a1ce9f17ff3a2adcb877390c Author: Carl Love <ce...@us...> Date: Wed Apr 19 13:45:19 2023 -0400 PowerPC:, Update test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c The commit: commit 20cc0680c3491e062c76605b24e76dc02e16ef47 Author: Carl Love <ce...@us...> Date: Mon Apr 17 17:12:25 2023 -0400 PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c Fixes an issue with the PAD_ORI used in the the tests by explicitly adding SAVE_REGS and RESTORE_REGS macros. The macros ensure that the block of immediate OR instructions don't inadvertently change the contents of the registers. John Reiser suggested that the PAD_ORI asm statements in the PAD_ORI macro be updated to inform the compiler which register the ori instruction is clobbering. The compiler will then generate the code to save and restore the register automatically. This is a cleaner solution then explicitly adding the macros to store and restore the registers. It is functionally cleaner in that the value fetched by the instruction under test is not modified by the PAD_ORI instructions. This patch removes the SAVE_REG and RESTORE_REG macros and updates the PAD_ORI macro. Diff: --- none/tests/ppc64/isa_3_1_helpers.h | 40 +++- none/tests/ppc64/test_isa_3_1_R1_RT.c | 246 ------------------------- none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp | 12 +- none/tests/ppc64/test_isa_3_1_R1_XT.c | 177 ------------------ none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp | 28 +-- 5 files changed, 52 insertions(+), 451 deletions(-) diff --git a/none/tests/ppc64/isa_3_1_helpers.h b/none/tests/ppc64/isa_3_1_helpers.h index 716a6277b9..b559e730e7 100644 --- a/none/tests/ppc64/isa_3_1_helpers.h +++ b/none/tests/ppc64/isa_3_1_helpers.h @@ -67,14 +67,38 @@ extern void initialize_buffer(int); #define RELOC_BUFFER_SIZE 0x1000 extern unsigned long long pcrelative_buff_addr(int); #define PAD_ORI \ - __asm__ __volatile__ ("ori 21,21,21"); \ - __asm__ __volatile__ ("ori 22,22,22");\ - __asm__ __volatile__ ("ori 23,23,23");\ - __asm__ __volatile__ ("ori 24,24,24");\ - __asm__ __volatile__ ("ori 25,25,25");\ - __asm__ __volatile__ ("ori 26,26,26");\ - __asm__ __volatile__ ("ori 27,27,27");\ - __asm__ __volatile__ ("ori 28,28,28"); + __asm__ __volatile__ ("ori 21,21,21" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "21" /* clobbers register 21 */); \ + __asm__ __volatile__ ("ori 22,22,22" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "22" /* clobbers register 22 */); \ + __asm__ __volatile__ ("ori 23,23,23" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "23" /* clobbers register 23 */); \ + __asm__ __volatile__ ("ori 24,24,24" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "24" /* clobbers register 24 */); \ + __asm__ __volatile__ ("ori 25,25,25" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "25" /* clobbers register 25 */); \ + __asm__ __volatile__ ("ori 26,26,26" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "26" /* clobbers register 26 */); \ + __asm__ __volatile__ ("ori 27,27,27" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "27" /* clobbers register 27 */); \ + __asm__ __volatile__ ("ori 28,28,28" \ + : /* empty: no outputs from asm to C */ \ + : /* empty: no inputs from C to asm */ \ + : "28" /* clobbers register 28 */); extern int verbose; #define debug_printf(X) if (verbose>0) printf(X); diff --git a/none/tests/ppc64/test_isa_3_1_R1_RT.c b/none/tests/ppc64/test_isa_3_1_R1_RT.c index 33dcddc3e5..241d6cf41f 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_RT.c +++ b/none/tests/ppc64/test_isa_3_1_R1_RT.c @@ -49,304 +49,108 @@ struct test_list_t current_test; #include "isa_3_1_helpers.h" -#ifdef __powerpc64__ -typedef uint64_t HWord_t; -/* Save and restore all of the registers but rt which is the result of the instruction - under test. Need to ensure the PAD_ORI does not change the other registers. This - really shouldn't be needed but the optimization gets messed up when it inlines the - test function. */ -#define SAVE_REGS(addr) \ - asm volatile( \ - " std 21, 0(%0) \n" \ - " std 22, 8(%0) \n" \ - " std 23, 16(%0) \n" \ - " std 24, 24(%0) \n" \ - " std 25, 32(%0) \n" \ - " std 28, 56(%0) \n" \ - " std 29, 64(%0) \n" \ - " std 30, 72(%0) \n" \ - " std 31, 80(%0) \n" \ - ::"b"(addr)) - -#define SAVE_REG_RT(addr) \ - asm volatile( \ - " std 26, 40(%0) \n" \ - ::"b"(addr)) -#define SAVE_REG_27(addr) \ - asm volatile( \ - " std 27, 40(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REGS(addr) \ - asm volatile( \ - " ld 21, 0(%0) \n" \ - " ld 22, 8(%0) \n" \ - " ld 23, 16(%0) \n" \ - " ld 24, 24(%0) \n" \ - " ld 25, 32(%0) \n" \ - " ld 28, 56(%0) \n" \ - " ld 29, 64(%0) \n" \ - " ld 30, 72(%0) \n" \ - " ld 31, 80(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_RT(addr) \ - asm volatile( \ - " ld 26, 40(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_27(addr) \ - asm volatile( \ - " ld 27, 40(%0) \n" \ - ::"b"(addr)) - -#else /* !__powerpc64__ */ - -typedef uint32_t HWord_t; -#define SAVE_REGS(addr) \ - asm volatile( \ - " stw 21, 0(%0) \n" \ - " stw 22, 4(%0) \n" \ - " stw 23, 8(%0) \n" \ - " stw 24, 12(%0) \n" \ - " stw 25, 16(%0) \n" \ - " stw 28, 28(%0) \n" \ - " stw 29, 32(%0) \n" \ - " stw 30, 36(%0) \n" \ - " stw 31, 40(%0) \n" \ - ::"b"(addr)) - -#define SAVE_REG_RT(addr) \ - asm volatile( \ - " stw 26, 20(%0) \n" \ - ::"b"(addr)) - -#define SAVE_REG_27(addr) \ - asm volatile( \ - " stw 27, 20(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REGS(addr) \ - asm volatile( \ - " lwz 21, 0(%0) \n" \ - " lwz 22, 4(%0) \n" \ - " lwz 23, 8(%0) \n" \ - " lwz 24, 12(%0) \n" \ - " lwz 25, 16(%0) \n" \ - " lwz 28, 28(%0) \n" \ - " lwz 29, 32(%0) \n" \ - " lwz 30, 36(%0) \n" \ - " lwz 31, 400(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_RT(addr) \ - asm volatile( \ - " lwz 26, 40(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_27(addr) \ - asm volatile( \ - " lwz 27, 40(%0) \n" \ - ::"b"(addr)) - -#endif /* __powerpc64__ */ - -#define NUM_ENTRIES_SAVE_RESTORE 11 - -HWord_t temp[NUM_ENTRIES_SAVE_RESTORE]; - static void test_plxvp_off0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +0(0),1" ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); - RESTORE_REG_27(temp); } static void test_plxvp_off8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +8(0),1" ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); - RESTORE_REG_27(temp); } static void test_plxvp_off16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +16(0),1" ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); - RESTORE_REG_27(temp); } static void test_plxvp_off24_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +24(0),1" ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); - RESTORE_REG_27(temp); } static void test_plxvp_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +32(0),1" ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); - RESTORE_REG_27(temp); } static void test_plbz_off0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +0(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plbz_off8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +8(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plbz_off16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +16(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plbz_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plbz_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plhz_off0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +0(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plhz_off8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +8(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plhz_off16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +16(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plhz_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plhz_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plha_off0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +0(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plha_off8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +8(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plha_off16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +16(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plha_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plha_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REG_27(temp); - RESTORE_REGS(temp); } static void test_plwz_off0_R1 (void) { __asm__ __volatile__ ("plwz %0, +0(0), 1" : "=r" (rt) ); @@ -358,23 +162,15 @@ static void test_plwz_off16_R1 (void) { __asm__ __volatile__ ("plwz %0, +16(0), 1" : "=r" (rt) ); } static void test_plwz_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plwz_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plwa_off0_R1 (void) { __asm__ __volatile__ ("plwa %0, +0(0), 1" : "=r" (rt) ); @@ -386,69 +182,41 @@ static void test_plwa_off16_R1 (void) { __asm__ __volatile__ ("plwa %0, +16(0), 1" : "=r" (rt) ); } static void test_plwa_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwa %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_plwa_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwa %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pld_off0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +0(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pld_off8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +8(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pld_off16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +16(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pld_off32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pld_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_27(temp); } static void test_pstb_off0_R1 (void) { __asm__ __volatile__ ("pstb %0, -0x1f400+0(0), 1" :: "r" (rs) ); @@ -534,49 +302,35 @@ static void test_paddi_98_R1 (void) { rt = 0xffff0098; } static void test_plq_off0_R1 (void) { - SAVE_REGS(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +0(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); } static void test_plq_off8_R1 (void) { - SAVE_REGS(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +8(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); } static void test_plq_off16_R1 (void) { - SAVE_REGS(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +16(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); } static void test_plq_off32_R1 (void) { - SAVE_REGS(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +32(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); } static void test_plq_off48_R1 (void) { - SAVE_REGS(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +48(0), 1" : "=r" (rt) ); PAD_ORI - RESTORE_REGS(temp); } static void test_plq_off64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_RT(temp); PAD_ORI __asm__ __volatile__ ("plq %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_RT(temp); } static void test_pstq_off0_R1 (void) { __asm__ __volatile__ ("pstq 24, -0x1f400+0(0), 1" ); diff --git a/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp b/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp index 19011bc08d..b6f17f8dcc 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp +++ b/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp @@ -52,11 +52,11 @@ plq off8_R1 => 62d6001662b5001f 6318001862f7001f plq off16_R1 => 6318001862f7001f 635a001a6339001b -plq off32_R1 => 639c001c637b001b eac90008eaa9001b +plq off32_R1 => 639c001c637b001b eb81ffe0eae1ffbb -plq off48_R1 => eb090018eae9001a eb890038eb29003b +plq off48_R1 => 4e80003a 9000000001b -plq off64_R1 => 1111111111111111 eac90008eaa9001b +plq off64_R1 => 639c001c637b001b eb81ffe0eae1ffbb plwa off0_R1 => 4100000 @@ -82,11 +82,11 @@ plxvp off0_R1 => 6318001862f70017 635a001a63390019 ea80000004100000 62d6001662b5 plxvp off8_R1 => 635a001a63390019 639c001c637b001b 62d6001662b50015 6318001862f70017 -plxvp off16_R1 => 639c001c637b001b eac90008eaa90000 6318001862f70017 635a001a63390019 +plxvp off16_R1 => 639c001c637b001b eb81ffe0eae1ffb8 6318001862f70017 635a001a63390019 -plxvp off24_R1 => eac90008eaa90000 eb090018eae90010 635a001a63390019 639c001c637b001b +plxvp off24_R1 => eb81ffe0eae1ffb8 000000004e800020 635a001a63390019 639c001c637b001b -plxvp off32_R1 => eb090018eae90010 eb890038eb290020 639c001c637b001b eac90008eaa90000 +plxvp off32_R1 => 000000004e800020 0000090000000000 639c001c637b001b eb81ffe0eae1ffb8 pstb off0_R1 102030405060708 => 08 diff --git a/none/tests/ppc64/test_isa_3_1_R1_XT.c b/none/tests/ppc64/test_isa_3_1_R1_XT.c index 6c06ee64e4..bd30bfd62f 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_XT.c +++ b/none/tests/ppc64/test_isa_3_1_R1_XT.c @@ -48,95 +48,6 @@ unsigned long current_fpscr; struct test_list_t current_test; #include "isa_3_1_helpers.h" -#ifdef __powerpc64__ -typedef uint64_t HWord_t; - -/* Save and restore all of the registers. Need to ensure the PAD_ORI does not change - the other registers. This really shouldn't be needed but the optimization gets - messed up when it inlines the test function. */ -#define SAVE_REGS(addr) \ - asm volatile( \ - " std 21, 0(%0) \n" \ - " std 22, 8(%0) \n" \ - " std 23, 16(%0) \n" \ - " std 24, 24(%0) \n" \ - " std 25, 32(%0) \n" \ - " std 26, 40(%0) \n" \ - " std 27, 48(%0) \n" \ - " std 29, 64(%0) \n" \ - " std 30, 72(%0) \n" \ - " std 31, 80(%0) \n" \ - ::"b"(addr)) - -#define SAVE_REG_28(addr) \ - asm volatile( \ - " std 28, 56(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REGS(addr) \ - asm volatile( \ - " ld 21, 0(%0) \n" \ - " ld 22, 8(%0) \n" \ - " ld 23, 16(%0) \n" \ - " ld 24, 24(%0) \n" \ - " ld 25, 32(%0) \n" \ - " ld 26, 40(%0) \n" \ - " ld 27, 48(%0) \n" \ - " ld 29, 64(%0) \n" \ - " ld 30, 72(%0) \n" \ - " ld 31, 80(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_28(addr) \ - asm volatile( \ - " ld 28, 56(%0) \n" \ - ::"b"(addr)) - -#else /* !__powerpc64__ */ - -typedef uint32_t HWord_t; -#define SAVE_REGS(addr) \ - asm volatile( \ - " stw 21, 0(%0) \n" \ - " stw 22, 4(%0) \n" \ - " stw 23, 8(%0) \n" \ - " stw 24, 12(%0) \n" \ - " stw 25, 16(%0) \n" \ - " stw 26, 20(%0) \n" \ - " stw 27, 24(%0) \n" \ - " stw 29, 32(%0) \n" \ - " stw 30, 36(%0) \n" \ - " stw 31, 40(%0) \n" \ - ::"b"(addr)) - -#define SAVE_REG_28(addr) \ - asm volatile( \ - " stw 28, 28(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REGS(addr) \ - asm volatile( \ - " lwz 21, 0(%0) \n" \ - " lwz 22, 4(%0) \n" \ - " lwz 23, 8(%0) \n" \ - " lwz 24, 12(%0) \n" \ - " lwz 25, 16(%0) \n" \ - " lwz 26, 20(%0) \n" \ - " lwz 27, 24(%0) \n" \ - " lwz 29, 32(%0) \n" \ - " lwz 30, 36(%0) \n" \ - " lwz 31, 400(%0) \n" \ - ::"b"(addr)) - -#define RESTORE_REG_28(addr) \ - asm volatile( \ - " lwz 28, 28(%0) \n" \ - ::"b"(addr)) -#endif /* __powerpc64__ */ - -#define NUM_ENTRIES_SAVE_RESTORE 11 - -HWord_t temp[NUM_ENTRIES_SAVE_RESTORE]; static void test_pstxvp_off0_R1 (void) { __asm__ __volatile__ ("pstxvp 20, -0x1f400+0(0),1"); @@ -151,78 +62,54 @@ static void test_pstxvp_off48_R1 (void) { __asm__ __volatile__ ("pstxvp 20, -0x1f400+48(0),1"); } static void test_plfd_64_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +64(0), 1"); PAD_ORI PAD_ORI - RESTORE_REGS(temp); } static void test_plfd_32_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +32(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfd_16_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +16(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfd_8_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +8(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfd_4_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +4(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfd_0_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +0(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_64_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +64(0), 1"); PAD_ORI PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_32_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +32(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_16_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +16(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_8_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +8(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_4_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +4(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_plfs_0_R1 (void) { - SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +0(0), 1"); PAD_ORI - RESTORE_REGS(temp); } static void test_pstfd_32_R1 (void) { __asm__ __volatile__ ("pstfd 26, -0x1f400+32(0), 1"); @@ -255,102 +142,54 @@ static void test_pstfs_0_R1 (void) { __asm__ __volatile__ ("pstfs 26, -0x1f400+0(0), 1"); } static void test_plxsd_64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +64(0), 1" : "=v" (vrt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxsd_32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ (".align 2 ; plxsd %0, +32(0), 1" : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxsd_16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +16(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxsd_8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +8(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxsd_4_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +4(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxsd_0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +0(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_64_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +64(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_32_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +32(0), 1; pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +16(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +8(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_4_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +4(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxssp_0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +0(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } /* Follow the short-range plxv instructions with nop in order to pad out subsequent instructions. When written there are found @@ -358,36 +197,20 @@ static void test_plxssp_0_R1 (void) { into the target variable. (pla,pstxv...). */ static void test_plxv_16_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +16(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxv_8_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +8(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxv_4_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +4(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_plxv_0_R1 (void) { - SAVE_REGS(temp); - SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +0(0), 1; pnop;pnop;pnop; " : "=wa" (vec_xt) ); PAD_ORI - RESTORE_REGS(temp); - RESTORE_REG_28(temp); } static void test_pstxsd_64_R1 (void) { __asm__ __volatile__ (".align 2 ; pstxsd 22, -0x1f400+64(0), 1" ); diff --git a/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp b/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp index cef5c773fb..fc088cecf6 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp +++ b/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp @@ -28,7 +28,7 @@ plxsd 4_R1 => 7000000a8000004,0000000000000000 5.77662562e-275 +Zer plxsd 8_R1 => 7000000,0000000000000000 +Den +Zero -plxsd 16_R1 => 700000060000000,0000000000000000 5.77662407e-275 +Zero +plxsd 16_R1 => 7000000,0000000000000000 +Den +Zero plxsd 32_R1 => 6339001963180018,0000000000000000 9.43505226e+169 +Zero @@ -38,21 +38,21 @@ plxssp 0_R1 => 3882000000000000,0000000000000000 6.19888e-05 +Zero plxssp 4_R1 => bd80000080000000,0000000000000000 -6.25000e-02 -Zero +Zero +Zero -plxssp 8_R1 => 4400000000000000,0000000000000000 5.12000e+02 +Zero +Zero +Zero +plxssp 8_R1 => 38e0000000000000,0000000000000000 1.06812e-04 +Zero +Zero +Zero plxssp 16_R1 => 38e0000000000000,0000000000000000 1.06812e-04 +Zero +Zero +Zero plxssp 32_R1 => 445ac002c0000000,0000000000000000 8.75000e+02 -2.00000e+00 +Zero +Zero -plxssp 64_R1 => 4467200320000000,0000000000000000 9.24500e+02 1.08420e-19 +Zero +Zero +plxssp 64_R1 => 446b400340000000,0000000000000000 9.41000e+02 2.00000e+00 +Zero +Zero -plxv 0_R1 => c800000004100000 700000060000000 +plxv 0_R1 => c800000004100000 7000000 -plxv 4_R1 => 60000000c8000004 7000000 +plxv 4_R1 => 7000000c8000004 6000000000000000 -plxv 8_R1 => 700000060000000 700000000000000 +plxv 8_R1 => 7000000 7000000 -plxv 16_R1 => 700000000000000 700000000000000 +plxv 16_R1 => 7000000 7000000 pstfd 0_R1 43dfe000003fe000 43eff000000ff000 => e000003fe00043df pstfd 0_R1 43eff000000ff000 43efefffffcff000 => f000000ff00043ef @@ -86,15 +86,15 @@ pstfs 32_R1 000000005f7f8000 000000005f7f8000 => 80005f7f pstxsd 0_R1 => 0000000000000000 -pstxsd 4_R1 => 00000000 00000000 +pstxsd 4_R1 => 0000000000000000 -pstxsd 8_R1 => 0000000000000000 +pstxsd 8_R1 => 00000000 00000000 -pstxsd 16_R1 => 0000000000000000 +pstxsd 16_R1 => 00000000 00000000 -pstxsd 32_R1 => 0000000000000000 +pstxsd 32_R1 => 00000000 00000000 -pstxsd 64_R1 => 0000000000000000 +pstxsd 64_R1 => 00000000 00000000 pstxssp 0_R1 => 00000000 @@ -116,9 +116,9 @@ pstxvp off32_R1 0180055e0180077e 0080000e8080000e ff7ffffe7f7ffffe ff8000007f800 pstxvp off48_R1 0180055e0180077e 0080000e8080000e ff7ffffe7f7ffffe ff8000007f800000 => fffe7f7ffffeff7f 00007f800000ff80 077e0180055e0180 000e8080000e0080 -pstxv 0_R1 ff7ffffe7f7ffffe,ff8000007f800000 => fffe7f7ffffeff7f 00007f800000ff80 +pstxv 0_R1 ff7ffffe7f7ffffe,ff8000007f800000 => fffe7f7f fffeff7f00007f80 0000ff80 -pstxv 4_R1 ff7ffffe7f7ffffe,ff8000007f800000 => fffe7f7ffffeff7f 00007f800000ff80 +pstxv 4_R1 ff7ffffe7f7ffffe,ff8000007f800000 => fffe7f7f fffeff7f00007f80 0000ff80 pstxv 8_R1 ff7ffffe7f7ffffe,ff8000007f800000 => fffe7f7ffffeff7f 00007f800000ff80 |
|
From: Carl L. <ce...@us...> - 2023-04-19 18:31:31
|
On Wed, 2023-04-19 at 08:08 -0700, John Reiser wrote:
> >
<snip>
> On 4/18/23 12:53, Carl Love via Valgrind-developers wrote:
>
> At the lowest level, the problem is that a statement such as
> __asm__ __volatile__ ("ori 21,21,21");
Yes, that is the issue because it is using registers that the compiler
is not aware of.
> in the PAD_ORI macro of none/tests/ppc64/isa_3_1_helpers.h
> is incomplete because it does not specify the CLOBBERS portion of
> __asm__.
> [See
> https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
> e= ]
> Here is a more-complete statement that tells the compiler
> that the __asm__ scribbles on register 21:
> __asm__ __volatile__ ("ori 21,21,21"
> : /* empty: no outputs from asm to C */
> : /* empty: no inputs from C to asm */
> : "21" /* clobbers register 21 */
> );
>
I wasn't aware of the full specification on the asm command. This is
really helpful.
> Omitting the CLOBBERS is surprising because other __asm__ in the same
> file
> do use it, such as:
> __asm__ __volatile__ ("mtcr %0" : : "b"(_arg) : ALLCR );
>
> If the CLOBBERS are specified, the the compiler automatically saves
> and
> restores the clobbered registers, if required because "callee saved"
> by the
> global usage convention.
Yup, I tried removing the SAVE_REG and RESTORE_REG macros I added and
updated the PAD_ORI macro instead. This works as you said. It does
have the added advantage of removing the modification of the fetched
register by the instructions under test which my save/restore registers
macro didn't do.
I have created an additional patch to change the fix for the tests
using your approach which I will commit shortly.
Thanks for the help.
Carl
|
|
From: Mark W. <ma...@kl...> - 2023-04-19 15:41:45
|
Hi Philippe,
On Sun, 2023-04-16 at 14:35 +0200, Philippe Waroquiers wrote:
> > > Not too sure what is going wrong/what I am doing wrong ...
> >
> > Nothing. There is something about sleepers that causes this. It also
> > happens for me. I'll try to debug it. But it works when you give vgdb
> > -d -d debug options...
> Humph, race condition/timing related bugs are hard to debug :(.
Finally found where this happens (sometimes):
diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index 7ed9a8b2e..2373bf335 100644
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -398,7 +398,9 @@ int read_buf(int fd, char* buf, const char* desc)
{
int nrread;
DEBUG(2, "reading %s\n", desc);
- nrread = read(fd, buf, PBUFSIZ);
+ do {
+ nrread = read(fd, buf, PBUFSIZ);
+ } while (nrread == -1 && errno == EAGAIN);
if (nrread == -1) {
ERROR(errno, "error reading %s\n", desc);
return -1;
This means the pipe isn't actually ready to be read from. Which really
shouldn't happen because we do a poll on the fd to make sure we get an
POLLIN event before starting to read from it.
I'll check in the above if I cannot find a more elegant solution.
Cheers,
Mark
|
|
From: John R. <jr...@bi...> - 2023-04-19 15:08:19
|
On 4/18/23 12:53, Carl Love via Valgrind-developers wrote:
> Mark:
>
> On Mon, 2023-04-17 at 09:22 -0700, Carl Love via Valgrind-developers
> wrote:
>> The test_isa_3_1_R1_RT and test_isa_3_1_R1_XT tests seem to run
>> differently then expected. The tests generate multiple lines of
>> output
>> when only one line was expected. For example:
>
> I have pushed a fix for the two tests. The issue is the tests are
> testing load instructions that load relative to the current PC address.
> The tests of these instructions adds blocks of OR immediate
> instructions before the assembly for the instruction under test.
> Unfortunately, the test didn't save and restore the registers touched
> by the OR immediate instructions. This is fine as long as you are
> calling a function, the touched registers are volitile across a
> function call. It seems the more recent GCC is a bit more aggressive
> in inlining the test functions. However, the compiler doesn't realize
> that the inline OR immediate instructions are touching registers. The
> OR immediate instructions were inadvertently changing the value of the
> register that held the for loop variable. Thus the loops would execute
> more times then expected.
>
> The commit is:
>
> commit 20cc0680c3491e062c76605b24e76dc02e16ef47 (HEAD -> master)
> Author: Carl Love <ce...@us...>
> Date: Mon Apr 17 17:12:25 2023 -0400
>
> PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c
>
> If the commit gets into the current release, great. If not, it is not
> an issue. The problem is completely isolated to the test case.
At the lowest level, the problem is that a statement such as
__asm__ __volatile__ ("ori 21,21,21");
in the PAD_ORI macro of none/tests/ppc64/isa_3_1_helpers.h
is incomplete because it does not specify the CLOBBERS portion of __asm__.
[See https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html ]
Here is a more-complete statement that tells the compiler
that the __asm__ scribbles on register 21:
__asm__ __volatile__ ("ori 21,21,21"
: /* empty: no outputs from asm to C */
: /* empty: no inputs from C to asm */
: "21" /* clobbers register 21 */
);
Omitting the CLOBBERS is surprising because other __asm__ in the same file
do use it, such as:
__asm__ __volatile__ ("mtcr %0" : : "b"(_arg) : ALLCR );
If the CLOBBERS are specified, the the compiler automatically saves and
restores the clobbered registers, if required because "callee saved" by the
global usage convention. This can be seen by compiling a test such as:
int fn2(int,int,int,int,int,int,int,int,
int,int,int,int,int,int,int,int); /* external */
int fn1(int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)
{
int b1=1, b2=2, b3=3, b4=4, b5=5, b6=6, b7=7, b8=8;
fn2(a1,b1,a2,b2,a3,b3,a4,b4,a5,b5,a6,b6,a7,b7,a8,b8);
__asm__ __volatile__ (
"ori 21,21,21; ori 22,22,22; ori 23,23,23; ori 24,24,24"
: /* empty: no outputs from asm to C */
: /* empty: no inputs from C to asm */
: "21", "22", "23", "24" /* clobbers these registers */
);
fn2(a1,b1,a2,b2,a3,b3,a4,b4,a5,b5,a6,b6,a7,b7,a8,b8);
}
and inspecting the generated code; I used gcc 4.9.2:
fn1:
stwu 1,-160(1)
mflr 0
stw 0,164(1)
stw 21,116(1)
stw 22,120(1)
stw 23,124(1)
stw 24,128(1)
<<snip>>
bl fn2
#APP
# 8 "asm2.c" 1
ori 21,21,21; ori 22,22,22; ori 23,23,23; ori 24,24,24
# 0 "" 2
#NO_APP
<<snip>>
lwz 21,-44(11)
lwz 22,-40(11)
lwz 23,-36(11)
lwz 24,-32(11)
lwz 31,-4(11)
mr 1,11
blr
Not specifying the CLOBBERS may be deliberate, perhaps because the
compiler's automatic save and restore interferes with some other
property of the testing. In that case, there should be a comment
in the code, such as:
/* The __asm__ CLOBBERS are omitted because auto-save and restore
* gets in the way of computing offsets between code blocks.
* Therefore we must save and restore "by hand".
*/
|
|
From: Nicholas N. <nj...@so...> - 2023-04-18 23:26:27
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=b0e9fef2019e8ccc04bfa012077e41fc5e41f5b4 commit b0e9fef2019e8ccc04bfa012077e41fc5e41f5b4 Author: Nicholas Nethercote <n.n...@gm...> Date: Mon Apr 17 09:34:11 2023 +1000 cg_annotate: Remove the `-I`/`--include` option. For much the same reasons that I removed user annotations recently: it's rarely/never used, and complicates things. Diff: --- cachegrind/cg_annotate.in | 60 +++++++-------------------------- cachegrind/tests/Makefile.am | 1 - cachegrind/tests/ann-diff1.post.exp | 1 - cachegrind/tests/ann-diff2.post.exp | 1 - cachegrind/tests/ann-merge1.post.exp | 1 - cachegrind/tests/ann1a.post.exp | 1 - cachegrind/tests/ann1b.post.exp | 1 - cachegrind/tests/ann2-aux/ann2-via-I.rs | 1 - cachegrind/tests/ann2-past-the-end.rs | 2 ++ cachegrind/tests/ann2.cgout | 11 +++--- cachegrind/tests/ann2.post.exp | 35 +++++++------------ cachegrind/tests/ann2.vgtest | 2 +- 12 files changed, 33 insertions(+), 84 deletions(-) diff --git a/cachegrind/cg_annotate.in b/cachegrind/cg_annotate.in index 0b68e094c3..5e64a94485 100755 --- a/cachegrind/cg_annotate.in +++ b/cachegrind/cg_annotate.in @@ -51,7 +51,6 @@ class Args(Namespace): show_percs: bool annotate: bool context: int - include: list[str] cgout_filename: list[str] @staticmethod @@ -142,14 +141,6 @@ class Args(Namespace): help="print N lines of context before and after annotated lines " "(default: %(default)s)", ) - p.add_argument( - "-I", - "--include", - action="append", - default=[], - metavar="D", - help="add D to the list of searched source file directories", - ) p.add_argument( "cgout_filename", nargs=1, @@ -663,14 +654,6 @@ def print_metadata(desc: str, cmd: str, events: Events) -> None: print("Events shown: ", *events.show_events) print("Event sort order:", *events.sort_events) print("Threshold: ", args.threshold) - - if len(args.include) == 0: - print("Include dirs: ") - else: - print(f"Include dirs: {args.include[0]}") - for include_dirname in args.include[1:]: - print(f" {include_dirname}") - print("Annotation: ", "on" if args.annotate else "off") print() @@ -920,40 +903,23 @@ def print_annotated_src_files( dict_line_cc = dict_fl_dict_line_cc.pop("???", None) add_dict_line_cc_to_cc(dict_line_cc, annotated_ccs.files_unknown_cc) - # Prepend "" to the include dirnames so things work in the case where the - # filename has the full path. - include_dirnames = args.include.copy() - include_dirnames.insert(0, "") - def print_ann_fancy(src_filename: str) -> None: print_fancy(f"Annotated source file: {src_filename}") for src_filename in sorted(ann_src_filenames): - readable = False - for include_dirname in include_dirnames: - if include_dirname == "": - full_src_filename = src_filename - else: - full_src_filename = os.path.join(include_dirname, src_filename) - - try: - with open(full_src_filename, "r", encoding="utf-8") as src_file: - dict_line_cc = dict_fl_dict_line_cc.pop(src_filename, None) - assert dict_line_cc is not None - print_ann_fancy(src_file.name) # includes full path - print_annotated_src_file( - events, - dict_line_cc, - src_file, - annotated_ccs, - summary_cc, - ) - readable = True - break - except OSError: - pass - - if not readable: + try: + with open(src_filename, "r", encoding="utf-8") as src_file: + dict_line_cc = dict_fl_dict_line_cc.pop(src_filename, None) + assert dict_line_cc is not None + print_ann_fancy(src_filename) + print_annotated_src_file( + events, + dict_line_cc, + src_file, + annotated_ccs, + summary_cc, + ) + except OSError: dict_line_cc = dict_fl_dict_line_cc.pop(src_filename, None) add_dict_line_cc_to_cc(dict_line_cc, annotated_ccs.unreadable_cc) diff --git a/cachegrind/tests/Makefile.am b/cachegrind/tests/Makefile.am index 0c7219a9bc..d38d300b90 100644 --- a/cachegrind/tests/Makefile.am +++ b/cachegrind/tests/Makefile.am @@ -24,7 +24,6 @@ EXTRA_DIST = \ ann2.post.exp ann2.stderr.exp ann2.vgtest ann2.cgout \ ann2-basic.rs ann2-more-recent-than-cgout.rs \ ann2-negatives.rs ann2-past-the-end.rs \ - ann2-aux/ann2-via-I.rs \ chdir.vgtest chdir.stderr.exp \ clreq.vgtest clreq.stderr.exp \ dlclose.vgtest dlclose.stderr.exp dlclose.stdout.exp \ diff --git a/cachegrind/tests/ann-diff1.post.exp b/cachegrind/tests/ann-diff1.post.exp index 4e13f0c089..54962b513d 100644 --- a/cachegrind/tests/ann-diff1.post.exp +++ b/cachegrind/tests/ann-diff1.post.exp @@ -8,7 +8,6 @@ Events recorded: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Events shown: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Event sort order: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Threshold: 0.1 -Include dirs: Annotation: on -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann-diff2.post.exp b/cachegrind/tests/ann-diff2.post.exp index bcf09ea9ca..e1060dbd23 100644 --- a/cachegrind/tests/ann-diff2.post.exp +++ b/cachegrind/tests/ann-diff2.post.exp @@ -8,7 +8,6 @@ Events recorded: One Two Events shown: One Two Event sort order: One Two Threshold: 0.1 -Include dirs: Annotation: on -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann-merge1.post.exp b/cachegrind/tests/ann-merge1.post.exp index f12f1c235d..1f47332b8a 100644 --- a/cachegrind/tests/ann-merge1.post.exp +++ b/cachegrind/tests/ann-merge1.post.exp @@ -9,7 +9,6 @@ Events recorded: A Events shown: A Event sort order: A Threshold: 0.1 -Include dirs: Annotation: on -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann1a.post.exp b/cachegrind/tests/ann1a.post.exp index a83767cb0a..bde53e6501 100644 --- a/cachegrind/tests/ann1a.post.exp +++ b/cachegrind/tests/ann1a.post.exp @@ -10,7 +10,6 @@ Events recorded: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Events shown: Ir I1mr ILmr Event sort order: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Threshold: 0.1 -Include dirs: Annotation: on -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann1b.post.exp b/cachegrind/tests/ann1b.post.exp index b76b4236f2..3ec4288cb4 100644 --- a/cachegrind/tests/ann1b.post.exp +++ b/cachegrind/tests/ann1b.post.exp @@ -10,7 +10,6 @@ Events recorded: Ir I1mr ILmr Dr D1mr DLmr Dw D1mw DLmw Events shown: Dw Dr Ir Event sort order: Dr Threshold: 0.1 -Include dirs: Annotation: off -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann2-aux/ann2-via-I.rs b/cachegrind/tests/ann2-aux/ann2-via-I.rs deleted file mode 100644 index 5626abf0f7..0000000000 --- a/cachegrind/tests/ann2-aux/ann2-via-I.rs +++ /dev/null @@ -1 +0,0 @@ -one diff --git a/cachegrind/tests/ann2-past-the-end.rs b/cachegrind/tests/ann2-past-the-end.rs index 4cb29ea38f..b2f931a673 100644 --- a/cachegrind/tests/ann2-past-the-end.rs +++ b/cachegrind/tests/ann2-past-the-end.rs @@ -1,3 +1,5 @@ one two three +four +five diff --git a/cachegrind/tests/ann2.cgout b/cachegrind/tests/ann2.cgout index b5d62b1806..2be80fe1ea 100644 --- a/cachegrind/tests/ann2.cgout +++ b/cachegrind/tests/ann2.cgout @@ -71,9 +71,11 @@ fn=neg4 # File with source newer than the cgout file. fl=ann2-past-the-end.rs -# This filename is repeated in ann2-could-not-be-found.rs above. +# No `fn=` line yet, so the function name falls back to `<unspecified>`. +1 1000 500 0 +# This funcname is repeated in ann2-could-not-be-found.rs above. fn=f1 -1 200 100 0 +2 200 100 0 20 300 100 0 21 300 100 0 22 200 0 -1000 @@ -85,11 +87,6 @@ fn=f1 101 3000 2000 0 102 3000 2000 0 -# File found in ann2-aux/, via -I. -fl=ann2-via-I.rs -# No `fn=` line, so the function name falls back to `<unspecified>`. -1 1000 500 0 - # File below the threshold. (It also doesn't exist, but that doesn't matter. We # don't try to open it because it's below the threshold.) fl=ann2-below-threshold.rs diff --git a/cachegrind/tests/ann2.post.exp b/cachegrind/tests/ann2.post.exp index 5cfdbfd1df..8db35f136f 100644 --- a/cachegrind/tests/ann2.post.exp +++ b/cachegrind/tests/ann2.post.exp @@ -7,9 +7,6 @@ Events recorded: A SomeCount VeryLongEventName Events shown: A SomeCount VeryLongEventName Event sort order: A SomeCount VeryLongEventName Threshold: 0.5 -Include dirs: ann2-no-such-dir - ann2-no-such-dir-2 - ann2-aux Annotation: on -------------------------------------------------------------------------------- @@ -33,9 +30,9 @@ A_______________ SomeCount_______ VeryLongEventName > 9,000 (9.0%, 95.6%) 6,000 (6.0%, 99.0%) 0 (n/a, n/a) ann2-could-not-be-found.rs:f1 -> 1,000 (1.0%, 96.6%) 500 (0.5%, 99.5%) 0 (n/a, n/a) ann2-via-I.rs:<unspecified> - -> 1,000 (1.0%, 97.6%) 300 (0.3%, 99.8%) -1,000 (n/a, n/a) ann2-past-the-end.rs:f1 +> 2,000 (2.0%, 97.6%) 800 (0.8%, 99.8%) -1,000 (n/a, n/a) ann2-past-the-end.rs: + 1,000 (1.0%) 500 (0.5%) 0 <unspecified> + 1,000 (1.0%) 300 (0.3%) -1,000 (n/a) f1 > 1,000 (1.0%, 98.6%) 0 (0.0%, 99.8%) 0 (n/a, n/a) ann2-more-recent-than-cgout.rs:new @@ -66,7 +63,7 @@ A_______________ SomeCount_______ VeryLongEventName > 2,000 (2.0%, 95.1%) 100 (0.1%, 97.3%) 0 (n/a, n/a) f2:ann2-basic.rs -> 1,000 (1.0%, 96.1%) 500 (0.5%, 97.8%) 0 (n/a, n/a) <unspecified>:ann2-via-I.rs +> 1,000 (1.0%, 96.1%) 500 (0.5%, 97.8%) 0 (n/a, n/a) <unspecified>:ann2-past-the-end.rs > 1,000 (1.0%, 97.1%) 0 (0.0%, 97.8%) 0 (n/a, n/a) unknown:??? @@ -154,16 +151,17 @@ A____________________ SomeCount_____ VeryLongEventName -------------------------------------------------------------------------------- -- Annotated source file: ann2-past-the-end.rs -------------------------------------------------------------------------------- -A_________ SomeCount_ VeryLongEventName +A___________ SomeCount_ VeryLongEventName -200 (0.2%) 100 (0.1%) 0 one - . . . two - . . . three --- line 3 ---------------------------------------- +1,000 (1.0%) 500 (0.5%) 0 one + 200 (0.2%) 100 (0.1%) 0 two + . . . three + . . . four +-- line 4 ---------------------------------------- -300 (0.3%) 100 (0.1%) 0 <bogus line 20> -300 (0.3%) 100 (0.1%) 0 <bogus line 21> -200 (0.2%) 0 -1,000 (n/a) <bogus line 22> + 300 (0.3%) 100 (0.1%) 0 <bogus line 20> + 300 (0.3%) 100 (0.1%) 0 <bogus line 21> + 200 (0.2%) 0 -1,000 (n/a) <bogus line 22> @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ @@ WARNING @@ WARNING @@ WARNING @@ WARNING @@ WARNING @@ WARNING @@ WARNING @@ @@ -171,13 +169,6 @@ A_________ SomeCount_ VeryLongEventName @@ Information recorded about lines past the end of 'ann2-past-the-end.rs'. @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ --------------------------------------------------------------------------------- --- Annotated source file: ann2-aux/ann2-via-I.rs --------------------------------------------------------------------------------- -A___________ SomeCount_ VeryLongEventName - -1,000 (1.0%) 500 (0.5%) 0 one - -------------------------------------------------------------------------------- -- Annotation summary -------------------------------------------------------------------------------- diff --git a/cachegrind/tests/ann2.vgtest b/cachegrind/tests/ann2.vgtest index 9fb0d1b86f..4add2fe4cc 100644 --- a/cachegrind/tests/ann2.vgtest +++ b/cachegrind/tests/ann2.vgtest @@ -8,6 +8,6 @@ vgopts: --cachegrind-out-file=cachegrind.out # The `sleep` is to ensure the mtime of the second touched file is greater than # the mtime of the first touched file. -post: touch ann2.cgout && sleep 0.1 && touch ann2-more-recent-than-cgout.rs && python3 ../cg_annotate --context 2 --annotate --show-percs=yes --threshold=0.5 -Iann2-no-such-dir --include ann2-no-such-dir-2 -I=ann2-aux ann2.cgout +post: touch ann2.cgout && sleep 0.1 && touch ann2-more-recent-than-cgout.rs && python3 ../cg_annotate --context 2 --annotate --show-percs=yes --threshold=0.5 ann2.cgout cleanup: rm cachegrind.out |
|
From: Paul F. <pa...@so...> - 2023-04-18 20:28:44
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1e784548a17aec105ed79d53c91c3fd023b2c364 commit 1e784548a17aec105ed79d53c91c3fd023b2c364 Author: Paul Floyd <pj...@wa...> Date: Tue Apr 18 22:27:55 2023 +0200 Bug 468606 - build: remove "Valgrind relies on GCC" check/output Diff: --- NEWS | 1 + configure.ac | 6 ------ 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 696720e97e..4b18ad9d0a 100644 --- a/NEWS +++ b/NEWS @@ -154,6 +154,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 467839 Gdbserver: Improve compatibility of library directory name 468401 [PATCH] Add a style file for clang-format 468556 Build failure for vgdb +468606 build: remove "Valgrind relies on GCC" check/output 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/configure.ac b/configure.ac index a439ec85d9..8217136d9e 100755 --- a/configure.ac +++ b/configure.ac @@ -103,12 +103,6 @@ if test "x$LTO_AR" = "x"; then fi AC_ARG_VAR([LTO_AR],[Archiver command for link time optimisation]) - -# Check for the compiler support -if test "${GCC}" != "yes" ; then - AC_MSG_ERROR([Valgrind relies on GCC to be compiled]) -fi - # figure out where perl lives AC_PATH_PROG(PERL, perl) |
|
From: Carl L. <ce...@us...> - 2023-04-18 19:54:14
|
Mark:
On Mon, 2023-04-17 at 09:22 -0700, Carl Love via Valgrind-developers
wrote:
> The test_isa_3_1_R1_RT and test_isa_3_1_R1_XT tests seem to run
> differently then expected. The tests generate multiple lines of
> output
> when only one line was expected. For example:
I have pushed a fix for the two tests. The issue is the tests are
testing load instructions that load relative to the current PC address.
The tests of these instructions adds blocks of OR immediate
instructions before the assembly for the instruction under test.
Unfortunately, the test didn't save and restore the registers touched
by the OR immediate instructions. This is fine as long as you are
calling a function, the touched registers are volitile across a
function call. It seems the more recent GCC is a bit more aggressive
in inlining the test functions. However, the compiler doesn't realize
that the inline OR immediate instructions are touching registers. The
OR immediate instructions were inadvertently changing the value of the
register that held the for loop variable. Thus the loops would execute
more times then expected.
The commit is:
commit 20cc0680c3491e062c76605b24e76dc02e16ef47 (HEAD -> master)
Author: Carl Love <ce...@us...>
Date: Mon Apr 17 17:12:25 2023 -0400
PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c
If the commit gets into the current release, great. If not, it is not
an issue. The problem is completely isolated to the test case.
Carl
|
|
From: Carl L. <ca...@so...> - 2023-04-18 19:45:22
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=20cc0680c3491e062c76605b24e76dc02e16ef47 commit 20cc0680c3491e062c76605b24e76dc02e16ef47 Author: Carl Love <ce...@us...> Date: Mon Apr 17 17:12:25 2023 -0400 PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c Test adds a block of xori instructions for use with the PC relative tests. The registers used by the xori instructions need to be saved and restored, otherwise the register changes can impact the execution of the for loops in the test as registers are randomly changed. The issue occcurs when GCC is optimizing and inlining the test functions. Diff: --- none/tests/ppc64/isa_3_1_register_defines.h | 1 - none/tests/ppc64/test_isa_3_1_R1_RT.c | 264 ++++++++++++++++++++++++- none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp | 16 +- none/tests/ppc64/test_isa_3_1_R1_XT.c | 178 +++++++++++++++++ none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp | 16 +- 5 files changed, 451 insertions(+), 24 deletions(-) diff --git a/none/tests/ppc64/isa_3_1_register_defines.h b/none/tests/ppc64/isa_3_1_register_defines.h index a8c08f5910..ed74992e1f 100644 --- a/none/tests/ppc64/isa_3_1_register_defines.h +++ b/none/tests/ppc64/isa_3_1_register_defines.h @@ -46,4 +46,3 @@ extern unsigned long get_vsrhd_vs26(); extern unsigned long get_vsrhd_vs27(); extern unsigned long get_vsrhd_vs28(); extern unsigned long get_vsrhd_vs29(); - diff --git a/none/tests/ppc64/test_isa_3_1_R1_RT.c b/none/tests/ppc64/test_isa_3_1_R1_RT.c index d73b84b107..33dcddc3e5 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_RT.c +++ b/none/tests/ppc64/test_isa_3_1_R1_RT.c @@ -49,108 +49,304 @@ struct test_list_t current_test; #include "isa_3_1_helpers.h" +#ifdef __powerpc64__ +typedef uint64_t HWord_t; +/* Save and restore all of the registers but rt which is the result of the instruction + under test. Need to ensure the PAD_ORI does not change the other registers. This + really shouldn't be needed but the optimization gets messed up when it inlines the + test function. */ +#define SAVE_REGS(addr) \ + asm volatile( \ + " std 21, 0(%0) \n" \ + " std 22, 8(%0) \n" \ + " std 23, 16(%0) \n" \ + " std 24, 24(%0) \n" \ + " std 25, 32(%0) \n" \ + " std 28, 56(%0) \n" \ + " std 29, 64(%0) \n" \ + " std 30, 72(%0) \n" \ + " std 31, 80(%0) \n" \ + ::"b"(addr)) + +#define SAVE_REG_RT(addr) \ + asm volatile( \ + " std 26, 40(%0) \n" \ + ::"b"(addr)) +#define SAVE_REG_27(addr) \ + asm volatile( \ + " std 27, 40(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REGS(addr) \ + asm volatile( \ + " ld 21, 0(%0) \n" \ + " ld 22, 8(%0) \n" \ + " ld 23, 16(%0) \n" \ + " ld 24, 24(%0) \n" \ + " ld 25, 32(%0) \n" \ + " ld 28, 56(%0) \n" \ + " ld 29, 64(%0) \n" \ + " ld 30, 72(%0) \n" \ + " ld 31, 80(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_RT(addr) \ + asm volatile( \ + " ld 26, 40(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_27(addr) \ + asm volatile( \ + " ld 27, 40(%0) \n" \ + ::"b"(addr)) + +#else /* !__powerpc64__ */ + +typedef uint32_t HWord_t; +#define SAVE_REGS(addr) \ + asm volatile( \ + " stw 21, 0(%0) \n" \ + " stw 22, 4(%0) \n" \ + " stw 23, 8(%0) \n" \ + " stw 24, 12(%0) \n" \ + " stw 25, 16(%0) \n" \ + " stw 28, 28(%0) \n" \ + " stw 29, 32(%0) \n" \ + " stw 30, 36(%0) \n" \ + " stw 31, 40(%0) \n" \ + ::"b"(addr)) + +#define SAVE_REG_RT(addr) \ + asm volatile( \ + " stw 26, 20(%0) \n" \ + ::"b"(addr)) + +#define SAVE_REG_27(addr) \ + asm volatile( \ + " stw 27, 20(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REGS(addr) \ + asm volatile( \ + " lwz 21, 0(%0) \n" \ + " lwz 22, 4(%0) \n" \ + " lwz 23, 8(%0) \n" \ + " lwz 24, 12(%0) \n" \ + " lwz 25, 16(%0) \n" \ + " lwz 28, 28(%0) \n" \ + " lwz 29, 32(%0) \n" \ + " lwz 30, 36(%0) \n" \ + " lwz 31, 400(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_RT(addr) \ + asm volatile( \ + " lwz 26, 40(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_27(addr) \ + asm volatile( \ + " lwz 27, 40(%0) \n" \ + ::"b"(addr)) + +#endif /* __powerpc64__ */ + +#define NUM_ENTRIES_SAVE_RESTORE 11 + +HWord_t temp[NUM_ENTRIES_SAVE_RESTORE]; + static void test_plxvp_off0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +0(0),1" ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); + RESTORE_REG_27(temp); } static void test_plxvp_off8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +8(0),1" ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); + RESTORE_REG_27(temp); } static void test_plxvp_off16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +16(0),1" ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); + RESTORE_REG_27(temp); } static void test_plxvp_off24_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +24(0),1" ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); + RESTORE_REG_27(temp); } static void test_plxvp_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plxvp 20, +32(0),1" ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); + RESTORE_REG_27(temp); } static void test_plbz_off0_R1 (void) { - PAD_ORI + SAVE_REGS(temp); + SAVE_REG_27(temp); + PAD_ORI __asm__ __volatile__ ("plbz %0, +0(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plbz_off8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +8(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plbz_off16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +16(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plbz_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plbz_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plbz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plhz_off0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +0(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plhz_off8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +8(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plhz_off16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +16(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plhz_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plhz_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plhz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plha_off0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +0(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plha_off8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +8(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plha_off16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +16(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plha_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plha_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plha %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REG_27(temp); + RESTORE_REGS(temp); } static void test_plwz_off0_R1 (void) { __asm__ __volatile__ ("plwz %0, +0(0), 1" : "=r" (rt) ); @@ -162,15 +358,23 @@ static void test_plwz_off16_R1 (void) { __asm__ __volatile__ ("plwz %0, +16(0), 1" : "=r" (rt) ); } static void test_plwz_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwz %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plwz_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwz %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plwa_off0_R1 (void) { __asm__ __volatile__ ("plwa %0, +0(0), 1" : "=r" (rt) ); @@ -182,37 +386,69 @@ static void test_plwa_off16_R1 (void) { __asm__ __volatile__ ("plwa %0, +16(0), 1" : "=r" (rt) ); } static void test_plwa_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwa %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_plwa_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("plwa %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pld_off0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); + PAD_ORI __asm__ __volatile__ ("pld %0, +0(0), 1" : "=r" (rt) ); + PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pld_off8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); + PAD_ORI __asm__ __volatile__ ("pld %0, +8(0), 1" : "=r" (rt) ); + PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pld_off16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +16(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pld_off32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pld_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_27(temp); PAD_ORI __asm__ __volatile__ ("pld %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_27(temp); } static void test_pstb_off0_R1 (void) { __asm__ __volatile__ ("pstb %0, -0x1f400+0(0), 1" :: "r" (rs) ); @@ -298,35 +534,49 @@ static void test_paddi_98_R1 (void) { rt = 0xffff0098; } static void test_plq_off0_R1 (void) { + SAVE_REGS(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +0(0), 1" ); + __asm__ __volatile__ ("plq %0, +0(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); } static void test_plq_off8_R1 (void) { + SAVE_REGS(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +8(0), 1" ); + __asm__ __volatile__ ("plq %0, +8(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); } static void test_plq_off16_R1 (void) { + SAVE_REGS(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +16(0), 1" ); + __asm__ __volatile__ ("plq %0, +16(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); } static void test_plq_off32_R1 (void) { + SAVE_REGS(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +32(0), 1" ); + __asm__ __volatile__ ("plq %0, +32(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); } static void test_plq_off48_R1 (void) { + SAVE_REGS(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +48(0), 1" ); + __asm__ __volatile__ ("plq %0, +48(0), 1" : "=r" (rt) ); PAD_ORI + RESTORE_REGS(temp); } static void test_plq_off64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_RT(temp); PAD_ORI - __asm__ __volatile__ ("plq 26, +64(0), 1" ); + __asm__ __volatile__ ("plq %0, +64(0), 1" : "=r" (rt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_RT(temp); } static void test_pstq_off0_R1 (void) { __asm__ __volatile__ ("pstq 24, -0x1f400+0(0), 1" ); diff --git a/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp b/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp index 87594748fd..19011bc08d 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp +++ b/none/tests/ppc64/test_isa_3_1_R1_RT.stdout.exp @@ -16,9 +16,9 @@ plbz off32_R1 => 1b plbz off64_R1 => 1b -pld off0_R1 => e740000004100000 +pld off0_R1 => e74000000410001a -pld off8_R1 => 4e800020 +pld off8_R1 => 62d6001662b5001f pld off16_R1 => 6318001862f7001f @@ -52,11 +52,11 @@ plq off8_R1 => 62d6001662b5001f 6318001862f7001f plq off16_R1 => 6318001862f7001f 635a001a6339001b -plq off32_R1 => 639c001c637b001b 4e80003b +plq off32_R1 => 639c001c637b001b eac90008eaa9001b -plq off48_R1 => 1a 62d6001662b5001f +plq off48_R1 => eb090018eae9001a eb890038eb29003b -plq off64_R1 => 639c001c637b001b 4e80003b +plq off64_R1 => 1111111111111111 eac90008eaa9001b plwa off0_R1 => 4100000 @@ -82,11 +82,11 @@ plxvp off0_R1 => 6318001862f70017 635a001a63390019 ea80000004100000 62d6001662b5 plxvp off8_R1 => 635a001a63390019 639c001c637b001b 62d6001662b50015 6318001862f70017 -plxvp off16_R1 => 639c001c637b001b 000000004e800020 6318001862f70017 635a001a63390019 +plxvp off16_R1 => 639c001c637b001b eac90008eaa90000 6318001862f70017 635a001a63390019 -plxvp off24_R1 => 000000004e800020 0000000000000000 635a001a63390019 639c001c637b001b +plxvp off24_R1 => eac90008eaa90000 eb090018eae90010 635a001a63390019 639c001c637b001b -plxvp off32_R1 => 0000000000000000 62d6001662b50015 639c001c637b001b 000000004e800020 +plxvp off32_R1 => eb090018eae90010 eb890038eb290020 639c001c637b001b eac90008eaa90000 pstb off0_R1 102030405060708 => 08 diff --git a/none/tests/ppc64/test_isa_3_1_R1_XT.c b/none/tests/ppc64/test_isa_3_1_R1_XT.c index 58885b8d30..6c06ee64e4 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_XT.c +++ b/none/tests/ppc64/test_isa_3_1_R1_XT.c @@ -48,6 +48,96 @@ unsigned long current_fpscr; struct test_list_t current_test; #include "isa_3_1_helpers.h" +#ifdef __powerpc64__ +typedef uint64_t HWord_t; + +/* Save and restore all of the registers. Need to ensure the PAD_ORI does not change + the other registers. This really shouldn't be needed but the optimization gets + messed up when it inlines the test function. */ +#define SAVE_REGS(addr) \ + asm volatile( \ + " std 21, 0(%0) \n" \ + " std 22, 8(%0) \n" \ + " std 23, 16(%0) \n" \ + " std 24, 24(%0) \n" \ + " std 25, 32(%0) \n" \ + " std 26, 40(%0) \n" \ + " std 27, 48(%0) \n" \ + " std 29, 64(%0) \n" \ + " std 30, 72(%0) \n" \ + " std 31, 80(%0) \n" \ + ::"b"(addr)) + +#define SAVE_REG_28(addr) \ + asm volatile( \ + " std 28, 56(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REGS(addr) \ + asm volatile( \ + " ld 21, 0(%0) \n" \ + " ld 22, 8(%0) \n" \ + " ld 23, 16(%0) \n" \ + " ld 24, 24(%0) \n" \ + " ld 25, 32(%0) \n" \ + " ld 26, 40(%0) \n" \ + " ld 27, 48(%0) \n" \ + " ld 29, 64(%0) \n" \ + " ld 30, 72(%0) \n" \ + " ld 31, 80(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_28(addr) \ + asm volatile( \ + " ld 28, 56(%0) \n" \ + ::"b"(addr)) + +#else /* !__powerpc64__ */ + +typedef uint32_t HWord_t; +#define SAVE_REGS(addr) \ + asm volatile( \ + " stw 21, 0(%0) \n" \ + " stw 22, 4(%0) \n" \ + " stw 23, 8(%0) \n" \ + " stw 24, 12(%0) \n" \ + " stw 25, 16(%0) \n" \ + " stw 26, 20(%0) \n" \ + " stw 27, 24(%0) \n" \ + " stw 29, 32(%0) \n" \ + " stw 30, 36(%0) \n" \ + " stw 31, 40(%0) \n" \ + ::"b"(addr)) + +#define SAVE_REG_28(addr) \ + asm volatile( \ + " stw 28, 28(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REGS(addr) \ + asm volatile( \ + " lwz 21, 0(%0) \n" \ + " lwz 22, 4(%0) \n" \ + " lwz 23, 8(%0) \n" \ + " lwz 24, 12(%0) \n" \ + " lwz 25, 16(%0) \n" \ + " lwz 26, 20(%0) \n" \ + " lwz 27, 24(%0) \n" \ + " lwz 29, 32(%0) \n" \ + " lwz 30, 36(%0) \n" \ + " lwz 31, 400(%0) \n" \ + ::"b"(addr)) + +#define RESTORE_REG_28(addr) \ + asm volatile( \ + " lwz 28, 28(%0) \n" \ + ::"b"(addr)) +#endif /* __powerpc64__ */ + +#define NUM_ENTRIES_SAVE_RESTORE 11 + +HWord_t temp[NUM_ENTRIES_SAVE_RESTORE]; + static void test_pstxvp_off0_R1 (void) { __asm__ __volatile__ ("pstxvp 20, -0x1f400+0(0),1"); } @@ -61,54 +151,78 @@ static void test_pstxvp_off48_R1 (void) { __asm__ __volatile__ ("pstxvp 20, -0x1f400+48(0),1"); } static void test_plfd_64_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +64(0), 1"); PAD_ORI PAD_ORI + RESTORE_REGS(temp); } static void test_plfd_32_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +32(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfd_16_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +16(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfd_8_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +8(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfd_4_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +4(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfd_0_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfd 28, +0(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_64_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +64(0), 1"); PAD_ORI PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_32_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +32(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_16_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +16(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_8_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +8(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_4_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +4(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_plfs_0_R1 (void) { + SAVE_REGS(temp); __asm__ __volatile__ ("plfs 28, +0(0), 1"); PAD_ORI + RESTORE_REGS(temp); } static void test_pstfd_32_R1 (void) { __asm__ __volatile__ ("pstfd 26, -0x1f400+32(0), 1"); @@ -141,54 +255,102 @@ static void test_pstfs_0_R1 (void) { __asm__ __volatile__ ("pstfs 26, -0x1f400+0(0), 1"); } static void test_plxsd_64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +64(0), 1" : "=v" (vrt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxsd_32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ (".align 2 ; plxsd %0, +32(0), 1" : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxsd_16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +16(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxsd_8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +8(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxsd_4_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +4(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxsd_0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxsd %0, +0(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_64_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +64(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_32_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +32(0), 1; pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +16(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +8(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_4_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +4(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxssp_0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxssp %0, +0(0), 1; pnop;pnop;pnop; " : "=v" (vrt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } /* Follow the short-range plxv instructions with nop in order to pad out subsequent instructions. When written there are found @@ -196,20 +358,36 @@ static void test_plxssp_0_R1 (void) { into the target variable. (pla,pstxv...). */ static void test_plxv_16_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +16(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxv_8_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +8(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxv_4_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +4(0), 1; pnop;pnop;pnop;" : "=wa" (vec_xt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_plxv_0_R1 (void) { + SAVE_REGS(temp); + SAVE_REG_28(temp); __asm__ __volatile__ ("plxv %x0, +0(0), 1; pnop;pnop;pnop; " : "=wa" (vec_xt) ); PAD_ORI + RESTORE_REGS(temp); + RESTORE_REG_28(temp); } static void test_pstxsd_64_R1 (void) { __asm__ __volatile__ (".align 2 ; pstxsd 22, -0x1f400+64(0), 1" ); diff --git a/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp b/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp index 48d591f4df..cef5c773fb 100644 --- a/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp +++ b/none/tests/ppc64/test_isa_3_1_R1_XT.stdout.exp @@ -26,9 +26,9 @@ plxsd 0_R1 => a800000004100000,0000000000000000 -5.07588375e-116 +Zer plxsd 4_R1 => 7000000a8000004,0000000000000000 5.77662562e-275 +Zero -plxsd 8_R1 => 700000060000000,0000000000000000 5.77662407e-275 +Zero +plxsd 8_R1 => 7000000,0000000000000000 +Den +Zero -plxsd 16_R1 => 7000000,0000000000000000 +Den +Zero +plxsd 16_R1 => 700000060000000,0000000000000000 5.77662407e-275 +Zero plxsd 32_R1 => 6339001963180018,0000000000000000 9.43505226e+169 +Zero @@ -38,21 +38,21 @@ plxssp 0_R1 => 3882000000000000,0000000000000000 6.19888e-05 +Zero plxssp 4_R1 => bd80000080000000,0000000000000000 -6.25000e-02 -Zero +Zero +Zero -plxssp 8_R1 => 38e0000000000000,0000000000000000 1.06812e-04 +Zero +Zero +Zero +plxssp 8_R1 => 4400000000000000,0000000000000000 5.12000e+02 +Zero +Zero +Zero plxssp 16_R1 => 38e0000000000000,0000000000000000 1.06812e-04 +Zero +Zero +Zero plxssp 32_R1 => 445ac002c0000000,0000000000000000 8.75000e+02 -2.00000e+00 +Zero +Zero -plxssp 64_R1 => 446b400340000000,0000000000000000 9.41000e+02 2.00000e+00 +Zero +Zero +plxssp 64_R1 => 4467200320000000,0000000000000000 9.24500e+02 1.08420e-19 +Zero +Zero -plxv 0_R1 => c800000004100000 7000000 +plxv 0_R1 => c800000004100000 700000060000000 -plxv 4_R1 => 7000000c8000004 700000000000000 +plxv 4_R1 => 60000000c8000004 7000000 -plxv 8_R1 => 7000000 7000000 +plxv 8_R1 => 700000060000000 700000000000000 -plxv 16_R1 => 7000000 7000000 +plxv 16_R1 => 700000000000000 700000000000000 pstfd 0_R1 43dfe000003fe000 43eff000000ff000 => e000003fe00043df pstfd 0_R1 43eff000000ff000 43efefffffcff000 => f000000ff00043ef |
|
From: Paul F. <pa...@so...> - 2023-04-18 19:19:11
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=04054f36be59eeb337f23932424a7e70bbfeba70 commit 04054f36be59eeb337f23932424a7e70bbfeba70 Author: Paul Floyd <pj...@wa...> Date: Tue Apr 18 21:18:12 2023 +0200 regtest: try to make the nightly script independent of test times Diff: --- nightly/bin/nightly | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nightly/bin/nightly b/nightly/bin/nightly index d4783f95d4..e41510e51e 100755 --- a/nightly/bin/nightly +++ b/nightly/bin/nightly @@ -146,6 +146,9 @@ for logfile in old new ; do # Grab some indicative text for the short log file -- if the regtests # succeeded, show their results. If we didn't make it that far, show the # last 20 lines. + # Change 68cf3b5dbfecb96c618c371359000daaaf4293b5 added time information to the + # logs, which is fairly variable. The sed filter deletes this so that we don't + # generate spurious diffs. egrep -q '^== [0-9]+ tests' $logfile.verbose && ( echo >> $logfile.short echo "Regression test results follow" >> $logfile.short @@ -157,7 +160,7 @@ for logfile in old new ; do echo >> $logfile.short echo "Last 20 lines of verbose log follow" >> $logfile.short \ echo >> $logfile.short - tail -20 $logfile.verbose >> $logfile.short + tail -20 $logfile.verbose | sed 's/(in [0-9]* sec) -*//' >> $logfile.short fi ) || ( echo >> $logfile.short |
|
From: Philippe W. <phi...@sk...> - 2023-04-18 11:02:58
|
The nightly build script produces a mail that indicates if there is a difference between the results of one day ago and the new results. When no difference, the mail subject contains 'unchanged'. It looks like the 'unchanged' logic is broken due to the addition of the time taken to run tests. It would be good to keep the 'unchanged' marker only depending on the functional results. Thanks Philippe |
|
From: Julian S. <jse...@gm...> - 2023-04-18 07:32:11
|
On 18/04/2023 09:14, Paul Floyd wrote: > On 18-04-23 05:30, Nicholas Nethercote wrote: >> Is there any appetite for clang-formatting Valgrind's code? > All that to say is that I'm in favour of using clang-format, Me too; +1 for that. I've lived with the Firefox C++ auto-format stuff for some years now and that has worked out well. In hindsight I'd change the 3 char indents to 2; 2 works well for Fx, and maintain a max width of 80. J |
|
From: Nicholas N. <n.n...@gm...> - 2023-04-18 07:28:14
|
If I had a single vote for a single element of a new style, it would be to change the 3 space indents to either 4 or 2 :) Nick On Tue, 18 Apr 2023 at 16:49, Paul Floyd <pj...@wa...> wrote: > > > On 18-04-23 05:41, Eyal Soha wrote: > > The problem with doing this is that it really messes with the git blame, > > introducing a lot of changes! > > > > If you do this, you should probably add some sort of formatting check to > > a CI process somewhere, otherwise your work will get stale and you'll > > just be doing the clang-format again in a year from now. > > My goal would be to have some sort of git trigger that maintains > formatting. > > > Good luck to you trying to get everyone to agree on a format! Ha! > > I think that there the battle is mostly won. There are informal "house > rules" and most of the code is 3 space indented with cuddle braces. > There is still a bit of variation (e.g., whether braces indent after > switch or not). > > It is also possible to 'protect' blocks of code from clang-format e.g., > > > int formatted_code; > // clang-format off > void unformatted_code ; > // clang-format on > > (copied from https://clang.llvm.org/docs/ClangFormatStyleOptions.html) > > A+ > Paul > > > > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
From: Paul F. <pj...@wa...> - 2023-04-18 07:14:34
|
On 18-04-23 05:30, Nicholas Nethercote wrote: > Is there any appetite for clang-formatting Valgrind's code? At work, we have numerous projects that have been worked on by large numbers of people with probably just about every imaginable free code editor. And over 30 years or more. Some teams and sub-projects have fairly consistent formatting. Others have random mixes of tabs and spaces. It drives me mad (it also drives clang-tidy and gcc mad with lots of warnings about inconsistent indentation). All that to say is that I'm in favour of using clang-format, A+ Paul |
|
From: Paul F. <pj...@wa...> - 2023-04-18 06:48:38
|
On 18-04-23 05:41, Eyal Soha wrote:
> The problem with doing this is that it really messes with the git blame,
> introducing a lot of changes!
>
> If you do this, you should probably add some sort of formatting check to
> a CI process somewhere, otherwise your work will get stale and you'll
> just be doing the clang-format again in a year from now.
My goal would be to have some sort of git trigger that maintains formatting.
> Good luck to you trying to get everyone to agree on a format! Ha!
I think that there the battle is mostly won. There are informal "house
rules" and most of the code is 3 space indented with cuddle braces.
There is still a bit of variation (e.g., whether braces indent after
switch or not).
It is also possible to 'protect' blocks of code from clang-format e.g.,
int formatted_code;
// clang-format off
void unformatted_code ;
// clang-format on
(copied from https://clang.llvm.org/docs/ClangFormatStyleOptions.html)
A+
Paul
|
|
From: Nicholas N. <n.n...@gm...> - 2023-04-18 05:05:48
|
On Tue, 18 Apr 2023 at 13:41, Eyal Soha <eya...@gm...> wrote: > The problem with doing this is that it really messes with the git blame, > introducing a lot of changes! > That's always the first objection that is raised. Turns out there's a good solution <https://medium.com/codex/how-to-introduce-a-code-formatter-without-messing-up-git-history-4a16bd074c10> . > If you do this, you should probably add some sort of formatting check to a > CI process somewhere, otherwise your work will get stale and you'll just be > doing the clang-format again in a year from now. > Yes. > Good luck to you trying to get everyone to agree on a format! Ha! > It requires negotiation, but it's doable. Remember, all of this was done successfully for Firefox, which is a much bigger and gnarlier codebase than Valgrind. Nick |
|
From: Eyal S. <eya...@gm...> - 2023-04-18 03:41:47
|
The problem with doing this is that it really messes with the git blame, introducing a lot of changes! If you do this, you should probably add some sort of formatting check to a CI process somewhere, otherwise your work will get stale and you'll just be doing the clang-format again in a year from now. Good luck to you trying to get everyone to agree on a format! Ha! Eyal On Mon, Apr 17, 2023 at 9:31 PM Nicholas Nethercote <n.n...@gm...> wrote: > Is there any appetite for clang-formatting Valgrind's code? I've now used > auto-formatters in C++, Rust, and Python, and found it an excellent > experience, and I get annoyed when working on code without auto-formatting. > The C++ case was Firefox, where a large and old codebase was formatted. So > it is possible (and better, IMO) to not just limit it to new code. > > It could certainly be done in pieces, e.g. one directory at a time, > something like that. > > Nick > > On Tue, 18 Apr 2023 at 06:07, Paul Floyd <pa...@so...> wrote: > >> >> https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1b3430761f6bda43b8187dbd342b34cb5c99df3f >> >> commit 1b3430761f6bda43b8187dbd342b34cb5c99df3f >> Author: Paul Floyd <pj...@wa...> >> Date: Mon Apr 17 22:05:30 2023 +0200 >> >> Bug 468401 - [PATCH] Add a style file for clang-format >> >> Patch submitted by: >> Petr Pavlu <pet...@da...> >> >> Diff: >> --- >> .clang-format | 17 +++++++++++++++++ >> .gitignore | 1 + >> NEWS | 1 + >> README_DEVELOPERS | 18 ++++++++++++++++++ >> 4 files changed, 37 insertions(+) >> >> diff --git a/.clang-format b/.clang-format >> new file mode 100644 >> index 0000000000..4450e737ac >> --- /dev/null >> +++ b/.clang-format >> @@ -0,0 +1,17 @@ >> +--- >> +Language: Cpp >> +BasedOnStyle: LLVM >> + >> +AlignConsecutiveAssignments: true >> +AlignConsecutiveDeclarations: true >> +AlignConsecutiveMacros: true >> +AllowAllParametersOfDeclarationOnNextLine: true >> +BinPackParameters: false >> +BreakBeforeBraces: Linux >> +ContinuationIndentWidth: 3 >> +IndentWidth: 3 >> +PointerAlignment: Left >> +# Mark the VG_(), ML_() and tool macros as type declarations which they >> are >> +# sufficiently close to, otherwise clang-format gets confused by them. >> +TypenameMacros: [VG_, ML_, CLG_, DRD_, HG_, MC_] >> +... >> diff --git a/.gitignore b/.gitignore >> index a88ab4dd43..6622e7c59e 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -3,6 +3,7 @@ >> # / >> /.in_place >> /.vs >> +/.clang-format >> /acinclude.m4 >> /aclocal.m4 >> /autom4te-*.cache >> diff --git a/NEWS b/NEWS >> index 43ff9766bd..696720e97e 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -152,6 +152,7 @@ are not entered into bugzilla tend to get forgotten >> about or ignored. >> 467714 fdleak_* and rlimit tests fail when parent process has more than >> 64 descriptors opened >> 467839 Gdbserver: Improve compatibility of library directory name >> +468401 [PATCH] Add a style file for clang-format >> 468556 Build failure for vgdb >> n-i-bz FreeBSD rfork syscall fail with EINVAL or ENOSYS rather than >> VG_(unimplemented) >> >> diff --git a/README_DEVELOPERS b/README_DEVELOPERS >> index 9c04763d47..979ee13b4a 100644 >> --- a/README_DEVELOPERS >> +++ b/README_DEVELOPERS >> @@ -372,3 +372,21 @@ translated, and that includes the address. >> Then re-run with 999999 changed to the highest bb number shown. >> This will print the one line per block, and also will print a >> disassembly of the block in which the fault occurred. >> + >> + >> +Formatting the code with clang-format >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> +clang-format is a tool to format C/C++/... code. The root directory of >> the >> +Valgrind tree contains file .clang-format which is a configuration for >> this tool >> +and specifies a style for Valgrind. This gives you an option to use >> +clang-format to easily format Valgrind code which you are modifying. >> + >> +The Valgrind codebase is not globally formatted with clang-format. It >> means >> +that you should not use the tool to format a complete file after making >> changes >> +in it because that would lead to creating unrelated modifications. >> + >> +The right approach is to format only updated or new code. By using an >> +integration with a text editor, it is possible to reformat arbitrary >> blocks >> +of code with a single keystroke. Refer to the upstream documentation >> which >> +describes integration with various editors and IDEs: >> +https://clang.llvm.org/docs/ClangFormat.html. >> >> >> _______________________________________________ >> Valgrind-developers mailing list >> Val...@li... >> https://lists.sourceforge.net/lists/listinfo/valgrind-developers >> > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
From: Nicholas N. <n.n...@gm...> - 2023-04-18 03:30:26
|
Is there any appetite for clang-formatting Valgrind's code? I've now used auto-formatters in C++, Rust, and Python, and found it an excellent experience, and I get annoyed when working on code without auto-formatting. The C++ case was Firefox, where a large and old codebase was formatted. So it is possible (and better, IMO) to not just limit it to new code. It could certainly be done in pieces, e.g. one directory at a time, something like that. Nick On Tue, 18 Apr 2023 at 06:07, Paul Floyd <pa...@so...> wrote: > > https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1b3430761f6bda43b8187dbd342b34cb5c99df3f > > commit 1b3430761f6bda43b8187dbd342b34cb5c99df3f > Author: Paul Floyd <pj...@wa...> > Date: Mon Apr 17 22:05:30 2023 +0200 > > Bug 468401 - [PATCH] Add a style file for clang-format > > Patch submitted by: > Petr Pavlu <pet...@da...> > > Diff: > --- > .clang-format | 17 +++++++++++++++++ > .gitignore | 1 + > NEWS | 1 + > README_DEVELOPERS | 18 ++++++++++++++++++ > 4 files changed, 37 insertions(+) > > diff --git a/.clang-format b/.clang-format > new file mode 100644 > index 0000000000..4450e737ac > --- /dev/null > +++ b/.clang-format > @@ -0,0 +1,17 @@ > +--- > +Language: Cpp > +BasedOnStyle: LLVM > + > +AlignConsecutiveAssignments: true > +AlignConsecutiveDeclarations: true > +AlignConsecutiveMacros: true > +AllowAllParametersOfDeclarationOnNextLine: true > +BinPackParameters: false > +BreakBeforeBraces: Linux > +ContinuationIndentWidth: 3 > +IndentWidth: 3 > +PointerAlignment: Left > +# Mark the VG_(), ML_() and tool macros as type declarations which they > are > +# sufficiently close to, otherwise clang-format gets confused by them. > +TypenameMacros: [VG_, ML_, CLG_, DRD_, HG_, MC_] > +... > diff --git a/.gitignore b/.gitignore > index a88ab4dd43..6622e7c59e 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -3,6 +3,7 @@ > # / > /.in_place > /.vs > +/.clang-format > /acinclude.m4 > /aclocal.m4 > /autom4te-*.cache > diff --git a/NEWS b/NEWS > index 43ff9766bd..696720e97e 100644 > --- a/NEWS > +++ b/NEWS > @@ -152,6 +152,7 @@ are not entered into bugzilla tend to get forgotten > about or ignored. > 467714 fdleak_* and rlimit tests fail when parent process has more than > 64 descriptors opened > 467839 Gdbserver: Improve compatibility of library directory name > +468401 [PATCH] Add a style file for clang-format > 468556 Build failure for vgdb > n-i-bz FreeBSD rfork syscall fail with EINVAL or ENOSYS rather than > VG_(unimplemented) > > diff --git a/README_DEVELOPERS b/README_DEVELOPERS > index 9c04763d47..979ee13b4a 100644 > --- a/README_DEVELOPERS > +++ b/README_DEVELOPERS > @@ -372,3 +372,21 @@ translated, and that includes the address. > Then re-run with 999999 changed to the highest bb number shown. > This will print the one line per block, and also will print a > disassembly of the block in which the fault occurred. > + > + > +Formatting the code with clang-format > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > +clang-format is a tool to format C/C++/... code. The root directory of > the > +Valgrind tree contains file .clang-format which is a configuration for > this tool > +and specifies a style for Valgrind. This gives you an option to use > +clang-format to easily format Valgrind code which you are modifying. > + > +The Valgrind codebase is not globally formatted with clang-format. It > means > +that you should not use the tool to format a complete file after making > changes > +in it because that would lead to creating unrelated modifications. > + > +The right approach is to format only updated or new code. By using an > +integration with a text editor, it is possible to reformat arbitrary > blocks > +of code with a single keystroke. Refer to the upstream documentation > which > +describes integration with various editors and IDEs: > +https://clang.llvm.org/docs/ClangFormat.html. > > > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |