This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/4] catch syscall -- try 4 -- Architecture-independent part
- From: Daniel Jacobowitz <drow at false dot org>
- To: Sérgio Durigan Júnior <sergiodj at linux dot vnet dot ibm dot com>
- Cc: Pedro Alves <pedro at codesourcery dot com>, gdb-patches at sourceware dot org, teawater <teawater at gmail dot com>
- Date: Fri, 27 Feb 2009 17:11:33 -0500
- Subject: Re: [PATCH 1/4] catch syscall -- try 4 -- Architecture-independent part
- 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>
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