[patch] Add "debug jit" for easier tracing of JIT support internals

[patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

Greetings,

This is on the rather obvious side ...

Thanks,

--
Paul Pluzhnikov

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* jit.c (jit_debug): New variable.
	(show_jit_debug): New function.
	(jit_register_code, jit_inferior_init): Add debug output.
	(_initialize_jit): Register "debug jit" command.



Index: jit.c
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.8
diff -u -p -r1.8 jit.c
--- jit.c	1 Jan 2011 15:33:09 -0000	1.8
+++ jit.c	5 Jan 2011 19:35:32 -0000
@@ -21,6 +21,8 @@
 
 #include "jit.h"
 #include "breakpoint.h"
+#include "command.h"
+#include "gdbcmd.h"
 #include "gdbcore.h"
 #include "observer.h"
 #include "objfiles.h"
@@ -48,6 +50,17 @@ static CORE_ADDR jit_descriptor_addr = 0
 
 static int registering_code = 0;
 
+/* Non-zero if we want to see trace of jit level stuff.  */
+
+static int jit_debug = 0;
+
+static void
+show_jit_debug (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
+}
+
 /* Helper cleanup function to clear an integer flag like the one above.  */
 
 static void
@@ -219,6 +232,13 @@ jit_register_code (struct gdbarch *gdbar
   const struct bfd_arch_info *b;
   CORE_ADDR *entry_addr_ptr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_register_code, symfile_addr = %s, "
+			"symfile_size = %zu\n",
+			paddress (gdbarch, code_entry->symfile_addr),
+			code_entry->symfile_size);
+
   nbfd = bfd_open_from_target_memory (code_entry->symfile_addr,
                                       code_entry->symfile_size, gnutarget);
   old_cleanups = make_cleanup_bfd_close (nbfd);
@@ -314,6 +334,11 @@ jit_inferior_init (struct gdbarch *gdbar
   struct jit_code_entry cur_entry;
   CORE_ADDR cur_entry_addr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, registering_code = %d\n",
+			registering_code);
+
   /* When we register code, GDB resets its breakpoints in case symbols have
      changed.  That in turn calls this handler, which makes us look for new
      code again.  To avoid being re-entered, we check this flag.  */
@@ -329,6 +354,10 @@ jit_inferior_init (struct gdbarch *gdbar
   if (reg_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
+			paddress (gdbarch, reg_addr));
+
   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
      assume we are not attached to a JIT and return early.  */
   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
@@ -338,6 +367,11 @@ jit_inferior_init (struct gdbarch *gdbar
   if (jit_descriptor_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, jit_descriptor_addr = %s\n",
+			paddress (gdbarch, jit_descriptor_addr));
+
   /* Read the descriptor so we can check the version number and load any already
      JITed functions.  */
   jit_read_descriptor (gdbarch, &descriptor);
@@ -453,6 +487,15 @@ extern void _initialize_jit (void);
 void
 _initialize_jit (void)
 {
+  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
+Set JIT debugging."), _("\
+Show JIT debugging."), _("\
+When non-zero, JIT debugging is enabled.  Higher numbers are more\n\
+verbose."),
+			    NULL,
+			    show_jit_debug,
+			    &setdebuglist, &showdebuglist);
+
   observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   jit_objfile_data = register_objfile_data ();

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Doug Evans]

On Wed, Jan 5, 2011 at 11:36 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> This is on the rather obvious side ...
>
> Thanks,
>
> --
> Paul Pluzhnikov
>
> 2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>        * jit.c (jit_debug): New variable.
>        (show_jit_debug): New function.
>        (jit_register_code, jit_inferior_init): Add debug output.
>        (_initialize_jit): Register "debug jit" command.
>
>
>
> Index: jit.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/jit.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 jit.c
> --- jit.c       1 Jan 2011 15:33:09 -0000       1.8
> +++ jit.c       5 Jan 2011 19:35:32 -0000
> @@ -21,6 +21,8 @@
>
>  #include "jit.h"
>  #include "breakpoint.h"
> +#include "command.h"
> +#include "gdbcmd.h"
>  #include "gdbcore.h"
>  #include "observer.h"
>  #include "objfiles.h"
> @@ -48,6 +50,17 @@ static CORE_ADDR jit_descriptor_addr = 0
>
>  static int registering_code = 0;
>
> +/* Non-zero if we want to see trace of jit level stuff.  */
> +
> +static int jit_debug = 0;
> +
> +static void
> +show_jit_debug (struct ui_file *file, int from_tty,
> +               struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
> +}
> +
>  /* Helper cleanup function to clear an integer flag like the one above.  */
>
>  static void
> @@ -219,6 +232,13 @@ jit_register_code (struct gdbarch *gdbar
>   const struct bfd_arch_info *b;
>   CORE_ADDR *entry_addr_ptr;
>
> +  if (jit_debug)
> +    fprintf_unfiltered (gdb_stdlog,
> +                       "jit_register_code, symfile_addr = %s, "
> +                       "symfile_size = %zu\n",
> +                       paddress (gdbarch, code_entry->symfile_addr),
> +                       code_entry->symfile_size);
> +
>   nbfd = bfd_open_from_target_memory (code_entry->symfile_addr,
>                                       code_entry->symfile_size, gnutarget);
>   old_cleanups = make_cleanup_bfd_close (nbfd);
> @@ -314,6 +334,11 @@ jit_inferior_init (struct gdbarch *gdbar
>   struct jit_code_entry cur_entry;
>   CORE_ADDR cur_entry_addr;
>
> +  if (jit_debug)
> +    fprintf_unfiltered (gdb_stdlog,
> +                       "jit_inferior_init, registering_code = %d\n",
> +                       registering_code);
> +
>   /* When we register code, GDB resets its breakpoints in case symbols have
>      changed.  That in turn calls this handler, which makes us look for new
>      code again.  To avoid being re-entered, we check this flag.  */
> @@ -329,6 +354,10 @@ jit_inferior_init (struct gdbarch *gdbar
>   if (reg_addr == 0)
>     return;
>
> +  if (jit_debug)
> +    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
> +                       paddress (gdbarch, reg_addr));
> +
>   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
>      assume we are not attached to a JIT and return early.  */
>   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
> @@ -338,6 +367,11 @@ jit_inferior_init (struct gdbarch *gdbar
>   if (jit_descriptor_addr == 0)
>     return;
>
> +  if (jit_debug)
> +    fprintf_unfiltered (gdb_stdlog,
> +                       "jit_inferior_init, jit_descriptor_addr = %s\n",
> +                       paddress (gdbarch, jit_descriptor_addr));
> +
>   /* Read the descriptor so we can check the version number and load any already
>      JITed functions.  */
>   jit_read_descriptor (gdbarch, &descriptor);
> @@ -453,6 +487,15 @@ extern void _initialize_jit (void);
>  void
>  _initialize_jit (void)
>  {
> +  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
> +Set JIT debugging."), _("\
> +Show JIT debugging."), _("\
> +When non-zero, JIT debugging is enabled.  Higher numbers are more\n\
> +verbose."),
> +                           NULL,
> +                           show_jit_debug,
> +                           &setdebuglist, &showdebuglist);
> +
>   observer_attach_inferior_created (jit_inferior_created_observer);
>   observer_attach_inferior_exit (jit_inferior_exit_hook);
>   jit_objfile_data = register_objfile_data ();
>

"works for me".
I'd suggest removing "Higher numbers are more verbose."
and while I haven't tried it, I wonder if show_jit_debug is necessary
(bummer if it is, we should fix that).

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Pedro Alves]

On Wednesday 05 January 2011 20:01:50, Doug Evans wrote:
> Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> > +
> > +static void
> > +show_jit_debug (struct ui_file *file, int from_tty,
> > +               struct cmd_list_element *c, const char *value)
> > +{
> > +  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
> > +}
> > +

...

> and while I haven't tried it, I wonder if show_jit_debug is necessary
> (bummer if it is, we should fix that).

It's necessary for i18n.  The default callback (deprecated_show_value_hack)
does:

  /* Print doc minus "show" at start.  */
  print_doc_line (gdb_stdout, c->doc + 5);

which obviously only works in English.

Until and if we find a better way, an implemented show callback
should be a requirement for all new commands.

-- 
Pedro Alves

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

On Wed, Jan 5, 2011 at 2:51 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> On Wednesday 05 January 2011 21:53:11, Paul Pluzhnikov wrote:
>> One question: is '%zu' appropriate format for printing size_t ?
>
> Nope, %z is C99.  GDB is currently written in C90.
>
> AFAICS, the size_t you're talking about holds the size of an
> object on the target,

Right. It's actually specified as uint64_t in jit.h, not size_t ...

> while 'size_t' is about sizes of objects
> on the host.  E.g., on a 32-bit GDB (cross) debugging a 64-bit
> inferior, size_t will usually be a 32-bit type.  I think it'd be
> better to make that a ULONGEST throughout (and then use phex
> or pulongest)

Since this interface is frozen, and ULONGEST isn't available to clients,
I'll just cast symfile_size to ULONGEST and print it with pulongest.


On Wed, Jan 5, 2011 at 2:57 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> Until and if we find a better way, an implemented show callback
> should be a requirement for all new commands.

Restored to the original ...

Thanks,

-- 
Paul Pluzhnikov


ChangeLog:

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* jit.c (jit_debug): New variable.
	(show_jit_debug): New function.
	(jit_register_code, jit_inferior_init): Add debug output.
	(_initialize_jit): Register "debug jit" command.


doc/ChangeLog:

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texi (Debugging Output): Document "set debug jit"

[-- Attachment #2: gdb-jit-debug-20110105-3.txt --]
[-- Type: text/plain, Size: 4118 bytes --]

Index: jit.c
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.8
diff -u -p -r1.8 jit.c
--- jit.c	1 Jan 2011 15:33:09 -0000	1.8
+++ jit.c	5 Jan 2011 23:09:45 -0000
@@ -21,6 +21,8 @@
 
 #include "jit.h"
 #include "breakpoint.h"
+#include "command.h"
+#include "gdbcmd.h"
 #include "gdbcore.h"
 #include "observer.h"
 #include "objfiles.h"
@@ -48,6 +50,17 @@ static CORE_ADDR jit_descriptor_addr = 0
 
 static int registering_code = 0;
 
+/* Non-zero if we want to see trace of jit level stuff.  */
+
+static int jit_debug = 0;
+
+static void
+show_jit_debug (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
+}
+
 /* Helper cleanup function to clear an integer flag like the one above.  */
 
 static void
@@ -219,6 +232,13 @@ jit_register_code (struct gdbarch *gdbar
   const struct bfd_arch_info *b;
   CORE_ADDR *entry_addr_ptr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_register_code, symfile_addr = %s, "
+			"symfile_size = %s\n",
+			paddress (gdbarch, code_entry->symfile_addr),
+			pulongest ((ULONGEST) code_entry->symfile_size));
+
   nbfd = bfd_open_from_target_memory (code_entry->symfile_addr,
                                       code_entry->symfile_size, gnutarget);
   old_cleanups = make_cleanup_bfd_close (nbfd);
@@ -314,6 +334,11 @@ jit_inferior_init (struct gdbarch *gdbar
   struct jit_code_entry cur_entry;
   CORE_ADDR cur_entry_addr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, registering_code = %d\n",
+			registering_code);
+
   /* When we register code, GDB resets its breakpoints in case symbols have
      changed.  That in turn calls this handler, which makes us look for new
      code again.  To avoid being re-entered, we check this flag.  */
@@ -329,6 +354,10 @@ jit_inferior_init (struct gdbarch *gdbar
   if (reg_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
+			paddress (gdbarch, reg_addr));
+
   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
      assume we are not attached to a JIT and return early.  */
   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
@@ -338,6 +367,11 @@ jit_inferior_init (struct gdbarch *gdbar
   if (jit_descriptor_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, jit_descriptor_addr = %s\n",
+			paddress (gdbarch, jit_descriptor_addr));
+
   /* Read the descriptor so we can check the version number and load any already
      JITed functions.  */
   jit_read_descriptor (gdbarch, &descriptor);
@@ -453,6 +487,14 @@ extern void _initialize_jit (void);
 void
 _initialize_jit (void)
 {
+  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
+Set JIT debugging."), _("\
+Show JIT debugging."), _("\
+When non-zero, JIT debugging is enabled."),
+			    NULL,
+			    show_jit_debug,
+			    &setdebuglist, &showdebuglist);
+
   observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   jit_objfile_data = register_objfile_data ();
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.783
diff -u -p -r1.783 gdb.texinfo
--- doc/gdb.texinfo	5 Jan 2011 05:09:52 -0000	1.783
+++ doc/gdb.texinfo	5 Jan 2011 23:09:46 -0000
@@ -19955,6 +19955,11 @@ The default is off.  @file{infrun.c} con
 for implementing operations such as single-stepping the inferior.
 @item show debug infrun
 Displays the current state of @value{GDBN} inferior debugging.
+@item set debug jit
+@cindex just-in-time compilation
+Turns on or off debugging messages from JIT debug support.
+@item show debug jit
+Displays the current state of @value{GDBN} JIT debugging.
 @item set debug lin-lwp
 @cindex @sc{gnu}/Linux LWP debug messages
 @cindex Linux lightweight processes

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Pedro Alves]

On Wednesday 05 January 2011 23:15:31, Paul Pluzhnikov wrote:
> Right. It's actually specified as uint64_t in jit.h, not size_t ...
> 
> > while 'size_t' is about sizes of objects
> > on the host.  E.g., on a 32-bit GDB (cross) debugging a 64-bit
> > inferior, size_t will usually be a 32-bit type.  I think it'd be
> > better to make that a ULONGEST throughout (and then use phex
> > or pulongest)
> 
> Since this interface is frozen, and ULONGEST isn't available to clients,
> I'll just cast symfile_size to ULONGEST and print it with pulongest.

That's better than %z, but I don't see why making that uint64_t
be a ULONGEST would be a problem.  The definition of:

 struct jit_code_entry
 {
   CORE_ADDR next_entry;
   CORE_ADDR prev_entry;
   CORE_ADDR symfile_addr;
   uint64_t symfile_size;
 };

in jit.h, is not included by clients.

Clients should define it like so, as per the manual:

 struct jit_code_entry
 {
   struct jit_code_entry *next_entry;
   struct jit_code_entry *prev_entry;
   const char *symfile_addr;
   uint64_t symfile_size;
 };

GDB reads this out of the inferior like so:

  /* Fix the endianness to match the host.  */
  ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
  code_entry->next_entry = extract_typed_address (&entry_buf[0], ptr_type);
  code_entry->prev_entry =
      extract_typed_address (&entry_buf[ptr_size], ptr_type);
  code_entry->symfile_addr =
      extract_typed_address (&entry_buf[2 * ptr_size], ptr_type);
  code_entry->symfile_size =
      extract_unsigned_integer (&entry_buf[3 * ptr_size], 8, byte_order);

And, extract_unsigned_integer returns a ULONGEST...

-- 
Pedro Alves

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On Wed, Jan 5, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> I don't see why making that uint64_t be a ULONGEST would be a problem.

Done.

This exposed a bug -- bfd_open_from_target_memory() had size_t where
ULONGEST was intended.

Thanks,

-- 
Paul Pluzhnikov


2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* jit.h (struct jit_code_entry): use ULONGEST for symfile_size.
	* jit.c (jit_debug): New variable.
	(show_jit_debug): New function.
	(struct target_buffer): Use ULONGEST.
	(bfd_open_from_target_memory): Likewise.
	(jit_register_code, jit_inferior_init): Add debug output.
	(_initialize_jit): Register "debug jit" command.


doc/ChangeLog:

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texi (Debugging Output): Document "set debug jit"

[-- Attachment #2: gdb-jit-debug-20110105-4.txt --]
[-- Type: text/plain, Size: 5166 bytes --]

Index: jit.h
===================================================================
RCS file: /cvs/src/src/gdb/jit.h,v
retrieving revision 1.4
diff -u -p -r1.4 jit.h
--- jit.h	1 Jan 2011 15:33:09 -0000	1.4
+++ jit.h	5 Jan 2011 23:41:17 -0000
@@ -42,7 +42,7 @@ struct jit_code_entry
   CORE_ADDR next_entry;
   CORE_ADDR prev_entry;
   CORE_ADDR symfile_addr;
-  uint64_t symfile_size;
+  ULONGEST symfile_size;
 };
 
 /* This is the global descriptor that the inferior uses to communicate
Index: jit.c
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.8
diff -u -p -r1.8 jit.c
--- jit.c	1 Jan 2011 15:33:09 -0000	1.8
+++ jit.c	5 Jan 2011 23:41:17 -0000
@@ -21,6 +21,8 @@
 
 #include "jit.h"
 #include "breakpoint.h"
+#include "command.h"
+#include "gdbcmd.h"
 #include "gdbcore.h"
 #include "observer.h"
 #include "objfiles.h"
@@ -48,6 +50,17 @@ static CORE_ADDR jit_descriptor_addr = 0
 
 static int registering_code = 0;
 
+/* Non-zero if we want to see trace of jit level stuff.  */
+
+static int jit_debug = 0;
+
+static void
+show_jit_debug (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
+}
+
 /* Helper cleanup function to clear an integer flag like the one above.  */
 
 static void
@@ -59,7 +72,7 @@ clear_int (void *int_addr)
 struct target_buffer
 {
   CORE_ADDR base;
-  size_t size;
+  ULONGEST size;
 };
 
 /* Openning the file is a no-op.  */
@@ -118,7 +131,7 @@ mem_bfd_iovec_stat (struct bfd *abfd, vo
 /* Open a BFD from the target's memory.  */
 
 static struct bfd *
-bfd_open_from_target_memory (CORE_ADDR addr, size_t size, char *target)
+bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size, char *target)
 {
   const char *filename = xstrdup ("<in-memory>");
   struct target_buffer *buffer = xmalloc (sizeof (struct target_buffer));
@@ -219,6 +232,13 @@ jit_register_code (struct gdbarch *gdbar
   const struct bfd_arch_info *b;
   CORE_ADDR *entry_addr_ptr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_register_code, symfile_addr = %s, "
+			"symfile_size = %s\n",
+			paddress (gdbarch, code_entry->symfile_addr),
+			pulongest (code_entry->symfile_size));
+
   nbfd = bfd_open_from_target_memory (code_entry->symfile_addr,
                                       code_entry->symfile_size, gnutarget);
   old_cleanups = make_cleanup_bfd_close (nbfd);
@@ -314,6 +334,11 @@ jit_inferior_init (struct gdbarch *gdbar
   struct jit_code_entry cur_entry;
   CORE_ADDR cur_entry_addr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, registering_code = %d\n",
+			registering_code);
+
   /* When we register code, GDB resets its breakpoints in case symbols have
      changed.  That in turn calls this handler, which makes us look for new
      code again.  To avoid being re-entered, we check this flag.  */
@@ -329,6 +354,10 @@ jit_inferior_init (struct gdbarch *gdbar
   if (reg_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
+			paddress (gdbarch, reg_addr));
+
   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
      assume we are not attached to a JIT and return early.  */
   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
@@ -338,6 +367,11 @@ jit_inferior_init (struct gdbarch *gdbar
   if (jit_descriptor_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, jit_descriptor_addr = %s\n",
+			paddress (gdbarch, jit_descriptor_addr));
+
   /* Read the descriptor so we can check the version number and load any already
      JITed functions.  */
   jit_read_descriptor (gdbarch, &descriptor);
@@ -453,6 +487,14 @@ extern void _initialize_jit (void);
 void
 _initialize_jit (void)
 {
+  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
+Set JIT debugging."), _("\
+Show JIT debugging."), _("\
+When non-zero, JIT debugging is enabled."),
+			    NULL,
+			    show_jit_debug,
+			    &setdebuglist, &showdebuglist);
+
   observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   jit_objfile_data = register_objfile_data ();
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.783
diff -u -p -r1.783 gdb.texinfo
--- doc/gdb.texinfo	5 Jan 2011 05:09:52 -0000	1.783
+++ doc/gdb.texinfo	5 Jan 2011 23:41:18 -0000
@@ -19955,6 +19955,11 @@ The default is off.  @file{infrun.c} con
 for implementing operations such as single-stepping the inferior.
 @item show debug infrun
 Displays the current state of @value{GDBN} inferior debugging.
+@item set debug jit
+@cindex just-in-time compilation
+Turns on or off debugging messages from JIT debug support.
+@item show debug jit
+Displays the current state of @value{GDBN} JIT debugging.
 @item set debug lin-lwp
 @cindex @sc{gnu}/Linux LWP debug messages
 @cindex Linux lightweight processes

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Pedro Alves]

On Wednesday 05 January 2011 23:49:08, Paul Pluzhnikov wrote:
> On Wed, Jan 5, 2011 at 3:28 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> 
> > I don't see why making that uint64_t be a ULONGEST would be a problem.
> 
> Done.
> 
> This exposed a bug -- bfd_open_from_target_memory() had size_t where
> ULONGEST was intended.
> 
> Thanks,

Thank you.  This is fine with me.

-- 
Pedro Alves

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

On Wed, Jan 5, 2011 at 4:44 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> Thank you.  This is fine with me.

Eli, ok for doc change?

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Eli Zaretskii]

> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Date: Wed, 5 Jan 2011 17:09:08 -0800
> Cc: gdb-patches@sourceware.org, Doug Evans <dje@google.com>,
>         Pedro Alves <pedro@codesourcery.com>
> 
> Eli, ok for doc change?

Geez, guys, I need to sleep sometimes, don't I?  My timezone is 10
hours ahead of yours.

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Doug Evans]

On Wed, Jan 5, 2011 at 2:57 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 05 January 2011 20:01:50, Doug Evans wrote:
>> Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>> > +
>> > +static void
>> > +show_jit_debug (struct ui_file *file, int from_tty,
>> > +               struct cmd_list_element *c, const char *value)
>> > +{
>> > +  fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
>> > +}
>> > +
>
> ...
>
>> and while I haven't tried it, I wonder if show_jit_debug is necessary
>> (bummer if it is, we should fix that).
>
> It's necessary for i18n.  The default callback (deprecated_show_value_hack)
> does:
>
>  /* Print doc minus "show" at start.  */
>  print_doc_line (gdb_stdout, c->doc + 5);

Ah, forgot.

> which obviously only works in English.
>
> Until and if we find a better way, an implemented show callback
> should be a requirement for all new commands.

It feels like if *some* better way is that hard, we're not very clever. :-)

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Pedro Alves]

On Wednesday 05 January 2011 23:26:22, Doug Evans wrote:
> > Until and if we find a better way, an implemented show callback
> > should be a requirement for all new commands.
> 
> It feels like if some better way is that hard, we're not very clever. :-)

An alternative I can think of, would be to come up with a GDB specific
printf formatter for "replace by the value of the setting", and then
instead of a code callback, we'd install a show string.  That is, 
data driven instead of code driven, so instead, we'd register
the command like:

  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
Set JIT debugging."), _("\
Show JIT debugging."), _("\
When non-zero, JIT debugging is enabled."),
                           NULL,
                           _("\
JIT debugging is: %V"),
                           &setdebuglist, &showdebuglist);

With %V being the new format string in question.

We'd still need to write the "show" string, but it's more
centralized and readable than putting it in a separate callback
function.

-- 
Pedro Alves

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> An alternative I can think of, would be to come up with a GDB specific
Pedro> printf formatter for "replace by the value of the setting", and then
Pedro> instead of a code callback, we'd install a show string.  That is, 
Pedro> data driven instead of code driven, so instead, we'd register
Pedro> the command like:

Pedro>   add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
Pedro> Set JIT debugging."), _("\
Pedro> Show JIT debugging."), _("\
Pedro> When non-zero, JIT debugging is enabled."),
Pedro>                            NULL,
Pedro>                            _("\
Pedro> JIT debugging is: %V"),
Pedro>                            &setdebuglist, &showdebuglist);

Pedro> With %V being the new format string in question.

Pedro> We'd still need to write the "show" string, but it's more
Pedro> centralized and readable than putting it in a separate callback
Pedro> function.

Could we go even further, and just specify the name of the parameter and
then some generic documentation about it?  The set- and show-docs are
usually pretty useless.

Something like:

add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug,
                          _("JIT debugging"),
                          _("When non-zero, JIT debugging is enabled."),
                          &setdebuglist, &showdebuglist);

Then:

(gdb) help show debug jit
Show the "JIT debugging" setting.
Usage: show debug jit
When non-zero, JIT debugging is enabled.
(gdb) help set debug jit
Set the "JIT debugging" setting.
Usage: set debug jit VALUE
When non-zero, JIT debugging is enabled.
(gdb) show debug jit
The "JIT debugging" setting is currently 0.

Tom

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

On Thu, Jan 6, 2011 at 11:44 AM, Tom Tromey <tromey@redhat.com> wrote:

> Could we go even further, and just specify the name of the parameter and
> then some generic documentation about it?  The set- and show-docs are
> usually pretty useless.
>
> Something like:
>
> add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug,
>                          _("JIT debugging"),
>                          _("When non-zero, JIT debugging is enabled."),
>                          &setdebuglist, &showdebuglist);
>
> Then:
>
> (gdb) help show debug jit
> Show the "JIT debugging" setting.
> Usage: show debug jit
> When non-zero, JIT debugging is enabled.

Just to avoid any confusion, "JIT debugging" (as in "GDB will set breakpoint,
read in-memory JITed files, etc.") is (currently) always enabled.

What 'set debug jit 1' turns on is debugging of what "JIT debugging"
is doing internally :-)

-- 
Paul Pluzhnikov

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Eli Zaretskii]

