[PATCH 0/3] Handling multiple JITers

[PATCH 0/3] Handling multiple JITers

[Tankut Baris Aktemur]

Hi,

This short series is about handling multiple JITers.

GDB defines an interface that JITers are expected to conform to, so
that JITed code can be debugged.  In particular, a JITer should invoke
(empty) functions from that interface at important points in the JIT
flow.  GDB puts breakpoints at these functions to be notified about
essential JIT events.  See

  https://sourceware.org/gdb/current/onlinedocs/gdb/JIT-Interface.html

GDB's internal JIT-tracking mechanism assumes that there is a single
objfile that contains the JIT symbols on which GDB inserts the
notification breakpoints.  This brings the problem that if multiple
JITers exist, only the first one will be decorated with JIT breakpoints
and the JIT events from the others will be missed.

The series proposed here makes GDB track multiple objfiles with JIT
symbols.

Regards.
Baris



Tankut Baris Aktemur (3): gdb/jit: use a map to store objfile and jit
  breakpoint info gdb/jit: enable tracking multiple jitter objfiles
  gdb/testsuite: fix minor things in jit tests

 gdb/jit.c                                    | 229 ++++++++++---------
 gdb/testsuite/gdb.base/jit-elf-so.exp        |   2 +-
 gdb/testsuite/gdb.base/jit-reader-simple.exp |  47 +++-
 3 files changed, 171 insertions(+), 107 deletions(-)

-- 
2.17.1

[PATCH 1/3] gdb/jit: use a map to store objfile and jit breakpoint info

[Tankut Baris Aktemur]

GDB's JIT handler stores an objfile (and data associated with it) per
program space to keep track of JIT breakpoint information.  This
assumes that there is at most one jitter objfile in the program space.
However, there may be multiple.  If so, only the first JITer's hook
breakpoints would be realized and the JIT events from the other JITer
would be missed.  This patch replaces the single objfile pointer with
a map so that multiple objfiles can be tracked per program space.

This is a preparatory patch, where we still use a single objfile, but
just access it via the map.  The subsequent patch utilizes multiple
objfiles.

gdb/ChangeLog:
2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* jit.c: Include <map>.
	(struct jit_objfile_bp): New struct.
	(struct jit_program_space_data): Enable storing multiple objfiles.
	(jit_read_descriptor): Take an objfile*	instead of
	jit_program_space_data* as the last argument.
	(jit_breakpoint_deleted): Update to use the new jit_read_descriptor
	signature and the jit_program_space_data struct.
	(jit_breakpoint_re_set_internal): Ditto.
	(jit_inferior_init): Ditto.
	(jit_event_handler): Ditto.
	(free_objfile_data): Ditto.
---
 gdb/jit.c | 122 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 47 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 1b5ef46469e..fdb1248ed5b 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -42,6 +42,7 @@
 #include "readline/tilde.h"
 #include "completer.h"
 #include <forward_list>
+#include <map>
 
 static std::string jit_reader_dir;
 
@@ -241,17 +242,11 @@ jit_reader_unload_command (const char *args, int from_tty)
   loaded_jit_reader = NULL;
 }
 
-/* Per-program space structure recording which objfile has the JIT
-   symbols.  */
+/* Per-objfile structure recording JIT breakpoints.  */
 
-struct jit_program_space_data
+struct jit_objfile_bp
 {
-  /* The objfile.  This is NULL if no objfile holds the JIT
-     symbols.  */
-
-  struct objfile *objfile = nullptr;
-
-  /* If this program space has __jit_debug_register_code, this is the
+  /* If this objfile has __jit_debug_register_code, this is the
      cached address from the minimal symbol.  This is used to detect
      relocations requiring the breakpoint to be re-created.  */
 
@@ -260,7 +255,17 @@ struct jit_program_space_data
   /* This is the JIT event breakpoint, or NULL if it has not been
      set.  */
 
-  struct breakpoint *jit_breakpoint = nullptr;
+  breakpoint *jit_breakpoint = nullptr;
+};
+
+/* Per-program space structure recording the objfiles and their JIT
+   symbols.  */
+
+struct jit_program_space_data
+{
+  /* The JIT breakpoint informations associated to objfiles.  */
+
+  std::map<objfile *, jit_objfile_bp> objfile_and_bps;
 };
 
 static program_space_key<jit_program_space_data> jit_program_space_key;
@@ -332,9 +337,9 @@ get_jit_program_space_data ()
    memory.  Returns 1 if all went well, 0 otherwise.  */
 
 static int
-jit_read_descriptor (struct gdbarch *gdbarch,
-		     struct jit_descriptor *descriptor,
-		     struct jit_program_space_data *ps_data)
+jit_read_descriptor (gdbarch *gdbarch,
+		     jit_descriptor *descriptor,
+		     objfile *objf)
 {
   int err;
   struct type *ptr_type;
@@ -344,17 +349,17 @@ jit_read_descriptor (struct gdbarch *gdbarch,
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct jit_objfile_data *objf_data;
 
-  if (ps_data->objfile == NULL)
+  if (objf == nullptr)
     return 0;
-  objf_data = get_jit_objfile_data (ps_data->objfile);
+  objf_data = get_jit_objfile_data (objf);
   if (objf_data->descriptor == NULL)
     return 0;
 
+  CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (objf, objf_data->descriptor);
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
 			"jit_read_descriptor, descriptor_addr = %s\n",
-			paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (ps_data->objfile,
-								  objf_data->descriptor)));
+			paddress (gdbarch, addr));
 
   /* Figure out how big the descriptor is on the remote and how to read it.  */
   ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
