[PATCH v4 0/9] Handling multiple JITers

[PATCH v4 0/9] Handling multiple JITers

[Tankut Baris Aktemur]

Hi,

This is v4 of the series originally submitted at

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

On top of v3, which is available at

  https://sourceware.org/pipermail/gdb-patches/2020-July/170434.html

this version addresses Simon's comments.  The most significant change
is that `jiter_objfile_data` and `jited_objfile_data` structs have been
moved to `jit.h` from `objfiles.h`.  The other changes are minor format
fixes.

The patches are also available at

  https://github.com/barisaktemur/gdb/commits/multi-jit-v4

Regards.
Baris

Simon Marchi (7):
  gdb/jit: link to jit_objfile_data directly from the objfile struct
  gdb/jit: split jit_objfile_data in two
  gdb/jit: apply some simplifications and assertions
  gdb/jit: move cached_code_address and jit_breakpoint to
    jiter_objfile_data
  gdb/jit: remove jiter_objfile_data -> objfile back-link
  gdb/jit: apply minor cleanup and modernization
  gdb/jit: skip jit symbol lookup if already detected the symbols don't
    exist

Tankut Baris Aktemur (2):
  gdb/jit: pass the jiter objfile as an argument to jit_event_handler
  gdb/jit: enable tracking multiple JITer objfiles

 gdb/breakpoint.c                             |   3 +-
 gdb/jit.c                                    | 351 +++++++------------
 gdb/jit.h                                    |  43 ++-
 gdb/objfiles.h                               |  15 +
 gdb/testsuite/gdb.base/jit-reader-simple.exp |  43 ++-
 5 files changed, 236 insertions(+), 219 deletions(-)

-- 
2.17.1

[PATCH v4 1/9] gdb/jit: pass the jiter objfile as an argument to jit_event_handler

[Tankut Baris Aktemur]

This is a refactoring that adds a new parameter to the `jit_event_handler`
function: the JITer objfile.  The goal is to distinguish which JITer
triggered the JIT event, in case there are multiple JITers -- a capability
that is added in a subsequent patch.

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

	* jit.h: Forward-declare `struct objfile`.
	(jit_event_handler): Add a second parameter, the JITer objfile.
	* jit.c (jit_read_descriptor): Change the signature to take the
	JITer objfile as an argument instead of the jit_program_space_data.
	(jit_inferior_init): Update the call to jit_read_descriptor.
	(jit_event_handler): Use the new JITer objfile argument when calling
	jit_read_descriptor.
	* breakpoint.c (handle_jit_event): Update the call to
	jit_event_handler to pass the JITer objfile.
---
 gdb/breakpoint.c |  3 ++-
 gdb/jit.c        | 27 +++++++++++++++------------
 gdb/jit.h        |  7 +++++--
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6d81323dd92..414208469f9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5448,8 +5448,9 @@ handle_jit_event (void)
 
   frame = get_current_frame ();
   gdbarch = get_frame_arch (frame);
+  objfile *jiter = symbol_objfile (get_frame_function (frame));
 
-  jit_event_handler (gdbarch);
+  jit_event_handler (gdbarch, jiter);
 
   target_terminal::inferior ();
 }
diff --git a/gdb/jit.c b/gdb/jit.c
index e8a843de390..41ed81ab4b0 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -332,9 +332,9 @@ get_jit_program_space_data ()
    memory.  Returns true if all went well, false otherwise.  */
 
 static bool
-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 *jiter)
 {
   int err;
   struct type *ptr_type;
@@ -344,16 +344,16 @@ 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)
-    return false;
-  objf_data = get_jit_objfile_data (ps_data->objfile);
+  gdb_assert (jiter != nullptr);
+  objf_data = get_jit_objfile_data (jiter);
+
   if (objf_data->descriptor == NULL)
     return false;
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
 			"jit_read_descriptor, descriptor_addr = %s\n",
-			paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (ps_data->objfile,
+			paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (jiter,
 								  objf_data->descriptor)));
 
   /* Figure out how big the descriptor is on the remote and how to read it.  */
@@ -363,7 +363,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,
+  err = target_read_memory (MSYMBOL_VALUE_ADDRESS (jiter,
 						   objf_data->descriptor),
 			    desc_buf, desc_size);
   if (err)
@@ -1255,9 +1255,13 @@ 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;
+
   /* 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, jiter))
     return;
 
   /* Check that the version number agrees with that we support.  */
@@ -1330,7 +1334,7 @@ jit_inferior_exit_hook (struct inferior *inf)
 }
 
 void
