[RFC] Support command "catch syscall" properly on different targets

Message ID 1425047015-1906-1-git-send-email-qiyaoltc@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Feb. 27, 2015, 2:23 p.m. UTC
  From: Yao Qi <yao.qi@linaro.org>

Nowadays, "catch syscall" is supported on linux-nat target of
different gdbarch and inf-ttrace target.  However, in
breakpoint.c:catch_syscall_command_1, we have this check

   /* Checking if the feature if supported.  */
   if (gdbarch_get_syscall_number_p (gdbarch) == 0)
     error (_("The feature 'catch syscall' is not supported on \
this architecture yet."));

On one hand, gdbarch method get_syscall_number isn't installed on any
HP-UX targets.  That means users will get such error message even
syscall catchpoint is supported on HP-UX.  On the other hand, on linux
remote target (with GDBserver), "catch syscall" isn't supported
(PR 13585), but no error is emitted:

 (gdb) target remote :1234
 Remote debugging using :1234
 Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
 0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
 (gdb) catch syscall close
 Catchpoint 1 (syscall 'close' [3])

The fix in this patch is to add a new target method
supports_syscall_catchpoint, so that we can have different
implementations on different targets.  On inf-ttrace, we
can simply return one, while on linux-child,
gdbarch_get_syscall_number_p is called.

With this patch applied, on linux remote target, it becomes:

 (gdb) target remote :1234
 Remote debugging using :1234
 Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
 0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
 (gdb) catch syscall close
 The feature 'catch syscall' is not supported on this target yet.

which looks more reasonable to me.  However, this patch causes some
regressions in catch-syscall.exp,

 catch syscall nonsense_syscall^M
 The feature 'catch syscall' is not supported on this target yet.^M
 (gdb) FAIL: gdb.base/catch-syscall.exp: catch syscall to a nonsense syscall is prohibited

because syscall catchpoint isn't supported on exec target.
I can move these tests to the place where inferior is created, before
I go too far, I'd like to hear what do you think of this.

gdb:

2015-02-27  Yao Qi  <yao.qi@linaro.org>

	* breakpoint.c (catch_syscall_command_1): Call
	target_supports_syscall_catchpoint instead of
	gdbarch_get_syscall_number_p.
	* inf-ttrace.c (inf_ttrace_supports_syscall_catchpoint): New
	function.
	(inf_ttrace_target): Install field to_supports_syscall_catchpoint.
	* linux-nat.c (linux_child_supports_syscall_catchpoint): New function.
	(linux_target_install_ops): Install field
	to_supports_syscall_catchpoint.
	* target-delegates.c: Regenerated.
	* target.h (struct target_ops) <to_supports_syscall_catchpoint>:
	New field.
	(target_supports_syscall_catchpoint): New macro.
---
 gdb/breakpoint.c       |  4 ++--
 gdb/inf-ttrace.c       | 10 ++++++++++
 gdb/linux-nat.c        | 10 ++++++++++
 gdb/target-delegates.c | 33 +++++++++++++++++++++++++++++++++
 gdb/target.h           |  9 +++++++++
 5 files changed, 64 insertions(+), 2 deletions(-)
  

Comments

Doug Evans Feb. 27, 2015, 6:49 p.m. UTC | #1
On Fri, Feb 27, 2015 at 6:23 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> From: Yao Qi <yao.qi@linaro.org>
>
> Nowadays, "catch syscall" is supported on linux-nat target of
> different gdbarch and inf-ttrace target.  However, in
> breakpoint.c:catch_syscall_command_1, we have this check
>
>    /* Checking if the feature if supported.  */
>    if (gdbarch_get_syscall_number_p (gdbarch) == 0)
>      error (_("The feature 'catch syscall' is not supported on \
> this architecture yet."));
>
> On one hand, gdbarch method get_syscall_number isn't installed on any
> HP-UX targets.  That means users will get such error message even
> syscall catchpoint is supported on HP-UX.  On the other hand, on linux
> remote target (with GDBserver), "catch syscall" isn't supported
> (PR 13585), but no error is emitted:
>
>  (gdb) target remote :1234
>  Remote debugging using :1234
>  Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>  0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
>  (gdb) catch syscall close
>  Catchpoint 1 (syscall 'close' [3])
>
> The fix in this patch is to add a new target method
> supports_syscall_catchpoint, so that we can have different
> implementations on different targets.  On inf-ttrace, we
> can simply return one, while on linux-child,
> gdbarch_get_syscall_number_p is called.
>
> With this patch applied, on linux remote target, it becomes:
>
>  (gdb) target remote :1234
>  Remote debugging using :1234
>  Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>  0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
>  (gdb) catch syscall close
>  The feature 'catch syscall' is not supported on this target yet.
>
> which looks more reasonable to me.  However, this patch causes some
> regressions in catch-syscall.exp,
>
>  catch syscall nonsense_syscall^M
>  The feature 'catch syscall' is not supported on this target yet.^M
>  (gdb) FAIL: gdb.base/catch-syscall.exp: catch syscall to a nonsense syscall is prohibited
>
> because syscall catchpoint isn't supported on exec target.
> I can move these tests to the place where inferior is created, before
> I go too far, I'd like to hear what do you think of this.

