From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6671 invoked by alias); 19 Dec 2013 19:09:08 -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 6659 invoked by uid 89); 19 Dec 2013 19:09:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Dec 2013 19:09:06 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBJJ92QA031323 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 19 Dec 2013 14:09:02 -0500 Received: from psique (ovpn-113-148.phx2.redhat.com [10.3.113.148]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rBJJ8xww024382 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 19 Dec 2013 14:09:00 -0500 From: Sergio Durigan Junior To: Pedro Alves Cc: gdb-patches@sourceware.org, Gabriel Krisman Bertazi Subject: Re: [PATCH] Fix for PR breakpoints/16297: Fix catch syscall to work with syscall 0 References: <87fvpu4vgh.fsf@lestat.krisman.be> <52AF3DA0.3020406@redhat.com> <52AF3F0F.3030107@redhat.com> <52AF47D5.1040304@redhat.com> <52B31D02.8010601@redhat.com> X-URL: http://www.redhat.com Date: Thu, 19 Dec 2013 19:09:00 -0000 In-Reply-To: <52B31D02.8010601@redhat.com> (Pedro Alves's message of "Thu, 19 Dec 2013 16:21:22 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2013-12/txt/msg00798.txt.bz2 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: 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 + + PR breakpoints/16297 + * breakpoint.c (breakpoint_hit_catch_syscall): Return immediately + when expected syscall is hit. + 2013-12-19 Tom Tromey * 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 + + 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 * 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] }