-jit_event_handler (struct gdbarch *gdbarch)
+jit_event_handler (gdbarch *gdbarch, objfile *jiter)
 {
   struct jit_descriptor descriptor;
   struct jit_code_entry code_entry;
@@ -1338,8 +1342,7 @@ jit_event_handler (struct gdbarch *gdbarch)
   struct objfile *objf;
 
   /* Read the descriptor from remote memory.  */
-  if (!jit_read_descriptor (gdbarch, &descriptor,
-			    get_jit_program_space_data ()))
+  if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
     return;
   entry_addr = descriptor.relevant_entry;
 
diff --git a/gdb/jit.h b/gdb/jit.h
index cc135037812..71e78a5167d 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -20,6 +20,8 @@
 #ifndef JIT_H
 #define JIT_H
 
+struct objfile;
+
 /* When the JIT breakpoint fires, the inferior wants us to take one of
    these actions.  These values are used by the inferior, so the
    values of these enums cannot be changed.  */
@@ -76,8 +78,9 @@ extern void jit_inferior_created_hook (void);
 extern void jit_breakpoint_re_set (void);
 
 /* This function is called by handle_inferior_event when it decides
-   that the JIT event breakpoint has fired.  */
+   that the JIT event breakpoint has fired.  JITER is the objfile
+   whose JIT event breakpoint has been hit.  */
 
-extern void jit_event_handler (struct gdbarch *gdbarch);
+extern void jit_event_handler (gdbarch *gdbarch, objfile *jiter);
 
 #endif /* JIT_H */
-- 
2.17.1

[PATCH v4 2/9] gdb/jit: link to jit_objfile_data directly from the objfile struct

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

Remove the use of objfile_data to associate a jit_objfile_data with an
objfile.  Instead, directly link to a jit_objfile_data from an objfile
struct.  The goal is to eliminate unnecessary abstraction.

The free_objfile_data function naturally becomes the destructor of
jit_objfile_data.  However, free_objfile_data accesses the objfile to
which the data is attached, which the destructor of jit_objfile_data
doesn't have access to.  To work around this, add a backlink to the
owning objfile in jit_objfile_data.  This is however temporary, it goes
away in a subsequent patch.

gdb/ChangeLog:
2020-MM-DD  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.h: Forward-declare `struct minimal_symbol`.
	(struct jit_objfile_data): Migrate to here from jit.c; also add a
	constructor, destructor, and an objfile* field.
	* jit.c (jit_objfile_data): Remove.
	(struct jit_objfile_data): Migrate from here to jit.h.
	(jit_objfile_data::~jit_objfile_data): New destructor
	implementation with code moved from free_objfile_data.
	(free_objfile_data): Delete.
	(get_jit_objfile_data): Update to use the jit_data field of objfile.
	(jit_find_objf_with_entry_addr): Ditto.
	(jit_inferior_exit_hook): Ditto.
	(_initialize_jit): Remove the call to
	register_objfile_data_with_cleanup.
	* objfiles.h (struct objfile) <jit_data>: New field.
---
 gdb/jit.c      | 83 +++++++++++++-------------------------------------
 gdb/jit.h      | 29 ++++++++++++++++++
 gdb/objfiles.h |  4 +++
 3 files changed, 55 insertions(+), 61 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 41ed81ab4b0..39e364101e7 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -45,8 +45,6 @@
 
 static std::string jit_reader_dir;
 
-static const struct objfile_data *jit_objfile_data;
-
 static const char *const jit_break_name = "__jit_debug_register_code";
 
 static const char *const jit_descriptor_name = "__jit_debug_descriptor";
@@ -265,24 +263,25 @@ struct jit_program_space_data
 
 static program_space_key<jit_program_space_data> jit_program_space_key;
 
-/* Per-objfile structure recording the addresses in the program space.
-   This object serves two purposes: for ordinary objfiles, it may
-   cache some symbols related to the JIT interface; and for
-   JIT-created objfiles, it holds some information about the
-   jit_code_entry.  */
+/* Destructor for jit_objfile_data.  */
 
-struct jit_objfile_data
+jit_objfile_data::~jit_objfile_data ()
 {
-  /* Symbol for __jit_debug_register_code.  */
-  struct minimal_symbol *register_code;
-
-  /* Symbol for __jit_debug_descriptor.  */
-  struct minimal_symbol *descriptor;
+  /* Free the data allocated in the jit_program_space_data slot.  */
+  if (this->register_code != NULL)
+    {
+      struct jit_program_space_data *ps_data;
 
-  /* Address of struct jit_code_entry in this objfile.  This is only
-     non-zero for objfiles that represent code created by the JIT.  */
-  CORE_ADDR addr;
-};
+      ps_data = jit_program_space_key.get (this->objfile->pspace);
+      if (ps_data != NULL && ps_data->objfile == this->objfile)
+	{
+	  ps_data->objfile = NULL;
+	  if (ps_data->jit_breakpoint != NULL)
+	    delete_breakpoint (ps_data->jit_breakpoint);
+	  ps_data->cached_code_address = 0;
+	}
+    }
+}
 
 /* Fetch the jit_objfile_data associated with OBJF.  If no data exists
    yet, make a new structure and attach it.  */