> Cc: ppluzhnikov@google.com
> Date: Wed,  5 Jan 2011 11:36:21 -0800 (PST)
> From: ppluzhnikov@google.com (Paul Pluzhnikov)
> 
> 2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* jit.c (jit_debug): New variable.
> 	(show_jit_debug): New function.
> 	(jit_register_code, jit_inferior_init): Add debug output.
> 	(_initialize_jit): Register "debug jit" command.

Thanks.  Since this is user-level setting, it should be in the manual.

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

On Wed, Jan 5, 2011 at 12:01 PM, Doug Evans <dje@google.com> wrote:

> I'd suggest removing "Higher numbers are more verbose."

Done.

> and while I haven't tried it, I wonder if show_jit_debug is necessary

It's not.

On Wed, Jan 5, 2011 at 12:57 PM, Eli Zaretskii <eliz@gnu.org> wrote:


>>       (_initialize_jit): Register "debug jit" command.
>
> Since this is user-level setting, it should be in the manual.

Done.


One question: is '%zu' appropriate format for printing size_t ?


Thanks,

-- 
Paul Pluzhnikov


ChangeLog:

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* jit.c (jit_debug): New variable.
	(jit_register_code, jit_inferior_init): Add debug output.
	(_initialize_jit): Register "debug jit" command.


