Re: gdb/linux-record fixes

[Pedro Alves <palves at redhat dot com>]

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)?

BTW, you'll also need to include ChangeLog entries.  Please check here:

  https://sourceware.org/gdb/wiki/ContributionChecklist

> 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?

> 
> 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