Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
@ 2022-02-07 11:39 Jan Vrany via Gdb-patches
  2022-02-08 15:49 ` Andrew Burgess via Gdb-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Vrany via Gdb-patches @ 2022-02-07 11:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jan Vrany

This commit changes the output of 'maint info jit' command to print
jit_code_entry::symfile_addr value rather than address of jit_code_entry
itself. This makes it consistent with address included in objfile names
(commit 4a620b7e).

To access this address, we keep this value in jiter_objfile_data attached
to each objfile created by JIT reader API.
---
 gdb/jit.c | 10 +++++-----
 gdb/jit.h |  9 +++++++--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 7819d763ab3..420cc80b86a 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
@@ -644,7 +644,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;
 }
@@ -773,7 +773,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..10a10dade7f 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -95,12 +95,17 @@ 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).  */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
  2022-02-07 11:39 [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value Jan Vrany via Gdb-patches
@ 2022-02-08 15:49 ` Andrew Burgess via Gdb-patches
  2022-02-08 19:16   ` Simon Marchi via Gdb-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Burgess via Gdb-patches @ 2022-02-08 15:49 UTC (permalink / raw)
  To: Jan Vrany; +Cc: gdb-patches

Jan,

Thanks for this patch.  While reviewing this, I wondered about using
ui_out's table mechanism to format the output, rather than just
printing the addresses.

Additionally, while reminding myself about how the jit stuff works, I
thought, maybe we should keep the jit_code_entry address in the 'maint
info jit' output - after all, given a jit_code_entry, we can, in
theory, figure out the symfile address, but we can't (easily) go from
a symfile address back to a jit_code_entry, right?

Anyway, while I ended up with the patch below.  What do you think
about this instead?

Thanks,
Andrew

---

commit 3982fbc710447f9323c75f297c464c102f921e1e
Author: Jan Vrany via Gdb-patches <gdb-patches@sourceware.org>
Date:   Mon Feb 7 11:39:22 2022 +0000

    gdb: extend the information printed by 'maint info jit'
    
    This commit updates the output of 'maint info jit' to print not just
    the jit_code_entry address, but also the symfile address, and the
    symfile size.
    
    The new information could be obtained by looking into target memory at
    the contents of the jit_code_entry, but, by storing this information
    within gdb at the time the jit object is loaded, it is now possible to
    check if the jit_code_entry has been modified in target memory behind
    gdb's back.
    
    Additionally, the symfile address is the same address that is now used
    in the objfile names after commit 4a620b7e.
    
    One test that relies on the output of 'maint info jit' was updated to
    allow for the new output format.

diff --git a/gdb/jit.c b/gdb/jit.c
index 7819d763ab3..9f57d521a8b 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -82,6 +82,8 @@ maint_info_jit_cmd (const char *args, int from_tty)
   inferior *inf = current_inferior ();
   bool printed_header = false;
 
+  gdb::optional<ui_out_emit_table> table_emitter;
+
   /* Print a line for each JIT-ed objfile.  */
   for (objfile *obj : inf->pspace->objfiles ())
     {
@@ -90,11 +92,35 @@ maint_info_jit_cmd (const char *args, int from_tty)
 
       if (!printed_header)
 	{
-	  printf_filtered ("Base address of known JIT-ed objfiles:\n");
+	  table_emitter.emplace (current_uiout, 3, -1, "jit-created-objfiles");
+
+	  /* The +2 allows for the leading '0x', then one character for
+	     every 4-bits.  */
+	  int addr_width = 2 + (gdbarch_ptr_bit (obj->arch ()) / 4);
+
+	  /* The std::max here selects between the width of an address (as
+	     a string) and the width of the column header string.  */
+	  current_uiout->table_header (std::max (addr_width, 22), ui_left,
+				       "jit_code_entry-address",
+				       "jit_code_entry address");
+	  current_uiout->table_header (std::max (addr_width, 15), ui_left,
+				       "symfile-address", "symfile address");
+	  current_uiout->table_header (20, ui_left,
+				       "symfile-size", "symfile size");
+	  current_uiout->table_body ();
+
 	  printed_header = true;
 	}
 
-      printf_filtered ("  %s\n", paddress (obj->arch (), obj->jited_data->addr));
+      ui_out_emit_tuple tuple_emitter (current_uiout, "jit-objfile");
+
+      current_uiout->field_core_addr ("jit_code_entry-address", obj->arch (),
+				      obj->jited_data->addr);
+      current_uiout->field_core_addr ("symfile-address", obj->arch (),
+				      obj->jited_data->symfile_addr);
+      current_uiout->field_unsigned ("symfile-size",
+				      obj->jited_data->symfile_size);
+      current_uiout->text ("\n");
     }
 }
 
@@ -211,11 +237,13 @@ 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, ULONGEST symfile_size)
 {
   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,
+						     symfile_size));
 }
 
 /* Helper function for reading the global JIT descriptor from remote
@@ -644,7 +672,9 @@ 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,
+		     priv_data->entry.symfile_size);
 
   delete obj;
 }
@@ -773,7 +803,8 @@ 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,
+		     code_entry->symfile_size);
 }
 
 /* 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..274ce456f47 100644
--- a/gdb/jit.h
+++ b/gdb/jit.h
@@ -95,12 +95,21 @@ 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,
+		      ULONGEST symfile_size)
+    : addr (addr),
+      symfile_addr (symfile_addr),
+      symfile_size (symfile_size)
   {}
 
   /* 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;
+
+  /* Value of jit_code_entry->symfile_size for this objfile.  */
+  ULONGEST symfile_size;
 };
 
 /* Re-establish the jit breakpoint(s).  */
diff --git a/gdb/testsuite/gdb.base/jit-elf-fork.exp b/gdb/testsuite/gdb.base/jit-elf-fork.exp
index e5b9a18d00c..635248cd8e4 100644
--- a/gdb/testsuite/gdb.base/jit-elf-fork.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-fork.exp
@@ -81,7 +81,11 @@ proc do_setup { detach-on-fork follow-fork-mode } {
 	gdb_continue_to_breakpoint "continue to before fork" ".*break before fork.*"
 
 	# We should have one JIT object loaded.
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfiles before fork"
+	gdb_test "maint info jit" \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfiles before fork"
 
 	# Put a breakpoint just after the fork, continue there.
 	gdb_breakpoint [gdb_get_line_number "break after fork" $::main_srcfile]
@@ -89,7 +93,11 @@ proc do_setup { detach-on-fork follow-fork-mode } {
 
 	# We should still have one JIT object loaded in whatever inferior we are
 	# currently stopped in, regardless of the mode.
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfiles after fork"
+	gdb_test "maint info jit" \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfiles after fork"
 
 	# Delete our breakpoints.
 	delete_breakpoints
@@ -108,7 +116,11 @@ proc_with_prefix test_detach_on_fork_off_follow_fork_mode_parent { } {
 
 	# Switch to the child, verify there is a JIT-ed objfile.
 	gdb_test "inferior 2" "Switching to inferior 2.*"
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfile in child"
+	gdb_test "maint info jit" \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfile in child"
 
 	# Continue child past JIT unload, verify there are no more JIT-ed objfiles.
 	gdb_continue_to_breakpoint "continue to before return - child" ".*break before return.*"
@@ -116,7 +128,11 @@ proc_with_prefix test_detach_on_fork_off_follow_fork_mode_parent { } {
 
 	# Go back to parent, the JIT-ed objfile should still be there.
 	gdb_test "inferior 1" "Switching to inferior 1.*"
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfile in parent"
+	gdb_test "maint info jit"  \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfile in parent"
 
 	# Continue parent past JIT unload, verify there are no more JIT-ed objfiles.
 	gdb_continue_to_breakpoint "continue to before return - parent" ".*break before return.*"
@@ -135,7 +151,11 @@ proc_with_prefix test_detach_on_fork_off_follow_fork_mode_child { } {
 
 	# Switch to the parent, verify there is a JIT-ed objfile.
 	gdb_test "inferior 1" "Switching to inferior 1.*"
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfile in parent"
+	gdb_test "maint info jit"  \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfile in parent"
 
 	# Continue parent past JIT unload, verify there are no more JIT-ed objfiles.
 	gdb_continue_to_breakpoint "continue to before return - parent" ".*break before return.*"
@@ -143,7 +163,11 @@ proc_with_prefix test_detach_on_fork_off_follow_fork_mode_child { } {
 
 	# Go back to child, the JIT-ed objfile should still be there.
 	gdb_test "inferior 2" "Switching to inferior 2.*"
-	gdb_test "maint info jit" "  ${::hex}" "jit-ed objfile in child"
+	gdb_test "maint info jit"  \
+	    [multi_line \
+		 "jit_code_entry address\\s+symfile address\\s+symfile size\\s*" \
+		 "${::hex}\\s+${::hex}\\s+${::decimal}\\s*"] \
+	    "jit-ed objfile in child"
 
 	# Continue child past JIT unload, verify there are no more JIT-ed objfiles.
 	gdb_continue_to_breakpoint "continue to before return - child" ".*break before return.*"


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
  2022-02-08 15:49 ` Andrew Burgess via Gdb-patches