Hi.
Some questions come to mind,
and some not specific to the topic at hand.

1) Do we have a story yet for how to handle differences across
multiple targets/inferiors?
E.g., while not completely supported today, what if I'm
debugging two targets and one supports "catch syscall"
and one doesn't? "break" applies across all inferiors
(though I really don't like this as a default behaviour),
and it would be odd if "catch" didn't work similarly
by default.

We have "thread apply all ...".
We could also have "inferior apply all ...".
E.g., inferior apply all break foo
[I gather itsets can help with this, but I like the consistency.]

"catch syscall" is target(arch)-specific.
Multi-arch doesn't work today (except for special cases),
but we should understand how we want it to work tomorrow.
And similarly for all such target-specific commands.

2) Another issue that comes to mind is "catch syscall"
calls get_current_arch which is based on the current frame
if the inferior is live. If I'm debugging a multi-arch inferior (e.g., cell),
then when I do "catch syscall" I have to be cognizant of which frame
I'm in when I do that. [Right?]

3) Does this mean that I now won't be able to do:

bash$ gdb foo
(gdb) catch syscall open # <-- target == "exec" at this point
(gdb) run
  
Pedro Alves Feb. 27, 2015, 6:50 p.m. UTC | #2
On 02/27/2015 02:23 PM, Yao Qi wrote:

>  (gdb) target remote :1234
>  Remote debugging using :1234
>  Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>  0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
>  (gdb) catch syscall close
>  The feature 'catch syscall' is not supported on this target yet.
> 
> which looks more reasonable to me.  However, this patch causes some
> regressions in catch-syscall.exp,
> 
>  catch syscall nonsense_syscall^M
>  The feature 'catch syscall' is not supported on this target yet.^M
>  (gdb) FAIL: gdb.base/catch-syscall.exp: catch syscall to a nonsense syscall is prohibited
> 
> because syscall catchpoint isn't supported on exec target.
> I can move these tests to the place where inferior is created, before
> I go too far, I'd like to hear what do you think of this.

Yes, I guess we should do that.  If we're not connected to a
target yet, we have no clue whether the target that ends up
connected supports catch syscall or not.

Do we actually need to do anything when the inferior is created?
Supposedly once the inferior is created, we'll try to insert
the catchpoint, and that will fail is the target does not
support it.

Thanks,
Pedro Alves
  
Sergio Durigan Junior Feb. 27, 2015, 9:03 p.m. UTC | #3
On Friday, February 27 2015, Yao Qi wrote:

> Nowadays, "catch syscall" is supported on linux-nat target of
> different gdbarch and inf-ttrace target.  However, in
> breakpoint.c:catch_syscall_command_1, we have this check
>
>    /* Checking if the feature if supported.  */
>    if (gdbarch_get_syscall_number_p (gdbarch) == 0)
>      error (_("The feature 'catch syscall' is not supported on \
> this architecture yet."));
>
> On one hand, gdbarch method get_syscall_number isn't installed on any
> HP-UX targets.  That means users will get such error message even
> syscall catchpoint is supported on HP-UX.

You mean it's possible to use "catch syscall" on HP-UX targets?  I
wonder how it works.

>  On the other hand, on linux
> remote target (with GDBserver), "catch syscall" isn't supported
> (PR 13585), but no error is emitted:
>
>  (gdb) target remote :1234
>  Remote debugging using :1234
>  Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>  0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
>  (gdb) catch syscall close
>  Catchpoint 1 (syscall 'close' [3])

Yes, this is wrong.

> The fix in this patch is to add a new target method
> supports_syscall_catchpoint, so that we can have different
> implementations on different targets.  On inf-ttrace, we
> can simply return one, while on linux-child,
> gdbarch_get_syscall_number_p is called.
>
> With this patch applied, on linux remote target, it becomes:
>
>  (gdb) target remote :1234
>  Remote debugging using :1234
>  Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>  0x00007ffff7ddb2d0 in ?? () from /lib64/ld-linux-x86-64.so.2
>  (gdb) catch syscall close
>  The feature 'catch syscall' is not supported on this target yet.
>
> which looks more reasonable to me.  However, this patch causes some
> regressions in catch-syscall.exp,
>
>  catch syscall nonsense_syscall^M
>  The feature 'catch syscall' is not supported on this target yet.^M
>  (gdb) FAIL: gdb.base/catch-syscall.exp: catch syscall to a nonsense syscall is prohibited
>
> because syscall catchpoint isn't supported on exec target.
> I can move these tests to the place where inferior is created, before
> I go too far, I'd like to hear what do you think of this.