@@ -363,9 +368,7 @@ jit_read_descriptor (struct gdbarch *gdbarch,
   desc_buf = (gdb_byte *) alloca (desc_size);
 
   /* Read the descriptor.  */
-  err = target_read_memory (MSYMBOL_VALUE_ADDRESS (ps_data->objfile,
-						   objf_data->descriptor),
-			    desc_buf, desc_size);
+  err = target_read_memory (addr, desc_buf, desc_size);
   if (err)
     {
       printf_unfiltered (_("Unable to read JIT descriptor from "
@@ -941,11 +944,13 @@ jit_breakpoint_deleted (struct breakpoint *b)
       struct jit_program_space_data *ps_data;
 
       ps_data = jit_program_space_key.get (iter->pspace);
-      if (ps_data != NULL && ps_data->jit_breakpoint == iter->owner)
-	{
-	  ps_data->cached_code_address = 0;
-	  ps_data->jit_breakpoint = NULL;
-	}
+      if (ps_data != nullptr)
+	for (auto &objf_and_bp : ps_data->objfile_and_bps)
+	  if (objf_and_bp.second.jit_breakpoint == iter->owner)
+	    {
+	      objf_and_bp.second.cached_code_address = 0;
+	      objf_and_bp.second.jit_breakpoint = nullptr;
+	    }
     }
 }
 
@@ -961,7 +966,11 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;
 
-  if (ps_data->objfile == NULL)
+  objfile *the_objfile = nullptr;
+  if (!ps_data->objfile_and_bps.empty ())
+    the_objfile = ps_data->objfile_and_bps.begin ()->first;
+
+  if (the_objfile == nullptr)
     {
       /* Lookup the registration symbol.  If it is missing, then we
 	 assume we are not attached to a JIT.  */
@@ -980,12 +989,12 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
       objf_data->register_code = reg_symbol.minsym;
       objf_data->descriptor = desc_symbol.minsym;
 
-      ps_data->objfile = reg_symbol.objfile;
+      the_objfile = reg_symbol.objfile;
     }
   else
-    objf_data = get_jit_objfile_data (ps_data->objfile);
+    objf_data = get_jit_objfile_data (the_objfile);
 
-  addr = MSYMBOL_VALUE_ADDRESS (ps_data->objfile, objf_data->register_code);
+  addr = MSYMBOL_VALUE_ADDRESS (the_objfile, objf_data->register_code);
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
@@ -993,16 +1002,20 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 			"breakpoint_addr = %s\n",
 			paddress (gdbarch, addr));
 
-  if (ps_data->cached_code_address == addr)
+  /* Note that this will insert a fresh value if THE_OBJFILE does not
+     exist as a key.  */
+  jit_objfile_bp &info = ps_data->objfile_and_bps[the_objfile];
+
+  if (info.cached_code_address == addr)
     return 0;
 
   /* Delete the old breakpoint.  */
-  if (ps_data->jit_breakpoint != NULL)
-    delete_breakpoint (ps_data->jit_breakpoint);
+  if (info.jit_breakpoint != nullptr)
+    delete_breakpoint (info.jit_breakpoint);
 
   /* Put a breakpoint in the registration symbol.  */
-  ps_data->cached_code_address = addr;
-  ps_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+  info.cached_code_address = addr;
+  info.jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
 
   return 0;
 }
@@ -1255,9 +1268,14 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
     return;
 
+  /* Fetch the saved objfile.  */
+  objfile *objf = nullptr;
+  if (!ps_data->objfile_and_bps.empty ())
+    objf = (ps_data->objfile_and_bps.begin ())->first;
+
   /* Read the descriptor so we can check the version number and load
      any already JITed functions.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor, ps_data))
+  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
     return;
 
   /* Check that the version number agrees with that we support.  */
@@ -1269,16 +1287,17 @@ jit_inferior_init (struct gdbarch *gdbarch)
       return;
     }
 