@@ -290,16 +289,10 @@ struct jit_objfile_data
 static struct jit_objfile_data *
 get_jit_objfile_data (struct objfile *objf)
 {
-  struct jit_objfile_data *objf_data;
+  if (objf->jit_data == nullptr)
+    objf->jit_data.reset (new jit_objfile_data (objf));
 
-  objf_data = (struct jit_objfile_data *) objfile_data (objf, jit_objfile_data);
-  if (objf_data == NULL)
-    {
-      objf_data = XCNEW (struct jit_objfile_data);
-      set_objfile_data (objf, jit_objfile_data, objf_data);
-    }
-
-  return objf_data;
+  return objf->jit_data.get ();
 }
 
 /* Remember OBJFILE has been created for struct jit_code_entry located
@@ -915,13 +908,10 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
 {
   for (objfile *objf : current_program_space->objfiles ())
     {
-      struct jit_objfile_data *objf_data;
-
-      objf_data
-	= (struct jit_objfile_data *) objfile_data (objf, jit_objfile_data);
-      if (objf_data != NULL && objf_data->addr == entry_addr)
+      if (objf->jit_data != nullptr && objf->jit_data->addr == entry_addr)
 	return objf;
     }
+
   return NULL;
 }
 
@@ -1325,10 +1315,7 @@ jit_inferior_exit_hook (struct inferior *inf)
 {
   for (objfile *objf : current_program_space->objfiles_safe ())
     {
-      struct jit_objfile_data *objf_data
-	= (struct jit_objfile_data *) objfile_data (objf, jit_objfile_data);
-
-      if (objf_data != NULL && objf_data->addr != 0)
+      if (objf->jit_data != nullptr && objf->jit_data->addr != 0)
 	objf->unlink ();
     }
 }
@@ -1371,30 +1358,6 @@ jit_event_handler (gdbarch *gdbarch, objfile *jiter)
     }
 }
 
-/* Called to free the data allocated to the jit_program_space_data slot.  */
-
-static void
-free_objfile_data (struct objfile *objfile, void *data)
-{
-  struct jit_objfile_data *objf_data = (struct jit_objfile_data *) data;
-
-  if (objf_data->register_code != NULL)
-    {
-      struct jit_program_space_data *ps_data;
-
-      ps_data = jit_program_space_key.get (objfile->pspace);
-      if (ps_data != NULL && ps_data->objfile == objfile)
-	{
-	  ps_data->objfile = NULL;
-	  if (ps_data->jit_breakpoint != NULL)
-	    delete_breakpoint (ps_data->jit_breakpoint);
-	  ps_data->cached_code_address = 0;
-	}
-    }
-
-  xfree (data);
-}
-
 /* Initialize the jit_gdbarch_data slot with an instance of struct
    jit_gdbarch_data_type */
 
@@ -1427,8 +1390,6 @@ _initialize_jit ()
   gdb::observers::inferior_exit.attach (jit_inferior_exit_hook);
   gdb::observers::breakpoint_deleted.attach (jit_breakpoint_deleted);
 
-  jit_objfile_data =
-    register_objfile_data_with_cleanup (NULL, free_objfile_data);
   jit_gdbarch_data = gdbarch_data_register_pre_init (jit_gdbarch_data_init);
   if (is_dl_available ())
     {
diff --git a/gdb/jit.h b/gdb/jit.h
index 71e78a5167d..61092166391 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -21,6 +21,7 @@
 #define JIT_H
 
 struct objfile;
+struct minimal_symbol;
 
 /* When the JIT breakpoint fires, the inferior wants us to take one of
    these actions.  These values are used by the inferior, so the
@@ -66,6 +67,34 @@ struct jit_descriptor
   CORE_ADDR first_entry;
 };
 
+/* Per-objfile structure recording the addresses in the program space.
+   This object serves two purposes: for ordinary objfiles, it may
+   cache some symbols related to the JIT interface; and for
+   JIT-created objfiles, it holds some information about the
+   jit_code_entry.  */
+
+struct jit_objfile_data
+{
+  jit_objfile_data (struct objfile *objfile)
+    : objfile (objfile)
+  {}
+
+  ~jit_objfile_data ();
+
+  /* Back-link to the objfile. */
+  struct objfile *objfile;
+
+  /* Symbol for __jit_debug_register_code.  */
+  minimal_symbol *register_code = nullptr;
+
+  /* Symbol for __jit_debug_descriptor.  */
+  minimal_symbol *descriptor = nullptr;
+
+  /* Address of struct jit_code_entry in this objfile.  This is only
+     non-zero for objfiles that represent code created by the JIT.  */
+  CORE_ADDR addr = 0;
+};
+
 /* Looks for the descriptor and registration symbols and breakpoints
    the registration function.  If it finds both, it registers all the
    already JITed code.  If it has already found the symbols, then it
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 56ff52119dc..b21b4266360 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -36,6 +36,7 @@
 #include "bcache.h"
 #include "gdbarch.h"
 #include "gdbsupport/refcounted-object.h"
+#include "jit.h"
 
 struct htab;
 struct objfile_data;
@@ -697,6 +698,9 @@ struct objfile
      store these here rather than in struct block.  Static links must be
      allocated on the objfile's obstack.  */
   htab_up static_links;
+
+  /* JIT-related data for this objfile.  */
+  std::unique_ptr<jit_objfile_data> jit_data = nullptr;
 };
 
 /* A deleter for objfile.  */
-- 
2.17.1

[PATCH v4 3/9] gdb/jit: split jit_objfile_data in two

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

