[PATCH] gdb: include address in names of objfiles created by jit reader API

[PATCH] gdb: include address in names of objfiles created by jit reader API

[Jan Vrany]

This commit includes jited object address in the names of objfiles
created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
This allows one to at least differentiate one from another.
---
 gdb/jit.c                             | 8 ++++++--
 gdb/testsuite/gdb.base/jit-reader.exp | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 42776b95683..371cb8b1d48 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -624,12 +624,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
 		       struct gdb_object *obj)
 {
   struct objfile *objfile;
+  char objfile_name[64];
   jit_dbg_reader_data *priv_data;
 
   priv_data = (jit_dbg_reader_data *) cb->priv_data;
 
-  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
-			   OBJF_NOT_FILENAME);
+  snprintf (objfile_name, sizeof (objfile_name) - 1,
+            "<< JIT compiled code at 0x%" PRIxPTR " >>",
+            reinterpret_cast<uintptr_t> (priv_data));
+  objfile_name[sizeof (objfile_name) - 1] = '\0';
+  objfile = objfile::make (nullptr, objfile_name, OBJF_NOT_FILENAME);
   objfile->per_bfd->gdbarch = target_gdbarch ();
 
   for (gdb_symtab &symtab : obj->symtabs)
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index bcc85640ec0..d94360cd7d9 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -230,11 +230,11 @@ proc jit_reader_test {} {
 
 	    if { ![skip_python_tests] } {
 		gdb_test "python print(gdb.objfiles())" \
-		    "$any<gdb.Objfile filename=<< JIT compiled code >>>$any" \
+		    "$any<gdb.Objfile filename=<< JIT compiled code at $hex >>>$any" \
 		    "python gdb.Objfile.__repr__ ()"
 
 		gdb_test "python print(list(map(lambda objf : objf.filename, gdb.objfiles())))" \
-		    "$any'<< JIT compiled code >>'$any" \
+		    "$any'<< JIT compiled code at $hex >>'$any" \
 		    "python gdb.Objfile.filename"
 	    }
 	}
-- 
2.30.2

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Tom Tromey]

>>>>> "Jan" == Jan Vrany via Gdb-patches <gdb-patches@sourceware.org> writes:

Jan> This commit includes jited object address in the names of objfiles
Jan> created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
Jan> This allows one to at least differentiate one from another.

Jan>    struct objfile *objfile;
Jan> +  char objfile_name[64];
Jan>    jit_dbg_reader_data *priv_data;
 
Jan>    priv_data = (jit_dbg_reader_data *) cb->priv_data;
 
Jan> -  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
Jan> -			   OBJF_NOT_FILENAME);
Jan> +  snprintf (objfile_name, sizeof (objfile_name) - 1,
Jan> +            "<< JIT compiled code at 0x%" PRIxPTR " >>",
Jan> +            reinterpret_cast<uintptr_t> (priv_data));
Jan> +  objfile_name[sizeof (objfile_name) - 1] = '\0';

gdb normally uses xsnprintf for things like this.
Then you don't need to add the terminator, either.

However, it's also fine, and simpler, to just use string_printf.  This
allocates but it seems unimportant here.

Tom

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Simon Marchi]

On 2022-02-02 7:03 a.m., Jan Vrany via Gdb-patches wrote:
> This commit includes jited object address in the names of objfiles
> created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
> This allows one to at least differentiate one from another.
> ---
>  gdb/jit.c                             | 8 ++++++--
>  gdb/testsuite/gdb.base/jit-reader.exp | 4 ++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 42776b95683..371cb8b1d48 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -624,12 +624,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
>  		       struct gdb_object *obj)
>  {
>    struct objfile *objfile;
> +  char objfile_name[64];
>    jit_dbg_reader_data *priv_data;
>  
>    priv_data = (jit_dbg_reader_data *) cb->priv_data;
>  
> -  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
> -			   OBJF_NOT_FILENAME);
> +  snprintf (objfile_name, sizeof (objfile_name) - 1,
> +            "<< JIT compiled code at 0x%" PRIxPTR " >>",
> +            reinterpret_cast<uintptr_t> (priv_data));
> +  objfile_name[sizeof (objfile_name) - 1] = '\0';
> +  objfile = objfile::make (nullptr, objfile_name, OBJF_NOT_FILENAME);

I think this is printing a random stack address in GDB itself, doesn't it?
priv_data is initialized to point to a stack variable in
jit_reader_try_read_symtab.

