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

[Simon Marchi]

On 2020-07-15 4:16 a.m., Tankut Baris Aktemur wrote:
> 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: Include "objfiles.h".
> 	(jit_event_handler): Add a second parameter, the JITer objfile.
> 	* breakpoint.c (handle_jit_event): Update the call to
> 	jit_event_handler to pass 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.
> ---
>  gdb/breakpoint.c |  3 ++-
>  gdb/jit.c        | 27 +++++++++++++++------------
>  gdb/jit.h        |  4 +++-
>  3 files changed, 20 insertions(+), 14 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..dddded0236f 100644
> --- a/gdb/jit.h
> +++ b/gdb/jit.h
> @@ -20,6 +20,8 @@
>  #ifndef JIT_H
>  #define JIT_H
>  
> +#include "objfiles.h"

It's preferable to use forward declarations when possible:

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.  */
> @@ -78,6 +80,6 @@ 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.  */

Can you document the new parameter?  I suppose it would be something like
"JITER is the objfiles whose JIT event breakpoint has been hit".

Simon