public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 1/5] Implemement support for groups of syscalls in the xml-syscall interface.
@ 2016-05-17 16:34 Doug Evans
  2016-05-18  2:34 ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 4+ messages in thread
From: Doug Evans @ 2016-05-17 16:34 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches, palves, sergiodj

Gabriel Krisman Bertazi writes:
  > This is done by maintaining a list of syscall_group_desc for each
  > syscall group inside the syscalls_info structure.  Inside each
  > syscall_group_desc we have a vector of pointers to the syscalls that are
  > part of that group.
  >
  > gdb/
  >
  > 	* syscalls/gdb-syscalls.dtd: Include group attribute to the
  > 	syscall element.
  > 	* xml-syscall.c (get_syscalls_by_group): New.
  > 	(get_syscall_group_names): New.
  > 	(struct syscall_group_desc): New structure to store group data.
  > 	(struct syscalls_info): Include field to store the group list.
  > 	(sysinfo_free_syscall_group_desc): New.
  > 	(free_syscalls_info): Free group list.
  > 	(syscall_group_create_syscall_group_desc): New.
  > 	(syscall_group_add_syscall): New.
  > 	(syscall_create_syscall_desc): Add syscall to its groups.
  > 	(syscall_start_syscall): Load group attribute.
  > 	(syscall_group_get_group_by_name): New.
  > 	(xml_list_syscalls_by_group): New.
  > 	(xml_list_of_groups): New.
  > 	* xml-syscall.h (get_syscalls_by_group): Export function
  > 	to retrieve a list of syscalls filtered by the group name.
  > 	(get_syscall_group_names): Export function to retrieve the list
  > 	of syscall groups.