The jit_objfile_data is currently used to hold information about both
objfiles that are the result of JIT compilation (JITed) and objfiles
that can produce JITed objfiles (JITers).  I think that this double use
of the type is confusing, and that things would be more obvious if we
had one type for each role.

This patch splits it into:

- jited_objfile_data: for data about an objfile that is the result of a
  JIT compilation
- jiter_objfile_data: for data about an objfile which produces JITed
  objfiles

There are now two JIT-related fields in an objfile, one for each kind.
With this change, the following invariants hold:

- an objfile has a non-null `jiter_data` field iff it defines the required
  symbols of the JIT interface
- an objfile has a non-null `jited_data` field iff it is the product of
  JIT compilation (has been produced by some JITer)

gdb/ChangeLog:
2020-MM-DD  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.h (struct jit_objfile_data):  Split into...
	(struct jiter_objfile_data): ... this ...
	(struct jited_objfile_data): ... and this.
	* objfiles.h (struct objfile) <jit_data>: Remove.
	<jiter_data, jited_data>: New fields.
	* jit.c (jit_objfile_data::~jit_objfile_data): Rename to ...
	(jiter_objfile_data::~jiter_objfile_data): ... this.
	(get_jit_objfile_data): Rename to ...
	(get_jiter_objfile_data): ... this.
	(add_objfile_entry): Update.
	(jit_read_descriptor): Use get_jiter_objfile_data.
	(jit_find_objf_with_entry_addr): Use objfile's jited_data field.
	(jit_breakpoint_re_set_internal): Use get_jiter_objfile_data.
	(jit_inferior_exit_hook): Use objfile's jited_data field.
---
 gdb/jit.c      | 34 ++++++++++++++++------------------
 gdb/jit.h      | 28 +++++++++++++++++-----------
 gdb/objfiles.h |  9 +++++++--
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 39e364101e7..7c8bfcb51bb 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -263,9 +263,9 @@ struct jit_program_space_data
 
 static program_space_key<jit_program_space_data> jit_program_space_key;
 
-/* Destructor for jit_objfile_data.  */
+/* Destructor for jiter_objfile_data.  */
 
-jit_objfile_data::~jit_objfile_data ()
+jiter_objfile_data::~jiter_objfile_data ()
 {
   /* Free the data allocated in the jit_program_space_data slot.  */
   if (this->register_code != NULL)
@@ -283,16 +283,16 @@ jit_objfile_data::~jit_objfile_data ()
     }
 }
 
-/* Fetch the jit_objfile_data associated with OBJF.  If no data exists
+/* Fetch the jiter_objfile_data associated with OBJF.  If no data exists
    yet, make a new structure and attach it.  */
 
-static struct jit_objfile_data *
-get_jit_objfile_data (struct objfile *objf)
+static jiter_objfile_data *
+get_jiter_objfile_data (objfile *objf)
 {
-  if (objf->jit_data == nullptr)
-    objf->jit_data.reset (new jit_objfile_data (objf));
+  if (objf->jiter_data == nullptr)
+    objf->jiter_data.reset (new jiter_objfile_data (objf));
 
-  return objf->jit_data.get ();
+  return objf->jiter_data.get ();
 }
 
 /* Remember OBJFILE has been created for struct jit_code_entry located
@@ -301,10 +301,9 @@ get_jit_objfile_data (struct objfile *objf)
 static void
 add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
 {
-  struct jit_objfile_data *objf_data;
+  gdb_assert (objfile->jited_data == nullptr);
 
-  objf_data = get_jit_objfile_data (objfile);
-  objf_data->addr = entry;
+  objfile->jited_data.reset (new jited_objfile_data (entry));
 }
 
 /* Return jit_program_space_data for current program space.  Allocate
@@ -335,10 +334,9 @@ jit_read_descriptor (gdbarch *gdbarch,
   int desc_size;
   gdb_byte *desc_buf;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct jit_objfile_data *objf_data;
 
   gdb_assert (jiter != nullptr);
-  objf_data = get_jit_objfile_data (jiter);
+  jiter_objfile_data *objf_data = get_jiter_objfile_data (jiter);
 
   if (objf_data->descriptor == NULL)
     return false;
@@ -908,7 +906,7 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
 {
   for (objfile *objf : current_program_space->objfiles ())
     {
-      if (objf->jit_data != nullptr && objf->jit_data->addr == entry_addr)
+      if (objf->jited_data != nullptr && objf->jited_data->addr == entry_addr)
 	return objf;
     }
 
@@ -948,7 +946,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 {
   struct bound_minimal_symbol reg_symbol;
   struct bound_minimal_symbol desc_symbol;
-  struct jit_objfile_data *objf_data;
+  jiter_objfile_data *objf_data;
   CORE_ADDR addr;
 
   if (ps_data->objfile == NULL)
@@ -966,14 +964,14 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 	  || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
 	return false;
 
-      objf_data = get_jit_objfile_data (reg_symbol.objfile);
+      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_jit_objfile_data (ps_data->objfile);
+    objf_data = get_jiter_objfile_data (ps_data->objfile);
 
   addr = MSYMBOL_VALUE_ADDRESS (ps_data->objfile, objf_data->register_code);
 
@@ -1315,7 +1313,7 @@ jit_inferior_exit_hook (struct inferior *inf)
 {
   for (objfile *objf : current_program_space->objfiles_safe ())
     {
-      if (objf->jit_data != nullptr && objf->jit_data->addr != 0)
+      if (objf->jited_data != nullptr && objf->jited_data->addr != 0)
 	objf->unlink ();
     }
 }
diff --git a/gdb/jit.h b/gdb/jit.h
index 61092166391..fcef78d4991 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -67,19 +67,16 @@ struct jit_descriptor
   CORE_ADDR first_entry;
 };
 
-/* Per-objfile structure recording the addresses in the program space.
-   This object serves two purposes: for ordinary objfiles, it may
-   cache some symbols related to the JIT interface; and for
-   JIT-created objfiles, it holds some information about the
-   jit_code_entry.  */
+/* An objfile that defines the required symbols of the JIT interface has an
+   instance of this type attached to it.  */
 
