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

[Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>]

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 JITer 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 JITers
would be missed.  This patch replaces the single objfile pointer with
a list so that multiple objfiles can be tracked per program space.

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

Regression-tested on X86_64 Linux.

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

	* jit.c: (struct jiter_and_bp): New struct.
	(struct jit_program_space_data): Enable storing multiple objfiles.
	(jit_breakpoint_re_set_internal): Iterate over all objfiles in the
	program space to look for JIT symbols.
	(jit_breakpoint_deleted): Update to iterate over multiple jiters.
	(free_objfile_data): Ditto.
	(jit_inferior_init): Ditto.

gdb/testsuite/ChangeLog:
2020-06-15  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                                    | 172 +++++++++++--------
 gdb/testsuite/gdb.base/jit-reader-simple.exp |  43 ++++-
 2 files changed, 144 insertions(+), 71 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 5da0601b024..2a15aac9e70 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -241,17 +241,15 @@ 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.  */
+/* Structure recording JIT breakpoints per JITer objfile.  */
 
-struct jit_program_space_data
+struct jiter_and_bp
 {
-  /* The objfile.  This is NULL if no objfile holds the JIT
-     symbols.  */
+  /* The JITer objfile.  */
 
-  struct objfile *objfile = nullptr;
+  objfile *jiter = 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 +258,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 information associated with JITer objfiles.  */
+
+  std::forward_list<jiter_and_bp> jiter_and_bps;
 };
 
 static program_space_key<jit_program_space_data> jit_program_space_key;
@@ -941,16 +949,18 @@ 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 &info : ps_data->jiter_and_bps)
+	  if (info.jit_breakpoint == iter->owner)
+	    {
+	      info.cached_code_address = 0;
+	      info.jit_breakpoint = nullptr;
+	    }
     }
 }
 
-/* (Re-)Initialize the jit breakpoint if necessary.
-   Return true if the jit breakpoint has been successfully initialized.  */
+/* (Re-)Initialize the jit breakpoint if necessary.  Return true if
+   one or more jit breakpoints have been successfully initialized.  */
 
 static bool
 jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
@@ -961,50 +971,62 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;
 
-  if (ps_data->objfile == NULL)
+  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 false;
+	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 false;
+	continue;
 
       objf_data = get_jit_objfile_data (reg_symbol.objfile);
       objf_data->register_code = reg_symbol.minsym;
       objf_data->descriptor = desc_symbol.minsym;
 
-      ps_data->objfile = reg_symbol.objfile;
-    }
-  else
-    objf_data = get_jit_objfile_data (ps_data->objfile);
+      the_objfile = reg_symbol.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,
-			"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));
 
-  if (ps_data->cached_code_address == addr)
-    return true;
+      /* Search for an existing entry, or insert new if necessary.  */
+      auto iter = ps_data->jiter_and_bps.begin ();
+      for (; iter != ps_data->jiter_and_bps.end (); iter++)
+	if (iter->jiter == the_objfile)
+	  break;
 
-  /* Delete the old breakpoint.  */
-  if (ps_data->jit_breakpoint != NULL)
-    delete_breakpoint (ps_data->jit_breakpoint);
+      if (iter == ps_data->jiter_and_bps.end ())
+	{
+	  ps_data->jiter_and_bps.emplace_front ();
+	  iter = ps_data->jiter_and_bps.begin ();
+	  iter->jiter = the_objfile;
+	}
 
-  /* Put a breakpoint in the registration symbol.  */
-  ps_data->cached_code_address = addr;
-  ps_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+      if (iter->cached_code_address == addr)
+	continue;
 
-  return true;
+      /* Delete the old breakpoint.  */
+      if (iter->jit_breakpoint != nullptr)
+	delete_breakpoint (iter->jit_breakpoint);
+
+      /* Put a breakpoint in the registration symbol.  */
+      iter->cached_code_address = addr;
+      iter->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+    }
+
+  return !ps_data->jiter_and_bps.empty ();
 }
 
 /* The private data passed around in the frame unwind callback
@@ -1255,38 +1277,40 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (!jit_breakpoint_re_set_internal (gdbarch, ps_data))
     return;
 
-  /* There must be a JITer registered, otherwise we would exit early
-     above.  */
-  objfile *jiter = ps_data->objfile;
+  for (auto &info : ps_data->jiter_and_bps)
+    {
+      objfile *jiter = info.jiter;
 
-  /* Read the descriptor so we can check the version number and load
-     any already JITed functions.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
-    return;
+      /* Read the descriptor so we can check the version number and load
+	 any already JITed functions.  */
+      if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
+	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);
+	}
     }
 }
 
@@ -1383,12 +1407,20 @@ 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;
+	  /* Remove the data kept for objfile, but delete the
+	     breakpoint, too.  */
+	  ps_data->jiter_and_bps.remove_if ([=] (jiter_and_bp &info)
+	    {
+	      if (info.jiter == objfile)
+		{
+		  if (info.jit_breakpoint != nullptr)
+		    delete_breakpoint (info.jit_breakpoint);
+		  return true;
+		}
+	      return false;
+	    });
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
index 48bd326b533..d8a54a37ce7 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