I think what would speak more to the user would be the address in the
inferior where the JIT-ed code is.  The JIT engine is likely to have some
logging that says "I am JIT-ing some code and placing it at address 0x1234".
So having the objfile name say 0x1234 allows them to correlate the code they
generated with GDB's objfiles.  Maybe that was your intention, not sure.

Also, let's use string_printf instead of playing with char buffers.

What about the patch below?


From cb6a65b978424247e659e8f2d262fa18f5a7e77e Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 2 Feb 2022 10:54:03 -0500
Subject: [PATCH v2] gdb: include jit_code_entry::symfile_addr value in names
 of objfiles created by jit reader API

This commit includes jited object address in the names of objfiles
created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
This allows one to at least differentiate one from another.

The address is the one that the debugged program has put in
jit_code_entry::symfile_addr, and that the JIT reader's read function
receives.  As we can see in gdb.base/jit-reader-host.c and
gdb.base/jit-reader.c, that may not be the actual value of where the
JIT-ed code is.  But it is a value chosen by the author of the JIT
engine and the JIT reader, so including this value in the objfile name
may help them correlate the JIT objfiles created by with their logs /
data structures.

To access this field, we need to pass down a reference to the
jit_code_entry.  So make jit_dbg_reader_data a structure (instead of an
alias for a CORE_ADDR) that includes the address of the code entry in
the inferior's address space (the previous meaning of
jit_dbg_reader_data) plus a reference to the jit_code_entry as read into
GDB's address space.  And while at it, pass down the gdbarch, so that we
don't have to call target_gdbarch.

Co-Authored-By: Jan Vrany <jan.vrany@labware.com>
Change-Id: Ib26c4d1bd8de503d651aff89ad2e500cb312afa5
---
 gdb/jit.c                             | 41 ++++++++++++++++++---------
 gdb/testsuite/gdb.base/jit-reader.exp |  4 +--
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 42776b956832..356525443cb2 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -380,7 +380,16 @@ struct gdb_object
 /* The type of the `private' data passed around by the callback
    functions.  */

-typedef CORE_ADDR jit_dbg_reader_data;
+struct jit_dbg_reader_data
+{
+  /* Address of the jit_code_entry in the inferior's address space.  */
+  CORE_ADDR entry_addr;
+
+  /* The code entry, copied in our address space.  */
+  const jit_code_entry &entry;
+
+  struct gdbarch *gdbarch;
+};

 /* The reader calls into this function to read data off the targets
    address space.  */
@@ -623,19 +632,20 @@ static void
 jit_object_close_impl (struct gdb_symbol_callbacks *cb,
 		       struct gdb_object *obj)
 {
-  struct objfile *objfile;
-  jit_dbg_reader_data *priv_data;
-
-  priv_data = (jit_dbg_reader_data *) cb->priv_data;
+  jit_dbg_reader_data *priv_data = (jit_dbg_reader_data *) cb->priv_data;
+  std::string objfile_name
+    = string_printf ("<< JIT compiled code at %s >>",
+		     paddress (priv_data->gdbarch,
+			       priv_data->entry.symfile_addr));

-  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
-			   OBJF_NOT_FILENAME);
-  objfile->per_bfd->gdbarch = target_gdbarch ();
+  objfile *objfile = objfile::make (nullptr, objfile_name.c_str (),
+				    OBJF_NOT_FILENAME);
+  objfile->per_bfd->gdbarch = priv_data->gdbarch;

   for (gdb_symtab &symtab : obj->symtabs)
     finalize_symtab (&symtab, objfile);

-  add_objfile_entry (objfile, *priv_data);
+  add_objfile_entry (objfile, priv_data->entry_addr);

   delete obj;
 }
@@ -645,11 +655,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
    inferior address space.  */

 static int
-jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
+jit_reader_try_read_symtab (gdbarch *gdbarch, jit_code_entry *code_entry,
 			    CORE_ADDR entry_addr)
 {
   int status;
-  jit_dbg_reader_data priv_data;
+  jit_dbg_reader_data priv_data
+    {
+      entry_addr,
+      *code_entry,
+      gdbarch
+    };
   struct gdb_reader_funcs *funcs;
   struct gdb_symbol_callbacks callbacks =
     {
@@ -665,8 +680,6 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
       &priv_data
     };

-  priv_data = entry_addr;
-
   if (!loaded_jit_reader)
     return 0;

@@ -779,7 +792,7 @@ jit_register_code (struct gdbarch *gdbarch,
 		    paddress (gdbarch, code_entry->symfile_addr),
 		    pulongest (code_entry->symfile_size));

-  success = jit_reader_try_read_symtab (code_entry, entry_addr);
+  success = jit_reader_try_read_symtab (gdbarch, code_entry, entry_addr);

   if (!success)
     jit_bfd_try_read_symtab (code_entry, entry_addr, gdbarch);
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index bcc85640ec09..d94360cd7d9e 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -230,11 +230,11 @@ proc jit_reader_test {} {

 	    if { ![skip_python_tests] } {
 		gdb_test "python print(gdb.objfiles())" \
-		    "$any<gdb.Objfile filename=<< JIT compiled code >>>$any" \
+		    "$any<gdb.Objfile filename=<< JIT compiled code at $hex >>>$any" \
 		    "python gdb.Objfile.__repr__ ()"

 		gdb_test "python print(list(map(lambda objf : objf.filename, gdb.objfiles())))" \
-		    "$any'<< JIT compiled code >>'$any" \
+		    "$any'<< JIT compiled code at $hex >>'$any" \
 		    "python gdb.Objfile.filename"
 	    }
 	}
-- 
2.26.2

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Jan Vrany]

On Wed, 2022-02-02 at 11:17 -0500, Simon Marchi wrote:
> On 2022-02-02 7:03 a.m., Jan Vrany via Gdb-patches wrote:
> > This commit includes jited object address in the names of objfiles
> > created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
> > This allows one to at least differentiate one from another.
> > ---
> >  gdb/jit.c                             | 8 ++++++--
> >  gdb/testsuite/gdb.base/jit-reader.exp | 4 ++--
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/gdb/jit.c b/gdb/jit.c
> > index 42776b95683..371cb8b1d48 100644
> > --- a/gdb/jit.c
> > +++ b/gdb/jit.c
> > @@ -624,12 +624,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
> >  		       struct gdb_object *obj)
> >  {
> >    struct objfile *objfile;
> > +  char objfile_name[64];
> >    jit_dbg_reader_data *priv_data;
> >  
> > 
> > 
> > 
> >    priv_data = (jit_dbg_reader_data *) cb->priv_data;
> >  
> > 
> > 
> > 
> > -  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
> > -			   OBJF_NOT_FILENAME);
> > +  snprintf (objfile_name, sizeof (objfile_name) - 1,
> > +            "<< JIT compiled code at 0x%" PRIxPTR " >>",
> > +            reinterpret_cast<uintptr_t> (priv_data));
> > +  objfile_name[sizeof (objfile_name) - 1] = '\0';
> > +  objfile = objfile::make (nullptr, objfile_name, OBJF_NOT_FILENAME);
> 
> I think this is printing a random stack address in GDB itself, doesn't it?
> priv_data is initialized to point to a stack variable in
> jit_reader_try_read_symtab.

Yes, I misread the code, sorry. 

> 
> I think what would speak more to the user would be the address in the
> inferior where the JIT-ed code is.  The JIT engine is likely to have some
> logging that says "I am JIT-ing some code and placing it at address 0x1234".
> So having the objfile name say 0x1234 allows them to correlate the code they
> generated with GDB's objfiles.  Maybe that was your intention, not sure.

This seems tricky to me. IIUC, using JIT reader API, JIT (inferior) creates some 
debug info somewhere in its address space and then tell GDB to read it from there,
right? This address is the symfile_addr your patch below is putting into objfile's name.

But this address may differ from the location where the actual executable code is. I'd even
think in most cases it will be different.
Also, the jitted language may support nested functions / lambdas so it may produce multiple 
"machine" functions for a single "language" function. 

Even the jit-reader.exp does "JIT" two functions and register them at once.

> 
> Also, let's use string_printf instead of playing with char buffers.
> 
> What about the patch below?

Yes, this is better than my original attempt. But still, the value it
prints can be bit confusing in the context of "maintenance info jit"
command. When I applied your patch and run jit-reader.exp, then:

(gdb) maintenance info jit
Base address of known JIT-ed objfiles:
  0x5555555580a0
(gdb) print(gdb.objfiles()[-1])
No symbol "gdb" in current context.
(gdb) python print(gdb.objfiles()[-1])
<gdb.Objfile filename=<< JIT compiled code at 0x5555555592a0 >>>