-struct jit_objfile_data
+struct jiter_objfile_data
 {
-  jit_objfile_data (struct objfile *objfile)
+  jiter_objfile_data (struct objfile *objfile)
     : objfile (objfile)
   {}
 
-  ~jit_objfile_data ();
+  ~jiter_objfile_data ();
 
   /* Back-link to the objfile. */
   struct objfile *objfile;
@@ -89,10 +86,19 @@ struct jit_objfile_data
 
   /* Symbol for __jit_debug_descriptor.  */
   minimal_symbol *descriptor = nullptr;
+};
+
+/* An objfile that is the product of JIT compilation and was registered
+   using the JIT interface has an instance of this type attached to it.  */
+
+struct jited_objfile_data
+{
+  jited_objfile_data (CORE_ADDR addr)
+    : addr (addr)
+  {}
 
-  /* Address of struct jit_code_entry in this objfile.  This is only
-     non-zero for objfiles that represent code created by the JIT.  */
-  CORE_ADDR addr = 0;
+  /* Address of struct jit_code_entry for this objfile.  */
+  CORE_ADDR addr;
 };
 
 /* Looks for the descriptor and registration symbols and breakpoints
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index b21b4266360..3fbc6da0796 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -699,8 +699,13 @@ struct objfile
      allocated on the objfile's obstack.  */
   htab_up static_links;
 
-  /* JIT-related data for this objfile.  */
-  std::unique_ptr<jit_objfile_data> jit_data = nullptr;
+  /* JIT-related data for this objfile, if the objfile is a JITer;
+     that is, it produces JITed objfiles.  */
+  std::unique_ptr<jiter_objfile_data> jiter_data = nullptr;
+
+  /* JIT-related data for this objfile, if the objfile is JITed;
+     that is, it was produced by a JITer.  */
+  std::unique_ptr<jited_objfile_data> jited_data = nullptr;
 };
 
 /* A deleter for objfile.  */
-- 
2.17.1

[PATCH v4 4/9] gdb/jit: apply some simplifications and assertions

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

Following patch "gdb/jit: split jit_objfile_data in two", there are some
simplifications we can make.  The invariants described there mean that
we can assume / assert some things instead of checking them using
conditionals.

If an instance of jiter_objfile_data exists for a given objfile, it's
because the required JIT interface symbols were found.  Therefore, in
~jiter_objfile_data, the `register_code` field can't be NULL.  It was
previously used to differentiate a jit_objfile_data object used for a
JITer vs a JITed.  We can remove that check.

If an instance of jiter_objfile_data exists for a given objfile, it's
because it's the sole JITer objfile in the scope of its program space
(jit_program_space_data::objfile points to it).  At the moment,
jit_breakpoint_re_set_internal won't create a second instance of
jiter_objfile_data for a given program space.  Therefore, it's not
necessary to check for `ps_data != NULL` in ~jiter_objfile_data: we know
a jit_program_space_data for that program space exists.  We also don't
need to check for `ps_data->objfile == this->objfile`, because we know
the objfile is the sole JITer in this program space.  Replace these two
conditions with assertions.