I don't think not being able to specify the syscall catchpoint before
starting the inferior is a good thing.

I mean, catch syscall relies on ptrace, just like catch fork or catch
exec.  You can do a catch fork before starting the inferior, so I think
it's natural that you do a catch syscall in this scenario too (not to
mention that sometimes you may want to catch syscalls before main).

What happens if, instead of error'ing out, you emit a warning saying
that GDB does not know if the target supports syscall catchpoints, and
that it will try to use the catchpoint, but if the target doesn't
support it, then the catchpoint will not work (sorry, something like
that, but obviously not this phrase).

Other than that, the patch looks straightforward.

Thanks,

> gdb:
>
> 2015-02-27  Yao Qi  <yao.qi@linaro.org>
>
> 	* breakpoint.c (catch_syscall_command_1): Call
> 	target_supports_syscall_catchpoint instead of
> 	gdbarch_get_syscall_number_p.
> 	* inf-ttrace.c (inf_ttrace_supports_syscall_catchpoint): New
> 	function.
> 	(inf_ttrace_target): Install field to_supports_syscall_catchpoint.
> 	* linux-nat.c (linux_child_supports_syscall_catchpoint): New function.
> 	(linux_target_install_ops): Install field
> 	to_supports_syscall_catchpoint.
> 	* target-delegates.c: Regenerated.
> 	* target.h (struct target_ops) <to_supports_syscall_catchpoint>:
> 	New field.
> 	(target_supports_syscall_catchpoint): New macro.
> ---
>  gdb/breakpoint.c       |  4 ++--
>  gdb/inf-ttrace.c       | 10 ++++++++++
>  gdb/linux-nat.c        | 10 ++++++++++
>  gdb/target-delegates.c | 33 +++++++++++++++++++++++++++++++++
>  gdb/target.h           |  9 +++++++++
>  5 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index db4b872..8e0ffb8 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -12135,9 +12135,9 @@ catch_syscall_command_1 (char *arg, int from_tty,
>    struct gdbarch *gdbarch = get_current_arch ();
>  
>    /* Checking if the feature if supported.  */
> -  if (gdbarch_get_syscall_number_p (gdbarch) == 0)
> +  if (!target_supports_syscall_catchpoint (gdbarch))
>      error (_("The feature 'catch syscall' is not supported on \
> -this architecture yet."));
> +this target yet."));
>  
>    tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
>  
> diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
> index 080e167..9c8c561 100644
> --- a/gdb/inf-ttrace.c
> +++ b/gdb/inf-ttrace.c
> @@ -1180,6 +1180,15 @@ inf_ttrace_get_ada_task_ptid (struct target_ops *self, long lwp, long thread)
>    return ptid_build (ptid_get_pid (inferior_ptid), lwp, 0);
>  }
>  
> +/* Implement the supports_syscall_catchpoint target_ops method.  */
> +
> +static int
> +inf_ttrace_supports_syscall_catchpoint (struct target_ops *self,
> +					struct gdbarch *gdbarch)
> +{
> +  return 1;
> +}
> +
>  
>  struct target_ops *
>  inf_ttrace_target (void)
> @@ -1206,6 +1215,7 @@ inf_ttrace_target (void)
>    t->to_pid_to_str = inf_ttrace_pid_to_str;
>    t->to_xfer_partial = inf_ttrace_xfer_partial;
>    t->to_get_ada_task_ptid = inf_ttrace_get_ada_task_ptid;
> +  t->to_supports_syscall_catchpoint = inf_ttrace_supports_syscall_catchpoint;
>  
>    return t;
>  }
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 2e1133d..6ebcc3e 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -594,6 +594,15 @@ linux_child_set_syscall_catchpoint (struct target_ops *self,
>    return 0;
>  }
>  
> +/* Implement the supports_syscall_catchpoint target_ops method.  */
> +
> +static int
> +linux_child_supports_syscall_catchpoint (struct target_ops *ops,
> +					 struct gdbarch *gdbarch)
> +{
> +  return gdbarch_get_syscall_number_p (gdbarch);
> +}
> +
>  /* On GNU/Linux there are no real LWP's.  The closest thing to LWP's
>     are processes sharing the same VM space.  A multi-threaded process
>     is basically a group of such processes.  However, such a grouping
> @@ -4277,6 +4286,7 @@ linux_target_install_ops (struct target_ops *t)
>    t->to_insert_exec_catchpoint = linux_child_insert_exec_catchpoint;
>    t->to_remove_exec_catchpoint = linux_child_remove_exec_catchpoint;
>    t->to_set_syscall_catchpoint = linux_child_set_syscall_catchpoint;
> +  t->to_supports_syscall_catchpoint = linux_child_supports_syscall_catchpoint;
>    t->to_pid_to_exec_file = linux_child_pid_to_exec_file;
>    t->to_post_startup_inferior = linux_child_post_startup_inferior;
>    t->to_post_attach = linux_child_post_attach;
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index e026179..9011acd 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -1144,6 +1144,35 @@ debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int a
>  }
>  
>  static int
> +delegate_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
> +{
> +  self = self->beneath;
> +  return self->to_supports_syscall_catchpoint (self, arg1);
> +}
> +
> +static int
> +tdefault_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
> +{
> +  return 0;
> +}
> +
> +static int
> +debug_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
> +{
> +  int result;
> +  fprintf_unfiltered (gdb_stdlog, "-> %s->to_supports_syscall_catchpoint (...)\n", debug_target.to_shortname);
> +  result = debug_target.to_supports_syscall_catchpoint (&debug_target, arg1);
> +  fprintf_unfiltered (gdb_stdlog, "<- %s->to_supports_syscall_catchpoint (", debug_target.to_shortname);
> +  target_debug_print_struct_target_ops_p (&debug_target);
> +  fputs_unfiltered (", ", gdb_stdlog);
> +  target_debug_print_struct_gdbarch_p (arg1);
> +  fputs_unfiltered (") = ", gdb_stdlog);
> +  target_debug_print_int (result);
> +  fputs_unfiltered ("\n", gdb_stdlog);
> +  return result;
> +}
> +
> +static int
>  delegate_has_exited (struct target_ops *self, int arg1, int arg2, int *arg3)
>  {
>    self = self->beneath;
> @@ -3849,6 +3878,8 @@ install_delegators (struct target_ops *ops)
>      ops->to_remove_exec_catchpoint = delegate_remove_exec_catchpoint;
>    if (ops->to_set_syscall_catchpoint == NULL)
>      ops->to_set_syscall_catchpoint = delegate_set_syscall_catchpoint;
> +  if (ops->to_supports_syscall_catchpoint == NULL)
> +    ops->to_supports_syscall_catchpoint = delegate_supports_syscall_catchpoint;
>    if (ops->to_has_exited == NULL)
>      ops->to_has_exited = delegate_has_exited;
>    if (ops->to_mourn_inferior == NULL)
> @@ -4091,6 +4122,7 @@ install_dummy_methods (struct target_ops *ops)
>    ops->to_insert_exec_catchpoint = tdefault_insert_exec_catchpoint;
>    ops->to_remove_exec_catchpoint = tdefault_remove_exec_catchpoint;
>    ops->to_set_syscall_catchpoint = tdefault_set_syscall_catchpoint;
> +  ops->to_supports_syscall_catchpoint = tdefault_supports_syscall_catchpoint;
>    ops->to_has_exited = tdefault_has_exited;
>    ops->to_mourn_inferior = default_mourn_inferior;
>    ops->to_can_run = tdefault_can_run;
> @@ -4235,6 +4267,7 @@ init_debug_target (struct target_ops *ops)
>    ops->to_insert_exec_catchpoint = debug_insert_exec_catchpoint;
>    ops->to_remove_exec_catchpoint = debug_remove_exec_catchpoint;
>    ops->to_set_syscall_catchpoint = debug_set_syscall_catchpoint;
> +  ops->to_supports_syscall_catchpoint = debug_supports_syscall_catchpoint;
>    ops->to_has_exited = debug_has_exited;
>    ops->to_mourn_inferior = debug_mourn_inferior;
>    ops->to_can_run = debug_can_run;
> diff --git a/gdb/target.h b/gdb/target.h
> index 2811c47..0ef1ba2 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -541,6 +541,9 @@ struct target_ops
>      int (*to_set_syscall_catchpoint) (struct target_ops *,
>  				      int, int, int, int, int *)
>        TARGET_DEFAULT_RETURN (1);
> +    int (*to_supports_syscall_catchpoint) (struct target_ops *,
> +					   struct gdbarch *)
> +      TARGET_DEFAULT_RETURN (0);
>      int (*to_has_exited) (struct target_ops *, int, int, int *)
>        TARGET_DEFAULT_RETURN (0);
>      void (*to_mourn_inferior) (struct target_ops *)
> @@ -1523,6 +1526,12 @@ int target_follow_fork (int follow_child, int detach_fork);
>  						  pid, needed, any_count, \
>  						  table_size, table)
>  
> +/* Return true if GDBARCH on current target supports syscall catchpoint,
> +   otherwise, return false.  */
> +
> +#define target_supports_syscall_catchpoint(gdbarch)			\
> +  (*current_target.to_supports_syscall_catchpoint) (&current_target, gdbarch)
> +
>  /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
>     exit code of PID, if any.  */
>  
> -- 
> 1.9.1
  
