[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).  */