As you can see, the addresses differ. IIUC that's because "maint info jit" shows
address of struct jit_code_entry (in inferior address space) while the name 
of the objfile contains value of jit_code_entry->symfile_addr. 

What about changing "maintenance info jit" to print the symfile_addr instead? 
Or the other way round. See the patch below (to be applied after yours):


commit 2a233c590fa9dcccf5da8173dad037b9cb1137c0
Author: Jan Vrany <jan.vrany@labware.com>
Date:   Fri Feb 4 12:13:12 2022 +0000

    [WIP]: change "maint info jit" to print base address of jit symfile
    
    To be applied after https://sourceware.org/pipermail/gdb-patches/2022-February/185659.html

diff --git a/gdb/jit.c b/gdb/jit.c
index 356525443cb..832db11ac4a 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -94,7 +94,7 @@ maint_info_jit_cmd (const char *args, int from_tty)
 	  printed_header = true;
 	}
 
-      printf_filtered ("  %s\n", paddress (obj->arch (), obj->jited_data->addr));
+      printf_filtered ("  %s\n", paddress (obj->arch (), obj->jited_data->symfile_addr));
     }
 }
 
@@ -211,11 +211,11 @@ get_jiter_objfile_data (objfile *objf)
    at inferior address ENTRY.  */
 
 static void
-add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
+add_objfile_entry (struct objfile *objfile, CORE_ADDR entry, CORE_ADDR symfile_addr)
 {
   gdb_assert (objfile->jited_data == nullptr);
 
-  objfile->jited_data.reset (new jited_objfile_data (entry));
+  objfile->jited_data.reset (new jited_objfile_data (entry, symfile_addr));
 }
 
 /* Helper function for reading the global JIT descriptor from remote
@@ -645,7 +645,7 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
   for (gdb_symtab &symtab : obj->symtabs)
     finalize_symtab (&symtab, objfile);
 
-  add_objfile_entry (objfile, priv_data->entry_addr);
+  add_objfile_entry (objfile, priv_data->entry_addr, priv_data->entry.symfile_addr);
 
   delete obj;
 }
@@ -774,7 +774,7 @@ JITed symbol file is not an object file, ignoring it.\n"));
 				      &sai,
 				      OBJF_SHARED | OBJF_NOT_FILENAME, NULL);
 
-  add_objfile_entry (objfile, entry_addr);
+  add_objfile_entry (objfile, entry_addr, code_entry->symfile_addr);
 }
 
 /* This function registers code associated with a JIT code entry.  It uses the
diff --git a/gdb/jit.h b/gdb/jit.h
index 09dbce21f5c..a451c2b0e99 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -95,12 +95,18 @@ struct jiter_objfile_data
 
 struct jited_objfile_data
 {
-  jited_objfile_data (CORE_ADDR addr)
-    : addr (addr)
+  jited_objfile_data (CORE_ADDR addr, CORE_ADDR symfile_addr)
+    : addr (addr),
+      symfile_addr (symfile_addr)
   {}
 
   /* Address of struct jit_code_entry for this objfile.  */
   CORE_ADDR addr;
+
+  /* Value of jit_code_entry->symfile_addr for this objfile.  */
+  CORE_ADDR symfile_addr;
+
+
 };
 
 /* Re-establish the jit breakpoint(s).  */

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Simon Marchi]