Yao Qi March 3, 2015, 11:55 a.m. UTC | #4
Doug Evans <dje@google.com> writes:

> 1) Do we have a story yet for how to handle differences across
> multiple targets/inferiors?

I don't have such story in my mind.  Taking a look at
https://sourceware.org/gdb/wiki/MultiTarget and I don't see anything on
handling differences across multiple targets.

> E.g., while not completely supported today, what if I'm
> debugging two targets and one supports "catch syscall"
> and one doesn't? "break" applies across all inferiors
> (though I really don't like this as a default behaviour),
> and it would be odd if "catch" didn't work similarly
> by default.

When GDB goes to multi-target, syscall catchpoint should be skipped for
targets don't support it, IMO.

>
> We have "thread apply all ...".
> We could also have "inferior apply all ...".
> E.g., inferior apply all break foo
> [I gather itsets can help with this, but I like the consistency.]
>
> "catch syscall" is target(arch)-specific.
> Multi-arch doesn't work today (except for special cases),
> but we should understand how we want it to work tomorrow.
> And similarly for all such target-specific commands.

Oh, multi-arch is supported, isn't?  done by these two patch sets in
both GDB and GDBserver side respectively,

  https://sourceware.org/ml/gdb-patches/2012-11/msg00228.html
  https://sourceware.org/ml/gdb-patches/2013-05/msg01057.html

However, I'd say that multi-arch isn't widely used because it is hard to
find a case that more than one arch can exist in one single target.
Probably once multi-target is supported, multi-arch will be more used.

In the future, IMHO, each command has a scope, a term from itset
patches, and the command is applied to the given scope.  The scope
is a set of targets, inferiors and threads.  If "catch syscall" command
is applied to a scope in which one target doesn't support it, GDB can
report an error like "Target foo in scope bar doesn't support
'catch syscall'".

>
> 2) Another issue that comes to mind is "catch syscall"
> calls get_current_arch which is based on the current frame
> if the inferior is live. If I'm debugging a multi-arch inferior (e.g., cell),
> then when I do "catch syscall" I have to be cognizant of which frame
> I'm in when I do that. [Right?]