A pre-condition for calling the jit_read_descriptor function (which is
respected in the two call sites) is that the objfile `jiter` _is_ a
JITer - it already has a jiter_objfile_data attached to it.  When a
jiter_objfile_data exists, its `descriptor` field is necessarily set:
had the descriptor symbol not been found, jit_breakpoint_re_set_internal
would not have created the jiter_objfile_data.  Remove the check and
early return in jit_read_descriptor.  Access objfile's `jiter_data` field
directly instead of calling `get_jiter_objfile_data` (which creates the
jiter_objfile_data if it doesn't exist yet) and assert that the result
is not nullptr.

Finally, `jit_event_handler` is always passed a JITer objfile.  So, add
an assertion to ensure that.

gdb/ChangeLog:
2020-MM-DD  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.c (jiter_objfile_data::~jiter_objfile_data): Remove some
	checks.
	(jit_read_descriptor): Remove NULL check.
	(jit_event_handler): Add an assertion.
---
 gdb/jit.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 7c8bfcb51bb..9ac282ae534 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -267,20 +267,17 @@ static program_space_key<jit_program_space_data> jit_program_space_key;
 
 jiter_objfile_data::~jiter_objfile_data ()
 {
-  /* Free the data allocated in the jit_program_space_data slot.  */
-  if (this->register_code != NULL)
-    {
-      struct jit_program_space_data *ps_data;
+  jit_program_space_data *ps_data
+    = jit_program_space_key.get (this->objfile->pspace);
 
-      ps_data = jit_program_space_key.get (this->objfile->pspace);
-      if (ps_data != NULL && ps_data->objfile == this->objfile)
-	{
-	  ps_data->objfile = NULL;
-	  if (ps_data->jit_breakpoint != NULL)
-	    delete_breakpoint (ps_data->jit_breakpoint);
-	  ps_data->cached_code_address = 0;
-	}
-    }
+  gdb_assert (ps_data != nullptr);
+  gdb_assert (ps_data->objfile == this->objfile);
+
+  ps_data->objfile = NULL;
+  if (ps_data->jit_breakpoint != NULL)
+    delete_breakpoint (ps_data->jit_breakpoint);
+
+  ps_data->cached_code_address = 0;
 }
 
 /* Fetch the jiter_objfile_data associated with OBJF.  If no data exists
@@ -336,10 +333,8 @@ jit_read_descriptor (gdbarch *gdbarch,
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
   gdb_assert (jiter != nullptr);
-  jiter_objfile_data *objf_data = get_jiter_objfile_data (jiter);
-
-  if (objf_data->descriptor == NULL)
-    return false;
+  jiter_objfile_data *objf_data = jiter->jiter_data.get ();
+  gdb_assert (objf_data != nullptr);
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
@@ -1326,6 +1321,10 @@ jit_event_handler (gdbarch *gdbarch, objfile *jiter)
   CORE_ADDR entry_addr;
   struct objfile *objf;
 
+  /* If we get a JIT breakpoint event for this objfile, it is necessarily a
+     JITer.  */
+  gdb_assert (jiter->jiter_data != nullptr);
+
   /* Read the descriptor from remote memory.  */
   if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
     return;
-- 
2.17.1

[PATCH v4 5/9] gdb/jit: move cached_code_address and jit_breakpoint to jiter_objfile_data

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

This is in preparation for allowing more than one JITer objfile per
program space.  Once we do that, each JITer objfile will have its own
JIT breakpoint (on the __jit_debug_register_code function it provides).
The cached_code_address field is just the runtime / relocated address of
that symbol.

Since they are going to become JITer-objfile-specific and not
program-space-specific, move these fields from jit_program_space_data to
jiter_objfile_data.

gdb/ChangeLog:
2020-MM-DD  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.h (struct jiter_objfile_data) <cached_code_address,
	jit_breakpoint>: Move to here from ...
	* jit.c (jit_program_space_data): ... here.
	(jiter_objfile_data::~jiter_objfile_data): Update.
	(jit_breakpoint_deleted): Update.
	(jit_breakpoint_re_set_internal): Update.
---
 gdb/jit.c | 41 +++++++++++++++++------------------------
 gdb/jit.h |  8 ++++++++
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 9ac282ae534..562c76330cb 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -248,17 +248,6 @@ struct jit_program_space_data
      symbols.  */
 
   struct objfile *objfile = nullptr;
-
-  /* If this program space 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.  */
-
-  CORE_ADDR cached_code_address = 0;
-
-  /* This is the JIT event breakpoint, or NULL if it has not been
-     set.  */
-
-  struct breakpoint *jit_breakpoint = nullptr;
 };
 
 static program_space_key<jit_program_space_data> jit_program_space_key;
@@ -273,11 +262,9 @@ jiter_objfile_data::~jiter_objfile_data ()
   gdb_assert (ps_data != nullptr);
   gdb_assert (ps_data->objfile == this->objfile);
 
-  ps_data->objfile = NULL;
-  if (ps_data->jit_breakpoint != NULL)
-    delete_breakpoint (ps_data->jit_breakpoint);
-
-  ps_data->cached_code_address = 0;
+  ps_data->objfile = nullptr;
+  if (this->jit_breakpoint != nullptr)
+    delete_breakpoint (this->jit_breakpoint);
 }
 
 /* Fetch the jiter_objfile_data associated with OBJF.  If no data exists
@@ -924,10 +911,16 @@ 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)
+      if (ps_data != nullptr && ps_data->objfile != nullptr)
 	{
-	  ps_data->cached_code_address = 0;
-	  ps_data->jit_breakpoint = NULL;
+	  objfile *objf = ps_data->objfile;
+	  jiter_objfile_data *jiter_data = objf->jiter_data.get ();
+
+	  if (jiter_data->jit_breakpoint == iter->owner)
+	    {
+	      jiter_data->cached_code_address = 0;
+	      jiter_data->jit_breakpoint = nullptr;
+	    }
 	}
     }
 }
@@ -976,16 +969,16 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 			"breakpoint_addr = %s\n",
 			paddress (gdbarch, addr));
 
-  if (ps_data->cached_code_address == addr)
+  if (objf_data->cached_code_address == addr)
     return true;
 
   /* Delete the old breakpoint.  */