On 2022-02-04 07:39, Jan Vrany wrote:
> On Wed, 2022-02-02 at 11:17 -0500, Simon Marchi wrote:
>> On 2022-02-02 7:03 a.m., Jan Vrany via Gdb-patches wrote:
>>> This commit includes jited object address in the names of objfiles
>>> created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
>>> This allows one to at least differentiate one from another.
>>> ---
>>>  gdb/jit.c                             | 8 ++++++--
>>>  gdb/testsuite/gdb.base/jit-reader.exp | 4 ++--
>>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/gdb/jit.c b/gdb/jit.c
>>> index 42776b95683..371cb8b1d48 100644
>>> --- a/gdb/jit.c
>>> +++ b/gdb/jit.c
>>> @@ -624,12 +624,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
>>>  		       struct gdb_object *obj)
>>>  {
>>>    struct objfile *objfile;
>>> +  char objfile_name[64];
>>>    jit_dbg_reader_data *priv_data;
>>>  
>>>
>>>
>>>
>>>    priv_data = (jit_dbg_reader_data *) cb->priv_data;
>>>  
>>>
>>>
>>>
>>> -  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
>>> -			   OBJF_NOT_FILENAME);
>>> +  snprintf (objfile_name, sizeof (objfile_name) - 1,
>>> +            "<< JIT compiled code at 0x%" PRIxPTR " >>",
>>> +            reinterpret_cast<uintptr_t> (priv_data));
>>> +  objfile_name[sizeof (objfile_name) - 1] = '\0';
>>> +  objfile = objfile::make (nullptr, objfile_name, OBJF_NOT_FILENAME);
>>
>> I think this is printing a random stack address in GDB itself, doesn't it?
>> priv_data is initialized to point to a stack variable in
>> jit_reader_try_read_symtab.
> 
> Yes, I misread the code, sorry. 
> 
>>
>> I think what would speak more to the user would be the address in the
>> inferior where the JIT-ed code is.  The JIT engine is likely to have some
>> logging that says "I am JIT-ing some code and placing it at address 0x1234".
>> So having the objfile name say 0x1234 allows them to correlate the code they
>> generated with GDB's objfiles.  Maybe that was your intention, not sure.
> 
> This seems tricky to me. IIUC, using JIT reader API, JIT (inferior) creates some 
> debug info somewhere in its address space and then tell GDB to read it from there,
> right? This address is the symfile_addr your patch below is putting into objfile's name.
> 
> But this address may differ from the location where the actual executable code is. I'd even
> think in most cases it will be different.

Yes, that's my understanding.  If the program was not using a custom JIT
reader, it would put something readable by BFD (e.g. an ELF) at that
address.  But if using a custom JIT reader, it puts a data structure of
its choice, as long as the JIT (the inferior) and the custom JIT reader
agree on the format.

So you are right, we can't easily put the address of the actual code.  I
probably confused the two.

My only little concern is whether that symfile_addr is always going to
be unique.  Could you have a JIT and custom JIT reader where the JIT
always re-uses the same buffer to hold the symfile?  After you
registered one JIT object and the custom JIT reader has created the
objfile, I don't really see a requirement for keeping the symfile data
around.  I could imagine that the JIT could fill the buffer with data
for a second object an register ir.  So if we use the symfile_addr in
the objfile's name, we would have two objfiles with the same name.

Code entries are more likely to be unique, as the inferior is supposed
to keep code entries for existing JIT objects in a linked list, it can't
get rid of them after registering them.

> Also, the jitted language may support nested functions / lambdas so it may produce multiple 
> "machine" functions for a single "language" function. 
> 
> Even the jit-reader.exp does "JIT" two functions and register them at once.

Yes, it's two function in a single object file, as if you had two
functions in one ELF.

> 
>>
>> Also, let's use string_printf instead of playing with char buffers.
>>
>> What about the patch below?
> 
> Yes, this is better than my original attempt. But still, the value it
> prints can be bit confusing in the context of "maintenance info jit"
> command. When I applied your patch and run jit-reader.exp, then:
> 
> (gdb) maintenance info jit
> Base address of known JIT-ed objfiles:
>   0x5555555580a0
> (gdb) print(gdb.objfiles()[-1])
> No symbol "gdb" in current context.
> (gdb) python print(gdb.objfiles()[-1])
> <gdb.Objfile filename=<< JIT compiled code at 0x5555555592a0 >>>
> 
> As you can see, the addresses differ. IIUC that's because "maint info jit" shows
> address of struct jit_code_entry (in inferior address space) while the name 
> of the objfile contains value of jit_code_entry->symfile_addr. 
> 
> What about changing "maintenance info jit" to print the symfile_addr instead? 
> Or the other way round. See the patch below (to be applied after yours):

I agree, it would make sense for "maintenance info jit" and the objfile
name to display the same addresses.  Given the concern I shared above,
maybe the code entry address is better in the end?

In any case, maybe can change "maint info jit" to display both values in
a way that makes it clear which is which.  If I was debugging my JIT
stuff and was confused with all those addresses, I think that would help
me if it said, for each jit object:

  - Code entry address: 0x1234
  - Symfile address: 0x2345

That's with the understanding that the symfile address could be re-used
for multiple objects, but at least it shows what was the address given
at the time of registration, which can still be helpful to debug
problems.