Yeah, I think so.  "catch syscall" support varies on different arches,
and the frames in the same call chain may have different arches.
so "catch syscall" is supported on some frames, and not supported on
other frames.

>
> 3) Does this mean that I now won't be able to do:
>
> bash$ gdb foo
> (gdb) catch syscall open # <-- target == "exec" at this point
> (gdb) run

Yes.
  
Yao Qi March 3, 2015, 12:06 p.m. UTC | #5
Pedro Alves <palves@redhat.com> writes:

> Do we actually need to do anything when the inferior is created?
> Supposedly once the inferior is created, we'll try to insert
> the catchpoint, and that will fail is the target does not
> support it.

No, but we need to tweak test case to emit UNSUPPORTED instead of FAIL
if the target doesn't support "catch syscall".
  
Yao Qi March 3, 2015, 12:11 p.m. UTC | #6
Sergio Durigan Junior <sergiodj@redhat.com> writes:

> You mean it's possible to use "catch syscall" on HP-UX targets?  I
> wonder how it works.

Yes, that is what I meant, at least that is what I can tell from the
source.
  
Pedro Alves March 3, 2015, 12:14 p.m. UTC | #7
On 03/03/2015 11:55 AM, Yao Qi wrote:
> Doug Evans <dje@google.com> writes:
> 
>> 1) Do we have a story yet for how to handle differences across
>> multiple targets/inferiors?
> 
> I don't have such story in my mind.  Taking a look at
> https://sourceware.org/gdb/wiki/MultiTarget and I don't see anything on
> handling differences across multiple targets.

Pasting from:

https://sourceware.org/bugzilla/show_bug.cgi?id=10737#c5

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A related question is how to handle syscalls by name ("catch syscall open"):

#1 - when the user kills the program, and then starts another one, of a different architecture, in the same inferior.  E.g.,

  - file prog64
  - start
  - catch syscall open
  - kill
  - file prog32
  - run

#2 - similarly, when a 64-bit inferior execs a 32-bit inferior.

  - file prog64
  - start
  - catch syscall open (2 on 64-bit)
  - continue
  - inferior execs prog32
  - inferior now calls syscall 2, which is something else on i386.

These two cases may end up handled by the breakpoint_re_set machinery.

#3 - in the presence of multiple inferiors, each with its own arch.

  - file prog64
  - start
  - add-inferior -exec prog32
  - inferior 2
  - start
  - catch syscall open
  - set schedule-multiple on
  - c
  - both inferiors call "open"

Here, I don't think we'll catch inferior 1's.