Hi. Just some nits.
[I haven't read the entire series yet though.]

  > diff --git a/gdb/syscalls/gdb-syscalls.dtd  
b/gdb/syscalls/gdb-syscalls.dtd
  > index 3deda12..de47d4d 100644
  > --- a/gdb/syscalls/gdb-syscalls.dtd
  > +++ b/gdb/syscalls/gdb-syscalls.dtd
  > @@ -11,4 +11,5 @@
  >  <!ELEMENT syscall		EMPTY>
  >  <!ATTLIST syscall
  >  	name			CDATA	#REQUIRED
  > -	number			CDATA	#REQUIRED>
  > +	number			CDATA	#REQUIRED
  > +	groups			CDATA	#IMPLIED>

I'm assuming "IMPLIED" is ok here, I don't actually know.

  > diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
  > index ceaf750..dafbb1e 100644
  > --- a/gdb/xml-syscall.c
  > +++ b/gdb/xml-syscall.c
  > @@ -77,6 +77,20 @@ get_syscall_names (struct gdbarch *gdbarch)
  >    return NULL;
  >  }
  >
  > +struct syscall *
  > +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
  > +{
  > +  syscall_warn_user ();
  > +  return NULL;
  > +}
  > +
  > +const char **
  > +get_syscall_group_names (struct gdbarch *gdbarch)
  > +{
  > +  syscall_warn_user ();
  > +  return NULL;
  > +}
  > +
  >  #else /* ! HAVE_LIBEXPAT */
  >
  >  /* Structure which describes a syscall.  */
  > @@ -92,6 +106,19 @@ typedef struct syscall_desc
  >  } *syscall_desc_p;
  >  DEF_VEC_P(syscall_desc_p);
  >
  > +/* Structure of a syscall group.  */
  > +typedef struct syscall_group_desc
  > +{
  > +  /* The group name.  */
  > +
  > +  char *name;
  > +
  > +  /* The syscalls that are part of the group.  */
  > +
  > +  VEC(syscall_desc_p) *syscalls;
  > +} *syscall_group_desc_p;
  > +DEF_VEC_P(syscall_group_desc_p);
  > +
  >  /* Structure that represents syscalls information.  */
  >  struct syscalls_info
  >  {
  > @@ -99,6 +126,10 @@ struct syscalls_info
  >
  >    VEC(syscall_desc_p) *syscalls;
  >
  > +  /* The syscall groups.  */
  > +
  > +  VEC(syscall_group_desc_p) *groups;
  > +
  >    /* Variable that will hold the last known data-directory.  This is
  >       useful to know whether we should re-read the XML info for the
  >       target.  */
  > @@ -126,11 +157,21 @@ syscalls_info_free_syscalls_desc (struct  
syscall_desc *sd)
  >    xfree (sd->name);
  >  }
  >
  > +/* Free syscall_group_desc members but not the structure itself.  */
  > +
  > +static void
  > +syscalls_info_free_syscall_group_desc (struct syscall_group_desc *sd)
  > +{
  > +  VEC_free (syscall_desc_p, sd->syscalls);
  > +  xfree (sd->name);
  > +}
  > +
  >  static void
  >  free_syscalls_info (void *arg)
  >  {
  >    struct syscalls_info *syscalls_info = (struct syscalls_info *) arg;
  >    struct syscall_desc *sysdesc;
  > +  struct syscall_group_desc *groupdesc;
  >    int i;
  >
  >    xfree (syscalls_info->my_gdb_datadir);
  > @@ -144,6 +185,17 @@ free_syscalls_info (void *arg)
  >        VEC_free (syscall_desc_p, syscalls_info->syscalls);
  >      }
  >
  > +  if (syscalls_info->groups != NULL)
  > +    {
  > +      for (i = 0;
  > +	   VEC_iterate (syscall_group_desc_p,
  > +			syscalls_info->groups, i, groupdesc);
  > +	   i++)
  > +	syscalls_info_free_syscall_group_desc (groupdesc);
  > +
  > +      VEC_free (syscall_group_desc_p, syscalls_info->groups);
  > +    }
  > +
  >    xfree (syscalls_info);
  >  }
  >
  > @@ -153,16 +205,73 @@ make_cleanup_free_syscalls_info (struct  
syscalls_info *syscalls_info)
  >    return make_cleanup (free_syscalls_info, syscalls_info);
  >  }
  >
  > +/* Create a new syscall group.  Return pointer to the
  > +   syscall_group_desc structure that represents the new group.  */
  > +
  > +static struct syscall_group_desc *
  > +syscall_group_create_syscall_group_desc (struct syscalls_info  
*syscalls_info,
  > +					 const char *group)
  > +{
  > +  struct syscall_group_desc *groupdesc = XCNEW (struct  
syscall_group_desc);
  > +
  > +  groupdesc->name = xstrdup (group);
  > +
  > +  VEC_safe_push (syscall_group_desc_p, syscalls_info->groups,  
groupdesc);
  > +
  > +  return groupdesc;
  > +}
  > +
  > +/* Add a syscall to the group.  If group doesn't exist, create it.  */
  > +
  > +static void
  > +syscall_group_add_syscall (struct syscalls_info *syscalls_info,
  > +			   struct syscall_desc *syscall,
  > +			   const char *group)
  > +{
  > +  struct syscall_group_desc *groupdesc;
  > +  int i;
  > +
  > +  /* Search for an existing group.  */
  > +  for (i = 0;
  > +       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i,  
groupdesc);
  > +       i++)
  > +    {
  > +      if (strcmp (groupdesc->name, group) == 0)
  > +	break;
  > +    }
  > +
  > +  if (groupdesc == NULL)
  > +    {
  > +      /* No group was found with this name.  We must create a new
  > +	 one.  */
  > +      groupdesc = syscall_group_create_syscall_group_desc  
(syscalls_info,
  > +							   group);
  > +    }
  > +
  > +  VEC_safe_push (syscall_desc_p, groupdesc->syscalls, syscall);
  > +}
  > +
  >  static void
  >  syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
  > -                             const char *name, int number)
  > +			     const char *name, int number,
  > +			     char *groups)
  >  {
  >    struct syscall_desc *sysdesc = XCNEW (struct syscall_desc);
  > +  char *group;
  >
  >    sysdesc->name = xstrdup (name);
  >    sysdesc->number = number;
  >
  >    VEC_safe_push (syscall_desc_p, syscalls_info->syscalls, sysdesc);
  > +
  > +  /*  Add syscall to its groups.  */
  > +  if (groups != NULL)
  > +    {
  > +      for (group = strtok (groups, ",");
  > +	   group != NULL;
  > +	   group = strtok (NULL, ","))
  > +	syscall_group_add_syscall (syscalls_info, sysdesc, group);
  > +    }
  >  }
  >
  >  /* Handle the start of a <syscall> element.  */
  > @@ -177,6 +286,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
  >    /* syscall info.  */
  >    char *name = NULL;
  >    int number = 0;
  > +  char *groups = NULL;
  >
  >    len = VEC_length (gdb_xml_value_s, attributes);
  >
  > @@ -186,13 +296,15 @@ syscall_start_syscall (struct gdb_xml_parser  
*parser,
  >          name = (char *) attrs[i].value;
  >        else if (strcmp (attrs[i].name, "number") == 0)
  >          number = * (ULONGEST *) attrs[i].value;
  > +      else if (strcmp (attrs[i].name, "groups") == 0)
  > +        groups = (char *) attrs[i].value;
  >        else
  >          internal_error (__FILE__, __LINE__,
  >                          _("Unknown attribute name '%s'."),  
attrs[i].name);
  >      }
  >
  >    gdb_assert (name);
  > -  syscall_create_syscall_desc (data->syscalls_info, name, number);
  > +  syscall_create_syscall_desc (data->syscalls_info, name, number,  
groups);
  >  }
  >
  >
  > @@ -200,6 +312,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
  >  static const struct gdb_xml_attribute syscall_attr[] = {
  >    { "number", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
  >    { "name", GDB_XML_AF_NONE, NULL, NULL },
  > +  { "groups", GDB_XML_AF_OPTIONAL, NULL, NULL },
  >    { NULL, GDB_XML_AF_NONE, NULL, NULL }
  >  };
  >
  > @@ -321,6 +434,32 @@ init_syscalls_info (struct gdbarch *gdbarch)
  >    set_gdbarch_syscalls_info (gdbarch, syscalls_info);
  >  }
  >
  > +/* Search for a syscall group by its name.  Return syscall_group_desc
  > +   structure for the group if found or NULL otherwise.  */
  > +
  > +static struct syscall_group_desc *
  > +syscall_group_get_group_by_name (const struct syscalls_info  
*syscalls_info,
  > +				 const char *group)
  > +{
  > +  struct syscall_group_desc *groupdesc;
  > +  int i;
  > +
  > +  if (syscalls_info == NULL)
  > +    return NULL;
  > +
  > +  if (group == NULL)
  > +    return NULL;
  > +
  > +   /* Search for existing group.  */
  > +  for (i = 0;
  > +       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i,  
groupdesc);
  > +       i++)
  > +    if (strcmp (groupdesc->name, group) == 0)
  > +      return groupdesc;