Simon

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Jan Vrany]

On Fri, 2022-02-04 at 08:15 -0500, Simon Marchi wrote:
> 
> On 2022-02-04 07:39, Jan Vrany wrote:
> > On Wed, 2022-02-02 at 11:17 -0500, Simon Marchi wrote:
> > > On 2022-02-02 7:03 a.m., Jan Vrany via Gdb-patches wrote:
> > > > This commit includes jited object address in the names of objfiles
> > > > created by jit reader API (e.g., << JIT compiled code at 0x7ffd8a0c77a0 >>).
> > > > This allows one to at least differentiate one from another.
> > > > ---
> > > >  gdb/jit.c                             | 8 ++++++--
> > > >  gdb/testsuite/gdb.base/jit-reader.exp | 4 ++--
> > > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/gdb/jit.c b/gdb/jit.c
> > > > index 42776b95683..371cb8b1d48 100644
> > > > --- a/gdb/jit.c
> > > > +++ b/gdb/jit.c
> > > > @@ -624,12 +624,16 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
> > > >  		       struct gdb_object *obj)
> > > >  {
> > > >    struct objfile *objfile;
> > > > +  char objfile_name[64];
> > > >    jit_dbg_reader_data *priv_data;
> > > >  
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > >    priv_data = (jit_dbg_reader_data *) cb->priv_data;
> > > >  
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > -  objfile = objfile::make (nullptr, "<< JIT compiled code >>",
> > > > -			   OBJF_NOT_FILENAME);
> > > > +  snprintf (objfile_name, sizeof (objfile_name) - 1,
> > > > +            "<< JIT compiled code at 0x%" PRIxPTR " >>",
> > > > +            reinterpret_cast<uintptr_t> (priv_data));
> > > > +  objfile_name[sizeof (objfile_name) - 1] = '\0';
> > > > +  objfile = objfile::make (nullptr, objfile_name, OBJF_NOT_FILENAME);
> > > 
> > > I think this is printing a random stack address in GDB itself, doesn't it?
> > > priv_data is initialized to point to a stack variable in
> > > jit_reader_try_read_symtab.
> > 
> > Yes, I misread the code, sorry. 
> > 
> > > 
> > > I think what would speak more to the user would be the address in the
> > > inferior where the JIT-ed code is.  The JIT engine is likely to have some
> > > logging that says "I am JIT-ing some code and placing it at address 0x1234".
> > > So having the objfile name say 0x1234 allows them to correlate the code they
> > > generated with GDB's objfiles.  Maybe that was your intention, not sure.
> > 
> > This seems tricky to me. IIUC, using JIT reader API, JIT (inferior) creates some 
> > debug info somewhere in its address space and then tell GDB to read it from there,
> > right? This address is the symfile_addr your patch below is putting into objfile's name.
> > 
> > But this address may differ from the location where the actual executable code is. I'd even
> > think in most cases it will be different.
> 
> Yes, that's my understanding.  If the program was not using a custom JIT
> reader, it would put something readable by BFD (e.g. an ELF) at that
> address.  But if using a custom JIT reader, it puts a data structure of
> its choice, as long as the JIT (the inferior) and the custom JIT reader
> agree on the format.
> 
> So you are right, we can't easily put the address of the actual code.  I
> probably confused the two.
> 
> My only little concern is whether that symfile_addr is always going to
> be unique.  Could you have a JIT and custom JIT reader where the JIT
> always re-uses the same buffer to hold the symfile?  After you
> registered one JIT object and the custom JIT reader has created the
> objfile, I don't really see a requirement for keeping the symfile data
> around.  I could imagine that the JIT could fill the buffer with data
> for a second object an register ir.  So if we use the symfile_addr in
> the objfile's name, we would have two objfiles with the same name.
> 
> Code entries are more likely to be unique, as the inferior is supposed
> to keep code entries for existing JIT objects in a linked list, it can't
> get rid of them after registering them.

Hmm, I actually think it has to be unique in order to make things work. 
The JIT (inferior) is - generally - oblivious of whether or not it's being 
debugged. It does not know whether or not the "symfile" put into linked list 
of jit_code_entry has been read by GDB or not. Or it may even be read multiple 
times, such as when you jit-reader-load, jit-reader-unload and jit-reader-load 
again during debugging. So one cannot reuse same memory for multiple entries.

> 
> > Also, the jitted language may support nested functions / lambdas so it may produce multiple 
> > "machine" functions for a single "language" function. 
> > 
> > Even the jit-reader.exp does "JIT" two functions and register them at once.
> 
> Yes, it's two function in a single object file, as if you had two
> functions in one ELF.
> 
> > 
> > > 
> > > Also, let's use string_printf instead of playing with char buffers.
> > > 
> > > What about the patch below?
> > 
> > Yes, this is better than my original attempt. But still, the value it
> > prints can be bit confusing in the context of "maintenance info jit"
> > command. When I applied your patch and run jit-reader.exp, then:
> > 
> > (gdb) maintenance info jit
> > Base address of known JIT-ed objfiles:
> >   0x5555555580a0
> > (gdb) print(gdb.objfiles()[-1])
> > No symbol "gdb" in current context.
> > (gdb) python print(gdb.objfiles()[-1])
> > <gdb.Objfile filename=<< JIT compiled code at 0x5555555592a0 >>>
> > 
> > As you can see, the addresses differ. IIUC that's because "maint info jit" shows
> > address of struct jit_code_entry (in inferior address space) while the name 
> > of the objfile contains value of jit_code_entry->symfile_addr. 
> > 
> > What about changing "maintenance info jit" to print the symfile_addr instead? 
> > Or the other way round. See the patch below (to be applied after yours):
> 
> I agree, it would make sense for "maintenance info jit" and the objfile
> name to display the same addresses.  Given the concern I shared above,
> maybe the code entry address is better in the end?

I think it's matter of choice, I fine with either. 

> 
> In any case, maybe can change "maint info jit" to display both values in
> a way that makes it clear which is which.  If I was debugging my JIT
> stuff and was confused with all those addresses, I think that would help
> me if it said, for each jit object:
> 
>   - Code entry address: 0x1234
>   - Symfile address: 0x2345

Yes, we can print both as both can be useful when debugging JIT reader stuff.

Jan

> 
> That's with the understanding that the symfile address could be re-used
> for multiple objects, but at least it shows what was the address given
> at the time of registration, which can still be helpful to debug
> problems.
> 
> Simon

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Simon Marchi]