-  if (ps_data->jit_breakpoint != NULL)
-    delete_breakpoint (ps_data->jit_breakpoint);
+  if (objf_data->jit_breakpoint != nullptr)
+    delete_breakpoint (objf_data->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);
+  objf_data->cached_code_address = addr;
+  objf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);
 
   return true;
 }
diff --git a/gdb/jit.h b/gdb/jit.h
index fcef78d4991..b78c35d5184 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -86,6 +86,14 @@ struct jiter_objfile_data
 
   /* Symbol for __jit_debug_descriptor.  */
   minimal_symbol *descriptor = nullptr;
+
+  /* This is the relocated address of the __jit_debug_register_code function
+     provided by this objfile.  This is used to detect relocations changes
+     requiring the breakpoint to be re-created.  */
+  CORE_ADDR cached_code_address = 0;
+
+  /* This is the JIT event breakpoint, or nullptr if it has been deleted.  */
+  breakpoint *jit_breakpoint = nullptr;
 };
 
 /* An objfile that is the product of JIT compilation and was registered
-- 
2.17.1

[PATCH v4 6/9] gdb/jit: enable tracking multiple JITer objfiles

[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 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 562c76330cb..0e1cb200618 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.  */
 
@@ -908,15 +874,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;
@@ -925,62 +888,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
@@ -1219,7 +1175,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)
@@ -1227,42 +1182,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);
+	}
     }
 }
 
@@ -1288,8 +1244,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

[PATCH v4 7/9] gdb/jit: remove jiter_objfile_data -> objfile back-link

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

This is no longer needed, remove it.

gdb/ChangeLog:
2020-MM-DD  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.h (struct jiter_objfile_data) <jiter_objfile_data, objfile>:
	Remove.
	* jit.c (get_jiter_objfile_data): Update.
---
 gdb/jit.c | 2 +-
 gdb/jit.h | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 0e1cb200618..85b644dde06 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -254,7 +254,7 @@ static jiter_objfile_data *
 get_jiter_objfile_data (objfile *objf)
 {
   if (objf->jiter_data == nullptr)
-    objf->jiter_data.reset (new jiter_objfile_data (objf));
+    objf->jiter_data.reset (new jiter_objfile_data ());
 
   return objf->jiter_data.get ();
 }