Wrap if in {}.
If more than one line is involved, it goes in braces (even if just one  
statement).

  > +
  > +  return NULL;
  > +}
  > +
  >  static int
  >  xml_get_syscall_number (struct gdbarch *gdbarch,
  >                          const char *syscall_name)
  > @@ -388,6 +527,75 @@ xml_list_of_syscalls (struct gdbarch *gdbarch)
  >    return names;
  >  }
  >
  > +/* Iterate over the syscall_group_desc element to return a list of
  > +   syscalls that are part of the given group, terminated by an empty
  > +   element.  If the syscall group doesn't exist, return NULL.  */
  > +
  > +static struct syscall *
  > +xml_list_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
  > +{
  > +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
  > +  struct syscall_group_desc *groupdesc;
  > +  struct syscall_desc *sysdesc;
  > +  struct syscall *syscalls = NULL;
  > +  int nsyscalls;
  > +  int i;
  > +
  > +  if (syscalls_info == NULL)
  > +    return NULL;
  > +
  > +  groupdesc = syscall_group_get_group_by_name (syscalls_info, group);
  > +  if (groupdesc == NULL)
  > +    return NULL;
  > +
  > +  nsyscalls = VEC_length (syscall_desc_p, groupdesc->syscalls);
  > +  syscalls = (struct syscall*) xmalloc ((nsyscalls + 1)
  > +					* sizeof (struct syscall));
  > +
  > +  for (i = 0;
  > +       VEC_iterate (syscall_desc_p, groupdesc->syscalls, i, sysdesc);
  > +       i++)
  > +    {
  > +      syscalls[i].name = sysdesc->name;
  > +      syscalls[i].number = sysdesc->number;
  > +    }
  > +
  > +  /* Add final element marker.  */
  > +  syscalls[i].name = NULL;
  > +  syscalls[i].number = 0;
  > +
  > +  return syscalls;
  > +}
  > +
  > +/* Return a NULL terminated list of syscall groups or an empty list, if
  > +   no syscall group is available.  Return NULL, if there is no syscall
  > +   information available.  */
  > +
  > +static const char **
  > +xml_list_of_groups (struct gdbarch *gdbarch)
  > +{
  > +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
  > +  struct syscall_group_desc *groupdesc;
  > +  const char **names = NULL;
  > +  int i;
  > +  int ngroups;
  > +
  > +  if (syscalls_info == NULL)
  > +    return NULL;
  > +
  > +  ngroups = VEC_length (syscall_group_desc_p, syscalls_info->groups);
  > +  names = (const char**) xmalloc ((ngroups + 1) * sizeof (char *));
  > +
  > +  for (i = 0;
  > +       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i,  
groupdesc);
  > +       i++)
  > +    names[i] = groupdesc->name;
  > +
  > +  names[i] = NULL;
  > +
  > +  return names;
  > +}
  > +
  >  void
  >  set_xml_syscall_file_name (struct gdbarch *gdbarch, const char *name)
  >  {
  > @@ -422,4 +630,24 @@ get_syscall_names (struct gdbarch *gdbarch)
  >    return xml_list_of_syscalls (gdbarch);
  >  }
  >
  > +/* See comment in xml-syscall.h.  */
  > +
  > +struct syscall *
  > +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
  > +{
  > +  init_syscalls_info (gdbarch);
  > +
  > +  return xml_list_syscalls_by_group (gdbarch, group);
  > +}
  > +
  > +/* See comment in xml-syscall.h.  */
  > +
  > +const char **
  > +get_syscall_group_names (struct gdbarch *gdbarch)
  > +{
  > +  init_syscalls_info (gdbarch);
  > +
  > +  return xml_list_of_groups (gdbarch);
  > +}
  > +
  >  #endif /* ! HAVE_LIBEXPAT */
  > diff --git a/gdb/xml-syscall.h b/gdb/xml-syscall.h
  > index b0dd401..407613e 100644
  > --- a/gdb/xml-syscall.h
  > +++ b/gdb/xml-syscall.h
  > @@ -50,4 +50,20 @@ void get_syscall_by_name (struct gdbarch *gdbarch,
  >
  >  const char **get_syscall_names (struct gdbarch *gdbarch);
  >
  > +/* Function used to retrieve the list of syscalls of a given group in
  > +   the system.  Return a list of syscalls that are element of the
  > +   group, terminated by an empty element. The list is malloc'ed
  > +   and must be freed by the caller.  If group doesn't exist, return
  > +   NULL.  */
  > +
  > +struct syscall *get_syscalls_by_group (struct gdbarch *gdbarch,
  > +				       const char *group);
  > +
  > +/* Function used to retrieve the list of syscall groups in the system.
  > +   Return an array of strings terminated by a NULL element.  The list
  > +   must be freed by the caller.  Return NULL if there is no syscall
  > +   information available.  */
  > +
  > +const char **get_syscall_group_names (struct gdbarch *gdbarch);
  > +
  >  #endif /* XML_SYSCALL_H */
  > --
  > 2.4.3

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH v5 0/5] Catch syscall groups
@ 2016-05-15 20:32 Gabriel Krisman Bertazi
  2016-05-15 20:32 ` [PATCH v5 1/5] Implemement support for groups of syscalls in the xml-syscall interface Gabriel Krisman Bertazi
  0 siblings, 1 reply; 4+ messages in thread
From: Gabriel Krisman Bertazi @ 2016-05-15 20:32 UTC (permalink / raw)
  To: gdb-patches; +Cc: palves, sergiodj, dje, Gabriel Krisman Bertazi

Hi,

Last year, I submitted a proposal to include support to catch groups of
related syscalls in GDB.  My goal was to allow users to just write
"catch syscall group:network" to catch all network related syscalls,
instead of listing every single one of them in the command line.  This
idea came after what is done by the "strace -e trace=" kind of
expressions, and a suggestion made by Tom Tromey in the Sourceware
bugzilla.

Anyway, the patch iterated a little in this list and got positive
feedback and great suggestions from Sergio, Pedro and Doug.  Eventually,
I got very busy with college/work stuff, and had to put this on
hold.. until now.  I'm looking to put more effort on this to get it
upstream anytime soon.

There are just a few differences from the last iteration I submitted.
In summary, I followed Doug's suggestion and made the XML generation
happen only when building in maintainer's mode.  On that mode, we invoke
xsltproc to transform the xml.in files to .xml files that get read by
GDB.  I also plan to keep the generated xmls in the repository, such
that when building outside of maintainers mode, we don't depend on
xltproc.

As usual, the patches don't iclude the generated files.  Therefore, to
test it, you need to use maintainer mode at least once to get the xml
files generated.  When I push the patch set upstream, I'll ammend the
commit to include generated files, such that it doesn't break bisectable
builds.

If anyone is feeling lazy and just want to see things working, I have a
version that includes the generated files at my git:

git clone git://git.krisman.be/binutils-gdb.git -b syscall-group

Thanks,


Gabriel Krisman Bertazi (5):
  Implemement support for groups of syscalls in the xml-syscall    
    interface.
  Add support to catch groups of syscalls.
  Add tests for catching groups of syscalls on supported    
    architectures.
  Include group information to xml syscall files.
  Update documentation on catching a group of related syscalls.

 gdb/NEWS                                           |   5 +
 gdb/break-catch-syscall.c                          | 104 +++++++--
 gdb/configure.ac                                   |  10 +
 gdb/data-directory/Makefile.in                     |   4 +
 gdb/doc/gdb.texinfo                                |  28 ++-
 .../{aarch64-linux.xml => aarch64-linux.xml.in}    |   0
 .../{amd64-linux.xml => amd64-linux.xml.in}        |   0
 gdb/syscalls/apply-defaults.xsl                    |  27 +++
 gdb/syscalls/{arm-linux.xml => arm-linux.xml.in}   |   0
 gdb/syscalls/{bfin-linux.xml => bfin-linux.xml.in} |   0
 gdb/syscalls/gdb-syscalls.dtd                      |   3 +-
 gdb/syscalls/{i386-linux.xml => i386-linux.xml.in} |   0
 gdb/syscalls/linux-defaults.xml.in                 | 243 +++++++++++++++++++++
 .../{mips-n32-linux.xml => mips-n32-linux.xml.in}  |   0
 .../{mips-n64-linux.xml => mips-n64-linux.xml.in}  |   0
 .../{mips-o32-linux.xml => mips-o32-linux.xml.in}  |   0
 gdb/syscalls/{ppc-linux.xml => ppc-linux.xml.in}   |   0
 .../{ppc64-linux.xml => ppc64-linux.xml.in}        |   0
 gdb/syscalls/{s390-linux.xml => s390-linux.xml.in} |   0
 .../{s390x-linux.xml => s390x-linux.xml.in}        |   0
 .../{sparc-linux.xml => sparc-linux.xml.in}        |   0
 .../{sparc64-linux.xml => sparc64-linux.xml.in}    |   0
 gdb/testsuite/gdb.base/catch-syscall.exp           |  39 ++++
 gdb/xml-syscall.c                                  | 232 +++++++++++++++++++-
 gdb/xml-syscall.h                                  |  16 ++
 25 files changed, 693 insertions(+), 18 deletions(-)
 rename gdb/syscalls/{aarch64-linux.xml => aarch64-linux.xml.in} (100%)
 rename gdb/syscalls/{amd64-linux.xml => amd64-linux.xml.in} (100%)
 create mode 100644 gdb/syscalls/apply-defaults.xsl
 rename gdb/syscalls/{arm-linux.xml => arm-linux.xml.in} (100%)
 rename gdb/syscalls/{bfin-linux.xml => bfin-linux.xml.in} (100%)
 rename gdb/syscalls/{i386-linux.xml => i386-linux.xml.in} (100%)
 create mode 100644 gdb/syscalls/linux-defaults.xml.in
 rename gdb/syscalls/{mips-n32-linux.xml => mips-n32-linux.xml.in} (100%)
 rename gdb/syscalls/{mips-n64-linux.xml => mips-n64-linux.xml.in} (100%)
 rename gdb/syscalls/{mips-o32-linux.xml => mips-o32-linux.xml.in} (100%)
 rename gdb/syscalls/{ppc-linux.xml => ppc-linux.xml.in} (100%)
 rename gdb/syscalls/{ppc64-linux.xml => ppc64-linux.xml.in} (100%)
 rename gdb/syscalls/{s390-linux.xml => s390-linux.xml.in} (100%)
 rename gdb/syscalls/{s390x-linux.xml => s390x-linux.xml.in} (100%)
 rename gdb/syscalls/{sparc-linux.xml => sparc-linux.xml.in} (100%)
 rename gdb/syscalls/{sparc64-linux.xml => sparc64-linux.xml.in} (100%)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-18 16:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 16:34 [PATCH v5 1/5] Implemement support for groups of syscalls in the xml-syscall interface Doug Evans
2016-05-18  2:34 ` Gabriel Krisman Bertazi
2016-05-18 16:10   ` Doug Evans
  -- strict thread matches above, loose matches on Subject: below --
2016-05-15 20:32 [PATCH v5 0/5] Catch syscall groups Gabriel Krisman Bertazi
2016-05-15 20:32 ` [PATCH v5 1/5] Implemement support for groups of syscalls in the xml-syscall interface Gabriel Krisman Bertazi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).