-  /* If we've attached to a running program, we need to check the descriptor
-     to register any functions that were already generated.  */
+  /* If we've attached to a running program, we need to check the
+     descriptor to register any functions that were already
+     generated.  */
   for (cur_entry_addr = descriptor.first_entry;
        cur_entry_addr != 0;
        cur_entry_addr = cur_entry.next_entry)
     {
       jit_read_code_entry (gdbarch, cur_entry_addr, &cur_entry);
 
-      /* This hook may be called many times during setup, so make sure we don't
-	 add the same symbol file twice.  */
+      /* This hook may be called many times during setup, so make sure
+	 we don't add the same symbol file twice.  */
       if (jit_find_objf_with_entry_addr (cur_entry_addr) != NULL)
 	continue;
 
@@ -1335,11 +1354,16 @@ jit_event_handler (struct gdbarch *gdbarch)
   struct jit_descriptor descriptor;
   struct jit_code_entry code_entry;
   CORE_ADDR entry_addr;
-  struct objfile *objf;
+
+  jit_program_space_data *ps_data = get_jit_program_space_data ();
+
+  /* Fetch the saved objfile.  */
+  objfile *objf = nullptr;
+  if (!ps_data->objfile_and_bps.empty ())
+    objf = (ps_data->objfile_and_bps.begin ())->first;
 
   /* Read the descriptor from remote memory.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor,
-			    get_jit_program_space_data ()))
+  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
     return;
   entry_addr = descriptor.relevant_entry;
 
@@ -1380,12 +1404,16 @@ free_objfile_data (struct objfile *objfile, void *data)
       struct jit_program_space_data *ps_data;
 
       ps_data = jit_program_space_key.get (objfile->pspace);
-      if (ps_data != NULL && ps_data->objfile == objfile)
+      if (ps_data != nullptr)
 	{
-	  ps_data->objfile = NULL;
-	  if (ps_data->jit_breakpoint != NULL)
-	    delete_breakpoint (ps_data->jit_breakpoint);
-	  ps_data->cached_code_address = 0;
+	  auto search = ps_data->objfile_and_bps.find (objfile);
+	  if (search != ps_data->objfile_and_bps.end ())
+	    {
+	      if (search->second.jit_breakpoint != nullptr)
+		delete_breakpoint (search->second.jit_breakpoint);
+
+	      ps_data->objfile_and_bps.erase (search);
+	    }
 	}
     }
 
-- 
2.17.1

[PATCH 2/3] gdb/jit: enable tracking multiple jitter objfiles

[Tankut Baris Aktemur]

Keep track of multiple JITer objfiles and their JIT breakpoints.

This patch is viewed better with 'git diff -w'.

Regression-tested on X86_64 Linux.

gdb/ChangeLog:
2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* jit.c (jit_breakpoint_re_set_internal): Iterate over all objfiles
	in the program space to look for JIT symbols.
	(jit_inferior_init): Iterate over all objfiles with JIT breakpoints.
	(jit_event_handler): Ditto.

gdb/testsuite/ChangeLog:
2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.base/jit-reader-simple.exp: Add a scenario for a binary that
	loads two JITers.
---
 gdb/jit.c                                    | 171 +++++++++----------
 gdb/testsuite/gdb.base/jit-reader-simple.exp |  43 ++++-
 2 files changed, 125 insertions(+), 89 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index fdb1248ed5b..c689ac3f392 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -966,58 +966,53 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;
 
-  objfile *the_objfile = nullptr;
-  if (!ps_data->objfile_and_bps.empty ())
-    the_objfile = ps_data->objfile_and_bps.begin ()->first;
-
-  if (the_objfile == nullptr)
+  for (objfile *the_objfile : current_program_space->objfiles ())
     {
       /* Lookup the registration symbol.  If it is missing, then we
 	 assume we are not attached to a JIT.  */
-      reg_symbol = lookup_bound_minimal_symbol (jit_break_name);
+      reg_symbol = lookup_minimal_symbol (jit_break_name, nullptr,
+					  the_objfile);
       if (reg_symbol.minsym == NULL
 	  || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
-	return 1;
+	continue;
 
       desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
 					   reg_symbol.objfile);
       if (desc_symbol.minsym == NULL
 	  || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
-	return 1;
+	continue;
 
       objf_data = get_jit_objfile_data (reg_symbol.objfile);
       objf_data->register_code = reg_symbol.minsym;
       objf_data->descriptor = desc_symbol.minsym;
 
       the_objfile = reg_symbol.objfile;
-    }
-  else
-    objf_data = get_jit_objfile_data (the_objfile);
 
-  addr = MSYMBOL_VALUE_ADDRESS (the_objfile, objf_data->register_code);
+      addr = MSYMBOL_VALUE_ADDRESS (the_objfile, objf_data->register_code);
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_breakpoint_re_set_internal, "
-			"breakpoint_addr = %s\n",
-			paddress (gdbarch, addr));
+      if (jit_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "jit_breakpoint_re_set_internal, "
+			    "breakpoint_addr = %s\n",
+			    paddress (gdbarch, addr));
 
-  /* Note that this will insert a fresh value if THE_OBJFILE does not
-     exist as a key.  */
-  jit_objfile_bp &info = ps_data->objfile_and_bps[the_objfile];
+      /* Note that this will insert a fresh value if THE_OBJFILE does not
+	 exist as a key.  */
+      jit_objfile_bp &info = ps_data->objfile_and_bps[the_objfile];
 
-  if (info.cached_code_address == addr)
-    return 0;
+      if (info.cached_code_address == addr)
+	continue;
 
-  /* Delete the old breakpoint.  */
-  if (info.jit_breakpoint != nullptr)
-    delete_breakpoint (info.jit_breakpoint);
+      /* Delete the old breakpoint.  */
+      if (info.jit_breakpoint != nullptr)
+	delete_breakpoint (info.jit_breakpoint);
 
-  /* Put a breakpoint in the registration symbol.  */
-  info.cached_code_address = addr;
-  info.jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+      /* Put a breakpoint in the registration symbol.  */
+      info.cached_code_address = addr;
+      info.jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+    }
 
-  return 0;
+  return (ps_data->objfile_and_bps.empty ()) ? 1 : 0;
 }
 
 /* The private data passed around in the frame unwind callback
@@ -1268,40 +1263,40 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
     return;
 
-  /* Fetch the saved objfile.  */
-  objfile *objf = nullptr;
-  if (!ps_data->objfile_and_bps.empty ())
-    objf = (ps_data->objfile_and_bps.begin ())->first;
+  for (auto &objf_and_bp : ps_data->objfile_and_bps)
+    {
+      objfile *objf = objf_and_bp.first;
 
-  /* Read the descriptor so we can check the version number and load
-     any already JITed functions.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
-    return;
+      /* Read the descriptor so we can check the version number and load
+	 any already JITed functions.  */
+      if (!jit_read_descriptor (gdbarch, &descriptor, objf))
+	continue;
 
-  /* Check that the version number agrees with that we support.  */
-  if (descriptor.version != 1)
-    {
-      printf_unfiltered (_("Unsupported JIT protocol version %ld "
-			   "in descriptor (expected 1)\n"),
-			 (long) descriptor.version);
-      return;
-    }
+      /* Check that the version number agrees with that we support.  */
+      if (descriptor.version != 1)
+	{
+	  printf_unfiltered (_("Unsupported JIT protocol version %ld "
+			       "in descriptor (expected 1)\n"),
+			     (long) descriptor.version);
+	  continue;
+	}
 
-  /* If we've attached to a running program, we need to check the
-     descriptor to register any functions that were already
-     generated.  */
-  for (cur_entry_addr = descriptor.first_entry;
-       cur_entry_addr != 0;
-       cur_entry_addr = cur_entry.next_entry)
-    {
-      jit_read_code_entry (gdbarch, cur_entry_addr, &cur_entry);
+      /* If we've attached to a running program, we need to check the
+	 descriptor to register any functions that were already
+	 generated.  */
+      for (cur_entry_addr = descriptor.first_entry;
+	   cur_entry_addr != 0;
+	   cur_entry_addr = cur_entry.next_entry)
+	{
+	  jit_read_code_entry (gdbarch, cur_entry_addr, &cur_entry);
 
-      /* This hook may be called many times during setup, so make sure
-	 we don't add the same symbol file twice.  */
-      if (jit_find_objf_with_entry_addr (cur_entry_addr) != NULL)
-	continue;
+	  /* This hook may be called many times during setup, so make sure
+	     we don't add the same symbol file twice.  */
+	  if (jit_find_objf_with_entry_addr (cur_entry_addr) != NULL)
+	    continue;
 
-      jit_register_code (gdbarch, cur_entry_addr, &cur_entry);
+	  jit_register_code (gdbarch, cur_entry_addr, &cur_entry);
+	}
     }
 }
 