@ 2022-02-08 19:16   ` Simon Marchi via Gdb-patches
  2022-02-08 19:32     ` Jan Vrany via Gdb-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi via Gdb-patches @ 2022-02-08 19:16 UTC (permalink / raw)
  To: Andrew Burgess, Jan Vrany; +Cc: gdb-patches



On 2022-02-08 10:49, Andrew Burgess via Gdb-patches wrote:
> Jan,
> 
> Thanks for this patch.  While reviewing this, I wondered about using
> ui_out's table mechanism to format the output, rather than just
> printing the addresses.
> 
> Additionally, while reminding myself about how the jit stuff works, I
> thought, maybe we should keep the jit_code_entry address in the 'maint
> info jit' output - after all, given a jit_code_entry, we can, in
> theory, figure out the symfile address, but we can't (easily) go from
> a symfile address back to a jit_code_entry, right?

Why not print both?

Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
  2022-02-08 19:16   ` Simon Marchi via Gdb-patches
@ 2022-02-08 19:32     ` Jan Vrany via Gdb-patches
  2022-02-08 20:15       ` Simon Marchi via Gdb-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Vrany via Gdb-patches @ 2022-02-08 19:32 UTC (permalink / raw)
  To: Simon Marchi, Andrew Burgess; +Cc: gdb-patches

On Tue, 2022-02-08 at 14:16 -0500, Simon Marchi wrote:
> 
> On 2022-02-08 10:49, Andrew Burgess via Gdb-patches wrote:
> > Jan,
> > 
> > Thanks for this patch.  While reviewing this, I wondered about using
> > ui_out's table mechanism to format the output, rather than just
> > printing the addresses.
> > 
> > Additionally, while reminding myself about how the jit stuff works, I
> > thought, maybe we should keep the jit_code_entry address in the 'maint
> > info jit' output - after all, given a jit_code_entry, we can, in
> > theory, figure out the symfile address, but we can't (easily) go from
> > a symfile address back to a jit_code_entry, right?
> 
> Why not print both?

I believe this is what Andrew's patch does. 

> > Anyway, while I ended up with the patch below.  What do you think
> > about this instead?

Andrew, I'm happy with it, of course. 

Jan

> 
> Simon



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
  2022-02-08 19:32     ` Jan Vrany via Gdb-patches