syscall catchpoints is presently inferior-specific; while I think it
should end up with a location per inferior instead, and "open" should be
parsed in each inferior's/location's arch.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> 
>> E.g., while not completely supported today, what if I'm
>> debugging two targets and one supports "catch syscall"
>> and one doesn't? "break" applies across all inferiors
>> (though I really don't like this as a default behaviour),
>> and it would be odd if "catch" didn't work similarly
>> by default.
> 
> When GDB goes to multi-target, syscall catchpoint should be skipped for
> targets don't support it, IMO.

Agreed.

>> "catch syscall" is target(arch)-specific.
>> Multi-arch doesn't work today (except for special cases),
>> but we should understand how we want it to work tomorrow.
>> And similarly for all such target-specific commands.
> 
> Oh, multi-arch is supported, isn't?  done by these two patch sets in
> both GDB and GDBserver side respectively,
> 
>   https://sourceware.org/ml/gdb-patches/2012-11/msg00228.html
>   https://sourceware.org/ml/gdb-patches/2013-05/msg01057.html

Yeah.

> In the future, IMHO, each command has a scope, a term from itset
> patches, and the command is applied to the given scope.  The scope
> is a set of targets, inferiors and threads.  If "catch syscall" command
> is applied to a scope in which one target doesn't support it, GDB can
> report an error like "Target foo in scope bar doesn't support
> 'catch syscall'".
> 

*nod*

FYI, I'm getting very close to posting the all-stop-on-top-of-non-stop
patch.  The remaining dependency (that I had identified) was the
moribund locations rework posted recently.  Itsets will follow.

Thanks,
Pedro Alves
  
Pedro Alves March 3, 2015, 12:29 p.m. UTC | #8
On 03/03/2015 12:11 PM, Yao Qi wrote:
> Sergio Durigan Junior <sergiodj@redhat.com> writes:
> 
>> You mean it's possible to use "catch syscall" on HP-UX targets?  I
>> wonder how it works.
> 
> Yes, that is what I meant, at least that is what I can tell from the
> source.

TARGET_WAITKIND_SYSCALL_ENTRY / TARGET_WAITKIND_SYSCALL_RETURN
was supported by the HP-UX target long before GNU/Linux gained
support.  See inf-ttrace.c.  However, I don't think "catch syscall"
itself ever did.  inf_ttrace_enable_syscall_events is only used
for watchpoints.  Not that I find we should go fix it, mind you.

In gdb 6.8, for example, we had:

      /* Be careful not to try to gather much state about a thread
         that's in a syscall.  It's frequently a losing proposition.  */
    case TARGET_WAITKIND_SYSCALL_ENTRY:
      if (debug_infrun)
        fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_ENTRY\n");
      resume (0, TARGET_SIGNAL_0);
      prepare_to_wait (ecs);
      return;

      /* Before examining the threads further, step this thread to
         get it entirely out of the syscall.  (We get notice of the
         event when the thread is just on the verge of exiting a
         syscall.  Stepping one instruction seems to get it back
         into user code.)  */
    case TARGET_WAITKIND_SYSCALL_RETURN:
      if (debug_infrun)
        fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
      prepare_to_wait (ecs);
      return;

I'd guess once this step after TARGET_WAITKIND_SYSCALL_RETURN
finishes, watchpoints end up processed.  Sounds like watchpoints
are broken on hpux then.  Not that anyone really cares though...

Thanks,
Pedro Alves
  
Pedro Alves March 3, 2015, 12:38 p.m. UTC | #9
On 03/03/2015 12:06 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Do we actually need to do anything when the inferior is created?
>> Supposedly once the inferior is created, we'll try to insert
>> the catchpoint, and that will fail is the target does not
>> support it.
> 
> No, but we need to tweak test case to emit UNSUPPORTED instead of FAIL
> if the target doesn't support "catch syscall".

Yeah, I'm imagining that we'll just detect it like we detect failures
to insert hardware breakpoints / watchpoints, at resume time.
Something like:

    set test "continue with catch syscall"
    gdb_test_multiple "continue" $test {
	-re "Could not insert syscall catchpoint.*$gdb_prompt $" {
              ...
	}
	-re "Breakpoint .*$gdb_prompt $" {
              ...
	}
    }

Thanks,
Pedro Alves
  
Sergio Durigan Junior March 3, 2015, 6:06 p.m. UTC | #10
On Tuesday, March 03 2015, Pedro Alves wrote:

> On 03/03/2015 12:11 PM, Yao Qi wrote:
>> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>> 
>>> You mean it's possible to use "catch syscall" on HP-UX targets?  I
>>> wonder how it works.
>> 
>> Yes, that is what I meant, at least that is what I can tell from the
>> source.
>
> TARGET_WAITKIND_SYSCALL_ENTRY / TARGET_WAITKIND_SYSCALL_RETURN
> was supported by the HP-UX target long before GNU/Linux gained
> support.  See inf-ttrace.c.  However, I don't think "catch syscall"
> itself ever did.  inf_ttrace_enable_syscall_events is only used
> for watchpoints.  Not that I find we should go fix it, mind you.

Exactly, that's what I meant by my question.  Sorry for not being
clear.

I remember seeing TARGET_WAITKIND_SYSCALL_{ENTRY,RETURN} when
implementing "catch syscall", and I even remember making a decision to
use them instead of creating other TARGET_WAITKIND's.  However, the
"catch syscall" itself doesn't work on HP-UX.

Anyway, I just raised this point because we shouldn't "return 1" when
asked if inf-ttrace.c supports "catch syscall".
  
Pedro Alves March 3, 2015, 6:18 p.m. UTC | #11
On 03/03/2015 06:06 PM, Sergio Durigan Junior wrote:
> On Tuesday, March 03 2015, Pedro Alves wrote:
> 
>> On 03/03/2015 12:11 PM, Yao Qi wrote:
>>> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>>>
>>>> You mean it's possible to use "catch syscall" on HP-UX targets?  I
>>>> wonder how it works.
>>>
>>> Yes, that is what I meant, at least that is what I can tell from the
>>> source.
>>
>> TARGET_WAITKIND_SYSCALL_ENTRY / TARGET_WAITKIND_SYSCALL_RETURN
>> was supported by the HP-UX target long before GNU/Linux gained
>> support.  See inf-ttrace.c.  However, I don't think "catch syscall"
>> itself ever did.  inf_ttrace_enable_syscall_events is only used
>> for watchpoints.  Not that I find we should go fix it, mind you.
> 
> Exactly, that's what I meant by my question.  Sorry for not being
> clear.
> 

Ahah, looks like we've been through this before:

  https://sourceware.org/ml/gdb-patches/2013-12/msg00587.html

I apologize on behalf of my former self.  He was silly.  :-)

