[PATCH 1/3] Making syscall information be per-arch, instead of global

[PATCH 1/3] Making syscall information be per-arch, instead of global

[Sergio Durigan Junior]

This commit basically moves the "struct syscalls_info" to gdbarch,
instead of making it global inside xml-syscall.c.  This allows GDB to
maintain the knowledge about each architecture's syscalls
independently.

This is important because the relationship syscall name <-> number
differs from arch to arch; for example, before this patch you would
see:

  (gdb) set architecture i386
  The target architecture is assumed to be i386
  (gdb) catch syscall 5
  Catchpoint 1 (syscall 'open' [5])
  (gdb) set architecture i386:x86-64
  The target architecture is assumed to be i386:x86-64
  (gdb) catch syscall 5
  Catchpoint 2 (syscall 'open' [5])

But with the patch:

  (gdb) set architecture i386
  The target architecture is assumed to be i386
  (gdb) catch syscall 5
  Catchpoint 1 (syscall 'open' [5])
  (gdb) set architecture i386:x86-64
  The target architecture is assumed to be i386:x86-64
  (gdb) catch syscall 5
  Catchpoint 2 (syscall 'fstat' [5])

I.e., when the architecture is changed, GDB is now aware of the
syscalls in the new arch and doesn't think we are still referring to
the old arch.

This patch does not fix all of the bugs that exist on the "catch
syscall" command for multi-arch/multi-inferior scenarios; Pedro
outlined a few more issues on
<https://sourceware.org/bugzilla/show_bug.cgi?id=10737#c5>.
Nevertheless, it is a good step towards a "catch syscall" that works
in all scenarios.

gdb/
2014-11-12  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/10737
	* gdbarch.c: Regenerate.
	* gdbarch.h: Likewise.
	* gdbarch.sh: Forward declare "struct syscalls_info".
	(xml_syscall_file): New variable.
	(syscalls_info): Likewise.
	* xml-syscall.c: Include gdbarch.h.
	(set_xml_syscall_file_name): Accept gdbarch parameter.
	(get_syscall_by_number): Likewise.
	(get_syscall_by_name): Likewise.
	(get_syscall_names): Likewise.
	(my_gdb_datadir): Delete global variable.
	(struct syscalls_info) <my_gdb_datadir>: New variable.
	(struct syscalls_info) <sysinfo>: Rename variable to
	"syscalls_info".
	(sysinfo): Delete global variable.
	(have_initialized_sysinfo): Likewise.
	(xml_syscall_file): Likewise.
	(sysinfo_free_syscalls_desc): Rename to...
	(syscalls_info_free_syscalls_desc): ... this.
	(free_syscalls_info): Rename "sysinfo" to "syscalls_info".  Adjust
	code to the new layout of "struct syscalls_info".
	(make_cleanup_free_syscalls_info): Rename parameter "sysinfo" to
	"syscalls_info".
	(syscall_create_syscall_desc): Likewise.
	(syscall_start_syscall): Likewise.
	(syscall_parse_xml): Likewise.
	(xml_init_syscalls_info): Likewise.
	(init_sysinfo): Rename to...
	(init_syscalls_info): ...this.  Add gdbarch as a parameter.
	Adjust function to deal with gdbarch.
	(xml_get_syscall_number): Delete parameter sysinfo.  Accept
	gdbarch as a parameter.  Adjust code.
	(xml_get_syscall_name): Likewise.
	(xml_list_of_syscalls): Likewise.
	(set_xml_syscall_file_name): Accept gdbarch as parameter.
	(get_syscall_by_number): Likewise.
	(get_syscall_by_name): Likewise.
	(get_syscall_names): Likewise.
	* xml-syscall.h (set_xml_syscall_file_name): Likewise.
	(get_syscall_by_number): Likewise.
	(get_syscall_by_name): Likewise.
	(get_syscall_names): Likewise.
---
 gdb/gdbarch.c     |  44 +++++++++++++++
 gdb/gdbarch.h     |  11 ++++
 gdb/gdbarch.sh    |   7 +++
 gdb/xml-syscall.c | 166 +++++++++++++++++++++++++++++-------------------------
 gdb/xml-syscall.h |  11 ++--
 5 files changed, 157 insertions(+), 82 deletions(-)

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd53acc..f89a6d2 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -288,6 +288,8 @@ struct gdbarch
   gdbarch_get_siginfo_type_ftype *get_siginfo_type;
   gdbarch_record_special_symbol_ftype *record_special_symbol;
   gdbarch_get_syscall_number_ftype *get_syscall_number;
+  const char * xml_syscall_file;
+  struct syscalls_info * syscalls_info;
   const char *const * stap_integer_prefixes;
   const char *const * stap_integer_suffixes;
   const char *const * stap_register_prefixes;
@@ -603,6 +605,8 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of get_siginfo_type, has predicate.  */
   /* Skip verify of record_special_symbol, has predicate.  */
   /* Skip verify of get_syscall_number, has predicate.  */
+  /* Skip verify of xml_syscall_file, invalid_p == 0 */
+  /* Skip verify of syscalls_info, invalid_p == 0 */
   /* Skip verify of stap_integer_prefixes, invalid_p == 0 */
   /* Skip verify of stap_integer_suffixes, invalid_p == 0 */
   /* Skip verify of stap_register_prefixes, invalid_p == 0 */
@@ -1266,6 +1270,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: static_transform_name = <%s>\n",
                       host_address_to_string (gdbarch->static_transform_name));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: syscalls_info = %s\n",
+                      host_address_to_string (gdbarch->syscalls_info));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: target_desc = %s\n",
                       host_address_to_string (gdbarch->target_desc));
   fprintf_unfiltered (file,
@@ -1304,6 +1311,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: write_pc = <%s>\n",
                       host_address_to_string (gdbarch->write_pc));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: xml_syscall_file = %s\n",
+                      pstring (gdbarch->xml_syscall_file));
   if (gdbarch->dump_tdep != NULL)
     gdbarch->dump_tdep (gdbarch, file);
 }
@@ -3887,6 +3897,40 @@ set_gdbarch_get_syscall_number (struct gdbarch *gdbarch,
   gdbarch->get_syscall_number = get_syscall_number;
 }
 
+const char *
+gdbarch_xml_syscall_file (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  /* Skip verify of xml_syscall_file, invalid_p == 0 */
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_xml_syscall_file called\n");
+  return gdbarch->xml_syscall_file;
+}
+
+void
+set_gdbarch_xml_syscall_file (struct gdbarch *gdbarch,
+                              const char * xml_syscall_file)
+{
+  gdbarch->xml_syscall_file = xml_syscall_file;
+}
+
+struct syscalls_info *
+gdbarch_syscalls_info (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  /* Skip verify of syscalls_info, invalid_p == 0 */
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_syscalls_info called\n");
+  return gdbarch->syscalls_info;
+}
+
+void
+set_gdbarch_syscalls_info (struct gdbarch *gdbarch,
+                           struct syscalls_info * syscalls_info)
+{
+  gdbarch->syscalls_info = syscalls_info;
+}
+
 const char *const *
 gdbarch_stap_integer_prefixes (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5cd4197..0bd1d56 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -60,6 +60,7 @@ struct stap_parse_info;
 struct ravenscar_arch_ops;
 struct elf_internal_linux_prpsinfo;
 struct mem_range;
+struct syscalls_info;
 
 /* The architecture associated with the inferior through the
    connection to the target.
@@ -1056,6 +1057,16 @@ typedef LONGEST (gdbarch_get_syscall_number_ftype) (struct gdbarch *gdbarch, pti
 extern LONGEST gdbarch_get_syscall_number (struct gdbarch *gdbarch, ptid_t ptid);
 extern void set_gdbarch_get_syscall_number (struct gdbarch *gdbarch, gdbarch_get_syscall_number_ftype *get_syscall_number);
 
+/* The filename of the XML syscall for this architecture. */
+
+extern const char * gdbarch_xml_syscall_file (struct gdbarch *gdbarch);
+extern void set_gdbarch_xml_syscall_file (struct gdbarch *gdbarch, const char * xml_syscall_file);
+
+/* Information about system calls from this architecture */
+
+extern struct syscalls_info * gdbarch_syscalls_info (struct gdbarch *gdbarch);
+extern void set_gdbarch_syscalls_info (struct gdbarch *gdbarch, struct syscalls_info * syscalls_info);
+
 /* SystemTap related fields and functions.
    A NULL-terminated array of prefixes used to mark an integer constant
    on the architecture's assembly.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index d9b55c2..8aeb394 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -845,6 +845,12 @@ M:void:record_special_symbol:struct objfile *objfile, asymbol *sym:objfile, sym
 # Get architecture-specific system calls information from registers.
 M:LONGEST:get_syscall_number:ptid_t ptid:ptid
 
+# The filename of the XML syscall for this architecture.
+v:const char *:xml_syscall_file:::0:0::0:pstring (gdbarch->xml_syscall_file)
+
+# Information about system calls from this architecture
+v:struct syscalls_info *:syscalls_info:::0:0::0:host_address_to_string (gdbarch->syscalls_info)
+
 # SystemTap related fields and functions.
 
 # A NULL-terminated array of prefixes used to mark an integer constant
@@ -1156,6 +1162,7 @@ struct stap_parse_info;
 struct ravenscar_arch_ops;
 struct elf_internal_linux_prpsinfo;
 struct mem_range;
+struct syscalls_info;
 
 /* The architecture associated with the inferior through the
    connection to the target.
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 3824468..0d948b1 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -22,6 +22,7 @@
 #include "gdbtypes.h"
 #include "xml-support.h"
 #include "xml-syscall.h"
+#include "gdbarch.h"
 
 /* For the struct syscall definition.  */
 #include "target.h"
@@ -46,14 +47,15 @@ syscall_warn_user (void)
 }
 
 void