@@ -1357,38 +1352,38 @@ jit_event_handler (struct gdbarch *gdbarch)
 
   jit_program_space_data *ps_data = get_jit_program_space_data ();
 
-  /* Fetch the saved objfile.  */
-  objfile *objf = nullptr;
-  if (!ps_data->objfile_and_bps.empty ())
-    objf = (ps_data->objfile_and_bps.begin ())->first;
-
-  /* Read the descriptor from remote memory.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
-    return;
-  entry_addr = descriptor.relevant_entry;
-
-  /* Do the corresponding action.  */
-  switch (descriptor.action_flag)
+  for (auto &objf_and_bp : ps_data->objfile_and_bps)
     {
-    case JIT_NOACTION:
-      break;
-    case JIT_REGISTER:
-      jit_read_code_entry (gdbarch, entry_addr, &code_entry);
-      jit_register_code (gdbarch, entry_addr, &code_entry);
-      break;
-    case JIT_UNREGISTER:
-      objf = jit_find_objf_with_entry_addr (entry_addr);
-      if (objf == NULL)
-	printf_unfiltered (_("Unable to find JITed code "
-			     "entry at address: %s\n"),
-			   paddress (gdbarch, entry_addr));
-      else
-	objf->unlink ();
+      objfile *objf = objf_and_bp.first;
 
-      break;
-    default:
-      error (_("Unknown action_flag value in JIT descriptor!"));
-      break;
+      /* Read the descriptor from remote memory.  */
+      if (!jit_read_descriptor (gdbarch, &descriptor, objf))
+	continue;
+      entry_addr = descriptor.relevant_entry;
+
+      /* Do the corresponding action.  */
+      switch (descriptor.action_flag)
+	{
+	case JIT_NOACTION:
+	  break;
+	case JIT_REGISTER:
+	  jit_read_code_entry (gdbarch, entry_addr, &code_entry);
+	  jit_register_code (gdbarch, entry_addr, &code_entry);
+	  break;
+	case JIT_UNREGISTER:
+	  objf = jit_find_objf_with_entry_addr (entry_addr);
+	  if (objf == NULL)
+	    printf_unfiltered (_("Unable to find JITed code "
+				 "entry at address: %s\n"),
+			       paddress (gdbarch, entry_addr));
+	  else
+	    objf->unlink ();
+
+	  break;
+	default:
+	  error (_("Unknown action_flag value in JIT descriptor!"));
+	  break;
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
index c036e71c3fb..930c59c0124 100644
--- a/gdb/testsuite/gdb.base/jit-reader-simple.exp
+++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp
@@ -34,6 +34,7 @@ standard_testfile
 set libname $testfile-jit
 set srcfile_lib $srcdir/$subdir/$libname.c
 set binfile_lib [standard_output_file $libname.so]
+set binfile_lib2 [standard_output_file ${libname}2.so]
 
 # Build a standalone JIT binary.
 
@@ -53,12 +54,15 @@ proc build_standalone_jit {{options ""}} {
 
 proc build_shared_jit {{options ""}} {
     global testfile
-    global srcfile_lib binfile_lib
+    global srcfile_lib binfile_lib binfile_lib2
 
     lappend options "debug additional_flags=-fPIC"
     if { [gdb_compile_shlib $srcfile_lib $binfile_lib $options] != "" } {
 	return -1
     }
+    if { [gdb_compile_shlib $srcfile_lib $binfile_lib2 $options] != "" } {
+	return -1
+    }
 
     return 0
 }
@@ -83,6 +87,15 @@ if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \
     return -1
 }
 
+# Build the program that loads *two* JIT libraries.
+set binfile_dl2 $binfile-dl2
+set options [list debug shlib=${binfile_lib} shlib=${binfile_lib2}]
+if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl2 executable \
+	 $options] == -1 } {
+    untested "failed to compile two-jitter binary"
+    return -1
+}
+
 # STANDALONE is true when the JIT reader is included directly in the
 # main program.  False when the JIT reader is in a separate shared
 # library.  If CHANGE_ADDR is true, force changing the JIT descriptor
@@ -160,3 +173,31 @@ foreach standalone {1 0} {
 	}
     }
 }
+
+# Now start the program that loads two JITer libraries and expect to
+# see JIT breakpoints defined for both.
+
+with_test_prefix "two JITers" {
+    clean_restart $binfile_dl2
+
+    if {![runto_main]} {
+	untested "could not run to main for the two-JITer test"
+	return -1
+    }
+
+    set num_bps 0
+    set ws "\[ \t\]+"
+    gdb_test_multiple "maint info breakpoints" "have two jit breakpoints" {
+	-re "jit events${ws}keep y${ws}$hex <__jit_debug_register_code> inf 1\r\n" {
+	    incr num_bps
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    if {$num_bps == 2} {
+		pass $gdb_test_name
+	    } else {
+		fail $gdb_test_name
+	    }
+	}
+    }
+}
-- 
2.17.1