> I remember seeing TARGET_WAITKIND_SYSCALL_{ENTRY,RETURN} when
> implementing "catch syscall", and I even remember making a decision to
> use them instead of creating other TARGET_WAITKIND's.  However, the
> "catch syscall" itself doesn't work on HP-UX.
> 
> Anyway, I just raised this point because we shouldn't "return 1" when
> asked if inf-ttrace.c supports "catch syscall".

Yeah.  In any case, AFAIR, we were going to remove support for hpux,
as nobody really cares about it anymore...

Thanks,
Pedro Alves
  
Yao Qi March 6, 2015, 4:36 p.m. UTC | #12
Sergio Durigan Junior <sergiodj@redhat.com> writes:

> Anyway, I just raised this point because we shouldn't "return 1" when
> asked if inf-ttrace.c supports "catch syscall".

Sergio,
I'll remove inf_ttrace_supports_syscall_catchpoint from my updated
patch.  Thanks for pointing this out.

In my patch, "catch syscall" command is allowed on the target which
supports catch syscall.  On exec target, GDB will error on command
"catch syscall", so it causes regressions in catch-syscall.exp.

The proc test_catch_syscall_multi_arch in catch-syscall.exp tests
syscall number is mapped to different syscall number on different
gdbarch, but it has:

	# We are not interested in loading any binary here, and in
	# some systems (PowerPC, for example), if we load a binary
	# there is no way to set other architecture.

As you mentioned in
https://sourceware.org/bugzilla/show_bug.cgi?id=10737#c4 , when GDB
starts an inferior, the arch is set correspondingly, so it is unable
reproduce the bug.  test_catch_syscall_multi_arch is the simpler
reproducer, IMO.  How about 'upgrade' test_catch_syscall_multi_arch like
this?

 - file 32-bit program
 - run to main
 - catch syscall 5
 - kill
 - file 64-bit program
 - run to main
 - catch syscall 5
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index db4b872..8e0ffb8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12135,9 +12135,9 @@  catch_syscall_command_1 (char *arg, int from_tty,
   struct gdbarch *gdbarch = get_current_arch ();
 
   /* Checking if the feature if supported.  */
-  if (gdbarch_get_syscall_number_p (gdbarch) == 0)
+  if (!target_supports_syscall_catchpoint (gdbarch))
     error (_("The feature 'catch syscall' is not supported on \
-this architecture yet."));
+this target yet."));
 
   tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
 
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 080e167..9c8c561 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -1180,6 +1180,15 @@  inf_ttrace_get_ada_task_ptid (struct target_ops *self, long lwp, long thread)
   return ptid_build (ptid_get_pid (inferior_ptid), lwp, 0);
 }
 
+/* Implement the supports_syscall_catchpoint target_ops method.  */
+
+static int
+inf_ttrace_supports_syscall_catchpoint (struct target_ops *self,
+					struct gdbarch *gdbarch)
+{
+  return 1;
+}
+
 
 struct target_ops *
 inf_ttrace_target (void)
@@ -1206,6 +1215,7 @@  inf_ttrace_target (void)
   t->to_pid_to_str = inf_ttrace_pid_to_str;
   t->to_xfer_partial = inf_ttrace_xfer_partial;
   t->to_get_ada_task_ptid = inf_ttrace_get_ada_task_ptid;
+  t->to_supports_syscall_catchpoint = inf_ttrace_supports_syscall_catchpoint;
 
   return t;
 }
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 2e1133d..6ebcc3e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -594,6 +594,15 @@  linux_child_set_syscall_catchpoint (struct target_ops *self,
   return 0;
 }
 