> Hmm, I actually think it has to be unique in order to make things work. 
> The JIT (inferior) is - generally - oblivious of whether or not it's being 
> debugged. It does not know whether or not the "symfile" put into linked list 
> of jit_code_entry has been read by GDB or not. Or it may even be read multiple 
> times, such as when you jit-reader-load, jit-reader-unload and jit-reader-load 
> again during debugging. So one cannot reuse same memory for multiple entries.

Ah you're right.  And in case GDB attaches to a process that already has
some JIT objects, it will read them all.

>> I agree, it would make sense for "maintenance info jit" and the objfile
>> name to display the same addresses.  Given the concern I shared above,
>> maybe the code entry address is better in the end?
> 
> I think it's matter of choice, I fine with either. 

Symfile address is fine, given what you said above.

Thanks!

Simon

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Jan Vrany]

On Fri, 2022-02-04 at 09:52 -0500, Simon Marchi wrote:
> > Hmm, I actually think it has to be unique in order to make things work. 
> > The JIT (inferior) is - generally - oblivious of whether or not it's being 
> > debugged. It does not know whether or not the "symfile" put into linked list 
> > of jit_code_entry has been read by GDB or not. Or it may even be read multiple 
> > times, such as when you jit-reader-load, jit-reader-unload and jit-reader-load 
> > again during debugging. So one cannot reuse same memory for multiple entries.
> 
> Ah you're right.  And in case GDB attaches to a process that already has
> some JIT objects, it will read them all.
> 
> > > I agree, it would make sense for "maintenance info jit" and the objfile
> > > name to display the same addresses.  Given the concern I shared above,
> > > maybe the code entry address is better in the end?
> > 
> > I think it's matter of choice, I fine with either. 
> 
> Symfile address is fine, given what you said above.

Perfect! Will you push the patch or shall I submit a new version
based on yours? 

Thanks!

Jan

Re: [PATCH] gdb: include address in names of objfiles created by jit reader API

[Simon Marchi]

On 2022-02-04 10:32, Jan Vrany wrote:
> Perfect! Will you push the patch or shall I submit a new version
> based on yours? 

I pushed my version of the patch.  Could you provide an updated version
of the patch that modifies "maintenance info jit"?

Simon