doc/ChangeLog:

2010-01-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texi (Debugging Output): Document "set debug jit"

[-- Attachment #2: gdb-jit-debug-20110105-2.txt --]
[-- Type: text/plain, Size: 3890 bytes --]

Index: jit.c
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.8
diff -u -p -r1.8 jit.c
--- jit.c	1 Jan 2011 15:33:09 -0000	1.8
+++ jit.c	5 Jan 2011 21:46:10 -0000
@@ -21,6 +21,8 @@
 
 #include "jit.h"
 #include "breakpoint.h"
+#include "command.h"
+#include "gdbcmd.h"
 #include "gdbcore.h"
 #include "observer.h"
 #include "objfiles.h"
@@ -48,6 +50,10 @@ static CORE_ADDR jit_descriptor_addr = 0
 
 static int registering_code = 0;
 
+/* Non-zero if we want to see trace of jit level stuff.  */
+
+static int jit_debug = 0;
+
 /* Helper cleanup function to clear an integer flag like the one above.  */
 
 static void
@@ -219,6 +225,13 @@ jit_register_code (struct gdbarch *gdbar
   const struct bfd_arch_info *b;
   CORE_ADDR *entry_addr_ptr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_register_code, symfile_addr = %s, "
+			"symfile_size = %zu\n",
+			paddress (gdbarch, code_entry->symfile_addr),
+			code_entry->symfile_size);
+
   nbfd = bfd_open_from_target_memory (code_entry->symfile_addr,
                                       code_entry->symfile_size, gnutarget);
   old_cleanups = make_cleanup_bfd_close (nbfd);
@@ -314,6 +327,11 @@ jit_inferior_init (struct gdbarch *gdbar
   struct jit_code_entry cur_entry;
   CORE_ADDR cur_entry_addr;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, registering_code = %d\n",
+			registering_code);
+
   /* When we register code, GDB resets its breakpoints in case symbols have
      changed.  That in turn calls this handler, which makes us look for new
      code again.  To avoid being re-entered, we check this flag.  */
@@ -329,6 +347,10 @@ jit_inferior_init (struct gdbarch *gdbar
   if (reg_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init, reg_addr = %s\n",
+			paddress (gdbarch, reg_addr));
+
   /* Lookup the descriptor symbol and cache the addr.  If it is missing, we
      assume we are not attached to a JIT and return early.  */
   desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
@@ -338,6 +360,11 @@ jit_inferior_init (struct gdbarch *gdbar
   if (jit_descriptor_addr == 0)
     return;
 
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_inferior_init, jit_descriptor_addr = %s\n",
+			paddress (gdbarch, jit_descriptor_addr));
+
   /* Read the descriptor so we can check the version number and load any already
      JITed functions.  */
   jit_read_descriptor (gdbarch, &descriptor);
@@ -453,6 +480,13 @@ extern void _initialize_jit (void);
 void
 _initialize_jit (void)
 {
+  add_setshow_zinteger_cmd ("jit", class_maintenance, &jit_debug, _("\
+Set JIT debugging."), _("\
+Show JIT debugging."), _("\
+When non-zero, JIT debugging is enabled."),
+			    NULL, NULL,
+			    &setdebuglist, &showdebuglist);
+
   observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   jit_objfile_data = register_objfile_data ();
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.783
diff -u -p -r1.783 gdb.texinfo
--- doc/gdb.texinfo	5 Jan 2011 05:09:52 -0000	1.783
+++ doc/gdb.texinfo	5 Jan 2011 21:46:11 -0000
@@ -19955,6 +19955,11 @@ The default is off.  @file{infrun.c} con
 for implementing operations such as single-stepping the inferior.
 @item show debug infrun
 Displays the current state of @value{GDBN} inferior debugging.
+@item set debug jit
+@cindex just-in-time compilation
+Turns on or off debugging messages from JIT debug support.
+@item show debug jit
+Displays the current state of @value{GDBN} JIT debugging.
 @item set debug lin-lwp
 @cindex @sc{gnu}/Linux LWP debug messages
 @cindex Linux lightweight processes

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Pedro Alves]

On Wednesday 05 January 2011 21:53:11, Paul Pluzhnikov wrote:
> One question: is '%zu' appropriate format for printing size_t ?

Nope, %z is C99.  GDB is currently written in C90.

AFAICS, the size_t you're talking about holds the size of an
object on the target, while 'size_t' is about sizes of objects
on the host.  E.g., on a 32-bit GDB (cross) debugging a 64-bit
inferior, size_t will usually be a 32-bit type.  I think it'd be
better to make that a ULONGEST throughout (and then use phex
or pulongest)

-- 
Pedro Alves

Re: [patch] Add "debug jit" for easier tracing of JIT support internals

[Eli Zaretskii]

> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Date: Wed, 5 Jan 2011 13:53:11 -0800
> Cc: gdb-patches@sourceware.org
> 
> > Since this is user-level setting, it should be in the manual.
> 
> Done.

Thanks.  One comment below.

> One question: is '%zu' appropriate format for printing size_t ?

I think it's only in C9x, so no.

> +@item set debug jit
> +@cindex just-in-time compilation

Since this text does not describe just-in-time compilation in general,
the index entry should be qualified:

  @cindex just-in-time compilation, debugging messages

Okay with that change.