+/* Implement the supports_syscall_catchpoint target_ops method.  */
+
+static int
+linux_child_supports_syscall_catchpoint (struct target_ops *ops,
+					 struct gdbarch *gdbarch)
+{
+  return gdbarch_get_syscall_number_p (gdbarch);
+}
+
 /* On GNU/Linux there are no real LWP's.  The closest thing to LWP's
    are processes sharing the same VM space.  A multi-threaded process
    is basically a group of such processes.  However, such a grouping
@@ -4277,6 +4286,7 @@  linux_target_install_ops (struct target_ops *t)
   t->to_insert_exec_catchpoint = linux_child_insert_exec_catchpoint;
   t->to_remove_exec_catchpoint = linux_child_remove_exec_catchpoint;
   t->to_set_syscall_catchpoint = linux_child_set_syscall_catchpoint;
+  t->to_supports_syscall_catchpoint = linux_child_supports_syscall_catchpoint;
   t->to_pid_to_exec_file = linux_child_pid_to_exec_file;
   t->to_post_startup_inferior = linux_child_post_startup_inferior;
   t->to_post_attach = linux_child_post_attach;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index e026179..9011acd 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1144,6 +1144,35 @@  debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int a
 }
 
 static int
+delegate_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
+{
+  self = self->beneath;
+  return self->to_supports_syscall_catchpoint (self, arg1);
+}
+
+static int
+tdefault_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
+{
+  return 0;
+}
+
+static int
+debug_supports_syscall_catchpoint (struct target_ops *self, struct gdbarch *arg1)
+{
+  int result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_supports_syscall_catchpoint (...)\n", debug_target.to_shortname);
+  result = debug_target.to_supports_syscall_catchpoint (&debug_target, arg1);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_supports_syscall_catchpoint (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_struct_gdbarch_p (arg1);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_int (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
+static int
 delegate_has_exited (struct target_ops *self, int arg1, int arg2, int *arg3)
 {
   self = self->beneath;
@@ -3849,6 +3878,8 @@  install_delegators (struct target_ops *ops)
     ops->to_remove_exec_catchpoint = delegate_remove_exec_catchpoint;
   if (ops->to_set_syscall_catchpoint == NULL)
     ops->to_set_syscall_catchpoint = delegate_set_syscall_catchpoint;
+  if (ops->to_supports_syscall_catchpoint == NULL)
+    ops->to_supports_syscall_catchpoint = delegate_supports_syscall_catchpoint;
   if (ops->to_has_exited == NULL)
     ops->to_has_exited = delegate_has_exited;
   if (ops->to_mourn_inferior == NULL)
@@ -4091,6 +4122,7 @@  install_dummy_methods (struct target_ops *ops)
   ops->to_insert_exec_catchpoint = tdefault_insert_exec_catchpoint;
   ops->to_remove_exec_catchpoint = tdefault_remove_exec_catchpoint;
   ops->to_set_syscall_catchpoint = tdefault_set_syscall_catchpoint;
+  ops->to_supports_syscall_catchpoint = tdefault_supports_syscall_catchpoint;
   ops->to_has_exited = tdefault_has_exited;
   ops->to_mourn_inferior = default_mourn_inferior;
   ops->to_can_run = tdefault_can_run;
@@ -4235,6 +4267,7 @@  init_debug_target (struct target_ops *ops)
   ops->to_insert_exec_catchpoint = debug_insert_exec_catchpoint;
   ops->to_remove_exec_catchpoint = debug_remove_exec_catchpoint;
   ops->to_set_syscall_catchpoint = debug_set_syscall_catchpoint;
+  ops->to_supports_syscall_catchpoint = debug_supports_syscall_catchpoint;
   ops->to_has_exited = debug_has_exited;
   ops->to_mourn_inferior = debug_mourn_inferior;
   ops->to_can_run = debug_can_run;
diff --git a/gdb/target.h b/gdb/target.h
index 2811c47..0ef1ba2 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -541,6 +541,9 @@  struct target_ops
     int (*to_set_syscall_catchpoint) (struct target_ops *,
 				      int, int, int, int, int *)
       TARGET_DEFAULT_RETURN (1);
+    int (*to_supports_syscall_catchpoint) (struct target_ops *,
+					   struct gdbarch *)
+      TARGET_DEFAULT_RETURN (0);
     int (*to_has_exited) (struct target_ops *, int, int, int *)
       TARGET_DEFAULT_RETURN (0);
     void (*to_mourn_inferior) (struct target_ops *)
@@ -1523,6 +1526,12 @@  int target_follow_fork (int follow_child, int detach_fork);
 						  pid, needed, any_count, \
 						  table_size, table)
 
+/* Return true if GDBARCH on current target supports syscall catchpoint,
+   otherwise, return false.  */
+
+#define target_supports_syscall_catchpoint(gdbarch)			\
+  (*current_target.to_supports_syscall_catchpoint) (&current_target, gdbarch)
+
 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */