* [PATCH] gdb: include address in names of objfiles created by jit reader API @ 2022-02-02 12:03 Jan Vrany 2022-02-02 16:09 ` Tom Tromey 2022-02-02 16:17 ` Simon Marchi 0 siblings, 2 replies; 9+ messages in thread From: Jan Vrany @ 2022-02-02 12:03 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-02 12:03 [PATCH] gdb: include address in names of objfiles created by jit reader API Jan Vrany @ 2022-02-02 16:09 ` Tom Tromey 2022-02-02 16:17 ` Simon Marchi 1 sibling, 0 replies; 9+ messages in thread From: Tom Tromey @ 2022-02-02 16:09 UTC (permalink / raw) To: Jan Vrany via Gdb-patches; +Cc: Jan Vrany >>>>> "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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-02 12:03 [PATCH] gdb: include address in names of objfiles created by jit reader API Jan Vrany 2022-02-02 16:09 ` Tom Tromey @ 2022-02-02 16:17 ` Simon Marchi 2022-02-04 12:39 ` Jan Vrany 1 sibling, 1 reply; 9+ messages in thread From: Simon Marchi @ 2022-02-02 16:17 UTC (permalink / raw) To: Jan Vrany, gdb-patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-02 16:17 ` Simon Marchi @ 2022-02-04 12:39 ` Jan Vrany 2022-02-04 13:15 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Jan Vrany @ 2022-02-04 12:39 UTC (permalink / raw) To: Simon Marchi, gdb-patches 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). */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-04 12:39 ` Jan Vrany @ 2022-02-04 13:15 ` Simon Marchi 2022-02-04 13:42 ` Jan Vrany 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2022-02-04 13:15 UTC (permalink / raw) To: Jan Vrany, Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-04 13:15 ` Simon Marchi @ 2022-02-04 13:42 ` Jan Vrany 2022-02-04 14:52 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Jan Vrany @ 2022-02-04 13:42 UTC (permalink / raw) To: Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-04 13:42 ` Jan Vrany @ 2022-02-04 14:52 ` Simon Marchi 2022-02-04 15:32 ` Jan Vrany 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2022-02-04 14:52 UTC (permalink / raw) To: Jan Vrany, gdb-patches > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-04 14:52 ` Simon Marchi @ 2022-02-04 15:32 ` Jan Vrany 2022-02-04 16:09 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Jan Vrany @ 2022-02-04 15:32 UTC (permalink / raw) To: Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gdb: include address in names of objfiles created by jit reader API 2022-02-04 15:32 ` Jan Vrany @ 2022-02-04 16:09 ` Simon Marchi 0 siblings, 0 replies; 9+ messages in thread From: Simon Marchi @ 2022-02-04 16:09 UTC (permalink / raw) To: Jan Vrany, gdb-patches 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-04 16:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-02 12:03 [PATCH] gdb: include address in names of objfiles created by jit reader API Jan Vrany 2022-02-02 16:09 ` Tom Tromey 2022-02-02 16:17 ` Simon Marchi 2022-02-04 12:39 ` Jan Vrany 2022-02-04 13:15 ` Simon Marchi 2022-02-04 13:42 ` Jan Vrany 2022-02-04 14:52 ` Simon Marchi 2022-02-04 15:32 ` Jan Vrany 2022-02-04 16:09 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).