@ 2022-02-08 20:15       ` Simon Marchi via Gdb-patches
  2022-02-11 14:53         ` Andrew Burgess via Gdb-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi via Gdb-patches @ 2022-02-08 20:15 UTC (permalink / raw)
  To: Jan Vrany, Andrew Burgess; +Cc: gdb-patches



On 2022-02-08 14:32, Jan Vrany wrote:
> On Tue, 2022-02-08 at 14:16 -0500, Simon Marchi wrote:
>>
>> On 2022-02-08 10:49, Andrew Burgess via Gdb-patches wrote:
>>> Jan,
>>>
>>> Thanks for this patch.  While reviewing this, I wondered about using
>>> ui_out's table mechanism to format the output, rather than just
>>> printing the addresses.
>>>
>>> Additionally, while reminding myself about how the jit stuff works, I
>>> thought, maybe we should keep the jit_code_entry address in the 'maint
>>> info jit' output - after all, given a jit_code_entry, we can, in
>>> theory, figure out the symfile address, but we can't (easily) go from
>>> a symfile address back to a jit_code_entry, right?
>>
>> Why not print both?
> 
> I believe this is what Andrew's patch does. 
> 
>>> Anyway, while I ended up with the patch below.  What do you think
>>> about this instead?
> 
> Andrew, I'm happy with it, of course. 

Ah, I'm happy with it too then :).

Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value
  2022-02-08 20:15       ` Simon Marchi via Gdb-patches
@ 2022-02-11 14:53         ` Andrew Burgess via Gdb-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Burgess via Gdb-patches @ 2022-02-11 14:53 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Jan Vrany, gdb-patches

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2022-02-08 15:15:21 -0500]:

> 
> 
> On 2022-02-08 14:32, Jan Vrany wrote:
> > On Tue, 2022-02-08 at 14:16 -0500, Simon Marchi wrote:
> >>
> >> On 2022-02-08 10:49, Andrew Burgess via Gdb-patches wrote:
> >>> Jan,
> >>>
> >>> Thanks for this patch.  While reviewing this, I wondered about using
> >>> ui_out's table mechanism to format the output, rather than just
> >>> printing the addresses.
> >>>
> >>> Additionally, while reminding myself about how the jit stuff works, I
> >>> thought, maybe we should keep the jit_code_entry address in the 'maint
> >>> info jit' output - after all, given a jit_code_entry, we can, in
> >>> theory, figure out the symfile address, but we can't (easily) go from
> >>> a symfile address back to a jit_code_entry, right?
> >>
> >> Why not print both?
> > 
> > I believe this is what Andrew's patch does. 
> > 
> >>> Anyway, while I ended up with the patch below.  What do you think
> >>> about this instead?
> > 
> > Andrew, I'm happy with it, of course. 
> 
> Ah, I'm happy with it too then :).

Thanks both.  I pushed this patch.

Andrew


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-11 14:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-07 11:39 [PATCH] gdb: change "maint info jit" to print jit_code_entry::symfile_addr value Jan Vrany via Gdb-patches
2022-02-08 15:49 ` Andrew Burgess via Gdb-patches
2022-02-08 19:16   ` Simon Marchi via Gdb-patches
2022-02-08 19:32     ` Jan Vrany via Gdb-patches
2022-02-08 20:15       ` Simon Marchi via Gdb-patches
2022-02-11 14:53         ` Andrew Burgess via Gdb-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox