[PATCH v3 6/9] gdb/jit: enable tracking multiple JITer 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 removes that assumption, allowing an arbitrary number of
objfiles within a program space to be JITers.

- The "unique" program_space -> JITer objfile pointer in
  jit_program_space_data is removed.  In fact, jit_program_space_data
  becomes empty, so it is removed entirely.

- jit_breakpoint_deleted is modified, it now has to assume that any
  objfile in a program space is a potential JITer.  It now iterates on
  all objfiles, checking if they are indeed JITers, and if they are,
  whether the deleted breakpoint belongs to them.

- jit_breakpoint_re_set_internal also has to assume that any objfile in
  a program space is a potential JITer.  It creates (or updates) one
  jiter_objfile_data structure for each JITer it finds.

- Same for jit_inferior_init.  It now iterates all objfiles to read the
  initial JIT object list.

gdb/ChangeLog:
2020-MM-DD  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
	    Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.c (struct jit_program_space_data): Remove.
	(jit_program_space_key): Remove.
	(jiter_objfile_data::~jiter_objfile_data): Remove program space
	stuff.
	(get_jit_program_space_data): Remove.
	(jit_breakpoint_deleted): Iterate on all of the program space's
	objfiles.
	(jit_inferior_init): Likewise.
	(jit_breakpoint_re_set_internal): Likewise.  Also change return
	type to void.
	(jit_breakpoint_re_set): Pass current_program_space to
	jit_breakpoint_re_set_internal.

gdb/testsuite/ChangeLog:
2020-07-13  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, 105 insertions(+), 109 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index e51b6231946..87872ac9018 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -239,30 +239,10 @@ 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.  */
-
-struct jit_program_space_data
-{
-  /* The objfile.  This is NULL if no objfile holds the JIT
-     symbols.  */
-
-  struct objfile *objfile = nullptr;
-};
-
-static program_space_key<jit_program_space_data> jit_program_space_key;
-
 /* Destructor for jiter_objfile_data.  */
 
 jiter_objfile_data::~jiter_objfile_data ()
 {
-  jit_program_space_data *ps_data
-    = jit_program_space_key.get (this->objfile->pspace);
-
-  gdb_assert (ps_data != nullptr);
-  gdb_assert (ps_data->objfile == this->objfile);
-
-  ps_data->objfile = nullptr;
   if (this->jit_breakpoint != nullptr)
     delete_breakpoint (this->jit_breakpoint);
 }
@@ -290,20 +270,6 @@ add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
   objfile->jited_data.reset (new jited_objfile_data (entry));
 }
 
-/* Return jit_program_space_data for current program space.  Allocate
-   if not already present.  */
-
-static struct jit_program_space_data *
-get_jit_program_space_data ()
-{
-  struct jit_program_space_data *ps_data;
-
-  ps_data = jit_program_space_key.get (current_program_space);
-  if (ps_data == NULL)
-    ps_data = jit_program_space_key.emplace (current_program_space);
-  return ps_data;
-}
-
 /* Helper function for reading the global JIT descriptor from remote
    memory.  Returns true if all went well, false otherwise.  */
 