diff --git a/gdb/jit.h b/gdb/jit.h
index b78c35d5184..739a8f30e2c 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -72,15 +72,8 @@ struct jit_descriptor
 
 struct jiter_objfile_data
 {
-  jiter_objfile_data (struct objfile *objfile)
-    : objfile (objfile)
-  {}
-
   ~jiter_objfile_data ();
 
-  /* Back-link to the objfile. */
-  struct objfile *objfile;
-
   /* Symbol for __jit_debug_register_code.  */
   minimal_symbol *register_code = nullptr;
 
-- 
2.17.1

[PATCH v4 8/9] gdb/jit: apply minor cleanup and modernization

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

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

	* jit.c (jit_read_descriptor): Define the descriptor address once,
	use twice.
	(jit_breakpoint_deleted): Move the declaration of the loop variable
	`iter` into the loop header.
	(jit_breakpoint_re_set_internal): Move the declaration of the local
	variable `objf_data` to the first point of definition.
	(jit_event_handler): Move the declaration of local variables
	`code_entry`, `entry_addr`, and `objf` to their first point of use.
	Rename `objf` to `jited`.
---
 gdb/jit.c | 53 +++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 85b644dde06..4fe2acc2f94 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -289,11 +289,12 @@ jit_read_descriptor (gdbarch *gdbarch,
   jiter_objfile_data *objf_data = jiter->jiter_data.get ();
   gdb_assert (objf_data != nullptr);
 
+  CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (jiter, objf_data->descriptor);
+
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
 			"jit_read_descriptor, descriptor_addr = %s\n",
-			paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (jiter,
-								  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;
@@ -302,9 +303,7 @@ jit_read_descriptor (gdbarch *gdbarch,
   desc_buf = (gdb_byte *) alloca (desc_size);
 
   /* Read the descriptor.  */
-  err = target_read_memory (MSYMBOL_VALUE_ADDRESS (jiter,
-						   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 "
@@ -867,12 +866,10 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
 static void
 jit_breakpoint_deleted (struct breakpoint *b)
 {
-  struct bp_location *iter;
-
   if (b->type != bp_jit_event)
     return;
 
-  for (iter = b->loc; iter != NULL; iter = iter->next)
+  for (bp_location *iter = b->loc; iter != nullptr; iter = iter->next)
     {
       for (objfile *objf : iter->pspace->objfiles ())
 	{
@@ -894,8 +891,6 @@ jit_breakpoint_deleted (struct breakpoint *b)
 static void
 jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
 {
-  jiter_objfile_data *objf_data;
-
   for (objfile *the_objfile : pspace->objfiles ())
     {
       /* Lookup the registration symbol.  If it is missing, then we
@@ -912,7 +907,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
 	  || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
 	continue;
 
-      objf_data = get_jiter_objfile_data (reg_symbol.objfile);
+      jiter_objfile_data *objf_data
+	= get_jiter_objfile_data (reg_symbol.objfile);
       objf_data->register_code = reg_symbol.minsym;
       objf_data->descriptor = desc_symbol.minsym;
 
@@ -1265,9 +1261,6 @@ void
 jit_event_handler (gdbarch *gdbarch, objfile *jiter)
 {
   struct jit_descriptor descriptor;
-  struct jit_code_entry code_entry;
-  CORE_ADDR entry_addr;
-  struct objfile *objf;
 
   /* If we get a JIT breakpoint event for this objfile, it is necessarily a
      JITer.  */
@@ -1276,27 +1269,35 @@ jit_event_handler (gdbarch *gdbarch, objfile *jiter)
   /* Read the descriptor from remote memory.  */
   if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
     return;
-  entry_addr = descriptor.relevant_entry;
+  CORE_ADDR 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;
+      {
+	jit_code_entry code_entry;
+	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 *jited = jit_find_objf_with_entry_addr (entry_addr);
+	if (jited == nullptr)
+	  printf_unfiltered (_("Unable to find JITed code "
+			       "entry at address: %s\n"),
+			     paddress (gdbarch, entry_addr));
+	else
+	  jited->unlink ();
+
+	break;
+      }
 
-      break;
     default:
       error (_("Unknown action_flag value in JIT descriptor!"));
       break;
-- 
2.17.1

[PATCH v4 9/9] gdb/jit: skip jit symbol lookup if already detected the symbols don't exist

[Tankut Baris Aktemur]

From: Simon Marchi <simon.marchi@polymtl.ca>

To detect whether an objfile is a JITer, we lookup JIT interface
symbols in the objfile.  If an objfile does not have these symbols, we
conclude that it is not a JITer.  An objfile that does not have the
symbols will never have them.  Therefore, once we do a lookup and find
out that the objfile does not have JIT symbols, just set a flag so
that we can skip symbol lookup for that objfile the next time we reset
JIT breakpoints.

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

	* objfiles.h (struct objfile) <skip_jit_symbol_lookup>: New field.
	* jit.c (jit_breakpoint_re_set_internal): Use the
	`skip_jit_symbol_lookup` field.
---
 gdb/jit.c      | 15 +++++++++++++--
 gdb/objfiles.h |  6 ++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 4fe2acc2f94..024c66e7add 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -893,19 +893,30 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
 {
   for (objfile *the_objfile : pspace->objfiles ())
     {
+      if (the_objfile->skip_jit_symbol_lookup)
+	continue;
+
       /* Lookup the registration symbol.  If it is missing, then we
 	 assume we are not attached to a JIT.  */
       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)
-	continue;
+	{
+	  /* No need to repeat the lookup the next time.  */
+	  the_objfile->skip_jit_symbol_lookup = true;
+	  continue;
+	}
 
       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)
-	continue;
+	{
+	  /* No need to repeat the lookup the next time.  */
+	  the_objfile->skip_jit_symbol_lookup = true;
+	  continue;
+	}
 
       jiter_objfile_data *objf_data
 	= get_jiter_objfile_data (reg_symbol.objfile);
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 3fbc6da0796..549977ad257 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -706,6 +706,12 @@ struct objfile
   /* JIT-related data for this objfile, if the objfile is JITed;
      that is, it was produced by a JITer.  */
   std::unique_ptr<jited_objfile_data> jited_data = nullptr;
+
+  /* A flag that is set to true if the JIT interface symbols are not
+     found in this objfile, so that we can skip the symbol lookup the
+     next time.  If an objfile does not have the symbols, it will
+     never have them.  */
+  bool skip_jit_symbol_lookup = false;
 };
 
 /* A deleter for objfile.  */
-- 
2.17.1

Re: [PATCH v4 0/9] Handling multiple JITers

[Simon Marchi]

On 2020-07-21 2:06 p.m., Tankut Baris Aktemur wrote:
> Hi,
> 
> This is v4 of the series originally submitted at
> 
>   https://sourceware.org/pipermail/gdb-patches/2020-May/168959.html
> 
> On top of v3, which is available at
> 
>   https://sourceware.org/pipermail/gdb-patches/2020-July/170434.html
> 
> this version addresses Simon's comments.  The most significant change
> is that `jiter_objfile_data` and `jited_objfile_data` structs have been
> moved to `jit.h` from `objfiles.h`.  The other changes are minor format
> fixes.
> 
> The patches are also available at
> 
>   https://github.com/barisaktemur/gdb/commits/multi-jit-v4
> 
> Regards.
> Baris

Thanks, v3 was all good with my comments addressed, so this LGTM.

Simon

RE: [PATCH v4 0/9] Handling multiple JITers

[Aktemur, Tankut Baris]

On Wednesday, July 22, 2020 2:48 PM, Simon Marchi wrote:
> 
> Thanks, v3 was all good with my comments addressed, so this LGTM.
> 
> Simon

Thank you.  I pushed the series.

-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