From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4474 invoked by alias); 7 Nov 2008 03:30:30 -0000 Received: (qmail 4419 invoked by uid 22791); 7 Nov 2008 03:30:28 -0000 X-Spam-Check-By: sourceware.org Received: from igw2.br.ibm.com (HELO igw2.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 07 Nov 2008 03:29:42 +0000 Received: from d24relay01.br.ibm.com (unknown [9.8.31.16]) by igw2.br.ibm.com (Postfix) with ESMTP id 52BD817F418 for ; Fri, 7 Nov 2008 01:28:06 -0200 (BRDT) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.18.232.46]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mA73THEm3780666 for ; Fri, 7 Nov 2008 00:29:17 -0300 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mA73Tcsh005150 for ; Fri, 7 Nov 2008 01:29:38 -0200 Received: from [9.8.9.146] ([9.8.9.146]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id mA73TbVY005094; Fri, 7 Nov 2008 01:29:38 -0200 Subject: Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= To: Pedro Alves Cc: gdb-patches@sourceware.org In-Reply-To: <200811041617.10621.pedro@codesourcery.com> References: <1225773079.24532.52.camel@miki> <200811041617.10621.pedro@codesourcery.com> Content-Type: text/plain; charset=iso-8859-1 Date: Fri, 07 Nov 2008 03:30:00 -0000 Message-Id: <1226028565.32321.86.camel@miki> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2008-11/txt/msg00114.txt.bz2 Hi Pedro, On Tue, 2008-11-04 at 16:17 +0000, Pedro Alves wrote: > On Tuesday 04 November 2008 04:31:19, Sérgio Durigan Júnior wrote: > > +static enum print_stop_action > > +print_it_catch_syscall (struct breakpoint *b) > > +{ > > + /* This is needed because we want to know in which state a > > + syscall is. It can be in the TARGET_WAITKIND_SYSCALL_ENTRY > > + or TARGET_WAITKIND_SYSCALL_RETURN, and depending on it we > > + must print "called syscall" or "returned from syscall". */ > > + struct thread_info *th_info = find_thread_pid (inferior_ptid); > > + const char *syscall_name = > > + gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number); > > + > > + annotate_catchpoint (b->number); > > + printf_filtered (_("\nCatchpoint %d ("), b->number); > > + > > + if (th_info->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY) > > + printf_filtered (_("calling ")); > > + else > > + printf_filtered (_("returned from ")); > > This bit left me wondering about the below. Take these with a > grain of salt, please :-) Of course :-). And I'm sorry for taking so long to respond. > syscall_state has been placed in struct thread_info, and linux-nat.c > toggles it between entry/exit, but that's mainly because on linux, the > same trap is sent in both cases. In the ttrace case (inf-ttrace.c), for > example, you have distinct TTEVT_SYSCALL_ENTRY and TTEVT_SYSCALL_RETURN > events at the target level. Shouldn't we be doing the same on linux? That > is, move 'syscall_state' to 'struct lwp_info', thus making it > private to the linux-nat.c implementation, and have the core side > always distinguish them from the TARGET_WAITKIND_SYSCALL_* type > returned from target_wait? It looks weird for the target side to > be writing to a thread_info directly. I always tend to ponder how I'd > do these things in gdbserver to validate target_ops designs --- I guess > I wouldn't be able to tweak gdb's common code from there. :-) > > Was it because you need to access it in print_it_catch_syscall? You > could get it from the last target event like you do in > breakpoint_hit_catch_syscall, I guess. Ok, I'll try to put the syscall_state in 'struct lwp_info'. Honestly, I don't remember now why I've chosen to put this variable inside thread_info, but of course you're way more capable of telling me how to make my design be more clever (and look more like GDB) :-). > Also, I'm not 100% sure, but I think you can crash in > linux-nat.c:linux_handle_extended_wait if an lwp happens to hit a syscall > you're catching before it's corresponding thread has been added to the > thread list in linux-thread-db.c. I'm not sure I'm able to answer this, but that's a problem that I wasn't considering. Thanks for the hint. > Also, while we're on to speaking of these matters, would it make sense > to be able to catch only syscall entry or syscall return events > at the UI level? That is, separate "catch syscall entry", > "catch syscall return" or some such? IMHO, we could postpone this for now. Honestly I'm pretty satisfied with the feature right now, but of course my opinion shouldn't count that much ;-). Thanks for your comments, I'll fix my code as soon as I can. Regards, -- Sérgio Durigan Júnior Linux on Power Toolchain - Software Engineer Linux Technology Center - LTC IBM Brazil