This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix for PR breakpoints/16297: Fix catch syscall to work with syscall 0
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Gabriel Krisman Bertazi <gabriel at krisman dot be>
- Date: Thu, 19 Dec 2013 17:08:58 -0200
- Subject: Re: [PATCH] Fix for PR breakpoints/16297: Fix catch syscall to work with syscall 0
- Authentication-results: sourceware.org; auth=none
- References: <87fvpu4vgh dot fsf at lestat dot krisman dot be> <m3mwk24v5w dot fsf at redhat dot com> <52AF3DA0 dot 3020406 at redhat dot com> <52AF3F0F dot 3030107 at redhat dot com> <m338lszn95 dot fsf at redhat dot com> <52AF47D5 dot 1040304 at redhat dot com> <m3lhzhqz1j dot fsf at redhat dot com> <52B31D02 dot 8010601 at redhat dot com>
On Thursday, December 19 2013, Pedro Alves wrote:
> On 12/19/2013 03:50 AM, Sergio Durigan Junior wrote:
>> @@ -27,6 +29,8 @@ main (void)
>>
>> chroot (".");
>>
>> + read (0, NULL, 0);
>
> I think the C implementation (libc or the compiler) is
> free to skip actually calling the syscall, given bytes is 0.
> Something like creating a pipe, and reading a byte off
> of it might be safer. But I won't object to leaving
> this as is for now.
Aha, that's a safer solution indeed! Thanks for the insight.
>> static int chroot_syscall = SYS_chroot;
>> +/* The "read" syscall is zero on x86_64. */
>> +static int read_syscall = SYS_read;
>
> Future readers who might not be familiar with this bug
> probably won't realize that the emphasis should be on
> zero, rather than the comment just happening to be
> trying to be informative. I'd suggest extending the comment:
>
> +/* GDB had a bug where it couldn't catch syscall number 0. In most
> + Linux architectures, syscall number 0 is restart_syscall, which
> + can't be called from userspace. However, the "read" syscall
> + is zero on x86_64. */
> +static int read_syscall = SYS_read;
Done.
> Otherwise looks fine to me.
Thanks. Checked-in:
<https://sourceware.org/ml/gdb-cvs/2013-12/msg00101.html>
The full patch is below.
--
Sergio
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 374b8c5..2bae7ae 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2013-12-19 Gabriel Krisman Bertazi <gabriel@krisman.be>
+
+ PR breakpoints/16297
+ * breakpoint.c (breakpoint_hit_catch_syscall): Return immediately
+ when expected syscall is hit.
+
2013-12-19 Tom Tromey <tromey@redhat.com>
* ser-unix.c (hardwire_ops): New global.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 589aa19..6a11ddf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8325,10 +8325,9 @@ breakpoint_hit_catch_syscall (const struct bp_location *bl,
VEC_iterate (int, c->syscalls_to_be_caught, i, iter);
i++)
if (syscall_number == iter)
- break;
- /* Not the same. */
- if (!iter)
- return 0;
+ return 1;
+
+ return 0;
}
return 1;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3e1c756..97ad49b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,15 @@
+2013-12-19 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ PR breakpoints/16297
+ * gdb.base/catch-syscall.c (read_syscall, pipe_syscall)
+ (write_syscall): New variables.
+ (main): Create a pipe, write 1 byte in it, and read 1 byte from
+ it.
+ * gdb.base/catch-syscall.exp (all_syscalls): Include "pipe,
+ "write" and "read" syscalls.
+ (fill_all_syscalls_numbers): Improve the way to obtain syscalls
+ numbers.
+
2013-12-19 Keven Boell <keven.boell@intel.com>
* gdb.fortran/module.exp: Completion matches fortran module
diff --git a/gdb/testsuite/gdb.base/catch-syscall.c b/gdb/testsuite/gdb.base/catch-syscall.c
index 8f94191..aa5727a 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.c
+++ b/gdb/testsuite/gdb.base/catch-syscall.c
@@ -16,17 +16,33 @@
static int close_syscall = SYS_close;
static int chroot_syscall = SYS_chroot;
+/* GDB had a bug where it couldn't catch syscall number 0 (PR 16297).
+ In most GNU/Linux architectures, syscall number 0 is
+ restart_syscall, which can't be called from userspace. However,
+ the "read" syscall is zero on x86_64. */
+static int read_syscall = SYS_read;
+static int pipe_syscall = SYS_pipe;
+static int write_syscall = SYS_write;
static int exit_group_syscall = SYS_exit_group;
int
main (void)
{
+ int fd[2];
+ char buf1[2] = "a";
+ char buf2[2];
+
/* A close() with a wrong argument. We are only
interested in the syscall. */
close (-1);
chroot (".");
+ pipe (fd);
+
+ write (fd[1], buf1, sizeof (buf1));
+ read (fd[0], buf2, sizeof (buf2));
+
/* The last syscall. Do not change this. */
_exit (0);
}
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index fd7d2db..5925419 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -47,7 +47,7 @@ if { [prepare_for_testing ${testfile}.exp $testfile ${testfile}.c] } {
# All (but the last) syscalls from the example code
# They are ordered according to the file, so do not change this.
-set all_syscalls { "close" "chroot" }
+set all_syscalls { "close" "chroot" "pipe" "write" "read" }
set all_syscalls_numbers { }
# The last syscall (exit()) does not return, so
@@ -392,11 +392,12 @@ proc do_syscall_tests_without_xml {} {
# This procedure fills the vector "all_syscalls_numbers" with the proper
# numbers for the used syscalls according to the architecture.
proc fill_all_syscalls_numbers {} {
- global all_syscalls_numbers last_syscall_number
+ global all_syscalls_numbers last_syscall_number all_syscalls
+
+ foreach syscall $all_syscalls {
+ lappend all_syscalls_numbers [get_integer_valueof "${syscall}_syscall" -1]
+ }
- set close_syscall [get_integer_valueof "close_syscall" -1]
- set chroot_syscall [get_integer_valueof "chroot_syscall" -1]
- set all_syscalls_numbers [list $close_syscall $chroot_syscall]
set last_syscall_number [get_integer_valueof "exit_group_syscall" -1]
}