-set_xml_syscall_file_name (const char *name)
+set_xml_syscall_file_name (const struct gdbarch *gdbarch,
+			   const char *name)
 {
   return;
 }
 
 void
-get_syscall_by_number (int syscall_number,
-                       struct syscall *s)
+get_syscall_by_number (const struct gdbarch *gdbarch,
+		       int syscall_number, struct syscall *s)
 {
   syscall_warn_user ();
   s->number = syscall_number;
@@ -61,8 +63,8 @@ get_syscall_by_number (int syscall_number,
 }
 
 void
-get_syscall_by_name (const char *syscall_name,
-                     struct syscall *s)
+get_syscall_by_name (const struct gdbarch *gdbarch,
+		     const char *syscall_name, struct syscall *s)
 {
   syscall_warn_user ();
   s->number = UNKNOWN_SYSCALL;
@@ -70,7 +72,7 @@ get_syscall_by_name (const char *syscall_name,
 }
 
 const char **
-get_syscall_names (void)
+get_syscall_names (const struct gdbarch *gdbarch)
 {
   syscall_warn_user ();
   return NULL;
@@ -78,10 +80,6 @@ get_syscall_names (void)
 
 #else /* ! HAVE_LIBEXPAT */
 
-/* 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.  */
-static char *my_gdb_datadir = NULL;
-
 /* Structure which describes a syscall.  */
 typedef struct syscall_desc
 {
@@ -101,6 +99,12 @@ struct syscalls_info
   /* The syscalls.  */
 
   VEC(syscall_desc_p) *syscalls;
+
+  /* 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.  */
+
+  char *my_gdb_datadir;
 };
 
 /* Callback data for syscall information parsing.  */
@@ -108,19 +112,9 @@ struct syscall_parsing_data
 {
   /* The syscalls_info we are building.  */
 
-  struct syscalls_info *sysinfo;
+  struct syscalls_info *syscalls_info;
 };
 
-/* Structure used to store information about the available syscalls in
-   the system.  */
-static const struct syscalls_info *sysinfo = NULL;
-
-/* A flag to tell if we already initialized the structure above.  */
-static int have_initialized_sysinfo = 0;
-
-/* The filename of the syscall's XML.  */
-static const char *xml_syscall_file = NULL;
-
 static struct syscalls_info *
 allocate_syscalls_info (void)
 {
@@ -128,7 +122,7 @@ allocate_syscalls_info (void)
 }
 
 static void
-sysinfo_free_syscalls_desc (struct syscall_desc *sd)
+syscalls_info_free_syscalls_desc (struct syscall_desc *sd)
 {
   xfree (sd->name);
 }
@@ -136,27 +130,32 @@ sysinfo_free_syscalls_desc (struct syscall_desc *sd)
 static void
 free_syscalls_info (void *arg)
 {
-  struct syscalls_info *sysinfo = arg;
+  struct syscalls_info *syscalls_info = arg;
   struct syscall_desc *sysdesc;
   int i;
 
-  for (i = 0;
-       VEC_iterate (syscall_desc_p, sysinfo->syscalls, i, sysdesc);
-       i++)
-    sysinfo_free_syscalls_desc (sysdesc);
-  VEC_free (syscall_desc_p, sysinfo->syscalls);
+  xfree (syscalls_info->my_gdb_datadir);
+
+  if (syscalls_info->syscalls != NULL)
+    {
+      for (i = 0;
+	   VEC_iterate (syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
+	   i++)
+	syscalls_info_free_syscalls_desc (sysdesc);
+      VEC_free (syscall_desc_p, syscalls_info->syscalls);
+    }
 
-  xfree (sysinfo);
+  xfree (syscalls_info);
 }
 
 static struct cleanup *
-make_cleanup_free_syscalls_info (struct syscalls_info *sysinfo)
+make_cleanup_free_syscalls_info (struct syscalls_info *syscalls_info)
 {
-  return make_cleanup (free_syscalls_info, sysinfo);
+  return make_cleanup (free_syscalls_info, syscalls_info);
 }
 
 static void
-syscall_create_syscall_desc (struct syscalls_info *sysinfo,
+syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
                              const char *name, int number)
 {
   struct syscall_desc *sysdesc = XCNEW (struct syscall_desc);
@@ -164,7 +163,7 @@ syscall_create_syscall_desc (struct syscalls_info *sysinfo,
   sysdesc->name = xstrdup (name);
   sysdesc->number = number;
 
-  VEC_safe_push (syscall_desc_p, sysinfo->syscalls, sysdesc);
+  VEC_safe_push (syscall_desc_p, syscalls_info->syscalls, sysdesc);
 }
 
 /* Handle the start of a <syscall> element.  */
@@ -194,7 +193,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
     }
 
   gdb_assert (name);
-  syscall_create_syscall_desc (data->sysinfo, name, number);
+  syscall_create_syscall_desc (data->syscalls_info, name, number);
 }
 
 
@@ -225,15 +224,15 @@ syscall_parse_xml (const char *document, xml_fetch_another fetcher,
   struct cleanup *result_cleanup;
   struct syscall_parsing_data data;
 
-  data.sysinfo = allocate_syscalls_info ();
-  result_cleanup = make_cleanup_free_syscalls_info (data.sysinfo);
+  data.syscalls_info = allocate_syscalls_info ();
+  result_cleanup = make_cleanup_free_syscalls_info (data.syscalls_info);
 
   if (gdb_xml_parse_quick (_("syscalls info"), NULL,
 			   syselements, document, &data) == 0)
     {
       /* Parsed successfully.  */
       discard_cleanups (result_cleanup);
-      return data.sysinfo;
+      return data.syscalls_info;
     }
   else
     {
@@ -248,12 +247,12 @@ syscall_parse_xml (const char *document, xml_fetch_another fetcher,
    struct syscalls_info with the values.
    
    Returns the struct syscalls_info if the file is valid, NULL otherwise.  */
-static const struct syscalls_info *
+static struct syscalls_info *
 xml_init_syscalls_info (const char *filename)
 {
   char *full_file;
   char *dirname;
-  struct syscalls_info *sysinfo;
+  struct syscalls_info *syscalls_info;
   struct cleanup *back_to;
 
   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
@@ -266,41 +265,47 @@ xml_init_syscalls_info (const char *filename)
   if (dirname != NULL)
     make_cleanup (xfree, dirname);
 
-  sysinfo = syscall_parse_xml (full_file,
+  syscalls_info = syscall_parse_xml (full_file,
 			       xml_fetch_content_from_file, dirname);
   do_cleanups (back_to);
 
-  return sysinfo;
+  return syscalls_info;
 }
 
 /* Initializes the syscalls_info structure according to the
    architecture.  */
 static void
-init_sysinfo (void)
+init_syscalls_info (struct gdbarch *gdbarch)
 {
+  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
+  const char *xml_syscall_file = gdbarch_xml_syscall_file (gdbarch);
+
   /* Should we re-read the XML info for this target?  */
-  if (my_gdb_datadir && filename_cmp (my_gdb_datadir, gdb_datadir) != 0)
+  if (syscalls_info != NULL && syscalls_info->my_gdb_datadir != NULL
+      && filename_cmp (syscalls_info->my_gdb_datadir, gdb_datadir) != 0)
     {
       /* The data-directory changed from the last time we used it.
 	 It means that we have to re-read the XML info.  */
-      have_initialized_sysinfo = 0;
-      xfree (my_gdb_datadir);
-      my_gdb_datadir = NULL;
-      if (sysinfo)
-	free_syscalls_info ((void *) sysinfo);
+      free_syscalls_info (syscalls_info);
+      syscalls_info = NULL;
+      set_gdbarch_syscalls_info (gdbarch, NULL);
     }
 
-  /* Did we already try to initialize the structure?  */
-  if (have_initialized_sysinfo)
+  /* Did we succeed at initializing this?  */
+  if (syscalls_info != NULL)
     return;
 
-  sysinfo = xml_init_syscalls_info (xml_syscall_file);
+  syscalls_info = xml_init_syscalls_info (xml_syscall_file);
 
-  have_initialized_sysinfo = 1;
+  /* If there was some error reading the XML file, we initialize
+     gdbarch->syscalls_info anyway, in order to store information
+     about our attempt.  */
+  if (syscalls_info == NULL)
+      syscalls_info = allocate_syscalls_info ();
 
-  if (sysinfo == NULL)
+  if (syscalls_info->syscalls == NULL)
     {
-      if (xml_syscall_file)
+      if (xml_syscall_file != NULL)
 	warning (_("Could not load the syscall XML file `%s/%s'."),
 		 gdb_datadir, xml_syscall_file);
       else
@@ -312,22 +317,25 @@ init_sysinfo (void)
     }
 
   /* Saving the data-directory used to read this XML info.  */
-  my_gdb_datadir = xstrdup (gdb_datadir);
+  syscalls_info->my_gdb_datadir = xstrdup (gdb_datadir);
+
+  set_gdbarch_syscalls_info (gdbarch, syscalls_info);
 }
 
 static int
-xml_get_syscall_number (const struct syscalls_info *sysinfo,
+xml_get_syscall_number (struct gdbarch *gdbarch,
                         const char *syscall_name)
 {
+  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
   struct syscall_desc *sysdesc;
   int i;
 
-  if (sysinfo == NULL
+  if (syscalls_info == NULL
       || syscall_name == NULL)
     return UNKNOWN_SYSCALL;
 
   for (i = 0;
-       VEC_iterate(syscall_desc_p, sysinfo->syscalls, i, sysdesc);
+       VEC_iterate(syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
        i++)
     if (strcmp (sysdesc->name, syscall_name) == 0)
       return sysdesc->number;
@@ -336,18 +344,19 @@ xml_get_syscall_number (const struct syscalls_info *sysinfo,
 }
 
 static const char *
-xml_get_syscall_name (const struct syscalls_info *sysinfo,
+xml_get_syscall_name (struct gdbarch *gdbarch,
                       int syscall_number)
 {
+  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
   struct syscall_desc *sysdesc;
   int i;
 
-  if (sysinfo == NULL
+  if (syscalls_info == NULL
       || syscall_number < 0)
     return NULL;
 
   for (i = 0;
-       VEC_iterate(syscall_desc_p, sysinfo->syscalls, i, sysdesc);
+       VEC_iterate(syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
        i++)
     if (sysdesc->number == syscall_number)
       return sysdesc->name;
@@ -356,21 +365,22 @@ xml_get_syscall_name (const struct syscalls_info *sysinfo,
 }
 
 static const char **
-xml_list_of_syscalls (const struct syscalls_info *sysinfo)
+xml_list_of_syscalls (struct gdbarch *gdbarch)
 {
+  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
   struct syscall_desc *sysdesc;
   const char **names = NULL;
   int nsyscalls;
   int i;
 
-  if (sysinfo == NULL)
+  if (syscalls_info == NULL)
     return NULL;
 
-  nsyscalls = VEC_length (syscall_desc_p, sysinfo->syscalls);
+  nsyscalls = VEC_length (syscall_desc_p, syscalls_info->syscalls);
   names = xmalloc ((nsyscalls + 1) * sizeof (char *));
 
   for (i = 0;
-       VEC_iterate (syscall_desc_p, sysinfo->syscalls, i, sysdesc);
+       VEC_iterate (syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
        i++)
     names[i] = sysdesc->name;
 
@@ -380,37 +390,37 @@ xml_list_of_syscalls (const struct syscalls_info *sysinfo)
 }
 
 void
-set_xml_syscall_file_name (const char *name)
+set_xml_syscall_file_name (struct gdbarch *gdbarch, const char *name)
 {
-  xml_syscall_file = name;
+  set_gdbarch_xml_syscall_file (gdbarch, name);
 }
 
 void
-get_syscall_by_number (int syscall_number,
-                       struct syscall *s)
+get_syscall_by_number (struct gdbarch *gdbarch,
+		       int syscall_number, struct syscall *s)
 {
-  init_sysinfo ();
+  init_syscalls_info (gdbarch);
 
   s->number = syscall_number;
-  s->name = xml_get_syscall_name (sysinfo, syscall_number);
+  s->name = xml_get_syscall_name (gdbarch, syscall_number);
 }
 
 void
-get_syscall_by_name (const char *syscall_name,
-                     struct syscall *s)
+get_syscall_by_name (struct gdbarch *gdbarch,
+		     const char *syscall_name, struct syscall *s)
 {
-  init_sysinfo ();
+  init_syscalls_info (gdbarch);
 
-  s->number = xml_get_syscall_number (sysinfo, syscall_name);
+  s->number = xml_get_syscall_number (gdbarch, syscall_name);
   s->name = syscall_name;
 }
 
 const char **
-get_syscall_names (void)
+get_syscall_names (struct gdbarch *gdbarch)
 {
-  init_sysinfo ();
+  init_syscalls_info (gdbarch);
 
-  return xml_list_of_syscalls (sysinfo);
+  return xml_list_of_syscalls (gdbarch);
 }
 
 #endif /* ! HAVE_LIBEXPAT */
diff --git a/gdb/xml-syscall.h b/gdb/xml-syscall.h
index dff389d..42b9dea 100644
--- a/gdb/xml-syscall.h
+++ b/gdb/xml-syscall.h
@@ -29,22 +29,25 @@
    GDB won't be able to find the correct XML file to open and get
    the syscalls definitions.  */
 
-void set_xml_syscall_file_name (const char *name);
+void set_xml_syscall_file_name (struct gdbarch *gdbarch,
+				const char *name);
 
 /* Function that retrieves the syscall name corresponding to the given
    number.  It puts the requested information inside 'struct syscall'.  */
 
-void get_syscall_by_number (int syscall_number, struct syscall *s);
+void get_syscall_by_number (struct gdbarch *gdbarch,
+			    int syscall_number, struct syscall *s);
 
 /* Function that retrieves the syscall number corresponding to the given
    name.  It puts the requested information inside 'struct syscall'.  */
 
-void get_syscall_by_name (const char *syscall_name, struct syscall *s);
+void get_syscall_by_name (struct gdbarch *gdbarch,
+			  const char *syscall_name, struct syscall *s);
 
 /* Function used to retrieve the list of syscalls in the system.  This list
    is returned as an array of strings.  Returns the list of syscalls in the
    system, or NULL otherwise.  */
 
-const char **get_syscall_names (void);
+const char **get_syscall_names (struct gdbarch *gdbarch);
 
 #endif /* XML_SYSCALL_H */
-- 
1.9.3

[PATCH 2/3] Adjust calls for setting "catch syscall" information

[Sergio Durigan Junior]

This patch adjusts the calls to several functions exported by
gdb/xml-syscall.c.  Now, we should receive a "gdbarch" argument that
refers to the architecture being used.

It is the logical move after patch #1, so it's very trivial.

gdb/
2014-11-12  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/10737
	* amd64-linux-tdep.c (amd64_linux_init_abi_common): Adjust call to
	set_xml_syscall_file_name to provide gdbarch.
	* arm-linux-tdep.c (arm_linux_init_abi): Likewise.
	* bfin-linux-tdep.c (bfin_linux_init_abi): Likewise.
	* breakpoint.c (print_it_catch_syscall): Adjust call to
	get_syscall_by_number to provide gdbarch.
	(print_one_catch_syscall): Likewise.
	(print_mention_catch_syscall): Likewise.
	(print_recreate_catch_syscall): Likewise.
	(catch_syscall_split_args): Adjust calls to get_syscall_by_number
	and get_syscall_by_name to provide gdbarch.
	(catch_syscall_completer): Adjust call to get_syscall_names to
	provide gdbarch.
	* i386-linux-tdep.c (i386_linux_init_abi): Adjust call to
	set_xml_syscall_file_name to provide gdbarch.
	* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
	* ppc-linux-tdep.c (ppc_linux_init_abi): Likewise.
	* s390-linux-tdep.c (s390_gdbarch_init): Likewise.
	* sparc-linux-tdep.c (sparc32_linux_init_abi): Likewise.
	* sparc64-linux-tdep.c (sparc64_linux_init_abi): Likewise.
---
 gdb/amd64-linux-tdep.c   |  2 +-
 gdb/arm-linux-tdep.c     |  2 +-
 gdb/bfin-linux-tdep.c    |  2 +-
 gdb/breakpoint.c         | 21 +++++++++++++--------
 gdb/i386-linux-tdep.c    |  2 +-
 gdb/mips-linux-tdep.c    |  6 +++---
 gdb/ppc-linux-tdep.c     |  4 ++--
 gdb/s390-linux-tdep.c    |  4 ++--
 gdb/sparc-linux-tdep.c   |  2 +-
 gdb/sparc64-linux-tdep.c |  2 +-
 10 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index edbb1b3..27a3560 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1663,7 +1663,7 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
   tdep->register_reggroup_p = amd64_linux_register_reggroup_p;
 
   /* Functions for 'catch syscall'.  */
-  set_xml_syscall_file_name (XML_SYSCALL_FILENAME_AMD64);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_AMD64);
   set_gdbarch_get_syscall_number (gdbarch,
                                   amd64_linux_get_syscall_number);
 
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 2e79658..fa28695 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -1468,7 +1468,7 @@ arm_linux_init_abi (struct gdbarch_info info,
   tdep->syscall_next_pc = arm_linux_syscall_next_pc;
 
   /* `catch syscall' */
-  set_xml_syscall_file_name ("syscalls/arm-linux.xml");
+  set_xml_syscall_file_name (gdbarch, "syscalls/arm-linux.xml");
   set_gdbarch_get_syscall_number (gdbarch, arm_linux_get_syscall_number);
 
   /* Syscall record.  */
diff --git a/gdb/bfin-linux-tdep.c b/gdb/bfin-linux-tdep.c
index 8471ec7..c3e37c0 100644
--- a/gdb/bfin-linux-tdep.c
+++ b/gdb/bfin-linux-tdep.c
@@ -157,7 +157,7 @@ bfin_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tramp_frame_prepend_unwinder (gdbarch, &bfin_linux_sigframe);
 
   /* Functions for 'catch syscall'.  */
-  set_xml_syscall_file_name ("syscalls/bfin-linux.xml");
+  set_xml_syscall_file_name (gdbarch, "syscalls/bfin-linux.xml");
   set_gdbarch_get_syscall_number (gdbarch,
                                   bfin_linux_get_syscall_number);
 }
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3ebe9c9..2e19602 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8607,10 +8607,11 @@ print_it_catch_syscall (bpstat bs)
   ptid_t ptid;
   struct target_waitstatus last;
   struct syscall s;
+  struct gdbarch *gdbarch = target_gdbarch ();
 
   get_last_target_status (&ptid, &last);
 
-  get_syscall_by_number (last.value.syscall_number, &s);
+  get_syscall_by_number (gdbarch, last.value.syscall_number, &s);
 
   annotate_catchpoint (b->number);
 
@@ -8653,6 +8654,7 @@ print_one_catch_syscall (struct breakpoint *b,
   struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
   struct value_print_options opts;
   struct ui_out *uiout = current_uiout;
+  struct gdbarch *gdbarch = target_gdbarch ();
 
   get_user_print_options (&opts);
   /* Field 4, the address, is omitted (which makes the columns not
@@ -8679,7 +8681,7 @@ print_one_catch_syscall (struct breakpoint *b,
         {
           char *x = text;
           struct syscall s;
-          get_syscall_by_number (iter, &s);
+          get_syscall_by_number (gdbarch, iter, &s);
 
           if (s.name != NULL)
             text = xstrprintf ("%s%s, ", text, s.name);
@@ -8710,6 +8712,7 @@ static void
 print_mention_catch_syscall (struct breakpoint *b)
 {
   struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
+  struct gdbarch *gdbarch = target_gdbarch ();
 
   if (c->syscalls_to_be_caught)
     {
@@ -8725,7 +8728,7 @@ print_mention_catch_syscall (struct breakpoint *b)
            i++)
         {
           struct syscall s;
-          get_syscall_by_number (iter, &s);
+          get_syscall_by_number (gdbarch, iter, &s);
 
           if (s.name)
             printf_filtered (" '%s' [%d]", s.name, s.number);
@@ -8746,6 +8749,7 @@ static void
 print_recreate_catch_syscall (struct breakpoint *b, struct ui_file *fp)
 {
   struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
+  struct gdbarch *gdbarch = target_gdbarch ();
 
   fprintf_unfiltered (fp, "catch syscall");
 
@@ -8759,7 +8763,7 @@ print_recreate_catch_syscall (struct breakpoint *b, struct ui_file *fp)
         {
           struct syscall s;
 
-          get_syscall_by_number (iter, &s);
+          get_syscall_by_number (gdbarch, iter, &s);
           if (s.name)
             fprintf_unfiltered (fp, " %s", s.name);
           else
@@ -12035,6 +12039,7 @@ catch_syscall_split_args (char *arg)
 {
   VEC(int) *result = NULL;
   struct cleanup *cleanup = make_cleanup (VEC_cleanup (int), &result);
+  struct gdbarch *gdbarch = target_gdbarch ();
 
   while (*arg != '\0')
     {
@@ -12054,12 +12059,12 @@ catch_syscall_split_args (char *arg)
       /* Check if the user provided a syscall name or a number.  */
       syscall_number = (int) strtol (cur_name, &endptr, 0);
       if (*endptr == '\0')
-	get_syscall_by_number (syscall_number, &s);
+	get_syscall_by_number (gdbarch, syscall_number, &s);
       else
 	{
 	  /* We have a name.  Let's check if it's valid and convert it
 	     to a number.  */
-	  get_syscall_by_name (cur_name, &s);
+	  get_syscall_by_name (gdbarch, cur_name, &s);
 
 	  if (s.number == UNKNOWN_SYSCALL)
 	    /* Here we have to issue an error instead of a warning,
@@ -12100,7 +12105,7 @@ this architecture yet."));
      to get the syscall XML file loaded or, most important,
      to display a warning to the user if there's no XML file
      for his/her architecture.  */
-  get_syscall_by_number (0, &s);
+  get_syscall_by_number (gdbarch, 0, &s);
 
   /* The allowed syntax is:
      catch syscall
@@ -15343,7 +15348,7 @@ static VEC (char_ptr) *
 catch_syscall_completer (struct cmd_list_element *cmd,
                          const char *text, const char *word)
 {
-  const char **list = get_syscall_names ();
+  const char **list = get_syscall_names (target_gdbarch ());
   VEC (char_ptr) *retlist
     = (list == NULL) ? NULL : complete_on_enum (list, word, word);
 
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 7feb21d..b45c230 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -994,7 +994,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
                                        displaced_step_at_entry_point);
 
   /* Functions for 'catch syscall'.  */
-  set_xml_syscall_file_name (XML_SYSCALL_FILENAME_I386);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_I386);
   set_gdbarch_get_syscall_number (gdbarch,
                                   i386_linux_get_syscall_number);
 
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index c2cec3d..bc42921 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -1558,7 +1558,7 @@ mips_linux_init_abi (struct gdbarch_info info,
 	  (gdbarch, svr4_ilp32_fetch_link_map_offsets);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_o32_sigframe);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_o32_rt_sigframe);
-	set_xml_syscall_file_name ("syscalls/mips-o32-linux.xml");
+	set_xml_syscall_file_name (gdbarch, "syscalls/mips-o32-linux.xml");
 	break;
       case MIPS_ABI_N32:
 	set_gdbarch_get_longjmp_target (gdbarch,
@@ -1572,7 +1572,7 @@ mips_linux_init_abi (struct gdbarch_info info,
 	   does not distinguish between quiet and signalling NaNs).  */
 	set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_n32_rt_sigframe);
-	set_xml_syscall_file_name ("syscalls/mips-n32-linux.xml");
+	set_xml_syscall_file_name (gdbarch, "syscalls/mips-n32-linux.xml");
 	break;
       case MIPS_ABI_N64:
 	set_gdbarch_get_longjmp_target (gdbarch,
@@ -1586,7 +1586,7 @@ mips_linux_init_abi (struct gdbarch_info info,
 	   does not distinguish between quiet and signalling NaNs).  */
 	set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
 	tramp_frame_prepend_unwinder (gdbarch, &mips_linux_n64_rt_sigframe);
-	set_xml_syscall_file_name ("syscalls/mips-n64-linux.xml");
+	set_xml_syscall_file_name (gdbarch, "syscalls/mips-n64-linux.xml");
 	break;
       default:
 	break;
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index a997869..fa51ed0 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1294,7 +1294,7 @@ ppc_linux_init_abi (struct gdbarch_info info,
         (gdbarch, svr4_ilp32_fetch_link_map_offsets);
 
       /* Setting the correct XML syscall filename.  */
-      set_xml_syscall_file_name (XML_SYSCALL_FILENAME_PPC);
+      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_PPC);
 
       /* Trampolines.  */
       tramp_frame_prepend_unwinder (gdbarch,
@@ -1346,7 +1346,7 @@ ppc_linux_init_abi (struct gdbarch_info info,
         (gdbarch, svr4_lp64_fetch_link_map_offsets);
 
       /* Setting the correct XML syscall filename.  */
-      set_xml_syscall_file_name (XML_SYSCALL_FILENAME_PPC64);
+      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_PPC64);
 
       /* Trampolines.  */
       tramp_frame_prepend_unwinder (gdbarch,
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index abd2e40..b5d94ce 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -3107,7 +3107,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       set_solib_svr4_fetch_link_map_offsets
 	(gdbarch, svr4_ilp32_fetch_link_map_offsets);
 
-      set_xml_syscall_file_name (XML_SYSCALL_FILENAME_S390);
+      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390);
       break;
 
     case ABI_LINUX_ZSERIES:
@@ -3122,7 +3122,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 						    s390_address_class_type_flags_to_name);
       set_gdbarch_address_class_name_to_type_flags (gdbarch,
 						    s390_address_class_name_to_type_flags);
-      set_xml_syscall_file_name (XML_SYSCALL_FILENAME_S390);
+      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390);
       break;
     }
 
diff --git a/gdb/sparc-linux-tdep.c b/gdb/sparc-linux-tdep.c
index fd1fc3d..ec7d84d 100644
--- a/gdb/sparc-linux-tdep.c
+++ b/gdb/sparc-linux-tdep.c
@@ -455,7 +455,7 @@ sparc32_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_write_pc (gdbarch, sparc_linux_write_pc);
 
   /* Functions for 'catch syscall'.  */
-  set_xml_syscall_file_name (XML_SYSCALL_FILENAME_SPARC32);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC32);
   set_gdbarch_get_syscall_number (gdbarch,
                                   sparc32_linux_get_syscall_number);
 
diff --git a/gdb/sparc64-linux-tdep.c b/gdb/sparc64-linux-tdep.c
index e843eb3..0dd47d0 100644
--- a/gdb/sparc64-linux-tdep.c
+++ b/gdb/sparc64-linux-tdep.c
@@ -336,7 +336,7 @@ sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_write_pc (gdbarch, sparc64_linux_write_pc);
 
   /* Functions for 'catch syscall'.  */
-  set_xml_syscall_file_name (XML_SYSCALL_FILENAME_SPARC64);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC64);
   set_gdbarch_get_syscall_number (gdbarch,
                                   sparc64_linux_get_syscall_number);
 }
-- 
1.9.3

[PATCH 3/3] Testcase

[Sergio Durigan Junior]

This patch implements the testcase for this fix.  The test is very
simple: we just have to verify if the syscall number for each
architecture has different meanings.  I chose to test i386 and x86_64
here, but it could be any other architecture supported by the "catch
syscall" command.

gdb/testsuite/
2014-11-12  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/10737
	* gdb.base/catch-syscall.exp (do_syscall_tests): Call
	test_catch_syscall_multi_target.
	(test_catch_syscall_multi_target): New function.
---
 gdb/testsuite/gdb.base/catch-syscall.exp | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index a70534c..cec1d07 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -311,6 +311,10 @@ proc do_syscall_tests {} {
     # Testing the 'catch' syscall command during a restart of
     # the inferior.
     if [runto_main] then { test_catch_syscall_restarting_inferior }
+
+    # Testing if the 'catch syscall' command works when switching to
+    # different architectures on-the-fly (PR gdb/10737).
+    if [runto_main] then { test_catch_syscall_multi_target }
 }
 
 proc test_catch_syscall_without_args_noxml {} {
@@ -372,6 +376,32 @@ proc test_catch_syscall_with_wrong_args_noxml {} {
     }
 }
 
+proc test_catch_syscall_multi_target {} {
+    global decimal binfile
+
+    with_test_prefix "multiple targets" {
+	clean_restart $binfile
+
+	gdb_test "set architecture i386" \
+	    "The target architecture is assumed to be i386" \
+	    "set arch to i386"
+
+	gdb_test "catch syscall 1" \
+	    "Catchpoint $decimal \\(syscall .exit. \\\[1\\\]\\)" \
+	    "insert catch syscall on syscall 1 -- exit on i386"
+
+	gdb_test "set architecture i386:x86-64" \
+	    "The target architecture is assumed to be i386:x86-64" \
+	    "set arch to x86_64"
+
+	gdb_test "catch syscall 1" \
+	    "Catchpoint $decimal \\(syscall .write. \\\[1\\\]\\)" \
+	    "insert catch syscall on syscall 1 -- exit on i386"
+
+	clean_restart $binfile
+    }
+}
+
 proc do_syscall_tests_without_xml {} {
     # Make sure GDB doesn't load the syscalls xml from the system data
     # directory.
-- 
1.9.3

[PATCH] Partial fix for PR breakpoints/10737: Make syscall info be per-arch instead of global

[Sergio Durigan Junior]

Hello,

This patch series intends to partially fix PR breakpoints/10737, which
is about making the syscall information (for the "catch syscall"
command) be per-arch, instead of global.  This is not a full fix
because of the other issues pointed by Pedro here:

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

However, I consider it a good step towards the real fix.  It will also
help me fix <https://sourceware.org/bugzilla/show_bug.cgi?id=17402>,
so I think it is good if the patch goes in.

What this patch does, basically, is move the "syscalls_info"
struct to gdbarch.  Currently, the syscall information is stored in a
global variable inside gdb/xml-syscall.c, which means that there is no
easy way to correlate this info with the current target or
architecture being used, for example.  This causes strange behaviors,
because the syscall info is not re-read when the arch changes.  For
example, if you put a syscall catchpoint in syscall 5 on i386 (syscall
open), and then load a x86_64 program on GDB and put the same syscall
5 there (fstat on x86_64), you will still see that GDB tells you that
it is catching "open", even though it is not.  With this patch, GDB
correctly says that it will be catching fstat syscalls.

As I said, there are still some problems on the "catch syscall"
mechanism, because (for example) the user should be able to "catch
syscall open" on i386, and then expect "open" to be caught also on
x86_64.  Currently, it doesn't work.  I intend to work on this later.

Anyway, I would like to know what you think about the current patch.
I guess it is simple enough to understand, and should not pose any
difficulties to be accepted (I expect).

Re: [PATCH 1/3] Making syscall information be per-arch, instead of global

[Pedro Alves]

On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:

> 	(xml_init_syscalls_info): Likewise.

Should mention the dropped const.

>  xml_init_syscalls_info (const char *filename)
>  {
>    char *full_file;
>    char *dirname;
> -  struct syscalls_info *sysinfo;
> +  struct syscalls_info *syscalls_info;
>    struct cleanup *back_to;
>  
>    full_file = xml_fetch_content_from_file (filename, gdb_datadir);
> @@ -266,41 +265,47 @@ xml_init_syscalls_info (const char *filename)
>    if (dirname != NULL)
>      make_cleanup (xfree, dirname);
>  
> -  sysinfo = syscall_parse_xml (full_file,
> +  syscalls_info = syscall_parse_xml (full_file,
>  			       xml_fetch_content_from_file, dirname);

The line after needs a reindent.

>    do_cleanups (back_to);
>  
> -  return sysinfo;
> +  return syscalls_info;
>  }
>  

> -  have_initialized_sysinfo = 1;
> +  /* If there was some error reading the XML file, we initialize
> +     gdbarch->syscalls_info anyway, in order to store information
> +     about our attempt.  */
> +  if (syscalls_info == NULL)
> +      syscalls_info = allocate_syscalls_info ();

Indentation here looks odd.

>
> -  if (sysinfo == NULL)
> +  if (syscalls_info->syscalls == NULL)
>      {

Otherwise looks good.

Thanks for doing this.

Thanks,
Pedro Alves

Re: [PATCH 2/3] Adjust calls for setting "catch syscall" information

[Pedro Alves]

On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -8607,10 +8607,11 @@ print_it_catch_syscall (bpstat bs)
>    ptid_t ptid;
>    struct target_waitstatus last;
>    struct syscall s;
> +  struct gdbarch *gdbarch = target_gdbarch ();

I think this would better be bs->bp_location_at->gdbarch .

> @@ -8653,6 +8654,7 @@ print_one_catch_syscall (struct breakpoint *b,
>    struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
>    struct value_print_options opts;
>    struct ui_out *uiout = current_uiout;
> +  struct gdbarch *gdbarch = target_gdbarch ();

Here, we can use b->loc->gdbarch.

Later, to handle the case "catch syscall open" with multiple
inferiors of different archs, I think we'll end up with a
location for each inferior, or for each arch, and we'll iterate
over locations here.  For now, I think we can assume there's only
one location.

Similarly for other places.

> @@ -15343,7 +15348,7 @@ static VEC (char_ptr) *
>  catch_syscall_completer (struct cmd_list_element *cmd,
>                           const char *text, const char *word)
>  {
> -  const char **list = get_syscall_names ();
> +  const char **list = get_syscall_names (target_gdbarch ());
>    VEC (char_ptr) *retlist
>      = (list == NULL) ? NULL : complete_on_enum (list, word, word);

This one is handling user input, so should be "get_current_arch ()":

/* Return "current" architecture.  If the target is running, this is
   the architecture of the selected frame.  Otherwise, the "current"
   architecture defaults to the target architecture.

   This function should normally be called solely by the command
   interpreter routines to determine the architecture to execute a
   command in.  */
struct gdbarch *
get_current_arch (void)


Thanks,
Pedro Alves

Re: [PATCH 3/3] Testcase

[Pedro Alves]

On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:
> This patch implements the testcase for this fix.  The test is very
> simple: we just have to verify if the syscall number for each
> architecture has different meanings.  I chose to test i386 and x86_64
> here, but it could be any other architecture supported by the "catch
> syscall" command.

This only works if the built GDB has these architectures configured
in.

E.g., an --enable-targets=all build on x86:

(gdb) set architecture aarch64
The target architecture is assumed to be aarch64

while on a default x86 build:

(gdb) set architecture aarch64
Undefined item: "aarch64".

From that, you can see that:

 (gdb) set architecture i386

would fail on non-x86 builds that don't include x86 in --enable-targets=foo.

>  
> +proc test_catch_syscall_multi_target {} {

Please make this "multi_arch".  Let's leave "multi-target" for
https://sourceware.org/gdb/wiki/MultiTarget.

> +    global decimal binfile
> +
> +    with_test_prefix "multiple targets" {
> +	clean_restart $binfile
> +
> +	gdb_test "set architecture i386" \
> +	    "The target architecture is assumed to be i386" \
> +	    "set arch to i386"
> +
> +	gdb_test "catch syscall 1" \
> +	    "Catchpoint $decimal \\(syscall .exit. \\\[1\\\]\\)" \
> +	    "insert catch syscall on syscall 1 -- exit on i386"
> +
> +	gdb_test "set architecture i386:x86-64" \
> +	    "The target architecture is assumed to be i386:x86-64" \
> +	    "set arch to x86_64"
> +
> +	gdb_test "catch syscall 1" \
> +	    "Catchpoint $decimal \\(syscall .write. \\\[1\\\]\\)" \
> +	    "insert catch syscall on syscall 1 -- exit on i386"

The "exit on i386" part seems stale here.

I think we should do something like this:

if { [istarget "i*86-*-*"] || [istarget "x86_64-*-*"] } {
    set arch1 "i386"
    set syscall1 "exit"
    set arch2 "i386:x86-64"
    set syscall2 "write"
} elseif { [istarget "powerpc-*-linux*"] || [istarget "powerpc64-*-linux*"] } {
    ...
} elseif { [istarget "sparc-*-linux*"] && ![istarget "sparc64-*-linux*"] } {
    ...
} elseif { [istarget "mips*-linux*"] } {
    ...
} elseif { [istarget "arm*-linux*"] } {
    ...
} elseif { [istarget "s390*-linux*"] } {
    ...
} else {
    error "please port me"
}

Thanks,
Pedro Alves

Re: [PATCH 1/3] Making syscall information be per-arch, instead of global

[Sergio Durigan Junior]

On Friday, November 14 2014, Pedro Alves wrote:

> On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:
>
>> 	(xml_init_syscalls_info): Likewise.
>
> Should mention the dropped const.
>
>>  xml_init_syscalls_info (const char *filename)
>>  {
>>    char *full_file;
>>    char *dirname;
>> -  struct syscalls_info *sysinfo;
>> +  struct syscalls_info *syscalls_info;
>>    struct cleanup *back_to;
>>  
>>    full_file = xml_fetch_content_from_file (filename, gdb_datadir);
>> @@ -266,41 +265,47 @@ xml_init_syscalls_info (const char *filename)
>>    if (dirname != NULL)
>>      make_cleanup (xfree, dirname);
>>  
>> -  sysinfo = syscall_parse_xml (full_file,
>> +  syscalls_info = syscall_parse_xml (full_file,
>>  			       xml_fetch_content_from_file, dirname);
>
> The line after needs a reindent.
>
>>    do_cleanups (back_to);
>>  
>> -  return sysinfo;
>> +  return syscalls_info;
>>  }
>>  
>
>> -  have_initialized_sysinfo = 1;
>> +  /* If there was some error reading the XML file, we initialize
>> +     gdbarch->syscalls_info anyway, in order to store information
>> +     about our attempt.  */
>> +  if (syscalls_info == NULL)
>> +      syscalls_info = allocate_syscalls_info ();
>
> Indentation here looks odd.
>
>>
>> -  if (sysinfo == NULL)
>> +  if (syscalls_info->syscalls == NULL)
>>      {
>
> Otherwise looks good.
>
> Thanks for doing this.

Thanks for the review.  I fixed all the issues pointed.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 2/3] Adjust calls for setting "catch syscall" information

[Sergio Durigan Junior]

On Friday, November 14 2014, Pedro Alves wrote:

> On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -8607,10 +8607,11 @@ print_it_catch_syscall (bpstat bs)
>>    ptid_t ptid;
>>    struct target_waitstatus last;
>>    struct syscall s;
>> +  struct gdbarch *gdbarch = target_gdbarch ();
>
> I think this would better be bs->bp_location_at->gdbarch .
>
>> @@ -8653,6 +8654,7 @@ print_one_catch_syscall (struct breakpoint *b,
>>    struct syscall_catchpoint *c = (struct syscall_catchpoint *) b;
>>    struct value_print_options opts;
>>    struct ui_out *uiout = current_uiout;
>> +  struct gdbarch *gdbarch = target_gdbarch ();
>
> Here, we can use b->loc->gdbarch.
>
> Later, to handle the case "catch syscall open" with multiple
> inferiors of different archs, I think we'll end up with a
> location for each inferior, or for each arch, and we'll iterate
> over locations here.  For now, I think we can assume there's only
> one location.

Hm, thanks for this insight.  I was already wondering the best way to
deal with this problem.  I will give it a try later.

> Similarly for other places.
>
>> @@ -15343,7 +15348,7 @@ static VEC (char_ptr) *
>>  catch_syscall_completer (struct cmd_list_element *cmd,
>>                           const char *text, const char *word)
>>  {
>> -  const char **list = get_syscall_names ();
>> +  const char **list = get_syscall_names (target_gdbarch ());
>>    VEC (char_ptr) *retlist
>>      = (list == NULL) ? NULL : complete_on_enum (list, word, word);
>
> This one is handling user input, so should be "get_current_arch ()":
>
> /* Return "current" architecture.  If the target is running, this is
>    the architecture of the selected frame.  Otherwise, the "current"
>    architecture defaults to the target architecture.
>
>    This function should normally be called solely by the command
>    interpreter routines to determine the architecture to execute a
>    command in.  */
> struct gdbarch *
> get_current_arch (void)

Thanks for the review.  I fixed all the issues.  I will concentrate on
the testcase now in order to get this patch in.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 3/3] Testcase

[Sergio Durigan Junior]

On Friday, November 14 2014, Pedro Alves wrote:

> On 11/13/2014 12:18 AM, Sergio Durigan Junior wrote:
>> This patch implements the testcase for this fix.  The test is very
>> simple: we just have to verify if the syscall number for each
>> architecture has different meanings.  I chose to test i386 and x86_64
>> here, but it could be any other architecture supported by the "catch
>> syscall" command.
>
> This only works if the built GDB has these architectures configured
> in.
>
> E.g., an --enable-targets=all build on x86:
>
> (gdb) set architecture aarch64
> The target architecture is assumed to be aarch64
>
> while on a default x86 build:
>
> (gdb) set architecture aarch64
> Undefined item: "aarch64".
>
> From that, you can see that:
>
>  (gdb) set architecture i386
>
> would fail on non-x86 builds that don't include x86 in --enable-targets=foo.

True, thanks for catching this.  When I tested it in my machine, I made
some confusion and used the same GDB to test both scenarios.  Sorry
about that.

>>  
>> +proc test_catch_syscall_multi_target {} {
>
> Please make this "multi_arch".  Let's leave "multi-target" for
> https://sourceware.org/gdb/wiki/MultiTarget.

Ouch, sorry.  Fixed.

>
>> +    global decimal binfile
>> +
>> +    with_test_prefix "multiple targets" {
>> +	clean_restart $binfile
>> +
>> +	gdb_test "set architecture i386" \
>> +	    "The target architecture is assumed to be i386" \
>> +	    "set arch to i386"
>> +
>> +	gdb_test "catch syscall 1" \
>> +	    "Catchpoint $decimal \\(syscall .exit. \\\[1\\\]\\)" \
>> +	    "insert catch syscall on syscall 1 -- exit on i386"
>> +
>> +	gdb_test "set architecture i386:x86-64" \
>> +	    "The target architecture is assumed to be i386:x86-64" \
>> +	    "set arch to x86_64"
>> +
>> +	gdb_test "catch syscall 1" \
>> +	    "Catchpoint $decimal \\(syscall .write. \\\[1\\\]\\)" \
>> +	    "insert catch syscall on syscall 1 -- exit on i386"
>
> The "exit on i386" part seems stale here.

Fixed.

> I think we should do something like this:
>
> if { [istarget "i*86-*-*"] || [istarget "x86_64-*-*"] } {
>     set arch1 "i386"
>     set syscall1 "exit"
>     set arch2 "i386:x86-64"
>     set syscall2 "write"
> } elseif { [istarget "powerpc-*-linux*"] || [istarget "powerpc64-*-linux*"] } {
>     ...
> } elseif { [istarget "sparc-*-linux*"] && ![istarget "sparc64-*-linux*"] } {
>     ...
> } elseif { [istarget "mips*-linux*"] } {
>     ...
> } elseif { [istarget "arm*-linux*"] } {
>     ...
> } elseif { [istarget "s390*-linux*"] } {
>     ...
> } else {
>     error "please port me"
> }

That was my first idea, but I was trying to avoid having yet another set
of if...elseif... on catch-syscall.exp.  And due to my mistake when
testing this patch, I thought I wouldn't need to worry about that after
all.  Silly me.

Anyway, here is the updated patch.  WDYT?

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

gdb/testsuite/
2014-11-18  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/10737
	* gdb.base/catch-syscall.exp (do_syscall_tests): Call
	test_catch_syscall_multi_arch.
	(test_catch_syscall_multi_arch): New function.

diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
index a70534c..ed87d61 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -311,6 +311,10 @@ proc do_syscall_tests {} {
     # Testing the 'catch' syscall command during a restart of
     # the inferior.
     if [runto_main] then { test_catch_syscall_restarting_inferior }
+
+    # Testing if the 'catch syscall' command works when switching to
+    # different architectures on-the-fly (PR gdb/10737).
+    if [runto_main] then { test_catch_syscall_multi_arch }
 }
 
 proc test_catch_syscall_without_args_noxml {} {
@@ -372,6 +376,73 @@ proc test_catch_syscall_with_wrong_args_noxml {} {
     }
 }
 
+proc test_catch_syscall_multi_arch {} {
+    global decimal binfile
+
+    if { [istarget "i*86-*-*"] || [istarget "x86_64-*-*"] } {
+	set arch1 "i386"
+	set arch2 "i386:x86-64"
+	set syscall1_name "exit"
+	set syscall2_name "write"
+	set syscall_number 1
+    } elseif { [istarget "powerpc-*-linux*"] \
+		   || [istarget "powerpc64-*-linux*"] } {
+	set arch1 "powerpc:common"
+	set arch2 "powerpc:common64"
+	set syscall1_name "openat"
+	set syscall2_name "unlinkat"
+	set syscall_number 286
+    } elseif { [istarget "sparc-*-linux*"] \
+		   || [istarget "sparc64-*-linux*"] } {
+	set arch1 "sparc"
+	set arch2 "sparc:v9"
+	set syscall1_name "setresuid32"
+	set syscall2_name "setresuid"
+	set syscall_number 108
+    } elseif { [istarget "mips*-linux*"] } {
+	# MIPS does not use the same numbers for syscalls on 32 and 64
+	# bits.
+	verbose "Not testing MIPS for multi-arch syscall support"
+	return
+    } elseif { [istarget "arm*-linux*"] } {
+	# catch syscall supports only 32-bit ARM for now.
+	verbose "Not testing ARM for multi-arch syscall support"
+	return
+    } elseif { [istarget "s390*-linux*"] } {
+	set arch1 ""
+	set arch2 "s390:64-bit"
+	set syscall1_name "_newselect"
+	set syscall2_name "select"
+	set syscall_number 142
+    }
+
+    with_test_prefix "multiple targets" {
+	# 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.
+	gdb_exit
+	gdb_start
+
+	gdb_test "set architecture $arch1" \
+	    "The target architecture is assumed to be $arch1" \
+	    "set arch to $arch1"
+
+	gdb_test "catch syscall $syscall_number" \
+	    "Catchpoint $decimal \\(syscall .${syscall1_name}. \\\[${syscall_number}\\\]\\)" \
+	    "insert catch syscall on syscall $syscall_number -- $syscall1_name on $arch1"
+
+	gdb_test "set architecture $arch2" \
+	    "The target architecture is assumed to be $arch2" \
+	    "set arch to $arch2"
+
+	gdb_test "catch syscall $syscall_number" \
+	    "Catchpoint $decimal \\(syscall .${syscall2_name}. \\\[${syscall_number}\\\]\\)" \
+	    "insert catch syscall on syscall $syscall_number -- $syscall2_name on $arch2"
+
+	clean_restart $binfile
+    }
+}
+
 proc do_syscall_tests_without_xml {} {
     # Make sure GDB doesn't load the syscalls xml from the system data
     # directory.

Re: [PATCH 3/3] Testcase

[Pedro Alves]

On 11/18/2014 03:30 AM, Sergio Durigan Junior wrote:
> +    with_test_prefix "multiple targets" {

s/multiple targets/multiple architectures/

Otherwise looks good.  One question:

> +	# We are not interested in loading any binary here,

This is fine, though ...

> and in
> +	# some systems (PowerPC, for example), if we load a binary
> +	# there is no way to set other architecture.

... eh.  I wasn't aware of that.  What does GDB say?

> +	gdb_exit
> +	gdb_start

Thanks,
Pedro Alves

Re: [PATCH 3/3] Testcase

[Sergio Durigan Junior]

On Tuesday, November 18 2014, Pedro Alves wrote:

> On 11/18/2014 03:30 AM, Sergio Durigan Junior wrote:
>> +    with_test_prefix "multiple targets" {
>
> s/multiple targets/multiple architectures/

Sorry, fixed.

> Otherwise looks good.  One question:
>
>> +	# We are not interested in loading any binary here,
>
> This is fine, though ...
>
>> and in
>> +	# some systems (PowerPC, for example), if we load a binary
>> +	# there is no way to set other architecture.
>
> ... eh.  I wasn't aware of that.  What does GDB say?

Here is what I see on a Fedora 20 PPC64 box:

  (gdb) set architecture 
  Requires an argument. Valid arguments are rs6000:6000, rs6000:rs1,
  rs6000:rsc, rs6000:rs2, powerpc:common64, powerpc:common, powerpc:603,
  powerpc:EC603e, powerpc:604, powerpc:403, powerpc:601, powerpc:620,
  powerpc:630, powerpc:a35, powerpc:rs64ii, powerpc:rs64iii, powerpc:7400,
  powerpc:e500, powerpc:e500mc, powerpc:e500mc64, powerpc:MPC8XX,
  powerpc:750, powerpc:titan, powerpc:vle, powerpc:e5500, powerpc:e6500,
  auto.
  (gdb) set architecture powerpc:common
  Architecture `powerpc:common' not recognized.
  The target architecture is set automatically (currently
  powerpc:common64)

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

PowerPC: "set architecture" -> "Architecture not recognized" ? (was: Re: [PATCH 3/3] Testcase)

[Pedro Alves]

On 11/18/2014 08:20 PM, Sergio Durigan Junior wrote:

>>> and in
>>> +	# some systems (PowerPC, for example), if we load a binary
>>> +	# there is no way to set other architecture.
>>
>> ... eh.  I wasn't aware of that.  What does GDB say?
> 
> Here is what I see on a Fedora 20 PPC64 box:
> 
>   (gdb) set architecture 
>   Requires an argument. Valid arguments are rs6000:6000, rs6000:rs1,
>   rs6000:rsc, rs6000:rs2, powerpc:common64, powerpc:common, powerpc:603,
>   powerpc:EC603e, powerpc:604, powerpc:403, powerpc:601, powerpc:620,
>   powerpc:630, powerpc:a35, powerpc:rs64ii, powerpc:rs64iii, powerpc:7400,
>   powerpc:e500, powerpc:e500mc, powerpc:e500mc64, powerpc:MPC8XX,
>   powerpc:750, powerpc:titan, powerpc:vle, powerpc:e5500, powerpc:e6500,
>   auto.
>   (gdb) set architecture powerpc:common
>   Architecture `powerpc:common' not recognized.

Eh, this has the looks of a bug.  At the very least, if on purpose for
some reason, the error string is misleading.

>   The target architecture is set automatically (currently
>   powerpc:common64)

Thanks,
Pedro Alves

Re: PowerPC: "set architecture" -> "Architecture not recognized" ?

[Sergio Durigan Junior]

On Wednesday, November 19 2014, Pedro Alves wrote:

> On 11/18/2014 08:20 PM, Sergio Durigan Junior wrote:
>
>>>> and in
>>>> +	# some systems (PowerPC, for example), if we load a binary
>>>> +	# there is no way to set other architecture.
>>>
>>> ... eh.  I wasn't aware of that.  What does GDB say?
>> 
>> Here is what I see on a Fedora 20 PPC64 box:
>> 
>>   (gdb) set architecture 
>>   Requires an argument. Valid arguments are rs6000:6000, rs6000:rs1,
>>   rs6000:rsc, rs6000:rs2, powerpc:common64, powerpc:common, powerpc:603,
>>   powerpc:EC603e, powerpc:604, powerpc:403, powerpc:601, powerpc:620,
>>   powerpc:630, powerpc:a35, powerpc:rs64ii, powerpc:rs64iii, powerpc:7400,
>>   powerpc:e500, powerpc:e500mc, powerpc:e500mc64, powerpc:MPC8XX,
>>   powerpc:750, powerpc:titan, powerpc:vle, powerpc:e5500, powerpc:e6500,
>>   auto.
>>   (gdb) set architecture powerpc:common
>>   Architecture `powerpc:common' not recognized.
>
> Eh, this has the looks of a bug.  At the very least, if on purpose for
> some reason, the error string is misleading.

Yeah, that's what I thought, though I still did not investigate it.  I
plan to get to it soon, or file a bug.  Meanwhile, I believe I can check
this series in, right?

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: PowerPC: "set architecture" -> "Architecture not recognized" ?

[Pedro Alves]

On 11/19/2014 06:45 PM, Sergio Durigan Junior wrote:
> Yeah, that's what I thought, though I still did not investigate it.  I
> plan to get to it soon, or file a bug.  Meanwhile, I believe I can check
> this series in, right?

Yes.

(Please squash it to a single commit, to avoid breaking bisects.)

Thanks,
Pedro Alves

Re: PowerPC: "set architecture" -> "Architecture not recognized" ?

[Sergio Durigan Junior]

On Thursday, November 20 2014, Pedro Alves wrote:

> On 11/19/2014 06:45 PM, Sergio Durigan Junior wrote:
>> Yeah, that's what I thought, though I still did not investigate it.  I
>> plan to get to it soon, or file a bug.  Meanwhile, I believe I can check
>> this series in, right?
>
> Yes.

Thanks.

> (Please squash it to a single commit, to avoid breaking bisects.)

Sure; I was already going to do that, but forgot to mention.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Partial fix for PR breakpoints/10737: Make syscall info be per-arch instead of global

[Sergio Durigan Junior]

On Wednesday, November 12 2014, I wrote:

> Hello,
>
> This patch series intends to partially fix PR breakpoints/10737, which
> is about making the syscall information (for the "catch syscall"
> command) be per-arch, instead of global.  This is not a full fix
> because of the other issues pointed by Pedro here:
>
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=10737#c5>
>
> However, I consider it a good step towards the real fix.  It will also
> help me fix <https://sourceware.org/bugzilla/show_bug.cgi?id=17402>,
> so I think it is good if the patch goes in.

Pushed:

  <https://sourceware.org/ml/gdb-cvs/2014-11/msg00066.html>

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/