[PATCH] Fix for PR breakpoints/16297: Fix catch syscall to work with syscall 0

[Sergio Durigan Junior]

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