From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13107 invoked by alias); 27 Feb 2009 22:11:47 -0000 Received: (qmail 13099 invoked by uid 22791); 27 Feb 2009 22:11:46 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_32 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 27 Feb 2009 22:11:37 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id D55B310A5B; Fri, 27 Feb 2009 22:11:34 +0000 (GMT) Received: from caradoc.them.org (209.195.188.212.nauticom.net [209.195.188.212]) by nan.false.org (Postfix) with ESMTP id A513D10577; Fri, 27 Feb 2009 22:11:34 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1LdAvd-0004zb-Gl; Fri, 27 Feb 2009 17:11:33 -0500 Date: Sat, 28 Feb 2009 00:44:00 -0000 From: Daniel Jacobowitz To: =?iso-8859-1?Q?S=E9rgio_Durigan_J=FAnior?= Cc: Pedro Alves , gdb-patches@sourceware.org, teawater Subject: Re: [PATCH 1/4] catch syscall -- try 4 -- Architecture-independent part Message-ID: <20090227221133.GA12904@caradoc.them.org> Mail-Followup-To: =?iso-8859-1?Q?S=E9rgio_Durigan_J=FAnior?= , Pedro Alves , gdb-patches@sourceware.org, teawater References: <1232929831.26873.22.camel@miki> <200901260053.06295.pedro@codesourcery.com> <1232945747.26873.27.camel@miki> <1232989355.26873.39.camel@miki> <20090201193306.GJ4597@caradoc.them.org> <1235561189.14363.20.camel@miki> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1235561189.14363.20.camel@miki> User-Agent: Mutt/1.5.17 (2008-05-11) 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: 2009-02/txt/msg00503.txt.bz2 On Wed, Feb 25, 2009 at 08:26:29AM -0300, Sérgio Durigan Júnior wrote: > I'm not sure I understood what you said, so please correct me if I'm > wrong. IIUC, you want me to create another field in 'struct > thread_info', and this field would be a 'struct target_waitstatus', > right? This is because you think I could copy the whole 'struct > target_waitstatus' in 'deal_with_syscall_event', and use it someplace > else. Is it right? If it is, I have a question: would I have access to > the current 'struct thread_info' inside breakpoint.c? The copy in to the thread is not syscall-specific, so that's not where it should go. Maybe in infrun.c below this? /* Cache the last pid/waitstatus. */ target_last_wait_ptid = ecs->ptid; target_last_waitstatus = ecs->ws; ... ecs->event_thread = find_thread_pid (ecs->ptid); > > > + get_last_target_status (&ptid, &last); > > > + > > > + annotate_catchpoint (b->number); > > > + > > > + if (s.name == NULL) > > > + syscall_id = xstrprintf ("%d", b->syscall_number); > > > + else > > > + syscall_id = xstrprintf ("'%s'", s.name); > > > > For instance, here we've got the waitstatus in addition to the breakpoint. > > That's what confused me. I didn't get if you want me to use 'struct > thread_info' or 'struct target_waitstatus' to hold the syscall_number > info :-). It doesn't matter - as long as you don't put it in the breakpoint, and it comes from somewhere thread-specific. > > > +/* Implement the "print_one" breakpoint_ops method for syscall > > > + catchpoints. */ > > > + > > > +static void > > > +print_one_catch_syscall (struct breakpoint *b, CORE_ADDR *last_addr) > > > +{ > > > > Have you tried hitting a syscall catchpoint in MI mode, and is the > > output anything useful? > > No, unfortunately I haven't. Actually, I must first learn how to use the > MI interface, but that should not be hard :-). I'd suggest doing that as part of this submission so that we know you're on the right track. It isn't too hard; you can start by looking at the test logs from gdb.mi tests, if that helps. > > > +# Fills the struct syscall (passed as argument) with the corresponding > > > +# system call represented by syscall_number. > > > +M:void:get_syscall_by_number:int syscall_number, struct syscall *s:syscall_number, s > > > + > > > +# Fills the struct syscall (passed as argument) with the corresponding > > > +# system call represented by syscall_name. > > > +M:void:get_syscall_by_name:const char *syscall_name, struct syscall *s:syscall_name, s > > > + > > > +# Returns the array containing the syscall names for the architecture. > > > +M:const char **:get_syscall_names:void: > > > > If every target is going to use XML for this, these three do not need > > to be gdbarch methods and the support code can move from linux-tdep.c > > to xml-syscall.c. > > As far as I understood (from our discussion a few months ago), not every > target is supposed to use the XML for syscalls. That's specially true > for embedded systems and/or architectures for which the XML file is > missing (for some obscure reason, don't know). That's why I thought it > would be better not to generalize. I don't think this is a big deal. If it is, we can handle it the same way as for target-descriptions: pre-compile them into GDB. > > > + if (target_passed_by_entrypoint () > 0 > > > + && catch_syscall_enabled () > 0) > > > + request = PT_SYSCALL; > > > + else > > > + request = PT_CONTINUE; > > > > Why is target_passed_by_entrypoint still necessary? If we understand > > why, I think there'll be some other more appropriate flag. Is it to > > avoid using PTRACE_SYSCALL when the shell is running, before the > > application starts? > > It's been a long time since I added this check, but as far as I > remember, that's exactly the reason. I tried to remove this, and the > testcase simply freezes. Do you have another idea? :-) Not sure that the flag exists any more, but you're trying to avoid it when called by startup_inferior. I suppose you could use the inferior_created observer (not new_inferior! The distinction is not too clear in the manual but that one is too early). The problem is, again, that this flag needs to be per-inferior. Pedro, any thoughts? > > > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > > > index 9a7e39c..1d0f66f 100644 > > > --- a/gdb/linux-nat.c > > > +++ b/gdb/linux-nat.c > > > @@ -676,6 +676,7 @@ linux_child_post_attach (int pid) > > > { > > > linux_enable_event_reporting (pid_to_ptid (pid)); > > > check_for_thread_db (); > > > + linux_enable_tracesysgood (pid_to_ptid (pid)); > > > } > > > > > > static void > > > @@ -683,6 +684,7 @@ linux_child_post_startup_inferior (ptid_t ptid) > > > { > > > linux_enable_event_reporting (ptid); > > > check_for_thread_db (); > > > + linux_enable_tracesysgood (ptid); > > > } > > > > > > static int > > > @@ -4160,6 +4162,7 @@ linux_target_install_ops (struct target_ops *t) > > > t->to_follow_fork = linux_child_follow_fork; > > > t->to_find_memory_regions = linux_nat_find_memory_regions; > > > t->to_make_corefile_notes = linux_nat_make_corefile_notes; > > > + t->to_passed_by_entrypoint = linux_passed_by_entrypoint; > > > > > > super_xfer_partial = t->to_xfer_partial; > > > t->to_xfer_partial = linux_xfer_partial; > > > > These bits must be for another patch in the series :-) > > I'm sorry, I didn't understand what you meant by that :-(. These > modifications are all architecture-independent, so this is the right > place for them right? No - since they're specific to Linux. Also, I don't think they'll compile at this point, you haven't added the function yet. -- Daniel Jacobowitz CodeSourcery