You can subscribe to this list here.
| 2002 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
(1) |
Oct
(122) |
Nov
(152) |
Dec
(69) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2003 |
Jan
(6) |
Feb
(25) |
Mar
(73) |
Apr
(82) |
May
(24) |
Jun
(25) |
Jul
(10) |
Aug
(11) |
Sep
(10) |
Oct
(54) |
Nov
(203) |
Dec
(182) |
| 2004 |
Jan
(307) |
Feb
(305) |
Mar
(430) |
Apr
(312) |
May
(187) |
Jun
(342) |
Jul
(487) |
Aug
(637) |
Sep
(336) |
Oct
(373) |
Nov
(441) |
Dec
(210) |
| 2005 |
Jan
(385) |
Feb
(480) |
Mar
(636) |
Apr
(544) |
May
(679) |
Jun
(625) |
Jul
(810) |
Aug
(838) |
Sep
(634) |
Oct
(521) |
Nov
(965) |
Dec
(543) |
| 2006 |
Jan
(494) |
Feb
(431) |
Mar
(546) |
Apr
(411) |
May
(406) |
Jun
(322) |
Jul
(256) |
Aug
(401) |
Sep
(345) |
Oct
(542) |
Nov
(308) |
Dec
(481) |
| 2007 |
Jan
(427) |
Feb
(326) |
Mar
(367) |
Apr
(255) |
May
(244) |
Jun
(204) |
Jul
(223) |
Aug
(231) |
Sep
(354) |
Oct
(374) |
Nov
(497) |
Dec
(362) |
| 2008 |
Jan
(322) |
Feb
(482) |
Mar
(658) |
Apr
(422) |
May
(476) |
Jun
(396) |
Jul
(455) |
Aug
(267) |
Sep
(280) |
Oct
(253) |
Nov
(232) |
Dec
(304) |
| 2009 |
Jan
(486) |
Feb
(470) |
Mar
(458) |
Apr
(423) |
May
(696) |
Jun
(461) |
Jul
(551) |
Aug
(575) |
Sep
(134) |
Oct
(110) |
Nov
(157) |
Dec
(102) |
| 2010 |
Jan
(226) |
Feb
(86) |
Mar
(147) |
Apr
(117) |
May
(107) |
Jun
(203) |
Jul
(193) |
Aug
(238) |
Sep
(300) |
Oct
(246) |
Nov
(23) |
Dec
(75) |
| 2011 |
Jan
(133) |
Feb
(195) |
Mar
(315) |
Apr
(200) |
May
(267) |
Jun
(293) |
Jul
(353) |
Aug
(237) |
Sep
(278) |
Oct
(611) |
Nov
(274) |
Dec
(260) |
| 2012 |
Jan
(303) |
Feb
(391) |
Mar
(417) |
Apr
(441) |
May
(488) |
Jun
(655) |
Jul
(590) |
Aug
(610) |
Sep
(526) |
Oct
(478) |
Nov
(359) |
Dec
(372) |
| 2013 |
Jan
(467) |
Feb
(226) |
Mar
(391) |
Apr
(281) |
May
(299) |
Jun
(252) |
Jul
(311) |
Aug
(352) |
Sep
(481) |
Oct
(571) |
Nov
(222) |
Dec
(231) |
| 2014 |
Jan
(185) |
Feb
(329) |
Mar
(245) |
Apr
(238) |
May
(281) |
Jun
(399) |
Jul
(382) |
Aug
(500) |
Sep
(579) |
Oct
(435) |
Nov
(487) |
Dec
(256) |
| 2015 |
Jan
(338) |
Feb
(357) |
Mar
(330) |
Apr
(294) |
May
(191) |
Jun
(108) |
Jul
(142) |
Aug
(261) |
Sep
(190) |
Oct
(54) |
Nov
(83) |
Dec
(22) |
| 2016 |
Jan
(49) |
Feb
(89) |
Mar
(33) |
Apr
(50) |
May
(27) |
Jun
(34) |
Jul
(53) |
Aug
(53) |
Sep
(98) |
Oct
(206) |
Nov
(93) |
Dec
(53) |
| 2017 |
Jan
(65) |
Feb
(82) |
Mar
(102) |
Apr
(86) |
May
(187) |
Jun
(67) |
Jul
(23) |
Aug
(93) |
Sep
(65) |
Oct
(45) |
Nov
(35) |
Dec
(17) |
| 2018 |
Jan
(26) |
Feb
(35) |
Mar
(38) |
Apr
(32) |
May
(8) |
Jun
(43) |
Jul
(27) |
Aug
(30) |
Sep
(43) |
Oct
(42) |
Nov
(38) |
Dec
(67) |
| 2019 |
Jan
(32) |
Feb
(37) |
Mar
(53) |
Apr
(64) |
May
(49) |
Jun
(18) |
Jul
(14) |
Aug
(53) |
Sep
(25) |
Oct
(30) |
Nov
(49) |
Dec
(31) |
| 2020 |
Jan
(87) |
Feb
(45) |
Mar
(37) |
Apr
(51) |
May
(99) |
Jun
(36) |
Jul
(11) |
Aug
(14) |
Sep
(20) |
Oct
(24) |
Nov
(40) |
Dec
(23) |
| 2021 |
Jan
(14) |
Feb
(53) |
Mar
(85) |
Apr
(15) |
May
(19) |
Jun
(3) |
Jul
(14) |
Aug
(1) |
Sep
(57) |
Oct
(73) |
Nov
(56) |
Dec
(22) |
| 2022 |
Jan
(3) |
Feb
(22) |
Mar
(6) |
Apr
(55) |
May
(46) |
Jun
(39) |
Jul
(15) |
Aug
(9) |
Sep
(11) |
Oct
(34) |
Nov
(20) |
Dec
(36) |
| 2023 |
Jan
(79) |
Feb
(41) |
Mar
(99) |
Apr
(169) |
May
(48) |
Jun
(16) |
Jul
(16) |
Aug
(57) |
Sep
(19) |
Oct
|
Nov
|
Dec
|
| S | M | T | W | T | F | S |
|---|---|---|---|---|---|---|
|
|
|
|
|
|
|
1
(6) |
|
2
(6) |
3
(9) |
4
(4) |
5
(1) |
6
|
7
|
8
|
|
9
|
10
(2) |
11
(1) |
12
(2) |
13
(4) |
14
(6) |
15
(8) |
|
16
(9) |
17
(5) |
18
(13) |
19
(6) |
20
(15) |
21
(17) |
22
(19) |
|
23
(2) |
24
(4) |
25
(2) |
26
(10) |
27
(6) |
28
(9) |
29
(3) |
|
30
|
|
|
|
|
|
|
|
From: Nicholas N. <n.n...@gm...> - 2023-04-20 23:06:24
|
On Fri, 21 Apr 2023 at 07:06, Bart Van Assche <bva...@ac...> wrote: > No matter how > much time is spent on tuning the .clang-format file, there will always > be code for which the formatting is made worse by clang-format than the > existing code. > It's true that there are rare cases where an auto-formatter does a bad job, such as code in tabular form. Fortunately there's an easy workaround for that too: you just put `// clang-format off` at the start of the block and `// clang-format on` at the end of the block. Nick |
|
From: Bart V. A. <bva...@ac...> - 2023-04-20 21:06:51
|
On 4/20/23 13:51, Nicholas Nethercote wrote: > On Fri, 21 Apr 2023 at 06:02, Mark Wielaard <ma...@kl... > <mailto:ma...@kl...>> wrote: > I am not a fan, but also not dead against. > > Have you ever worked on a project that uses auto-formatting? Skepticism > followed by enthusiasm is common. Paul, Julian, and I all have used it > on other codebases and are now advocates. I'm using it for Cachegrind's > Python code right now. It makes life easier. I have worked on large projects that use auto-formatting. Despite this I'm strongly opposed against reformatting existing code. No matter how much time is spent on tuning the .clang-format file, there will always be code for which the formatting is made worse by clang-format than the existing code. Bart. |
|
From: Nicholas N. <n.n...@gm...> - 2023-04-20 20:52:09
|
On Fri, 21 Apr 2023 at 06:02, Mark Wielaard <ma...@kl...> wrote: > > Sure you can work around it, but I don't think that is a great > solution. It requires everyone to make some local changes. > You can send up a .gitconfig for the project, so it'll work automatically for everyone. > I am not a fan, but also not dead against. > Have you ever worked on a project that uses auto-formatting? Skepticism followed by enthusiasm is common. Paul, Julian, and I all have used it on other codebases and are now advocates. I'm using it for Cachegrind's Python code right now. It makes life easier. Personally I am happy with emacs M-x indent-region on the code I edit. > There is a .dir-locals.el in git which catches some (but certainly not > all) formatting things. > What about people who don't use emacs? Nick |
|
From: Paul F. <pa...@so...> - 2023-04-20 20:40:36
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=a2af9adec4536c3ce95d4ea5ddf15d00d7c55c6c commit a2af9adec4536c3ce95d4ea5ddf15d00d7c55c6c Author: Paul Floyd <pj...@wa...> Date: Thu Apr 20 22:11:31 2023 +0200 Bug 397083 - Likely false positive "uninitialised value(s)" for __wmemchr_avx2 and __wmemcmp_avx2_movbe Diff: --- .gitignore | 1 + NEWS | 1 + memcheck/tests/Makefile.am | 2 ++ memcheck/tests/wmemcmp.c | 17 +++++++++++++++++ memcheck/tests/wmemcmp.stderr.exp | 0 memcheck/tests/wmemcmp.vgtest | 2 ++ shared/vg_replace_strmem.c | 21 +++++++++++++++++++++ 7 files changed, 44 insertions(+) diff --git a/.gitignore b/.gitignore index 6622e7c59e..9e26e2fbf5 100644 --- a/.gitignore +++ b/.gitignore @@ -1004,6 +1004,7 @@ /memcheck/tests/wcs /memcheck/tests/weirdioctl /memcheck/tests/with space +/memcheck/tests/wmemcmp /memcheck/tests/wrap1 /memcheck/tests/wrap2 /memcheck/tests/wrap3 diff --git a/NEWS b/NEWS index 4ba0c31709..57e39f42a3 100644 --- a/NEWS +++ b/NEWS @@ -126,6 +126,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 351857 confusing error message about valid command line option 374596 inconsistent RDTSCP support on x86_64 392331 Spurious lock not held error from inside pthread_cond_timedwait +397083 Likely false positive "uninitialised value(s)" for __wmemchr_avx2 and __wmemcmp_avx2_movbe 400793 pthread_rwlock_timedwrlock false positive 419054 Unhandled syscall getcpu on arm32 433873 openat2 syscall unimplemented on Linux diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index b4d80f837a..faf9130bcd 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -376,6 +376,7 @@ EXTRA_DIST = \ vcpu_fnfns.stdout.exp-darwin vcpu_fnfns.stdout.exp-solaris \ vcpu_fnfns.stderr.exp vcpu_fnfns.vgtest \ wcs.vgtest wcs.stderr.exp wcs.stdout.exp \ + wmemcmp.vgtest wmemcmp.stderr.exp \ wrap1.vgtest wrap1.stdout.exp wrap1.stderr.exp \ wrap2.vgtest wrap2.stdout.exp wrap2.stderr.exp \ wrap3.vgtest wrap3.stdout.exp wrap3.stderr.exp \ @@ -478,6 +479,7 @@ check_PROGRAMS = \ vcpu_fbench vcpu_fnfns \ wcs \ xml1 \ + wmemcmp \ wrap1 wrap2 wrap3 wrap4 wrap5 wrap6 wrap7 wrap7so.so wrap8 \ wrapmalloc wrapmallocso.so wrapmallocstatic \ writev1 diff --git a/memcheck/tests/wmemcmp.c b/memcheck/tests/wmemcmp.c new file mode 100644 index 0000000000..d737620094 --- /dev/null +++ b/memcheck/tests/wmemcmp.c @@ -0,0 +1,17 @@ +#include <stdlib.h> +#include <wchar.h> + +int main () +{ + wchar_t *s = (wchar_t *) malloc (8 * sizeof (wchar_t)); + s[0] = '-'; + s[1] = 'N'; + s[2] = 'A'; + s[3] = 'N'; + s[4] = ' '; + s[5] = '3'; + s[6] = '3'; + s[7] = '\0'; + return wmemcmp (s + 1, L"NAN", 3) == 0; +} + diff --git a/memcheck/tests/wmemcmp.stderr.exp b/memcheck/tests/wmemcmp.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/wmemcmp.vgtest b/memcheck/tests/wmemcmp.vgtest new file mode 100644 index 0000000000..637a2335c0 --- /dev/null +++ b/memcheck/tests/wmemcmp.vgtest @@ -0,0 +1,2 @@ +prog: wmemcmp +vgopts: -q diff --git a/shared/vg_replace_strmem.c b/shared/vg_replace_strmem.c index 30065d537a..b32f13f76d 100644 --- a/shared/vg_replace_strmem.c +++ b/shared/vg_replace_strmem.c @@ -104,6 +104,7 @@ 20440 WCSNLEN 20450 WSTRNCMP 20460 MEMMEM + 20470 WMEMCMP */ #if defined(VGO_solaris) @@ -2262,6 +2263,26 @@ static inline void my_exit ( int x ) #if defined(VGO_freebsd) WMEMCHR(VG_Z_LIBC_SONAME, wmemchr) #endif + + +#define WMEMCMP(soname, fnname) \ + int VG_REPLACE_FUNCTION_EZU(20470,soname,fnname) \ + ( const Int *b1, const Int *b2, SizeT n ); \ + int VG_REPLACE_FUNCTION_EZU(20470,soname,fnname) \ + ( const Int *b1, const Int *b2, SizeT n ) \ + { \ + for (SizeT i = 0U; i < n; ++i) { \ + if (b1[i] != b2[i]) \ + return b1[i] > b2[i] ? 1 : -1; \ + } \ + return 0; \ + } + +#if defined(VGO_linux) + WMEMCMP(VG_Z_LIBC_SONAME, wmemcmp) +#endif + + /*------------------------------------------------------------*/ /*--- Improve definedness checking of process environment ---*/ /*------------------------------------------------------------*/ |
|
From: Bart V. A. <bva...@ac...> - 2023-04-20 20:23:04
|
On 4/17/23 20:30, Nicholas Nethercote wrote: > Is there any appetite for clang-formatting Valgrind's code? Why to reformat the entire code base? Auto-formatting can be used without reformatting the entire code base. This is how I reformat Linux kernel and Android patches before I publish these: git clang-format HEAD^ git diff # to review the changes git commit -a --amend --no-edit # to keep the changes git reset --hard # to discard the changes Plugins are available for many editors (including Emacs and vi) that support clang-format. Bart. |
|
From: Mark W. <ma...@kl...> - 2023-04-20 20:14:08
|
Hi Philippe,
On Tue, Apr 18, 2023 at 01:02:44PM +0200, Philippe Waroquiers via Valgrind-developers wrote:
> 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.
Sorry, I hadn't realized that would change the diff emails.
Paul has added a workaround:
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
Which adds a sed filter that deletes the timing info from the log
files.
It does have to be installed on some of the nightly builders though.
Cheers,
Mark
|
|
From: Mark W. <ma...@kl...> - 2023-04-20 20:02:59
|
Hi, On Tue, Apr 18, 2023 at 03:05:28PM +1000, Nicholas Nethercote wrote: > 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> > . (Note that article requires javascript to turn on, or you won't be able to read more than the first paragraph.) Sure you can work around it, but I don't think that is a great solution. It requires everyone to make some local 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. > > > > 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. I am not a fan, but also not dead against. Personally I am happy with emacs M-x indent-region on the code I edit. There is a .dir-locals.el in git which catches some (but certainly not all) formatting things. I played a bit with the .clang-format file. I am not sure I like using clang-format to reformat everything, it seems a little arbitrary. But if we could make the git-clang-format thing working then using that for formatting patches/regions that you changed might be OK. Cheers, Mark |
|
From: Mark W. <ma...@so...> - 2023-04-20 19:18:30
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=e1684bc775c2b253134354f1e903cab48df3758e commit e1684bc775c2b253134354f1e903cab48df3758e Author: Mark Wielaard <ma...@kl...> Date: Thu Apr 20 21:17:46 2023 +0200 Add 436413 Warn about realloc of size zero to NEWS Diff: --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 203b422784..4ba0c31709 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 +436413 Warn about realloc of size zero 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" |
|
From: Mark W. <ma...@kl...> - 2023-04-20 13:19:35
|
Hi Sasha,
On Thu, 2023-04-20 at 15:01 +0200, Mark Wielaard wrote:
> Looks like this was in response to Philippe's review. Thanks.
> I added one more comment at the top describing the three usages of
> vgdb. And fixed up a few places where tabs were used for indentation
> (we are not very consistent in that either, after the release we'll
> look into adopting something like clang-format so you don't have to do
> all this by hand). And a missing newline in coregrind/m_main.c to make
> none/tests/cmdline2 pass.
>
> Pushed with the following commit message to show what was changed:
And then I messed up and didn't include those small fixups I described,
sorry. Pushed a followup commit:
Author: Mark Wielaard <ma...@kl...>
Date: Thu Apr 20 15:04:03 2023 +0200
vgdb --multi: fix various typos, indentation and such (followup)
commit 56ccb1e36c4722b56e3e602b986bc45025cb685d missed a few small
fixlets:
- one more comment at the top describing the three usages of vgdb.
- fixed up a few places where tabs were used for indentation (we are
not very consistent in that either, after the release we'll look
into adopting something like clang-format so you don't have to do
all this by hand).
- Add a missing newline in coregrind/m_main.c to make
none/tests/cmdline2 pass.
Sorry for messing this up. One day git and I will become friends...
Cheers,
Mark
|
|
From: 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 |