Re: [PATCH 3/3] catch syscall -- try 5 -- Build-system, testcase, documentation and extra command

[Eli Zaretskii <eliz at gnu dot org>]

> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Date: Wed, 22 Apr 2009 21:33:02 -0300
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,18 @@
>  
>  *** Changes since GDB 6.8
>  
> +* GDB now has the new command `catch syscall'.  It can be used to
> +catch when the inferior calls a system call, or when the system call
> +returns.  Also, you can specify which system calls you would like GDB
> +to catch (or issue only a `catch syscall' without arguments, which will
> +make GDB catch every system call).  For instance, if you would like to
> +catch the system call close, you would issue a:
> +
> +    (gdb) catch syscall close
> +

This part is approved, but I suggest to add a sentence or two saying
what happens when such a catchpoint is hit.  Without such a
description, the reader will be left wondering why this feature is
useful.

> +This feature is available with a native GDB running on the following
> +architectures: x86, PowerPC and PowerPC64.

This makes it sound like system calls are a function of the hardware
architecture, which I think is not true.  I think you want to add
"Linux" somewhere here.

> gdb/doc/ChangeLog:
> 
> 2009-04-22  Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>
> 
> 	* gdb.texinfo (catch syscall): Documentation about the new
> 	feature.

This ChangeLog entry should have "Set Catchpoints" in parentheses
(without the quotes), because we state node names there, and this is
the name of the node where you add the description of "set syscall".

The patch for the manual is approved as well, with the following
comments:

> +@item syscall
> +@itemx syscall @r{[}@var{name} @r{|} @var{number}@r{]} @r{...}
> +@cindex break on a system call.
> +A call to or return from a @code{syscall}.

You never explain what a syscall is.  I suggest to rephrase and
expand:

  A call to or return from a system call, a.k.a.@: @dfn{syscall}.  A
  syscall is a mechanism for application programs to request a service
  from the operating system (OS) or one of the OS system services.
  @value{GDBN} can catch some or all of the syscalls issued by the
  debuggee, and show the related information for each syscall.

>                                       If no argument is specified,
> +then it catches a call to or return from any system call.

"and", not "or":

   If no argument is specified, calls to and returns from all system
   calls will be caught.

> +@var{name} can be any valid system call name in the system.  You can
> +use the @value{GDBN} command-line completion facilities to list the
> +available choices.  @xref{Completion,, Command Completion}, for
> +details on how to do this.
> +
> +You may also specify the system call numerically.  This may be useful
> +if @value{GDBN} does not fully support your system's list of system
> +calls.

This text is okay, but it could still leave the reader wondering about
too many things.  How does GDB know which syscalls are available? what
are the possible syscall numbers, and how do I know which ones are
valid? what happens if I use a name or number that are invalid?

The examples you give below answer some of these questions, and hint
to answers to others, but I think we still need some general text
about these issues, and we need a text that is as system-independent
as possible.  So I suggest this text instead:

  @var{name} can be any system call name that is valid for the
  underlying OS.  Just what syscalls are valid depends on the OS.  On
  GNU and Unix systems, you can find the full list of valid syscall
  names on <PUT HERE THE NAME OF SYSTEM FILE WHERE TO FIND THE LIST>.

  @c For MS-Windows, the syscall names and the corresponding numbers
  @c can be found, e.g., on this URL:
  @c http://www.metasploit.com/users/opcode/syscalls.html
  @c but we don't support Windows syscalls yet.

  Normally, @value{GDBN} knows in advance which syscalls are valid for
  each OS, so you can use the @value{GDBN} command-line completion
  facilities (@pxref{Completion,, command completion}) to list the
  available choices.

  You may also specify the system call numerically.  A syscall's
  number is the value passed to the OS's syscall dispatcher to
  identify the requested service.  When you specify the syscall by its
  name, @value{GDBN} uses its database of syscalls to convert the name
  into the corresponding numeric code, but using the number directly
  may be useful if @value{GDBN}'s database does not have the complete
  list of syscalls on your system (e.g., because @value{GDBN} lags
  behind the OS upgrades).

WDYT?

> +@smallexample
> +(@value{GDBP}) catch syscall chroot
> +Catchpoint 1 (syscall(s) 'chroot')

Why do you have the "(s)" part in this message?  You are announcing a
catchpoint for a single syscall, right?

> +Catchpoint 1 (call to syscall 'close'), \
> +	   0xffffe424 in __kernel_vsyscall ()

> +Catchpoint 1 (returned from syscall 'close'), \
> +	0xffffe424 in __kernel_vsyscall ()

Is this all GDB displays when a syscall is caught?  I thought it also
displayed the return code and the syscall parameters, like strace
does.  If we don't do that, this facility looks much less useful than
it sounds, doesn't it?  I realize that the user could glean all that
information by using the normal GDB commands, but for that, she would
need to know the some intimate details of the syscall dispatched
(__kernel_vsyscall in this case), right?

Also, how about showing the syscall number together with its name?

> +And last but not least, an example of specifying a system call
> +numerically:
> +
> +@smallexample
> +(@value{GDBP}) catch syscall 252
> +Catchpoint 1 (syscall(s) 'exit_group')
> +(@value{GDBP}) r
> +Starting program: /tmp/catch-syscall
> +
> +Catchpoint 1 (call to syscall 'exit_group'), \
> +		   0xffffe424 in __kernel_vsyscall ()

This example uses a number, but the XML database obviously has the
corresponding name of the syscall, right?  It would be more
interesting to have an example for a syscall whose name is not in the
XML file.