@@ -906,15 +872,12 @@ jit_breakpoint_deleted (struct breakpoint *b)
 
   for (iter = b->loc; iter != NULL; iter = iter->next)
     {
-      struct jit_program_space_data *ps_data;
-
-      ps_data = jit_program_space_key.get (iter->pspace);
-      if (ps_data != nullptr && ps_data->objfile != nullptr)
+      for (objfile *objf : iter->pspace->objfiles ())
 	{
-	  objfile *objf = ps_data->objfile;
 	  jiter_objfile_data *jiter_data = objf->jiter_data.get ();
 
-	  if (jiter_data->jit_breakpoint == iter->owner)
+	  if (jiter_data != nullptr
+	      && jiter_data->jit_breakpoint == iter->owner)
 	    {
 	      jiter_data->cached_code_address = 0;
 	      jiter_data->jit_breakpoint = nullptr;
@@ -923,62 +886,55 @@ jit_breakpoint_deleted (struct breakpoint *b)
     }
 }
 
-/* (Re-)Initialize the jit breakpoint if necessary.
-   Return true if the jit breakpoint has been successfully initialized.  */
+/* (Re-)Initialize the jit breakpoints for JIT-producing objfiles in
+   PSPACE.  */
 
-static bool
-jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
-				struct jit_program_space_data *ps_data)
+static void
+jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
 {
-  struct bound_minimal_symbol reg_symbol;
-  struct bound_minimal_symbol desc_symbol;
   jiter_objfile_data *objf_data;
-  CORE_ADDR addr;
 
-  if (ps_data->objfile == NULL)
+  for (objfile *the_objfile : pspace->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);
+      bound_minimal_symbol 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);
+      bound_minimal_symbol desc_symbol
+	= lookup_minimal_symbol (jit_descriptor_name, NULL, the_objfile);
       if (desc_symbol.minsym == NULL
 	  || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
-	return false;
+	continue;
 
       objf_data = get_jiter_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_jiter_objfile_data (ps_data->objfile);
-
-  addr = MSYMBOL_VALUE_ADDRESS (ps_data->objfile, objf_data->register_code);
-
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_breakpoint_re_set_internal, "
-			"breakpoint_addr = %s\n",
-			paddress (gdbarch, addr));
+      CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (the_objfile,
+					      objf_data->register_code);
 
-  if (objf_data->cached_code_address == addr)
-    return true;
+      if (jit_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "jit_breakpoint_re_set_internal, "
+			    "breakpoint_addr = %s\n",
+			    paddress (gdbarch, addr));
 
-  /* Delete the old breakpoint.  */
-  if (objf_data->jit_breakpoint != nullptr)
-    delete_breakpoint (objf_data->jit_breakpoint);
+      /* Check if we need to re-create the breakpoint.  */
+      if (objf_data->cached_code_address == addr)
+	continue;
 
-  /* Put a breakpoint in the registration symbol.  */
-  objf_data->cached_code_address = addr;
-  objf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+      /* Delete the old breakpoint.  */
+      if (objf_data->jit_breakpoint != nullptr)
+	delete_breakpoint (objf_data->jit_breakpoint);
 
-  return true;
+      /* Put a breakpoint in the registration symbol.  */
+      objf_data->cached_code_address = addr;
+      objf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
+    }
 }
 
 /* The private data passed around in the frame unwind callback
@@ -1217,7 +1173,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
 {
   struct jit_descriptor descriptor;
   struct jit_code_entry cur_entry;
-  struct jit_program_space_data *ps_data;
   CORE_ADDR cur_entry_addr;
 
   if (jit_debug)
@@ -1225,42 +1180,43 @@ jit_inferior_init (struct gdbarch *gdbarch)
 
   jit_prepend_unwinder (gdbarch);
 
-  ps_data = get_jit_program_space_data ();
-  if (!jit_breakpoint_re_set_internal (gdbarch, ps_data))
-    return;
+  jit_breakpoint_re_set_internal (gdbarch, current_program_space);
 
-  /* There must be a JITer registered, otherwise we would exit early
-     above.  */
-  objfile *jiter = ps_data->objfile;
+  for (objfile *jiter : current_program_space->objfiles ())
+    {
+      if (jiter->jiter_data == nullptr)
+	continue;
 
-  /* 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);
+	}
     }
 }
 
@@ -1286,8 +1242,7 @@ jit_inferior_created_hook (void)
 void
 jit_breakpoint_re_set (void)
 {
-  jit_breakpoint_re_set_internal (target_gdbarch (),
-				  get_jit_program_space_data ());
+  jit_breakpoint_re_set_internal (target_gdbarch (), current_program_space);
 }
 
 /* This function cleans up any code entries left over when the
diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
index 48bd326b533..a8f33c6d7a8 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"
+	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