[PATCH 3/3] gdb/testsuite: fix minor things in jit tests

[Tankut Baris Aktemur]

gdb/testsuite/ChangeLog:
2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.base/jit-elf-so.exp: Refer to the global main_loader_basename
	variable.
	* gdb.base/jit-reader-simple.exp: Fix typo ("Built" -> "Build"),
	and use the already-defined 'options' variable.
---
 gdb/testsuite/gdb.base/jit-elf-so.exp        | 2 +-
 gdb/testsuite/gdb.base/jit-reader-simple.exp | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
index 51c1d33ff9b..091fe5df812 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -50,7 +50,7 @@ set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
 # On success, return 0.
 # On failure, return -1.
 proc compile_jit_dlmain {options} {
-    global main_loader_srcfile main_loader_binfile
+    global main_loader_srcfile main_loader_binfile main_loader_basename
     set options [concat $options debug]
 
     if { [gdb_compile ${main_loader_srcfile} ${main_loader_binfile} \
diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
index 930c59c0124..d8a54a37ce7 100644
--- a/gdb/testsuite/gdb.base/jit-reader-simple.exp
+++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp
@@ -77,12 +77,12 @@ if {[build_shared_jit] == -1} {
     return
 }
 
-# Built the program that loads the JIT library.
+# Build the program that loads the JIT library.
 set srcfile_dl $testfile-dl.c
 set binfile_dl $binfile-dl
 set options [list debug shlib=${binfile_lib}]
 if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \
-	 [list debug shlib=$binfile_lib]] == -1 } {
+	 $options] == -1 } {
     untested "failed to compile"
     return -1
 }
-- 
2.17.1

RE: [PATCH 0/3] Handling multiple JITers

[Aktemur, Tankut Baris]

On Monday, May 25, 2020 11:39 AM, Tankut Baris Aktemur wrote:
> Hi,
> 
> This short series is about handling multiple JITers.
> 
> GDB defines an interface that JITers are expected to conform to, so
> that JITed code can be debugged.  In particular, a JITer should invoke
> (empty) functions from that interface at important points in the JIT
> flow.  GDB puts breakpoints at these functions to be notified about
> essential JIT events.  See
> 
>   https://sourceware.org/gdb/current/onlinedocs/gdb/JIT-Interface.html
> 
> GDB's internal JIT-tracking mechanism assumes that there is a single
> objfile that contains the JIT symbols on which GDB inserts the
> notification breakpoints.  This brings the problem that if multiple
> JITers exist, only the first one will be decorated with JIT breakpoints
> and the JIT events from the others will be missed.
> 
> The series proposed here makes GDB track multiple objfiles with JIT
> symbols.

Kindly pinging for

  https://sourceware.org/pipermail/gdb-patches/2020-May/168959.html

(Also CC'ed Simon this time because he looked at jit.c recently in detail.)

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 2/3] gdb/jit: enable tracking multiple jitter objfiles

[Simon Marchi]

On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> diff --git a/gdb/jit.c b/gdb/jit.c
> index fdb1248ed5b..c689ac3f392 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -966,58 +966,53 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,

The comment above jit_breakpoint_re_set_internal says:

  Return 0 if the jit breakpoint has been successfully initialized.

Can you adjust it to the new reality?  Something like "Return 0 if one
or more JIT breakpoints were successfully initialized".

I think it would also be good (as a separate cleanup) to make this function
return a bool, returning true when one or more JIT breakpoints were inserted.

> @@ -1357,38 +1352,38 @@ jit_event_handler (struct gdbarch *gdbarch)
>  
>    jit_program_space_data *ps_data = get_jit_program_space_data ();
>  
> -  /* Fetch the saved objfile.  */
> -  objfile *objf = nullptr;
> -  if (!ps_data->objfile_and_bps.empty ())
> -    objf = (ps_data->objfile_and_bps.begin ())->first;
> -
> -  /* Read the descriptor from remote memory.  */
> -  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
> -    return;
> -  entry_addr = descriptor.relevant_entry;
> -
> -  /* Do the corresponding action.  */
> -  switch (descriptor.action_flag)
> +  for (auto &objf_and_bp : ps_data->objfile_and_bps)
>      {
> -    case JIT_NOACTION:
> -      break;
> -    case JIT_REGISTER:
> -      jit_read_code_entry (gdbarch, entry_addr, &code_entry);
> -      jit_register_code (gdbarch, entry_addr, &code_entry);
> -      break;
> -    case JIT_UNREGISTER:
> -      objf = jit_find_objf_with_entry_addr (entry_addr);
> -      if (objf == NULL)
> -	printf_unfiltered (_("Unable to find JITed code "
> -			     "entry at address: %s\n"),
> -			   paddress (gdbarch, entry_addr));
> -      else
> -	objf->unlink ();
> +      objfile *objf = objf_and_bp.first;
>  
> -      break;
> -    default:
> -      error (_("Unknown action_flag value in JIT descriptor!"));
> -      break;
> +      /* Read the descriptor from remote memory.  */
> +      if (!jit_read_descriptor (gdbarch, &descriptor, objf))
> +	continue;
> +      entry_addr = descriptor.relevant_entry;
> +
> +      /* Do the corresponding action.  */
> +      switch (descriptor.action_flag)
> +	{
> +	case JIT_NOACTION:
> +	  break;
> +	case JIT_REGISTER:
> +	  jit_read_code_entry (gdbarch, entry_addr, &code_entry);
> +	  jit_register_code (gdbarch, entry_addr, &code_entry);
> +	  break;
> +	case JIT_UNREGISTER:
> +	  objf = jit_find_objf_with_entry_addr (entry_addr);
> +	  if (objf == NULL)
> +	    printf_unfiltered (_("Unable to find JITed code "
> +				 "entry at address: %s\n"),
> +			       paddress (gdbarch, entry_addr));
> +	  else
> +	    objf->unlink ();
> +
> +	  break;
> +	default:
> +	  error (_("Unknown action_flag value in JIT descriptor!"));
> +	  break;
> +	}

I don't think that iterating on all JIT-providing objfiles is the correct thing
to do here.  After all, we have hit the __jit_debug_register_code breakpoint of
exactly one of them, so we should only process the jit descriptor of that one.
At the same moment, the other JIT-providing objfiles could have prepared anything
in their descriptor (or there could be junk data), but they have not asked us
to process it.

I haven't tried, but I bet that you could build a test to trigger a bug here:

1. Have two jit-providing objfiles register one jit objfile each
2. Have the two jit-providing objfiles prepare their descriptor with JIT_UNREGISTER
   and the address of their jit objfile
3. Have one of them call __jit_debug_register_code.

We would forget about both jit objfiles, when in reality we should forget only about
one.

I think we'll need to know which jit event breakpoint was hit, so we can know which
jit-providing objfile asked us to process its descriptor.

Simon

Re: [PATCH 1/3] gdb/jit: use a map to store objfile and jit breakpoint info

[Simon Marchi]

On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 1b5ef46469e..fdb1248ed5b 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -42,6 +42,7 @@
>  #include "readline/tilde.h"
>  #include "completer.h"
>  #include <forward_list>
> +#include <map>
>  
>  static std::string jit_reader_dir;
>  
> @@ -241,17 +242,11 @@ jit_reader_unload_command (const char *args, int from_tty)
>    loaded_jit_reader = NULL;
>  }
>  
> -/* Per-program space structure recording which objfile has the JIT
> -   symbols.  */
> +/* Per-objfile structure recording JIT breakpoints.  */

