From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54501 invoked by alias); 20 Oct 2015 11:16:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 54489 invoked by uid 89); 20 Oct 2015 11:16:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_SBL_CSS,RP_MATCHES_RCVD,SPF_PASS autolearn=no version=3.3.2 X-HELO: xyzzy.0x04.net Received: from xyzzy.0x04.net (HELO xyzzy.0x04.net) (109.74.193.254) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Oct 2015 11:16:01 +0000 Received: from hogfather.0x04.net (89-65-84-110.dynamic.chello.pl [89.65.84.110]) by xyzzy.0x04.net (Postfix) with ESMTPS id C760F4012E; Tue, 20 Oct 2015 13:16:12 +0200 (CEST) Received: from [192.168.1.62] (84-10-2-59.static.chello.pl [84.10.2.59]) by hogfather.0x04.net (Postfix) with ESMTPSA id 176B95800AC; Tue, 20 Oct 2015 13:15:59 +0200 (CEST) Subject: Re: gdb/linux-record fixes To: Pedro Alves , Yao Qi References: <1445118081-10908-1-git-send-email-koriakin@0x04.net> <56250E2D.7020009@redhat.com> <562525C1.7000205@0x04.net> <5626206E.1000405@redhat.com> Cc: gdb-patches@sourceware.org From: =?UTF-8?Q?Marcin_Ko=c5=9bcielnicki?= Message-ID: <5626226D.3010203@0x04.net> Date: Tue, 20 Oct 2015 11:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5626206E.1000405@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2015-10/txt/msg00349.txt.bz2 On 20/10/15 13:07, Pedro Alves wrote: > On 10/19/2015 06:17 PM, Marcin Kościelnicki wrote: > >> Yeah, they're not covered by the testsuite. Actually, there seem to be >> only two tests in gdb.reverse suite that even touch syscalls: >> sigall-reverse (signal, sigprocmask, exit_group) and watch-reverse >> (read, write). No wonder that syscall handling is buggy... >> >> Stepping forward and backward over pipe/time/waitpid would indeed do the >> trick for patch #6. > > Can I convince you to add that to the patch (and likewise to others that > might not be overly hard)? I'll do that, if I'm not overcome by dejaGNU... I have no idea how that stuff works at the moment. > BTW, you'll also need to include ChangeLog entries. Please check here: > > https://sourceware.org/gdb/wiki/ContributionChecklist OK, will do. >> Some others are trickier to test, though - for >> example to test #3 (overlong sigaction/sigset_t) you'd need to manually >> invoke the actual linux syscalls, taking care to pull the right struct >> definitions from kernel headers, and make sure they're located near the >> end of segment bounduary (so that accessing after the end causes an >> error) - the glibc wrappers will always allocate relevant structs on >> stack, which almost certainly has extra bytes after the struct, hiding >> the bug. > > OOC, how did you first notice the bug? I'm working on s390 support, and noticed a lot of weirdness when filling the size_* fields for the new target and comparing with others. I dug down into syscall handling code and noticed a lot more problems. I also stepped through getgroups syscall on i386 to make sure size mismatches really cause problems (getgroups was recorded wrong because of the gid size issue). >> >> BTW, I was thinking of a self-testing approach for linux-record: >> >> - if gdb debugging is enabled, record changes on each instruction twice: >> before and after execution >> - memory/register contents recorded in "before" stage should match >> contents previously recorded in "after" stage, if any (ie. if it's not >> the first time we see a register or a memory byte). If there's a >> mismatch, record missed something. >> >> Unfortunately, that's a non-starter for multithreaded programs, or for >> progams involving shared memory :( > > Sounds like a good idea. I don't see multithreading being an issue, > as target record doesn't really work with multithreaded programs to begin > with. Also, I don't think we'd want it behind the usual "set debug record" > knob, as that sounds like it could actually change the record target's > behavior, and heisenbugs. Probably put it under its own > "maint set record self-check on" or some such knob. > > This reminds me that Yao once wrote a test that did something like > that (registers only, no gdb magic): > > https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html > > Not sure what happened to that. > > Thanks, > Pedro Alves >