This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 01/13] gdb/linux-record: Add testcases for a few syscalls.
- From: Marcin KoÅcielnicki <koriakin at 0x04 dot net>
- To: Pedro Alves <palves at redhat dot com>, qiyaoltc at gmail dot com
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 29 Oct 2015 13:31:04 +0100
- Subject: Re: [PATCH v2 01/13] gdb/linux-record: Add testcases for a few syscalls.
- Authentication-results: sourceware.org; auth=none
- References: <5628E5D6 dot 5020706 at 0x04 dot net> <1445521166-14492-1-git-send-email-koriakin at 0x04 dot net> <5632035E dot 30809 at redhat dot com>
On 29/10/15 12:30, Pedro Alves wrote:
On 10/22/2015 02:39 PM, Marcin KoÅcielnicki wrote:
gdb/testsuite/ChangeLog:
* gdb.reverse/linux-fstatat-reverse.c: New test.
* gdb.reverse/linux-fstatat-reverse.exp: New file.
* gdb.reverse/linux-getresuid-reverse.c: New test.
* gdb.reverse/linux-getresuid-reverse.exp: New file.
* gdb.reverse/linux-pipe-reverse.c: New test.
* gdb.reverse/linux-pipe-reverse.exp: New file.
* gdb.reverse/linux-readv-reverse.c: New test.
* gdb.reverse/linux-readv-reverse.exp: New file.
* gdb.reverse/linux-recvmsg-reverse.c: New test.
* gdb.reverse/linux-recvmsg-reverse.exp: New file.
* gdb.reverse/linux-time-reverse.c: New test.
* gdb.reverse/linux-time-reverse.exp: New file.
* gdb.reverse/linux-waitpid-reverse.c: New test.
* gdb.reverse/linux-waitpid-reverse.exp: New file.
Excellent, thank you!
I ran your tests against pristine/unfixed gdb, and linux-readv-reverse.exp
passes cleanly. This is on x86_64. Is that expected? The comments in
the test seem to suggest it would fail.
That's expected. The test would only fail for x32 ABI, since it had the
wrong size_iov, size_pointer, size_size_t.
+++ b/gdb/testsuite/gdb.reverse/linux-fstatat-reverse.c
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2008-2015 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+
+void marker1 ()
We follow GNU formatting in tests too, unless there's a good
reason not to. So, line break after void. Also, this is C,
so write "(void)" for parameters. Thus:
void
marker1 (void)
{
OK.
+{
+}
+
+void marker2 ()
Likewise.
+{
+}
+
+struct stat buf;
+
+int main() {
int
main ()
{
Not int main(void)?
+ marker1();
+ int fd = open("/", O_PATH);
+ fstatat(fd, ".", &buf, 0);
+ marker2();
Space before parens.
Several of the test programs have non-conforming indentation too.
OK.
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/linux-fstatat-reverse.exp b/gdb/testsuite/gdb.reverse/linux-fstatat-reverse.exp
new file mode 100644
index 0000000..23652e4
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/linux-fstatat-reverse.exp
@@ -0,0 +1,63 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+#
+# This test tests linux fstatat syscall for reverse execution.
Linux, uppercase. But,
+#
+
+if ![supports_reverse] {
+ return
+}
+
+
+if ![istarget "*linux*"] then {
+ verbose "Skipping linux syscall reverse tests."
+ return
+}
... I think most of these tests could/should drop this target check,
and drop the "linux-" in the filename as well. Any target/port
that supports record/reverse execution should be able to run
this, as long as it manages to compile the test program. And if the
test program doesn't compile in such a port, it'll be automatically
skipped.
OK. So a testcase that doesn't compile is not a problem? Because I
just noticed the time test doesn't compile on platforms that don't have
a time syscall, like s390x...
+
+gdb_test "break marker1" \
+ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
+ "set breakpoint at marker1"
+
+gdb_test "break marker2" \
+ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
+ "set breakpoint at marker2"
+
+gdb_continue_to_breakpoint "marker1" ".*$srcfile:.*"
+
+gdb_continue_to_breakpoint "marker2" ".*$srcfile:.*"
Most (all?) these tests run to marker1 and then immediately to
marker2. Can't they just continue to marker2 directly?
OK, I'll fix that.
+++ b/gdb/testsuite/gdb.reverse/linux-waitpid-reverse.exp
...
+# will be remembered. If not, new contents (status) will be # used, and
Spurious # after "will be".
+# the test will fail.
+
+gdb_test "print status" ".* = -1" "check waitpid record"
Thanks,
Pedro Alves
Thanks for the review, I'll resend a fixed version.