Just to help disambiguate, maybe precise here that we are talking about
the JIT-providing objfiles (those that define the magic interface
symbols), not the objfiles that are the result of the JIT.

>  
> -struct jit_program_space_data
> +struct jit_objfile_bp
>  {
> -  /* The objfile.  This is NULL if no objfile holds the JIT
> -     symbols.  */
> -
> -  struct objfile *objfile = nullptr;
> -
> -  /* If this program space has __jit_debug_register_code, this is the
> +  /* If this objfile has __jit_debug_register_code, this is the
>       cached address from the minimal symbol.  This is used to detect
>       relocations requiring the breakpoint to be re-created.  */
>  
> @@ -260,7 +255,17 @@ struct jit_program_space_data
>    /* This is the JIT event breakpoint, or NULL if it has not been
>       set.  */
>  
> -  struct breakpoint *jit_breakpoint = nullptr;
> +  breakpoint *jit_breakpoint = nullptr;
> +};
> +
> +/* Per-program space structure recording the objfiles and their JIT
> +   symbols.  */
> +
> +struct jit_program_space_data
> +{
> +  /* The JIT breakpoint informations associated to objfiles.  */
> +
> +  std::map<objfile *, jit_objfile_bp> objfile_and_bps;

If we don't care about key ordering, I'd use an std::unordered_map.

But really, if we expect just to have a handful of items, it would probably
be more efficient to have a list or vector.

Also, given my comment on the following patch, I think we'll have to do
lookups by breakpoint address, so we would have to iterate on the maps items
anyway.  Unless we use the breakpoint address as the key.

>  };
>  
>  static program_space_key<jit_program_space_data> jit_program_space_key;
> @@ -332,9 +337,9 @@ get_jit_program_space_data ()
>     memory.  Returns 1 if all went well, 0 otherwise.  */
>  
>  static int
> -jit_read_descriptor (struct gdbarch *gdbarch,
> -		     struct jit_descriptor *descriptor,
> -		     struct jit_program_space_data *ps_data)
> +jit_read_descriptor (gdbarch *gdbarch,
> +		     jit_descriptor *descriptor,
> +		     objfile *objf)
>  {
>    int err;
>    struct type *ptr_type;
> @@ -344,17 +349,17 @@ jit_read_descriptor (struct gdbarch *gdbarch,
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    struct jit_objfile_data *objf_data;
>  
> -  if (ps_data->objfile == NULL)
> +  if (objf == nullptr)
>      return 0;

I would probably change jit_read_descriptor to require a non-NULL objfile.

Simon

Re: [PATCH 3/3] gdb/testsuite: fix minor things in jit tests

[Simon Marchi]

On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> gdb/testsuite/ChangeLog:
> 2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* gdb.base/jit-elf-so.exp: Refer to the global main_loader_basename
> 	variable.
> 	* gdb.base/jit-reader-simple.exp: Fix typo ("Built" -> "Build"),
> 	and use the already-defined 'options' variable.
> ---
>  gdb/testsuite/gdb.base/jit-elf-so.exp        | 2 +-
>  gdb/testsuite/gdb.base/jit-reader-simple.exp | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
> index 51c1d33ff9b..091fe5df812 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
> @@ -50,7 +50,7 @@ set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
>  # On success, return 0.
>  # On failure, return -1.
>  proc compile_jit_dlmain {options} {
> -    global main_loader_srcfile main_loader_binfile
> +    global main_loader_srcfile main_loader_binfile main_loader_basename
>      set options [concat $options debug]
>  
>      if { [gdb_compile ${main_loader_srcfile} ${main_loader_binfile} \
> diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
> index 930c59c0124..d8a54a37ce7 100644
> --- a/gdb/testsuite/gdb.base/jit-reader-simple.exp
> +++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp
> @@ -77,12 +77,12 @@ if {[build_shared_jit] == -1} {
>      return
>  }
>  
> -# Built the program that loads the JIT library.
> +# Build the program that loads the JIT library.
>  set srcfile_dl $testfile-dl.c
>  set binfile_dl $binfile-dl
>  set options [list debug shlib=${binfile_lib}]
>  if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \
> -	 [list debug shlib=$binfile_lib]] == -1 } {
> +	 $options] == -1 } {
>      untested "failed to compile"
>      return -1
>  }
> -- 
> 2.17.1
> 

This LGTM, and it seems it could be pushed on its own?

Simon

RE: [PATCH 3/3] gdb/testsuite: fix minor things in jit tests

[Aktemur, Tankut Baris]

On Sunday, June 14, 2020 8:10 PM, Simon Marchi wrote:
> On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> > gdb/testsuite/ChangeLog:
> > 2020-05-25  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> >
> > 	* gdb.base/jit-elf-so.exp: Refer to the global main_loader_basename
> > 	variable.
> > 	* gdb.base/jit-reader-simple.exp: Fix typo ("Built" -> "Build"),
> > 	and use the already-defined 'options' variable.
> > ---
> >  gdb/testsuite/gdb.base/jit-elf-so.exp        | 2 +-
> >  gdb/testsuite/gdb.base/jit-reader-simple.exp | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
> > index 51c1d33ff9b..091fe5df812 100644
> > --- a/gdb/testsuite/gdb.base/jit-elf-so.exp
> > +++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
> > @@ -50,7 +50,7 @@ set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
> >  # On success, return 0.
> >  # On failure, return -1.
> >  proc compile_jit_dlmain {options} {
> > -    global main_loader_srcfile main_loader_binfile
> > +    global main_loader_srcfile main_loader_binfile main_loader_basename
> >      set options [concat $options debug]
> >
> >      if { [gdb_compile ${main_loader_srcfile} ${main_loader_binfile} \
> > diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
> > index 930c59c0124..d8a54a37ce7 100644
> > --- a/gdb/testsuite/gdb.base/jit-reader-simple.exp
> > +++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp
> > @@ -77,12 +77,12 @@ if {[build_shared_jit] == -1} {
> >      return
> >  }
> >
> > -# Built the program that loads the JIT library.
> > +# Build the program that loads the JIT library.
> >  set srcfile_dl $testfile-dl.c
> >  set binfile_dl $binfile-dl
> >  set options [list debug shlib=${binfile_lib}]
> >  if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \
> > -	 [list debug shlib=$binfile_lib]] == -1 } {
> > +	 $options] == -1 } {
> >      untested "failed to compile"
> >      return -1
> >  }
> > --
> > 2.17.1
> >
> 
> This LGTM, and it seems it could be pushed on its own?
> 
> Simon

Yes, it's independent from the other patches.  I pushed it and will not
include in the v2 of the series.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 1/3] gdb/jit: use a map to store objfile and jit breakpoint info

[Aktemur, Tankut Baris]

On Sunday, June 14, 2020 7:50 PM, Simon Marchi wrote:
> On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> > diff --git a/gdb/jit.c b/gdb/jit.c
> > index 1b5ef46469e..fdb1248ed5b 100644
> > --- a/gdb/jit.c
> > +++ b/gdb/jit.c
> > @@ -42,6 +42,7 @@
> >  #include "readline/tilde.h"
> >  #include "completer.h"
> >  #include <forward_list>
> > +#include <map>
> >
> >  static std::string jit_reader_dir;
> >
> > @@ -241,17 +242,11 @@ jit_reader_unload_command (const char *args, int from_tty)
> >    loaded_jit_reader = NULL;
> >  }
> >
> > -/* Per-program space structure recording which objfile has the JIT
> > -   symbols.  */
> > +/* Per-objfile structure recording JIT breakpoints.  */
> 
> Just to help disambiguate, maybe precise here that we are talking about
> the JIT-providing objfiles (those that define the magic interface
> symbols), not the objfiles that are the result of the JIT.

Updated the comment in v2 as

  /* Structure recording JIT breakpoints per JITer objfile.  */

> >
> > -struct jit_program_space_data
> > +struct jit_objfile_bp
> >  {
> > -  /* The objfile.  This is NULL if no objfile holds the JIT
> > -     symbols.  */
> > -
> > -  struct objfile *objfile = nullptr;
> > -
> > -  /* If this program space has __jit_debug_register_code, this is the
> > +  /* If this objfile has __jit_debug_register_code, this is the
> >       cached address from the minimal symbol.  This is used to detect
> >       relocations requiring the breakpoint to be re-created.  */
> >
> > @@ -260,7 +255,17 @@ struct jit_program_space_data
> >    /* This is the JIT event breakpoint, or NULL if it has not been
> >       set.  */
> >
> > -  struct breakpoint *jit_breakpoint = nullptr;
> > +  breakpoint *jit_breakpoint = nullptr;
> > +};
> > +
> > +/* Per-program space structure recording the objfiles and their JIT
> > +   symbols.  */
> > +
> > +struct jit_program_space_data
> > +{
> > +  /* The JIT breakpoint informations associated to objfiles.  */
> > +
> > +  std::map<objfile *, jit_objfile_bp> objfile_and_bps;
> 
> If we don't care about key ordering, I'd use an std::unordered_map.
> 
> But really, if we expect just to have a handful of items, it would probably
> be more efficient to have a list or vector.
> 
> Also, given my comment on the following patch, I think we'll have to do
> lookups by breakpoint address, so we would have to iterate on the maps items
> anyway.  Unless we use the breakpoint address as the key.

In several places we need to simply iterate over all.  In couple places we need to
lookup by the objfile, and in one place by the breakpoint.  In practice, I would
expect the number of JITer objfiles to be fewer than a handful.  Given all these,
I updated the patch to use a list.

> >  };
> >
> >  static program_space_key<jit_program_space_data> jit_program_space_key;
> > @@ -332,9 +337,9 @@ get_jit_program_space_data ()
> >     memory.  Returns 1 if all went well, 0 otherwise.  */
> >
> >  static int
> > -jit_read_descriptor (struct gdbarch *gdbarch,
> > -		     struct jit_descriptor *descriptor,
> > -		     struct jit_program_space_data *ps_data)
> > +jit_read_descriptor (gdbarch *gdbarch,
> > +		     jit_descriptor *descriptor,
> > +		     objfile *objf)
> >  {
> >    int err;
> >    struct type *ptr_type;
> > @@ -344,17 +349,17 @@ jit_read_descriptor (struct gdbarch *gdbarch,
> >    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> >    struct jit_objfile_data *objf_data;
> >
> > -  if (ps_data->objfile == NULL)
> > +  if (objf == nullptr)
> >      return 0;
> 
> I would probably change jit_read_descriptor to require a non-NULL objfile.
> 
> Simon

In v2, this spot now includes a gdb_assert.  There are two places that call this function.
In both, I believe it's guaranteed that the argument is non-null.

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 2/3] gdb/jit: enable tracking multiple jitter objfiles

[Aktemur, Tankut Baris]

On Sunday, June 14, 2020 7:09 PM, Simon Marchi wrote:
> On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> > diff --git a/gdb/jit.c b/gdb/jit.c
> > index fdb1248ed5b..c689ac3f392 100644
> > --- a/gdb/jit.c
> > +++ b/gdb/jit.c
> > @@ -966,58 +966,53 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
> 
> The comment above jit_breakpoint_re_set_internal says:
> 
>   Return 0 if the jit breakpoint has been successfully initialized.
> 
> Can you adjust it to the new reality?  Something like "Return 0 if one
> or more JIT breakpoints were successfully initialized".
> 
> I think it would also be good (as a separate cleanup) to make this function
> return a bool, returning true when one or more JIT breakpoints were inserted.

Updated the comment in v2.  Also added a separate patch to make the return type 'bool'.

> > @@ -1357,38 +1352,38 @@ jit_event_handler (struct gdbarch *gdbarch)
> >
> >    jit_program_space_data *ps_data = get_jit_program_space_data ();
> >
> > -  /* Fetch the saved objfile.  */
> > -  objfile *objf = nullptr;
> > -  if (!ps_data->objfile_and_bps.empty ())
> > -    objf = (ps_data->objfile_and_bps.begin ())->first;
> > -
> > -  /* Read the descriptor from remote memory.  */
> > -  if (!jit_read_descriptor (gdbarch, &descriptor, objf))
> > -    return;
> > -  entry_addr = descriptor.relevant_entry;
> > -
> > -  /* Do the corresponding action.  */
> > -  switch (descriptor.action_flag)
> > +  for (auto &objf_and_bp : ps_data->objfile_and_bps)
> >      {
> > -    case JIT_NOACTION:
> > -      break;
> > -    case JIT_REGISTER:
> > -      jit_read_code_entry (gdbarch, entry_addr, &code_entry);
> > -      jit_register_code (gdbarch, entry_addr, &code_entry);
> > -      break;
> > -    case JIT_UNREGISTER:
> > -      objf = jit_find_objf_with_entry_addr (entry_addr);
> > -      if (objf == NULL)
> > -	printf_unfiltered (_("Unable to find JITed code "
> > -			     "entry at address: %s\n"),
> > -			   paddress (gdbarch, entry_addr));
> > -      else
> > -	objf->unlink ();
> > +      objfile *objf = objf_and_bp.first;
> >
> > -      break;
> > -    default:
> > -      error (_("Unknown action_flag value in JIT descriptor!"));
> > -      break;
> > +      /* Read the descriptor from remote memory.  */
> > +      if (!jit_read_descriptor (gdbarch, &descriptor, objf))
> > +	continue;
> > +      entry_addr = descriptor.relevant_entry;
> > +
> > +      /* Do the corresponding action.  */
> > +      switch (descriptor.action_flag)
> > +	{
> > +	case JIT_NOACTION:
> > +	  break;
> > +	case JIT_REGISTER:
> > +	  jit_read_code_entry (gdbarch, entry_addr, &code_entry);
> > +	  jit_register_code (gdbarch, entry_addr, &code_entry);
> > +	  break;
> > +	case JIT_UNREGISTER:
> > +	  objf = jit_find_objf_with_entry_addr (entry_addr);
> > +	  if (objf == NULL)
> > +	    printf_unfiltered (_("Unable to find JITed code "
> > +				 "entry at address: %s\n"),
> > +			       paddress (gdbarch, entry_addr));
> > +	  else
> > +	    objf->unlink ();
> > +
> > +	  break;
> > +	default:
> > +	  error (_("Unknown action_flag value in JIT descriptor!"));
> > +	  break;
> > +	}
> 
> I don't think that iterating on all JIT-providing objfiles is the correct thing
> to do here.  After all, we have hit the __jit_debug_register_code breakpoint of
> exactly one of them, so we should only process the jit descriptor of that one.
> At the same moment, the other JIT-providing objfiles could have prepared anything
> in their descriptor (or there could be junk data), but they have not asked us
> to process it.
> 
> I haven't tried, but I bet that you could build a test to trigger a bug here:
> 
> 1. Have two jit-providing objfiles register one jit objfile each
> 2. Have the two jit-providing objfiles prepare their descriptor with JIT_UNREGISTER
>    and the address of their jit objfile
> 3. Have one of them call __jit_debug_register_code.
> 
> We would forget about both jit objfiles, when in reality we should forget only about
> one.

You're right.

> I think we'll need to know which jit event breakpoint was hit, so we can know which
> jit-providing objfile asked us to process its descriptor.
> 
> Simon

I added a separate patch in v2 to pass the JITer objfile as an argument to jit_event_handler.

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928