public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC][PATCH] Allow JIT unwinder provide symbol information
@ 2013-12-26 18:36 Sasha Smundak
  2014-01-13 18:25 ` Alexander Smundak
                   ` (3 more replies)
  0 siblings, 4 replies; 28+ messages in thread
From: Sasha Smundak @ 2013-12-26 18:36 UTC (permalink / raw)
  To: gdb-patches

The change proposed in this RFC is part of the effort to support
debugging applications containing Java code executed with the help of
Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
the backtrace of an application being debugged by the GDB modified to
provide such support:

#8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
#9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
#10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
#11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
#12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
#13 0x00007fffed000438 in <StubRoutines> ()

I.e., Myclass.main() calls Myclass.method1() calls
Util.callObjectMethod(), which is implemented as a C function
Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
Myclass.method1() is JIT-compiled.

The implementation uses GDB's JIT API, and this RFC is the GDB patch
needed for that. The implementation of the actual debugging support
for Java (that is, the code that unwinds Java frames and is able to
provide symbol information for them) is not presented here; hopefully
it will make it into OpenJDK at some point, or will be distributed
standalone.

GDB's current JIT API is sufficient to unwind the stack frames created
by the Java Virtual Machine (JVM). However, it needs to be extended in
order to be able to display correct symbol information, for two
reasons. First, there is no need to add and remove symbol information
as JIT compiles the code and disposes of it if JIT handler can provide
this information on demand. Second, when JVM runs Java code in the
interpreted mode, the code address in the frame points to the
interpreter code, which is not what should be displayed.

The solution proposed here is to add a "symbol provider" function to
the unwinder interface and modify the code displaying frames to get
the symbol information associated with the frame (function name,
source location, arguments) from the frame's unwinder if it has
symbol provider and use the current logic as a fallback strategy.

There are additional changes in this RFC exposing more GDB
functions to the JIT handler.

2013-12-19  Sasha Smundak  <asmundak@google.com>

	* gdb/frame-unwind.h (frame_symbol_type): New function type.
        (struct frame_unwind): Add the pointer to the frame_symbol_type.
        * gdb/frame.c (get_frame_symbol_info): New function.
        * gdb/frame.h (struct frame_symbol_info): Declare the struct
	containing symbol information returned by the unwinder.
	(get_frame_symbol_info): Declare it.
        * gdb/jit-reader.in (gdb_unwind_stash): New function type.
	(gdb_find_symbol): Ditto.
	(gdb_get_lwp): Ditto.
	(gdb_enumerate_shared): Ditto.
	(gdb_unwind_debug_flag): Ditto.
        (struct gdb_unwind_callbacks): Add members pointing to
	gdb_unwind_stash, gdb_find_symbol, gdb_get_lwp and
	gdb_unwind_debug_flag functions.
        (enum jit_frame_symbol_attr): New enum.
	(enum jit_frame_language): Ditto.
	(gdb_get_symbol_attr): New function type.
        (struct gdb_reader_funcs): Add member pointing to
	gdb_get_symbol_attr function.
        * gdb/jit.c: Include solist.h
	Include ptid.h
	(jit_prepend_unwinder): Declare.
	(jit_find_symbol_address): Declare.
	(jit_get_current_lwp): Declare.
	(jit_enumerate_shared): Declare.
	(jit_unwind_debug_flag): Declare.
	(jit_unwind_debug): Declare.
        (show_jit_unwind_debug): New function.
        (jit_breakpoint_re_set_internal): Add the call to
	jit_prepend_unwinder.
        (struct jit_unwind_private): Add a member containing the
	pointer to JIT reader's private area.
        (jit_get_register_from_frame): New function returning a
	register from the given frame (refactored from
	jit_unwind_reg_get_impl).
        (jit_unwind_reg_get_impl): Now a wrapper around
	jit_get_register_from_frame.
        (jit_unwind_get_cpu_register_value): New function to get
	"live" registers.
        (jit_stash_impl): Allocate private data area for the JIT reader.
        (jit_dealloc_cache): Free JIT reader private area.
        (jit_set_unwind_callbacks_vtable): New function, refactored
	from two different places where callbacks were set up.
        (jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
	(jit_frame_language_to_language): New function converting
	JIT reader's language type to the GDB internal language type.
	(jit_symbol): New function returning symbol information supplied
	by the JIT reader.
        (jit_frame_unwind): Add the pointer to jit_symbol.
        (jit_find_symbol_address): New GDB callback for the JIT reader
	returning symbols's address.
        (jit_get_current_lwp): New GDB callback for the JIT reader returning
	the current LWP ID.
        (jit_enumerate_shared): New GDB callback for the JIT reader to
	traverse inferior's shared objects.
        (jit_unwind_debug_flag): New GDB callback returning JIT unwind
	debugging flags.
        (_initialize_jit): Register 'show/set debug jitunwind' subcommand.
        * gdb/stack.c (find_frame_funname): If symbol information from
	the unwindeer if available.
        (find_frame_source_location): New function to return symbol source
	location from the unwinder if available.
        (print_frame): Use arguments information and source location from
	the unwinder if available.

diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 91ccf8c..ece6123 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct frame_info *self,
 typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
 						 void **this_prologue_cache);
 
+/* Return unwinder-specific symbol info or NULL.  */
+
+typedef const struct frame_symbol_info* (frame_symbol_ftype)
+    (struct frame_info *this_frame,
+     void **this_prologue_cache);
+
 struct frame_unwind
 {
   /* The frame's type.  Should this instead be a collection of
@@ -154,6 +160,9 @@ struct frame_unwind
   frame_sniffer_ftype *sniffer;
   frame_dealloc_cache_ftype *dealloc_cache;
   frame_prev_arch_ftype *prev_arch;
+  /* For the JIT interface to give external code a chance to supply
+     symbol information describing the frame.  */
+  frame_symbol_ftype *symbol_info;
 };
 
 /* Register a frame unwinder, _prepending_ it to the front of the
diff --git a/gdb/frame.c b/gdb/frame.c
index ed31c9e..31ee739 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2291,6 +2291,16 @@ find_frame_sal (struct frame_info *frame, struct symtab_and_line *sal)
   (*sal) = find_pc_line (pc, notcurrent);
 }
 
+/* If frame-specific unwinder has symbol info, return it.  */
+
+const struct frame_symbol_info *
+get_frame_symbol_info (struct frame_info *fi)
+{
+  return ((fi->unwind->symbol_info == NULL)
+      ? NULL
+      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
+}
+
 /* Per "frame.h", return the ``address'' of the frame.  Code should
    really be using get_frame_id().  */
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index b03f212..579210c 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -483,6 +483,23 @@ enum unwind_stop_reason
 #undef FIRST_ERROR
   };
 
+/* Unwinder-specific symbol information.  */
+
+struct frame_symbol_info
+{
+  const char *function;
+  const char *source_file;
+  const char *arguments;
+  enum language language;
+  int source_line;
+};
+
+/* Return symbol information supplied by the unwinder. If this frame's
+   unwinder implements frame_unwind.symbol_info method, calls it and
+   returns the result, otherwise returns NULL.  */
+
+const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
+
 /* Return the reason why we can't unwind past this frame.  */
 
 enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
index a42131b..6439e16 100644
--- a/gdb/jit-reader.in
+++ b/gdb/jit-reader.in
@@ -26,7 +26,7 @@ extern "C" {
 
 /* Versioning information.  See gdb_reader_funcs.  */
 
-#define GDB_READER_INTERFACE_VERSION 1
+#define GDB_READER_INTERFACE_VERSION 2
 
 /* Readers must be released under a GPL compatible license.  To
    declare that the reader is indeed released under a GPL compatible
@@ -260,6 +260,32 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
 typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
                                    struct gdb_reg_value *val);
 
+/* Provides access to the stashed data associated with the current frame.
+   On first call, allocates the memory and zeroes it. On subsequent calls
+   returns the pointer to the allocated memory.  */
+
+typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
+	                           unsigned size);
+
+/* Resolves the symbol and returns its address, or 0 if symbol is unknown.
+   The symbol is resolved as-is.  */
+
+typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol);
+
+/* Returns LWP ID of the current thread or 0.  */
+
+typedef long (gdb_get_lwp) (void);
+
+/* Iterates over shared objects present in the inferior, calling given
+   function for each shared object. Once the function returns non-zero,
+   stops iteration and returns shared object's path.  */
+
+typedef const char * (gdb_enumerate_shared) (int (*) (const char *, void *),
+	                                     void *);
+
+/* Returns debug flags setting for the unwinding.  */
+typedef unsigned (gdb_unwind_debug_flag) (void);
+
 /* This struct is passed to unwind in gdb_reader_funcs, and is to be
    used to unwind the current frame (current being the frame whose
    registers can be read using reg_get) into the earlier frame.  The
@@ -270,6 +296,12 @@ struct gdb_unwind_callbacks
   gdb_unwind_reg_get *reg_get;
   gdb_unwind_reg_set *reg_set;
   gdb_target_read *target_read;
+  gdb_unwind_stash *stash;
+  gdb_unwind_reg_get *cpu_reg_get;
+  gdb_find_symbol *find_symbol;
+  gdb_get_lwp *get_lwp;
+  gdb_enumerate_shared *enumerate_shared;
+  gdb_unwind_debug_flag *debug_flag;
 
   /* For internal use by GDB.  */
   void *priv_data;
@@ -306,6 +338,39 @@ typedef enum gdb_status (gdb_unwind_frame) (struct gdb_reader_funcs *self,
 typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
                                                 struct gdb_unwind_callbacks *c);
 
+enum jit_frame_symbol_attr {
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
+  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
+};
+
+enum jit_frame_language {
+  frame_language_unknown,           /* Language not known */
+  frame_language_auto,              /* Placeholder for automatic setting */
+  frame_language_c,                 /* C */
+  frame_language_cplus,             /* C++ */
+  frame_language_d,                 /* D */
+  frame_language_go,                /* Go */
+  frame_language_objc,              /* Objective-C */
+  frame_language_java,              /* Java */
+  frame_language_fortran,           /* Fortran */
+  frame_language_m2,                /* Modula-2 */
+  frame_language_asm,               /* Assembly language */
+  frame_language_pascal,            /* Pascal */
+  frame_language_ada,               /* Ada */
+  frame_language_opencl,            /* OpenCL */
+  frame_language_minimal,           /* All other languages, minimal support only */
+  frame_nr_languages
+};
+
+/* Return given attribute of a symbol associated with the current frame.  */
+
+typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self, 
+                                      struct gdb_unwind_callbacks *c,
+                                      enum jit_frame_symbol_attr attr);
+
 /* Called when a reader is being unloaded.  This function should also
    free SELF, if required.  */
 
@@ -336,6 +401,7 @@ struct gdb_reader_funcs
   gdb_read_debug_info *read;
   gdb_unwind_frame *unwind;
   gdb_get_frame_id *get_frame_id;
+  gdb_get_symbol_attr *get_symbol_attr;
   gdb_destroy_reader *destroy;
 };
 
diff --git a/gdb/jit.c b/gdb/jit.c
index fde79e5..d6abd40 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -33,6 +33,7 @@
 #include "observer.h"
 #include "objfiles.h"
 #include "regcache.h"
+#include "solist.h"
 #include "symfile.h"
 #include "symtab.h"
 #include "target.h"
@@ -40,6 +41,7 @@
 #include <sys/stat.h>
 #include "exceptions.h"
 #include "gdb_bfd.h"
+#include "ptid.h"
 
 static const char *jit_reader_dir = NULL;
 
@@ -53,6 +55,18 @@ static const struct program_space_data *jit_program_space_data = NULL;
 
 static void jit_inferior_init (struct gdbarch *gdbarch);
 
+static void jit_prepend_unwinder (struct gdbarch *gdbarch);
+
+static CORE_ADDR jit_find_symbol_address (const char *);
+
+static long jit_get_current_lwp (void);
+
+static const char *jit_enumerate_shared (int (*callback) (const char *,
+                                                          void *),
+                                         void *);
+
+static unsigned int jit_unwind_debug_flag (void);
+
 /* An unwinder is registered for every gdbarch.  This key is used to
    remember if the unwinder has been registered for a particular
    gdbarch.  */
@@ -70,6 +84,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
 }
 
+static unsigned int jit_unwind_debug;
+
+static void
+show_jit_unwind_debug (struct ui_file *file, int from_tty,
+                       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
+}
+
 struct target_buffer
 {
   CORE_ADDR base;
@@ -1020,6 +1043,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;
 
+  jit_prepend_unwinder (gdbarch);
+
   if (ps_data->objfile == NULL)
     {
       /* Lookup the registration symbol.  If it is missing, then we
@@ -1076,6 +1101,12 @@ struct jit_unwind_private
 
   /* The frame being unwound.  */
   struct frame_info *this_frame;
+
+  /* Symbol associated with this frame.  */
+  struct frame_symbol_info symbol_info;
+
+  /* Memory allocated on behalf of JIT handler.  */
+  void *stash;
 };
 
 /* Sets the value of a particular register in this frame.  */
@@ -1110,29 +1141,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
   xfree (value);
 }
 
-/* Get the value of register REGNUM in the previous frame.  */
+/* Get the value of register REGNUM in the given frame.  */
 
 static struct gdb_reg_value *
-jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
 {
-  struct jit_unwind_private *priv;
   struct gdb_reg_value *value;
   int gdb_reg, size;
   struct gdbarch *frame_arch;
 
-  priv = cb->priv_data;
-  frame_arch = get_frame_arch (priv->this_frame);
+  frame_arch = get_frame_arch (this_frame);
 
   gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
   size = register_size (frame_arch, gdb_reg);
   value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
-  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
+  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
 						   value->value);
   value->size = size;
   value->free = reg_value_free_impl;
   return value;
 }
 
+/* Get the value of register REGNUM in the previous frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  return jit_get_register_from_frame (
+      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
+}
+
+/* Get the value of the register REGNUM in the topmost frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  struct frame_info *frame
+      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
+  struct frame_info *next_frame;
+
+  while ((next_frame = get_next_frame (frame)) != NULL)
+    frame = next_frame;
+  return jit_get_register_from_frame (frame, regnum);
+}
+
+/* Return memory area allocated for the JIT reader.  The area is allocated on
+   demand (the first time this function is called with non-zero size).
+   Calling this function with size == 0 will not allocate memory but will
+   return its address if it has been already allocated.  */
+
+static void *
+jit_stash_impl (struct gdb_unwind_callbacks *cb, unsigned size)
+{
+  struct jit_unwind_private *priv_data = cb->priv_data;
+
+  if (!priv_data->stash && size)
+    priv_data->stash = xcalloc (size, 1);
+  return priv_data->stash;
+}
+
 /* gdb_reg_value has a free function, which must be called on each
    saved register value.  */
 
@@ -1151,9 +1218,25 @@ jit_dealloc_cache (struct frame_info *this_frame, void *cache)
       priv_data->registers[i]->free (priv_data->registers[i]);
 
   xfree (priv_data->registers);
+  xfree (priv_data->stash);
   xfree (priv_data);
 }
 
+static void
+jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
+                                 int allow_reg_set)
+{
+  callbacks->reg_get = jit_unwind_reg_get_impl;
+  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
+  callbacks->target_read = jit_target_read_impl;
+  callbacks->stash = jit_stash_impl;
+  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
+  callbacks->find_symbol = jit_find_symbol_address;
+  callbacks->get_lwp = jit_get_current_lwp;
+  callbacks->enumerate_shared = jit_enumerate_shared;
+  callbacks->debug_flag = jit_unwind_debug_flag;
+}
+
 /* The frame sniffer for the pseudo unwinder.
 
    While this is nominally a frame sniffer, in the case where the JIT
@@ -1170,9 +1253,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
   struct gdb_unwind_callbacks callbacks;
   struct gdb_reader_funcs *funcs;
 
-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = jit_unwind_reg_set_impl;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 1);
 
   if (loaded_jit_reader == NULL)
     return 0;
@@ -1226,9 +1307,7 @@ jit_frame_this_id (struct frame_info *this_frame, void **cache,
 
   /* We don't expect the frame_id function to set any registers, so we
      set reg_set to NULL.  */
-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = NULL;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
   callbacks.priv_data = &private;
 
   gdb_assert (loaded_jit_reader);
@@ -1258,6 +1337,72 @@ jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg)
     return frame_unwind_got_optimized (this_frame, reg);
 }
 
+/* Convert jit_frame_language enum to GDB's internal language enum.  */
+
+static enum language
+jit_frame_language_to_language (enum jit_frame_language frame_language)
+{
+#define LANG_CASE(x)  frame_language_##x: return language_##x
+  switch (frame_language)
+    {
+      LANG_CASE(auto);
+      LANG_CASE(c);
+      LANG_CASE(cplus);
+      LANG_CASE(d);
+      LANG_CASE(go);
+      LANG_CASE(objc);
+      LANG_CASE(java);
+      LANG_CASE(fortran);
+      LANG_CASE(m2);
+      LANG_CASE(asm);
+      LANG_CASE(pascal);
+      LANG_CASE(ada);
+      LANG_CASE(opencl);
+      LANG_CASE(minimal);
+      default:
+        return language_unknown;
+    }
+#undef LANG_CASE
+}
+
+/* Returns unwinder-specific symbol info.  */
+
+static const struct frame_symbol_info *
+jit_symbol (struct frame_info *this_frame, void **cache)
+{
+  struct gdb_reader_funcs *funcs;
+  struct gdb_unwind_callbacks callbacks;
+  struct jit_unwind_private *priv_data;
+
+  if (*cache == NULL)
+    return NULL;
+
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
+  priv_data = *cache;
+  callbacks.priv_data = priv_data;
+
+  gdb_assert (loaded_jit_reader);
+
+  funcs = loaded_jit_reader->functions;
+  priv_data->symbol_info.function
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
+  priv_data->symbol_info.language
+      = jit_frame_language_to_language (
+          (enum jit_frame_language)(funcs->get_symbol_attr (funcs, &callbacks,
+                                                            JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));
+  priv_data->symbol_info.source_file
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
+  priv_data->symbol_info.source_line = (int)(uintptr_t)
+      funcs->get_symbol_attr (funcs, &callbacks,
+                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
+  priv_data->symbol_info.arguments
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
+  return &priv_data->symbol_info;
+}
+
 /* Relay everything back to the unwinder registered by the JIT debug
    info reader.*/
 
@@ -1267,9 +1412,11 @@ static const struct frame_unwind jit_frame_unwind =
   default_frame_unwind_stop_reason,
   jit_frame_this_id,
   jit_frame_prev_register,
-  NULL,
+  NULL,  /* frame_data */
   jit_frame_sniffer,
-  jit_dealloc_cache
+  jit_dealloc_cache,
+  NULL,  /* prev_arch */
+  jit_symbol,
 };
 
 
@@ -1310,8 +1457,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");

-  jit_prepend_unwinder (gdbarch);
-
   ps_data = get_jit_program_space_data ();
   if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
     return;
@@ -1443,6 +1590,52 @@ free_objfile_data (struct objfile *objfile, void *data)
   xfree (data);
 }
 
+/* Locates exported symbol in the target and returns its address.  */
+
+CORE_ADDR
+jit_find_symbol_address (const char *symbol_name)
+{
+  struct minimal_symbol *min_symbol
+      = lookup_minimal_symbol (symbol_name, NULL, NULL);
+
+  return (min_symbol == NULL) ? 0 : SYMBOL_VALUE_ADDRESS (min_symbol);
+}
+
+/* Returns LWP of the inferior's current thread.  */
+
+long
+jit_get_current_lwp (void)
+{
+  long lwp = ptid_get_lwp (inferior_ptid);
+
+  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
+  // thread id then.
+  // TODO(asmundak): perhaps use thread id always?
+  return lwp ? lwp : ptid_get_tid (inferior_ptid);
+}
+
+/* Calls provided function for every shared object currently loaded into
+   the inferior until the function returns non zero.  */
+
+const char *
+jit_enumerate_shared (int (*callback) (const char *, void *), void *data)
+{
+  struct so_list *so;
+
+  for (so = master_so_list (); so; so = so->next)
+    {
+      if (callback (so->so_name, data))
+        return so->so_name;
+    }
+  return NULL;
+}
+
+static unsigned
+jit_unwind_debug_flag ()
+{
+  return jit_unwind_debug;
+}
+
 /* Initialize the jit_gdbarch_data slot with an instance of struct
    jit_gdbarch_data_type */
 
@@ -1472,6 +1665,13 @@ _initialize_jit (void)
 			     NULL,
 			     show_jit_debug,
 			     &setdebuglist, &showdebuglist);
+  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
+                             _("Set JIT frame unwinder debug."),
+                             _("Show JIT frame unwinder debug."),
+                             _("A collection of bit flags for debugging."),
+                             NULL,
+                             show_jit_unwind_debug,
+                             &setdebuglist, &showdebuglist);
 
   observer_attach_inferior_exit (jit_inferior_exit_hook);
   observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
diff --git a/gdb/stack.c b/gdb/stack.c
index f45bb80..d14eced 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1038,6 +1038,15 @@ find_frame_funname (struct frame_info *frame, char **funname,
 		    enum language *funlang, struct symbol **funcp)
 {
   struct symbol *func;
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      *funname = frame_symbol->function;
+      *funlang = frame_symbol->language;
+      *funcp = NULL;
+      return;
+    }
 
   *funname = NULL;
   *funlang = language_unknown;
@@ -1125,6 +1134,23 @@ find_frame_funname (struct frame_info *frame, char **funname,
     }
 }
 
+/* Fill unwinder-specific source location for the frame if available and
+   return 1. Otherwise return 0.  */
+
+static int
+find_frame_source_location (struct frame_info *fi, const char **file,
+                            int *line)
+{
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
+
+  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
+    return 0;
+
+  *file = frame_symbol->source_file;
+  *line = frame_symbol->source_line;
+  return 1;
+}
+
 static void
 print_frame (struct frame_info *frame, int print_level,
 	     enum print_what print_what, int print_args,
@@ -1140,6 +1166,8 @@ print_frame (struct frame_info *frame, int print_level,
   struct symbol *func;
   CORE_ADDR pc = 0;
   int pc_p;
+  const char *file;
+  int line;
 
   pc_p = get_frame_pc_if_available (frame, &pc);
 
@@ -1200,7 +1228,16 @@ print_frame (struct frame_info *frame, int print_level,
       args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
       TRY_CATCH (e, RETURN_MASK_ERROR)
 	{
-	  print_frame_args (func, frame, numargs, gdb_stdout);
+	  const struct frame_symbol_info *frame_symbol;
+	  frame_symbol = get_frame_symbol_info (frame);
+
+	  if (frame_symbol != NULL)
+	    {
+	      if (frame_symbol->arguments != NULL)
+		ui_out_text (uiout, frame_symbol->arguments);
+	    }
+	  else
+	    print_frame_args (func, frame, numargs, gdb_stdout);
 	}
       /* FIXME: ARGS must be a list.  If one argument is a string it
 	  will have " that will not be properly escaped.  */
@@ -1209,7 +1246,21 @@ print_frame (struct frame_info *frame, int print_level,
       QUIT;
     }
   ui_out_text (uiout, ")");
-  if (sal.symtab)
+
+  if (find_frame_source_location (frame, &file, &line))
+    {
+      annotate_frame_source_begin ();
+      ui_out_wrap_hint (uiout, "   ");
+      ui_out_text (uiout, " at ");
+      annotate_frame_source_file ();
+      ui_out_field_string (uiout, "file", file);
+      annotate_frame_source_file_end ();
+      ui_out_text (uiout, ":");
+      annotate_frame_source_line ();
+      ui_out_field_int (uiout, "line", line);
+      annotate_frame_source_end ();
+    }
+  else if (sal.symtab)
     {
       const char *filename_display;
       

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2013-12-26 18:36 [RFC][PATCH] Allow JIT unwinder provide symbol information Sasha Smundak
@ 2014-01-13 18:25 ` Alexander Smundak
  2014-02-07 21:54   ` Alexander Smundak
  2014-01-13 18:46 ` Doug Evans
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-01-13 18:25 UTC (permalink / raw)
  To: gdb-patches

Ping.

On Thu, Dec 26, 2013 at 10:36 AM, Sasha Smundak <asmundak@google.com> wrote:
> The change proposed in this RFC is part of the effort to support
> debugging applications containing Java code executed with the help of
> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
> the backtrace of an application being debugged by the GDB modified to
> provide such support:
>
> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
> #13 0x00007fffed000438 in <StubRoutines> ()
>
> I.e., Myclass.main() calls Myclass.method1() calls
> Util.callObjectMethod(), which is implemented as a C function
> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
> Myclass.method1() is JIT-compiled.
>
> The implementation uses GDB's JIT API, and this RFC is the GDB patch
> needed for that. The implementation of the actual debugging support
> for Java (that is, the code that unwinds Java frames and is able to
> provide symbol information for them) is not presented here; hopefully
> it will make it into OpenJDK at some point, or will be distributed
> standalone.
>
> GDB's current JIT API is sufficient to unwind the stack frames created
> by the Java Virtual Machine (JVM). However, it needs to be extended in
> order to be able to display correct symbol information, for two
> reasons. First, there is no need to add and remove symbol information
> as JIT compiles the code and disposes of it if JIT handler can provide
> this information on demand. Second, when JVM runs Java code in the
> interpreted mode, the code address in the frame points to the
> interpreter code, which is not what should be displayed.
>
> The solution proposed here is to add a "symbol provider" function to
> the unwinder interface and modify the code displaying frames to get
> the symbol information associated with the frame (function name,
> source location, arguments) from the frame's unwinder if it has
> symbol provider and use the current logic as a fallback strategy.
>
> There are additional changes in this RFC exposing more GDB
> functions to the JIT handler.
>
> 2013-12-19  Sasha Smundak  <asmundak@google.com>
>
>         * gdb/frame-unwind.h (frame_symbol_type): New function type.
>         (struct frame_unwind): Add the pointer to the frame_symbol_type.
>         * gdb/frame.c (get_frame_symbol_info): New function.
>         * gdb/frame.h (struct frame_symbol_info): Declare the struct
>         containing symbol information returned by the unwinder.
>         (get_frame_symbol_info): Declare it.
>         * gdb/jit-reader.in (gdb_unwind_stash): New function type.
>         (gdb_find_symbol): Ditto.
>         (gdb_get_lwp): Ditto.
>         (gdb_enumerate_shared): Ditto.
>         (gdb_unwind_debug_flag): Ditto.
>         (struct gdb_unwind_callbacks): Add members pointing to
>         gdb_unwind_stash, gdb_find_symbol, gdb_get_lwp and
>         gdb_unwind_debug_flag functions.
>         (enum jit_frame_symbol_attr): New enum.
>         (enum jit_frame_language): Ditto.
>         (gdb_get_symbol_attr): New function type.
>         (struct gdb_reader_funcs): Add member pointing to
>         gdb_get_symbol_attr function.
>         * gdb/jit.c: Include solist.h
>         Include ptid.h
>         (jit_prepend_unwinder): Declare.
>         (jit_find_symbol_address): Declare.
>         (jit_get_current_lwp): Declare.
>         (jit_enumerate_shared): Declare.
>         (jit_unwind_debug_flag): Declare.
>         (jit_unwind_debug): Declare.
>         (show_jit_unwind_debug): New function.
>         (jit_breakpoint_re_set_internal): Add the call to
>         jit_prepend_unwinder.
>         (struct jit_unwind_private): Add a member containing the
>         pointer to JIT reader's private area.
>         (jit_get_register_from_frame): New function returning a
>         register from the given frame (refactored from
>         jit_unwind_reg_get_impl).
>         (jit_unwind_reg_get_impl): Now a wrapper around
>         jit_get_register_from_frame.
>         (jit_unwind_get_cpu_register_value): New function to get
>         "live" registers.
>         (jit_stash_impl): Allocate private data area for the JIT reader.
>         (jit_dealloc_cache): Free JIT reader private area.
>         (jit_set_unwind_callbacks_vtable): New function, refactored
>         from two different places where callbacks were set up.
>         (jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
>         (jit_frame_language_to_language): New function converting
>         JIT reader's language type to the GDB internal language type.
>         (jit_symbol): New function returning symbol information supplied
>         by the JIT reader.
>         (jit_frame_unwind): Add the pointer to jit_symbol.
>         (jit_find_symbol_address): New GDB callback for the JIT reader
>         returning symbols's address.
>         (jit_get_current_lwp): New GDB callback for the JIT reader returning
>         the current LWP ID.
>         (jit_enumerate_shared): New GDB callback for the JIT reader to
>         traverse inferior's shared objects.
>         (jit_unwind_debug_flag): New GDB callback returning JIT unwind
>         debugging flags.
>         (_initialize_jit): Register 'show/set debug jitunwind' subcommand.
>         * gdb/stack.c (find_frame_funname): If symbol information from
>         the unwindeer if available.
>         (find_frame_source_location): New function to return symbol source
>         location from the unwinder if available.
>         (print_frame): Use arguments information and source location from
>         the unwinder if available.
>
> diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
> index 91ccf8c..ece6123 100644
> --- a/gdb/frame-unwind.h
> +++ b/gdb/frame-unwind.h
> @@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct frame_info *self,
>  typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
>                                                  void **this_prologue_cache);
>
> +/* Return unwinder-specific symbol info or NULL.  */
> +
> +typedef const struct frame_symbol_info* (frame_symbol_ftype)
> +    (struct frame_info *this_frame,
> +     void **this_prologue_cache);
> +
>  struct frame_unwind
>  {
>    /* The frame's type.  Should this instead be a collection of
> @@ -154,6 +160,9 @@ struct frame_unwind
>    frame_sniffer_ftype *sniffer;
>    frame_dealloc_cache_ftype *dealloc_cache;
>    frame_prev_arch_ftype *prev_arch;
> +  /* For the JIT interface to give external code a chance to supply
> +     symbol information describing the frame.  */
> +  frame_symbol_ftype *symbol_info;
>  };
>
>  /* Register a frame unwinder, _prepending_ it to the front of the
> diff --git a/gdb/frame.c b/gdb/frame.c
> index ed31c9e..31ee739 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2291,6 +2291,16 @@ find_frame_sal (struct frame_info *frame, struct symtab_and_line *sal)
>    (*sal) = find_pc_line (pc, notcurrent);
>  }
>
> +/* If frame-specific unwinder has symbol info, return it.  */
> +
> +const struct frame_symbol_info *
> +get_frame_symbol_info (struct frame_info *fi)
> +{
> +  return ((fi->unwind->symbol_info == NULL)
> +      ? NULL
> +      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
> +}
> +
>  /* Per "frame.h", return the ``address'' of the frame.  Code should
>     really be using get_frame_id().  */
>  CORE_ADDR
> diff --git a/gdb/frame.h b/gdb/frame.h
> index b03f212..579210c 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -483,6 +483,23 @@ enum unwind_stop_reason
>  #undef FIRST_ERROR
>    };
>
> +/* Unwinder-specific symbol information.  */
> +
> +struct frame_symbol_info
> +{
> +  const char *function;
> +  const char *source_file;
> +  const char *arguments;
> +  enum language language;
> +  int source_line;
> +};
> +
> +/* Return symbol information supplied by the unwinder. If this frame's
> +   unwinder implements frame_unwind.symbol_info method, calls it and
> +   returns the result, otherwise returns NULL.  */
> +
> +const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
> +
>  /* Return the reason why we can't unwind past this frame.  */
>
>  enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
> diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
> index a42131b..6439e16 100644
> --- a/gdb/jit-reader.in
> +++ b/gdb/jit-reader.in
> @@ -26,7 +26,7 @@ extern "C" {
>
>  /* Versioning information.  See gdb_reader_funcs.  */
>
> -#define GDB_READER_INTERFACE_VERSION 1
> +#define GDB_READER_INTERFACE_VERSION 2
>
>  /* Readers must be released under a GPL compatible license.  To
>     declare that the reader is indeed released under a GPL compatible
> @@ -260,6 +260,32 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
>  typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
>                                     struct gdb_reg_value *val);
>
> +/* Provides access to the stashed data associated with the current frame.
> +   On first call, allocates the memory and zeroes it. On subsequent calls
> +   returns the pointer to the allocated memory.  */
> +
> +typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
> +                                  unsigned size);
> +
> +/* Resolves the symbol and returns its address, or 0 if symbol is unknown.
> +   The symbol is resolved as-is.  */
> +
> +typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol);
> +
> +/* Returns LWP ID of the current thread or 0.  */
> +
> +typedef long (gdb_get_lwp) (void);
> +
> +/* Iterates over shared objects present in the inferior, calling given
> +   function for each shared object. Once the function returns non-zero,
> +   stops iteration and returns shared object's path.  */
> +
> +typedef const char * (gdb_enumerate_shared) (int (*) (const char *, void *),
> +                                            void *);
> +
> +/* Returns debug flags setting for the unwinding.  */
> +typedef unsigned (gdb_unwind_debug_flag) (void);
> +
>  /* This struct is passed to unwind in gdb_reader_funcs, and is to be
>     used to unwind the current frame (current being the frame whose
>     registers can be read using reg_get) into the earlier frame.  The
> @@ -270,6 +296,12 @@ struct gdb_unwind_callbacks
>    gdb_unwind_reg_get *reg_get;
>    gdb_unwind_reg_set *reg_set;
>    gdb_target_read *target_read;
> +  gdb_unwind_stash *stash;
> +  gdb_unwind_reg_get *cpu_reg_get;
> +  gdb_find_symbol *find_symbol;
> +  gdb_get_lwp *get_lwp;
> +  gdb_enumerate_shared *enumerate_shared;
> +  gdb_unwind_debug_flag *debug_flag;
>
>    /* For internal use by GDB.  */
>    void *priv_data;
> @@ -306,6 +338,39 @@ typedef enum gdb_status (gdb_unwind_frame) (struct gdb_reader_funcs *self,
>  typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
>                                                  struct gdb_unwind_callbacks *c);
>
> +enum jit_frame_symbol_attr {
> +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
> +  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
> +  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
> +  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
> +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
> +};
> +
> +enum jit_frame_language {
> +  frame_language_unknown,           /* Language not known */
> +  frame_language_auto,              /* Placeholder for automatic setting */
> +  frame_language_c,                 /* C */
> +  frame_language_cplus,             /* C++ */
> +  frame_language_d,                 /* D */
> +  frame_language_go,                /* Go */
> +  frame_language_objc,              /* Objective-C */
> +  frame_language_java,              /* Java */
> +  frame_language_fortran,           /* Fortran */
> +  frame_language_m2,                /* Modula-2 */
> +  frame_language_asm,               /* Assembly language */
> +  frame_language_pascal,            /* Pascal */
> +  frame_language_ada,               /* Ada */
> +  frame_language_opencl,            /* OpenCL */
> +  frame_language_minimal,           /* All other languages, minimal support only */
> +  frame_nr_languages
> +};
> +
> +/* Return given attribute of a symbol associated with the current frame.  */
> +
> +typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self,
> +                                      struct gdb_unwind_callbacks *c,
> +                                      enum jit_frame_symbol_attr attr);
> +
>  /* Called when a reader is being unloaded.  This function should also
>     free SELF, if required.  */
>
> @@ -336,6 +401,7 @@ struct gdb_reader_funcs
>    gdb_read_debug_info *read;
>    gdb_unwind_frame *unwind;
>    gdb_get_frame_id *get_frame_id;
> +  gdb_get_symbol_attr *get_symbol_attr;
>    gdb_destroy_reader *destroy;
>  };
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index fde79e5..d6abd40 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -33,6 +33,7 @@
>  #include "observer.h"
>  #include "objfiles.h"
>  #include "regcache.h"
> +#include "solist.h"
>  #include "symfile.h"
>  #include "symtab.h"
>  #include "target.h"
> @@ -40,6 +41,7 @@
>  #include <sys/stat.h>
>  #include "exceptions.h"
>  #include "gdb_bfd.h"
> +#include "ptid.h"
>
>  static const char *jit_reader_dir = NULL;
>
> @@ -53,6 +55,18 @@ static const struct program_space_data *jit_program_space_data = NULL;
>
>  static void jit_inferior_init (struct gdbarch *gdbarch);
>
> +static void jit_prepend_unwinder (struct gdbarch *gdbarch);
> +
> +static CORE_ADDR jit_find_symbol_address (const char *);
> +
> +static long jit_get_current_lwp (void);
> +
> +static const char *jit_enumerate_shared (int (*callback) (const char *,
> +                                                          void *),
> +                                         void *);
> +
> +static unsigned int jit_unwind_debug_flag (void);
> +
>  /* An unwinder is registered for every gdbarch.  This key is used to
>     remember if the unwinder has been registered for a particular
>     gdbarch.  */
> @@ -70,6 +84,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
>    fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
>  }
>
> +static unsigned int jit_unwind_debug;
> +
> +static void
> +show_jit_unwind_debug (struct ui_file *file, int from_tty,
> +                       struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
> +}
> +
>  struct target_buffer
>  {
>    CORE_ADDR base;
> @@ -1020,6 +1043,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>    struct jit_objfile_data *objf_data;
>    CORE_ADDR addr;
>
> +  jit_prepend_unwinder (gdbarch);
> +
>    if (ps_data->objfile == NULL)
>      {
>        /* Lookup the registration symbol.  If it is missing, then we
> @@ -1076,6 +1101,12 @@ struct jit_unwind_private
>
>    /* The frame being unwound.  */
>    struct frame_info *this_frame;
> +
> +  /* Symbol associated with this frame.  */
> +  struct frame_symbol_info symbol_info;
> +
> +  /* Memory allocated on behalf of JIT handler.  */
> +  void *stash;
>  };
>
>  /* Sets the value of a particular register in this frame.  */
> @@ -1110,29 +1141,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
>    xfree (value);
>  }
>
> -/* Get the value of register REGNUM in the previous frame.  */
> +/* Get the value of register REGNUM in the given frame.  */
>
>  static struct gdb_reg_value *
> -jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
> +jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
>  {
> -  struct jit_unwind_private *priv;
>    struct gdb_reg_value *value;
>    int gdb_reg, size;
>    struct gdbarch *frame_arch;
>
> -  priv = cb->priv_data;
> -  frame_arch = get_frame_arch (priv->this_frame);
> +  frame_arch = get_frame_arch (this_frame);
>
>    gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
>    size = register_size (frame_arch, gdb_reg);
>    value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
> -  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
> +  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
>                                                    value->value);
>    value->size = size;
>    value->free = reg_value_free_impl;
>    return value;
>  }
>
> +/* Get the value of register REGNUM in the previous frame.  */
> +
> +static struct gdb_reg_value *
> +jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
> +{
> +  return jit_get_register_from_frame (
> +      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
> +}
> +
> +/* Get the value of the register REGNUM in the topmost frame.  */
> +
> +static struct gdb_reg_value *
> +jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
> +{
> +  struct frame_info *frame
> +      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
> +  struct frame_info *next_frame;
> +
> +  while ((next_frame = get_next_frame (frame)) != NULL)
> +    frame = next_frame;
> +  return jit_get_register_from_frame (frame, regnum);
> +}
> +
> +/* Return memory area allocated for the JIT reader.  The area is allocated on
> +   demand (the first time this function is called with non-zero size).
> +   Calling this function with size == 0 will not allocate memory but will
> +   return its address if it has been already allocated.  */
> +
> +static void *
> +jit_stash_impl (struct gdb_unwind_callbacks *cb, unsigned size)
> +{
> +  struct jit_unwind_private *priv_data = cb->priv_data;
> +
> +  if (!priv_data->stash && size)
> +    priv_data->stash = xcalloc (size, 1);
> +  return priv_data->stash;
> +}
> +
>  /* gdb_reg_value has a free function, which must be called on each
>     saved register value.  */
>
> @@ -1151,9 +1218,25 @@ jit_dealloc_cache (struct frame_info *this_frame, void *cache)
>        priv_data->registers[i]->free (priv_data->registers[i]);
>
>    xfree (priv_data->registers);
> +  xfree (priv_data->stash);
>    xfree (priv_data);
>  }
>
> +static void
> +jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
> +                                 int allow_reg_set)
> +{
> +  callbacks->reg_get = jit_unwind_reg_get_impl;
> +  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
> +  callbacks->target_read = jit_target_read_impl;
> +  callbacks->stash = jit_stash_impl;
> +  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
> +  callbacks->find_symbol = jit_find_symbol_address;
> +  callbacks->get_lwp = jit_get_current_lwp;
> +  callbacks->enumerate_shared = jit_enumerate_shared;
> +  callbacks->debug_flag = jit_unwind_debug_flag;
> +}
> +
>  /* The frame sniffer for the pseudo unwinder.
>
>     While this is nominally a frame sniffer, in the case where the JIT
> @@ -1170,9 +1253,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
>    struct gdb_unwind_callbacks callbacks;
>    struct gdb_reader_funcs *funcs;
>
> -  callbacks.reg_get = jit_unwind_reg_get_impl;
> -  callbacks.reg_set = jit_unwind_reg_set_impl;
> -  callbacks.target_read = jit_target_read_impl;
> +  jit_set_unwind_callbacks_vtable (&callbacks, 1);
>
>    if (loaded_jit_reader == NULL)
>      return 0;
> @@ -1226,9 +1307,7 @@ jit_frame_this_id (struct frame_info *this_frame, void **cache,
>
>    /* We don't expect the frame_id function to set any registers, so we
>       set reg_set to NULL.  */
> -  callbacks.reg_get = jit_unwind_reg_get_impl;
> -  callbacks.reg_set = NULL;
> -  callbacks.target_read = jit_target_read_impl;
> +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
>    callbacks.priv_data = &private;
>
>    gdb_assert (loaded_jit_reader);
> @@ -1258,6 +1337,72 @@ jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg)
>      return frame_unwind_got_optimized (this_frame, reg);
>  }
>
> +/* Convert jit_frame_language enum to GDB's internal language enum.  */
> +
> +static enum language
> +jit_frame_language_to_language (enum jit_frame_language frame_language)
> +{
> +#define LANG_CASE(x)  frame_language_##x: return language_##x
> +  switch (frame_language)
> +    {
> +      LANG_CASE(auto);
> +      LANG_CASE(c);
> +      LANG_CASE(cplus);
> +      LANG_CASE(d);
> +      LANG_CASE(go);
> +      LANG_CASE(objc);
> +      LANG_CASE(java);
> +      LANG_CASE(fortran);
> +      LANG_CASE(m2);
> +      LANG_CASE(asm);
> +      LANG_CASE(pascal);
> +      LANG_CASE(ada);
> +      LANG_CASE(opencl);
> +      LANG_CASE(minimal);
> +      default:
> +        return language_unknown;
> +    }
> +#undef LANG_CASE
> +}
> +
> +/* Returns unwinder-specific symbol info.  */
> +
> +static const struct frame_symbol_info *
> +jit_symbol (struct frame_info *this_frame, void **cache)
> +{
> +  struct gdb_reader_funcs *funcs;
> +  struct gdb_unwind_callbacks callbacks;
> +  struct jit_unwind_private *priv_data;
> +
> +  if (*cache == NULL)
> +    return NULL;
> +
> +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
> +  priv_data = *cache;
> +  callbacks.priv_data = priv_data;
> +
> +  gdb_assert (loaded_jit_reader);
> +
> +  funcs = loaded_jit_reader->functions;
> +  priv_data->symbol_info.function
> +      = funcs->get_symbol_attr (funcs, &callbacks,
> +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
> +  priv_data->symbol_info.language
> +      = jit_frame_language_to_language (
> +          (enum jit_frame_language)(funcs->get_symbol_attr (funcs, &callbacks,
> +                                                            JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));
> +  priv_data->symbol_info.source_file
> +      = funcs->get_symbol_attr (funcs, &callbacks,
> +                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
> +  priv_data->symbol_info.source_line = (int)(uintptr_t)
> +      funcs->get_symbol_attr (funcs, &callbacks,
> +                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
> +  priv_data->symbol_info.arguments
> +      = funcs->get_symbol_attr (funcs, &callbacks,
> +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
> +  return &priv_data->symbol_info;
> +}
> +
>  /* Relay everything back to the unwinder registered by the JIT debug
>     info reader.*/
>
> @@ -1267,9 +1412,11 @@ static const struct frame_unwind jit_frame_unwind =
>    default_frame_unwind_stop_reason,
>    jit_frame_this_id,
>    jit_frame_prev_register,
> -  NULL,
> +  NULL,  /* frame_data */
>    jit_frame_sniffer,
> -  jit_dealloc_cache
> +  jit_dealloc_cache,
> +  NULL,  /* prev_arch */
> +  jit_symbol,
>  };
>
>
> @@ -1310,8 +1457,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
>    if (jit_debug)
>      fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
>
> -  jit_prepend_unwinder (gdbarch);
> -
>    ps_data = get_jit_program_space_data ();
>    if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
>      return;
> @@ -1443,6 +1590,52 @@ free_objfile_data (struct objfile *objfile, void *data)
>    xfree (data);
>  }
>
> +/* Locates exported symbol in the target and returns its address.  */
> +
> +CORE_ADDR
> +jit_find_symbol_address (const char *symbol_name)
> +{
> +  struct minimal_symbol *min_symbol
> +      = lookup_minimal_symbol (symbol_name, NULL, NULL);
> +
> +  return (min_symbol == NULL) ? 0 : SYMBOL_VALUE_ADDRESS (min_symbol);
> +}
> +
> +/* Returns LWP of the inferior's current thread.  */
> +
> +long
> +jit_get_current_lwp (void)
> +{
> +  long lwp = ptid_get_lwp (inferior_ptid);
> +
> +  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
> +  // thread id then.
> +  // TODO(asmundak): perhaps use thread id always?
> +  return lwp ? lwp : ptid_get_tid (inferior_ptid);
> +}
> +
> +/* Calls provided function for every shared object currently loaded into
> +   the inferior until the function returns non zero.  */
> +
> +const char *
> +jit_enumerate_shared (int (*callback) (const char *, void *), void *data)
> +{
> +  struct so_list *so;
> +
> +  for (so = master_so_list (); so; so = so->next)
> +    {
> +      if (callback (so->so_name, data))
> +        return so->so_name;
> +    }
> +  return NULL;
> +}
> +
> +static unsigned
> +jit_unwind_debug_flag ()
> +{
> +  return jit_unwind_debug;
> +}
> +
>  /* Initialize the jit_gdbarch_data slot with an instance of struct
>     jit_gdbarch_data_type */
>
> @@ -1472,6 +1665,13 @@ _initialize_jit (void)
>                              NULL,
>                              show_jit_debug,
>                              &setdebuglist, &showdebuglist);
> +  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
> +                             _("Set JIT frame unwinder debug."),
> +                             _("Show JIT frame unwinder debug."),
> +                             _("A collection of bit flags for debugging."),
> +                             NULL,
> +                             show_jit_unwind_debug,
> +                             &setdebuglist, &showdebuglist);
>
>    observer_attach_inferior_exit (jit_inferior_exit_hook);
>    observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
> diff --git a/gdb/stack.c b/gdb/stack.c
> index f45bb80..d14eced 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -1038,6 +1038,15 @@ find_frame_funname (struct frame_info *frame, char **funname,
>                     enum language *funlang, struct symbol **funcp)
>  {
>    struct symbol *func;
> +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
> +
> +  if (frame_symbol != NULL)
> +    {
> +      *funname = frame_symbol->function;
> +      *funlang = frame_symbol->language;
> +      *funcp = NULL;
> +      return;
> +    }
>
>    *funname = NULL;
>    *funlang = language_unknown;
> @@ -1125,6 +1134,23 @@ find_frame_funname (struct frame_info *frame, char **funname,
>      }
>  }
>
> +/* Fill unwinder-specific source location for the frame if available and
> +   return 1. Otherwise return 0.  */
> +
> +static int
> +find_frame_source_location (struct frame_info *fi, const char **file,
> +                            int *line)
> +{
> +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
> +
> +  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
> +    return 0;
> +
> +  *file = frame_symbol->source_file;
> +  *line = frame_symbol->source_line;
> +  return 1;
> +}
> +
>  static void
>  print_frame (struct frame_info *frame, int print_level,
>              enum print_what print_what, int print_args,
> @@ -1140,6 +1166,8 @@ print_frame (struct frame_info *frame, int print_level,
>    struct symbol *func;
>    CORE_ADDR pc = 0;
>    int pc_p;
> +  const char *file;
> +  int line;
>
>    pc_p = get_frame_pc_if_available (frame, &pc);
>
> @@ -1200,7 +1228,16 @@ print_frame (struct frame_info *frame, int print_level,
>        args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
>        TRY_CATCH (e, RETURN_MASK_ERROR)
>         {
> -         print_frame_args (func, frame, numargs, gdb_stdout);
> +         const struct frame_symbol_info *frame_symbol;
> +         frame_symbol = get_frame_symbol_info (frame);
> +
> +         if (frame_symbol != NULL)
> +           {
> +             if (frame_symbol->arguments != NULL)
> +               ui_out_text (uiout, frame_symbol->arguments);
> +           }
> +         else
> +           print_frame_args (func, frame, numargs, gdb_stdout);
>         }
>        /* FIXME: ARGS must be a list.  If one argument is a string it
>           will have " that will not be properly escaped.  */
> @@ -1209,7 +1246,21 @@ print_frame (struct frame_info *frame, int print_level,
>        QUIT;
>      }
>    ui_out_text (uiout, ")");
> -  if (sal.symtab)
> +
> +  if (find_frame_source_location (frame, &file, &line))
> +    {
> +      annotate_frame_source_begin ();
> +      ui_out_wrap_hint (uiout, "   ");
> +      ui_out_text (uiout, " at ");
> +      annotate_frame_source_file ();
> +      ui_out_field_string (uiout, "file", file);
> +      annotate_frame_source_file_end ();
> +      ui_out_text (uiout, ":");
> +      annotate_frame_source_line ();
> +      ui_out_field_int (uiout, "line", line);
> +      annotate_frame_source_end ();
> +    }
> +  else if (sal.symtab)
>      {
>        const char *filename_display;
>

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2013-12-26 18:36 [RFC][PATCH] Allow JIT unwinder provide symbol information Sasha Smundak
  2014-01-13 18:25 ` Alexander Smundak
@ 2014-01-13 18:46 ` Doug Evans
  2014-01-15  0:39   ` Alexander Smundak
  2014-02-08  7:08 ` Yao Qi
  2014-04-24 13:22 ` Pedro Alves
  3 siblings, 1 reply; 28+ messages in thread
From: Doug Evans @ 2014-01-13 18:46 UTC (permalink / raw)
  To: Sasha Smundak; +Cc: gdb-patches

Hi.
Comments inline.

Sasha Smundak writes:
 > The change proposed in this RFC is part of the effort to support
 > debugging applications containing Java code executed with the help of
 > Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
 > the backtrace of an application being debugged by the GDB modified to
 > provide such support:
 > 
 > #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
 > #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
 > #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
 > #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
 > #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
 > #13 0x00007fffed000438 in <StubRoutines> ()
 > 
 > I.e., Myclass.main() calls Myclass.method1() calls
 > Util.callObjectMethod(), which is implemented as a C function
 > Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
 > Myclass.method1() is JIT-compiled.
 > 
 > The implementation uses GDB's JIT API, and this RFC is the GDB patch
 > needed for that. The implementation of the actual debugging support
 > for Java (that is, the code that unwinds Java frames and is able to
 > provide symbol information for them) is not presented here; hopefully
 > it will make it into OpenJDK at some point, or will be distributed
 > standalone.
 > 
 > GDB's current JIT API is sufficient to unwind the stack frames created
 > by the Java Virtual Machine (JVM). However, it needs to be extended in
 > order to be able to display correct symbol information, for two
 > reasons. First, there is no need to add and remove symbol information
 > as JIT compiles the code and disposes of it if JIT handler can provide
 > this information on demand. Second, when JVM runs Java code in the
 > interpreted mode, the code address in the frame points to the
 > interpreter code, which is not what should be displayed.
 > 
 > The solution proposed here is to add a "symbol provider" function to
 > the unwinder interface and modify the code displaying frames to get
 > the symbol information associated with the frame (function name,
 > source location, arguments) from the frame's unwinder if it has
 > symbol provider and use the current logic as a fallback strategy.
 > 
 > There are additional changes in this RFC exposing more GDB
 > functions to the JIT handler.
 > 
 > 2013-12-19  Sasha Smundak  <asmundak@google.com>
 > 
 > 	* gdb/frame-unwind.h (frame_symbol_type): New function type.
 >         (struct frame_unwind): Add the pointer to the frame_symbol_type.
 >         * gdb/frame.c (get_frame_symbol_info): New function.
 >         * gdb/frame.h (struct frame_symbol_info): Declare the struct
 > 	containing symbol information returned by the unwinder.
 > 	(get_frame_symbol_info): Declare it.
 >         * gdb/jit-reader.in (gdb_unwind_stash): New function type.
 > 	(gdb_find_symbol): Ditto.
 > 	(gdb_get_lwp): Ditto.
 > 	(gdb_enumerate_shared): Ditto.
 > 	(gdb_unwind_debug_flag): Ditto.
 >         (struct gdb_unwind_callbacks): Add members pointing to
 > 	gdb_unwind_stash, gdb_find_symbol, gdb_get_lwp and
 > 	gdb_unwind_debug_flag functions.
 >         (enum jit_frame_symbol_attr): New enum.
 > 	(enum jit_frame_language): Ditto.
 > 	(gdb_get_symbol_attr): New function type.
 >         (struct gdb_reader_funcs): Add member pointing to
 > 	gdb_get_symbol_attr function.
 >         * gdb/jit.c: Include solist.h
 > 	Include ptid.h
 > 	(jit_prepend_unwinder): Declare.
 > 	(jit_find_symbol_address): Declare.
 > 	(jit_get_current_lwp): Declare.
 > 	(jit_enumerate_shared): Declare.
 > 	(jit_unwind_debug_flag): Declare.
 > 	(jit_unwind_debug): Declare.
 >         (show_jit_unwind_debug): New function.
 >         (jit_breakpoint_re_set_internal): Add the call to
 > 	jit_prepend_unwinder.
 >         (struct jit_unwind_private): Add a member containing the
 > 	pointer to JIT reader's private area.
 >         (jit_get_register_from_frame): New function returning a
 > 	register from the given frame (refactored from
 > 	jit_unwind_reg_get_impl).
 >         (jit_unwind_reg_get_impl): Now a wrapper around
 > 	jit_get_register_from_frame.
 >         (jit_unwind_get_cpu_register_value): New function to get
 > 	"live" registers.
 >         (jit_stash_impl): Allocate private data area for the JIT reader.
 >         (jit_dealloc_cache): Free JIT reader private area.
 >         (jit_set_unwind_callbacks_vtable): New function, refactored
 > 	from two different places where callbacks were set up.
 >         (jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
 > 	(jit_frame_language_to_language): New function converting
 > 	JIT reader's language type to the GDB internal language type.
 > 	(jit_symbol): New function returning symbol information supplied
 > 	by the JIT reader.
 >         (jit_frame_unwind): Add the pointer to jit_symbol.
 >         (jit_find_symbol_address): New GDB callback for the JIT reader
 > 	returning symbols's address.
 >         (jit_get_current_lwp): New GDB callback for the JIT reader returning
 > 	the current LWP ID.
 >         (jit_enumerate_shared): New GDB callback for the JIT reader to
 > 	traverse inferior's shared objects.
 >         (jit_unwind_debug_flag): New GDB callback returning JIT unwind
 > 	debugging flags.
 >         (_initialize_jit): Register 'show/set debug jitunwind' subcommand.
 >         * gdb/stack.c (find_frame_funname): If symbol information from
 > 	the unwindeer if available.
 >         (find_frame_source_location): New function to return symbol source
 > 	location from the unwinder if available.
 >         (print_frame): Use arguments information and source location from
 > 	the unwinder if available.

The ChangeLog entry has a mix of leading spaces and tabs.
Please use tabs exclusively.

I'm not sure a doc addition is needed, but a NEWS entry is.

 > diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
 > index 91ccf8c..ece6123 100644
 > --- a/gdb/frame-unwind.h
 > +++ b/gdb/frame-unwind.h
 > @@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct frame_info *self,
 >  typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
 >  						 void **this_prologue_cache);
 >  
 > +/* Return unwinder-specific symbol info or NULL.  */
 > +
 > +typedef const struct frame_symbol_info* (frame_symbol_ftype)
 > +    (struct frame_info *this_frame,
 > +     void **this_prologue_cache);
 > +
 >  struct frame_unwind
 >  {
 >    /* The frame's type.  Should this instead be a collection of
 > @@ -154,6 +160,9 @@ struct frame_unwind
 >    frame_sniffer_ftype *sniffer;
 >    frame_dealloc_cache_ftype *dealloc_cache;
 >    frame_prev_arch_ftype *prev_arch;
 > +  /* For the JIT interface to give external code a chance to supply
 > +     symbol information describing the frame.  */
 > +  frame_symbol_ftype *symbol_info;
 >  };
 >  
 >  /* Register a frame unwinder, _prepending_ it to the front of the
 > diff --git a/gdb/frame.c b/gdb/frame.c
 > index ed31c9e..31ee739 100644
 > --- a/gdb/frame.c
 > +++ b/gdb/frame.c
 > @@ -2291,6 +2291,16 @@ find_frame_sal (struct frame_info *frame, struct symtab_and_line *sal)
 >    (*sal) = find_pc_line (pc, notcurrent);
 >  }
 >  
 > +/* If frame-specific unwinder has symbol info, return it.  */
 > +
 > +const struct frame_symbol_info *
 > +get_frame_symbol_info (struct frame_info *fi)
 > +{
 > +  return ((fi->unwind->symbol_info == NULL)
 > +      ? NULL
 > +      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
 > +}
 > +
 >  /* Per "frame.h", return the ``address'' of the frame.  Code should
 >     really be using get_frame_id().  */
 >  CORE_ADDR
 > diff --git a/gdb/frame.h b/gdb/frame.h
 > index b03f212..579210c 100644
 > --- a/gdb/frame.h
 > +++ b/gdb/frame.h
 > @@ -483,6 +483,23 @@ enum unwind_stop_reason
 >  #undef FIRST_ERROR
 >    };
 >  
 > +/* Unwinder-specific symbol information.  */
 > +
 > +struct frame_symbol_info
 > +{
 > +  const char *function;
 > +  const char *source_file;
 > +  const char *arguments;
 > +  enum language language;
 > +  int source_line;
 > +};
 > +
 > +/* Return symbol information supplied by the unwinder. If this frame's
 > +   unwinder implements frame_unwind.symbol_info method, calls it and
 > +   returns the result, otherwise returns NULL.  */
 > +
 > +const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
 > +
 >  /* Return the reason why we can't unwind past this frame.  */
 >  
 >  enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
 > diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
 > index a42131b..6439e16 100644
 > --- a/gdb/jit-reader.in
 > +++ b/gdb/jit-reader.in
 > @@ -26,7 +26,7 @@ extern "C" {
 >  
 >  /* Versioning information.  See gdb_reader_funcs.  */
 >  
 > -#define GDB_READER_INTERFACE_VERSION 1
 > +#define GDB_READER_INTERFACE_VERSION 2
 >  
 >  /* Readers must be released under a GPL compatible license.  To
 >     declare that the reader is indeed released under a GPL compatible
 > @@ -260,6 +260,32 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
 >  typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
 >                                     struct gdb_reg_value *val);
 >  
 > +/* Provides access to the stashed data associated with the current frame.
 > +   On first call, allocates the memory and zeroes it. On subsequent calls
 > +   returns the pointer to the allocated memory.  */
 > +
 > +typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
 > +	                           unsigned size);
 > +
 > +/* Resolves the symbol and returns its address, or 0 if symbol is unknown.
 > +   The symbol is resolved as-is.  */

"as-is" needs some elaboration, I think.
I think symbol is the mangled form (if there is one).

Plus, I suggest being clearer regarding what the search context is.
E.g. Only global symbols are searched and the first one in any objfile
is found.

 > +
 > +typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol);
 > +
 > +/* Returns LWP ID of the current thread or 0.  */
 > +
 > +typedef long (gdb_get_lwp) (void);

The term "lwp" might be an insufficiently portable concept that we either
need to pick a different name or use a different value here.
It may be sufficient to stick with "lwp" but add further comments
specifying how it's used so that one can translate it to whatever else
is used on a non-lwp-using host.
Alas I don't have a good answer.  Hopefully someone else here does.

 > +
 > +/* Iterates over shared objects present in the inferior, calling given
 > +   function for each shared object. Once the function returns non-zero,
 > +   stops iteration and returns shared object's path.  */
 > +
 > +typedef const char * (gdb_enumerate_shared) (int (*) (const char *, void *),
 > +	                                     void *);

I can infer that the first argument to the callback is the shared object's
path.  Suggest explicitly saying so.  Plus, in gdb-land we have to deal
with the difference between the "real path" (all symlinks expanded, etc),
and the path as originally specified.  Suggest documenting what form the
path is.

 > +
 > +/* Returns debug flags setting for the unwinding.  */
 > +typedef unsigned (gdb_unwind_debug_flag) (void);
 > +
 >  /* This struct is passed to unwind in gdb_reader_funcs, and is to be
 >     used to unwind the current frame (current being the frame whose
 >     registers can be read using reg_get) into the earlier frame.  The
 > @@ -270,6 +296,12 @@ struct gdb_unwind_callbacks
 >    gdb_unwind_reg_get *reg_get;
 >    gdb_unwind_reg_set *reg_set;
 >    gdb_target_read *target_read;
 > +  gdb_unwind_stash *stash;
 > +  gdb_unwind_reg_get *cpu_reg_get;
 > +  gdb_find_symbol *find_symbol;
 > +  gdb_get_lwp *get_lwp;
 > +  gdb_enumerate_shared *enumerate_shared;
 > +  gdb_unwind_debug_flag *debug_flag;
 >  
 >    /* For internal use by GDB.  */
 >    void *priv_data;
 > @@ -306,6 +338,39 @@ typedef enum gdb_status (gdb_unwind_frame) (struct gdb_reader_funcs *self,
 >  typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
 >                                                  struct gdb_unwind_callbacks *c);
 >  
 > +enum jit_frame_symbol_attr {
 > +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
 > +  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
 > +  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
 > +  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
 > +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
 > +};
 > +
 > +enum jit_frame_language {
 > +  frame_language_unknown,           /* Language not known */
 > +  frame_language_auto,              /* Placeholder for automatic setting */
 > +  frame_language_c,                 /* C */
 > +  frame_language_cplus,             /* C++ */
 > +  frame_language_d,                 /* D */
 > +  frame_language_go,                /* Go */
 > +  frame_language_objc,              /* Objective-C */
 > +  frame_language_java,              /* Java */
 > +  frame_language_fortran,           /* Fortran */
 > +  frame_language_m2,                /* Modula-2 */
 > +  frame_language_asm,               /* Assembly language */
 > +  frame_language_pascal,            /* Pascal */
 > +  frame_language_ada,               /* Ada */
 > +  frame_language_opencl,            /* OpenCL */
 > +  frame_language_minimal,           /* All other languages, minimal support only */
 > +  frame_nr_languages
 > +};
 > +
 > +/* Return given attribute of a symbol associated with the current frame.  */
 > +
 > +typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self, 
 > +                                      struct gdb_unwind_callbacks *c,
 > +                                      enum jit_frame_symbol_attr attr);
 > +

I'd be interested in hearing other's opinions of going this route.
i.e. having one function returning a void* which is then interpreted
according to what was passed for enum jit_frame_symbol_attr.
My impression is that the community does not like this approach,
but I could be wrong.

 >  /* Called when a reader is being unloaded.  This function should also
 >     free SELF, if required.  */
 >  
 > @@ -336,6 +401,7 @@ struct gdb_reader_funcs
 >    gdb_read_debug_info *read;
 >    gdb_unwind_frame *unwind;
 >    gdb_get_frame_id *get_frame_id;
 > +  gdb_get_symbol_attr *get_symbol_attr;
 >    gdb_destroy_reader *destroy;
 >  };
 >  
 > diff --git a/gdb/jit.c b/gdb/jit.c
 > index fde79e5..d6abd40 100644
 > --- a/gdb/jit.c
 > +++ b/gdb/jit.c
 > @@ -33,6 +33,7 @@
 >  #include "observer.h"
 >  #include "objfiles.h"
 >  #include "regcache.h"
 > +#include "solist.h"
 >  #include "symfile.h"
 >  #include "symtab.h"
 >  #include "target.h"
 > @@ -40,6 +41,7 @@
 >  #include <sys/stat.h>
 >  #include "exceptions.h"
 >  #include "gdb_bfd.h"
 > +#include "ptid.h"
 >  
 >  static const char *jit_reader_dir = NULL;
 >  
 > @@ -53,6 +55,18 @@ static const struct program_space_data *jit_program_space_data = NULL;
 >  
 >  static void jit_inferior_init (struct gdbarch *gdbarch);
 >  
 > +static void jit_prepend_unwinder (struct gdbarch *gdbarch);
 > +
 > +static CORE_ADDR jit_find_symbol_address (const char *);
 > +
 > +static long jit_get_current_lwp (void);
 > +
 > +static const char *jit_enumerate_shared (int (*callback) (const char *,
 > +                                                          void *),
 > +                                         void *);
 > +
 > +static unsigned int jit_unwind_debug_flag (void);
 > +
 >  /* An unwinder is registered for every gdbarch.  This key is used to
 >     remember if the unwinder has been registered for a particular
 >     gdbarch.  */
 > @@ -70,6 +84,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
 >    fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
 >  }
 >  
 > +static unsigned int jit_unwind_debug;
 > +
 > +static void
 > +show_jit_unwind_debug (struct ui_file *file, int from_tty,
 > +                       struct cmd_list_element *c, const char *value)
 > +{
 > +  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
 > +}
 > +
 >  struct target_buffer
 >  {
 >    CORE_ADDR base;
 > @@ -1020,6 +1043,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 >    struct jit_objfile_data *objf_data;
 >    CORE_ADDR addr;
 >  
 > +  jit_prepend_unwinder (gdbarch);
 > +
 >    if (ps_data->objfile == NULL)
 >      {
 >        /* Lookup the registration symbol.  If it is missing, then we
 > @@ -1076,6 +1101,12 @@ struct jit_unwind_private
 >  
 >    /* The frame being unwound.  */
 >    struct frame_info *this_frame;
 > +
 > +  /* Symbol associated with this frame.  */
 > +  struct frame_symbol_info symbol_info;
 > +
 > +  /* Memory allocated on behalf of JIT handler.  */
 > +  void *stash;
 >  };
 >  
 >  /* Sets the value of a particular register in this frame.  */
 > @@ -1110,29 +1141,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
 >    xfree (value);
 >  }
 >  
 > -/* Get the value of register REGNUM in the previous frame.  */
 > +/* Get the value of register REGNUM in the given frame.  */
 >  
 >  static struct gdb_reg_value *
 > -jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
 > +jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
 >  {
 > -  struct jit_unwind_private *priv;
 >    struct gdb_reg_value *value;
 >    int gdb_reg, size;
 >    struct gdbarch *frame_arch;
 >  
 > -  priv = cb->priv_data;
 > -  frame_arch = get_frame_arch (priv->this_frame);
 > +  frame_arch = get_frame_arch (this_frame);
 >  
 >    gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
 >    size = register_size (frame_arch, gdb_reg);
 >    value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
 > -  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
 > +  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
 >  						   value->value);
 >    value->size = size;
 >    value->free = reg_value_free_impl;
 >    return value;
 >  }
 >  
 > +/* Get the value of register REGNUM in the previous frame.  */
 > +
 > +static struct gdb_reg_value *
 > +jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
 > +{
 > +  return jit_get_register_from_frame (
 > +      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
 > +}
 > +
 > +/* Get the value of the register REGNUM in the topmost frame.  */

"top"/"bottom" terms for frames can be confusing, and gdb tries to
avoid them. Suggest "innermost" instead of "topmost", I believe that's
the canonical term gdb uses.

 > +
 > +static struct gdb_reg_value *
 > +jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
 > +{
 > +  struct frame_info *frame
 > +      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
 > +  struct frame_info *next_frame;
 > +
 > +  while ((next_frame = get_next_frame (frame)) != NULL)
 > +    frame = next_frame;
 > +  return jit_get_register_from_frame (frame, regnum);
 > +}
 > +
 > +/* Return memory area allocated for the JIT reader.  The area is allocated on
 > +   demand (the first time this function is called with non-zero size).
 > +   Calling this function with size == 0 will not allocate memory but will
 > +   return its address if it has been already allocated.  */
 > +
 > +static void *
 > +jit_stash_impl (struct gdb_unwind_callbacks *cb, unsigned size)

I believe the rule is s/unsigned/size_t/.

Probably should assert-fail if called a second time with a different size,
but I think that could be left for later.

 > +{
 > +  struct jit_unwind_private *priv_data = cb->priv_data;
 > +
 > +  if (!priv_data->stash && size)
 > +    priv_data->stash = xcalloc (size, 1);
 > +  return priv_data->stash;
 > +}
 > +
 >  /* gdb_reg_value has a free function, which must be called on each
 >     saved register value.  */
 >  
 > @@ -1151,9 +1218,25 @@ jit_dealloc_cache (struct frame_info *this_frame, void *cache)
 >        priv_data->registers[i]->free (priv_data->registers[i]);
 >  
 >    xfree (priv_data->registers);
 > +  xfree (priv_data->stash);
 >    xfree (priv_data);
 >  }
 >  
 > +static void
 > +jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
 > +                                 int allow_reg_set)
 > +{
 > +  callbacks->reg_get = jit_unwind_reg_get_impl;
 > +  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
 > +  callbacks->target_read = jit_target_read_impl;
 > +  callbacks->stash = jit_stash_impl;
 > +  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
 > +  callbacks->find_symbol = jit_find_symbol_address;
 > +  callbacks->get_lwp = jit_get_current_lwp;
 > +  callbacks->enumerate_shared = jit_enumerate_shared;
 > +  callbacks->debug_flag = jit_unwind_debug_flag;
 > +}

I realize this is a bit nitpicky, but there's a style here that's
inconsistently used, namely naming the functions jit_unwind_${name}_impl,
or jit_${name}_impl.  Suggest picking one and being consistent throughout.

 > +
 >  /* The frame sniffer for the pseudo unwinder.
 >  
 >     While this is nominally a frame sniffer, in the case where the JIT
 > @@ -1170,9 +1253,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
 >    struct gdb_unwind_callbacks callbacks;
 >    struct gdb_reader_funcs *funcs;
 >  
 > -  callbacks.reg_get = jit_unwind_reg_get_impl;
 > -  callbacks.reg_set = jit_unwind_reg_set_impl;
 > -  callbacks.target_read = jit_target_read_impl;
 > +  jit_set_unwind_callbacks_vtable (&callbacks, 1);
 >  
 >    if (loaded_jit_reader == NULL)
 >      return 0;
 > @@ -1226,9 +1307,7 @@ jit_frame_this_id (struct frame_info *this_frame, void **cache,
 >  
 >    /* We don't expect the frame_id function to set any registers, so we
 >       set reg_set to NULL.  */
 > -  callbacks.reg_get = jit_unwind_reg_get_impl;
 > -  callbacks.reg_set = NULL;
 > -  callbacks.target_read = jit_target_read_impl;
 > +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
 >    callbacks.priv_data = &private;
 >  
 >    gdb_assert (loaded_jit_reader);
 > @@ -1258,6 +1337,72 @@ jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg)
 >      return frame_unwind_got_optimized (this_frame, reg);
 >  }
 >  
 > +/* Convert jit_frame_language enum to GDB's internal language enum.  */
 > +
 > +static enum language
 > +jit_frame_language_to_language (enum jit_frame_language frame_language)
 > +{
 > +#define LANG_CASE(x)  frame_language_##x: return language_##x
 > +  switch (frame_language)
 > +    {

Space after LANG_CASE below:

 > +      LANG_CASE(auto);
 > +      LANG_CASE(c);
 > +      LANG_CASE(cplus);
 > +      LANG_CASE(d);
 > +      LANG_CASE(go);
 > +      LANG_CASE(objc);
 > +      LANG_CASE(java);
 > +      LANG_CASE(fortran);
 > +      LANG_CASE(m2);
 > +      LANG_CASE(asm);
 > +      LANG_CASE(pascal);
 > +      LANG_CASE(ada);
 > +      LANG_CASE(opencl);
 > +      LANG_CASE(minimal);
 > +      default:
 > +        return language_unknown;
 > +    }
 > +#undef LANG_CASE
 > +}
 > +
 > +/* Returns unwinder-specific symbol info.  */
 > +
 > +static const struct frame_symbol_info *
 > +jit_symbol (struct frame_info *this_frame, void **cache)
 > +{
 > +  struct gdb_reader_funcs *funcs;
 > +  struct gdb_unwind_callbacks callbacks;
 > +  struct jit_unwind_private *priv_data;
 > +
 > +  if (*cache == NULL)
 > +    return NULL;
 > +
 > +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
 > +  priv_data = *cache;
 > +  callbacks.priv_data = priv_data;
 > +
 > +  gdb_assert (loaded_jit_reader);
 > +
 > +  funcs = loaded_jit_reader->functions;
 > +  priv_data->symbol_info.function
 > +      = funcs->get_symbol_attr (funcs, &callbacks,
 > +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
 > +  priv_data->symbol_info.language
 > +      = jit_frame_language_to_language (
 > +          (enum jit_frame_language)(funcs->get_symbol_attr (funcs, &callbacks,
 > +                                                            JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));

Suggest newline after (enum jit_frame_language) cast to keep text
within 80 cols.

 > +  priv_data->symbol_info.source_file
 > +      = funcs->get_symbol_attr (funcs, &callbacks,
 > +                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
 > +  priv_data->symbol_info.source_line = (int)(uintptr_t)
 > +      funcs->get_symbol_attr (funcs, &callbacks,
 > +                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
 > +  priv_data->symbol_info.arguments
 > +      = funcs->get_symbol_attr (funcs, &callbacks,
 > +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
 > +  return &priv_data->symbol_info;
 > +}
 > +
 >  /* Relay everything back to the unwinder registered by the JIT debug
 >     info reader.*/
 >  
 > @@ -1267,9 +1412,11 @@ static const struct frame_unwind jit_frame_unwind =
 >    default_frame_unwind_stop_reason,
 >    jit_frame_this_id,
 >    jit_frame_prev_register,
 > -  NULL,
 > +  NULL,  /* frame_data */
 >    jit_frame_sniffer,
 > -  jit_dealloc_cache
 > +  jit_dealloc_cache,
 > +  NULL,  /* prev_arch */
 > +  jit_symbol,
 >  };
 >  
 >  
 > @@ -1310,8 +1457,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
 >    if (jit_debug)
 >      fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
 > 
 > -  jit_prepend_unwinder (gdbarch);
 > -
 >    ps_data = get_jit_program_space_data ();
 >    if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
 >      return;
 > @@ -1443,6 +1590,52 @@ free_objfile_data (struct objfile *objfile, void *data)
 >    xfree (data);
 >  }
 >  
 > +/* Locates exported symbol in the target and returns its address.  */
 > +
 > +CORE_ADDR
 > +jit_find_symbol_address (const char *symbol_name)
 > +{
 > +  struct minimal_symbol *min_symbol
 > +      = lookup_minimal_symbol (symbol_name, NULL, NULL);
 > +
 > +  return (min_symbol == NULL) ? 0 : SYMBOL_VALUE_ADDRESS (min_symbol);
 > +}
 > +
 > +/* Returns LWP of the inferior's current thread.  */
 > +
 > +long
 > +jit_get_current_lwp (void)
 > +{
 > +  long lwp = ptid_get_lwp (inferior_ptid);
 > +
 > +  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
 > +  // thread id then.
 > +  // TODO(asmundak): perhaps use thread id always?
 > +  return lwp ? lwp : ptid_get_tid (inferior_ptid);
 > +}

In what situation does ptid_get_lwp return 0 for remote debugging?

Also, getting back to whether "lwp" is sufficient portable,
how does the external code use the value?

 > +
 > +/* Calls provided function for every shared object currently loaded into
 > +   the inferior until the function returns non zero.  */
 > +
 > +const char *
 > +jit_enumerate_shared (int (*callback) (const char *, void *), void *data)
 > +{
 > +  struct so_list *so;
 > +
 > +  for (so = master_so_list (); so; so = so->next)
 > +    {
 > +      if (callback (so->so_name, data))
 > +        return so->so_name;
 > +    }
 > +  return NULL;
 > +}
 > +
 > +static unsigned
 > +jit_unwind_debug_flag ()
 > +{
 > +  return jit_unwind_debug;
 > +}
 > +
 >  /* Initialize the jit_gdbarch_data slot with an instance of struct
 >     jit_gdbarch_data_type */
 >  
 > @@ -1472,6 +1665,13 @@ _initialize_jit (void)
 >  			     NULL,
 >  			     show_jit_debug,
 >  			     &setdebuglist, &showdebuglist);
 > +  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
 > +                             _("Set JIT frame unwinder debug."),
 > +                             _("Show JIT frame unwinder debug."),
 > +                             _("A collection of bit flags for debugging."),
 > +                             NULL,
 > +                             show_jit_unwind_debug,
 > +                             &setdebuglist, &showdebuglist);
 >  
 >    observer_attach_inferior_exit (jit_inferior_exit_hook);
 >    observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
 > diff --git a/gdb/stack.c b/gdb/stack.c
 > index f45bb80..d14eced 100644
 > --- a/gdb/stack.c
 > +++ b/gdb/stack.c
 > @@ -1038,6 +1038,15 @@ find_frame_funname (struct frame_info *frame, char **funname,
 >  		    enum language *funlang, struct symbol **funcp)
 >  {
 >    struct symbol *func;
 > +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
 > +
 > +  if (frame_symbol != NULL)
 > +    {
 > +      *funname = frame_symbol->function;
 > +      *funlang = frame_symbol->language;
 > +      *funcp = NULL;
 > +      return;
 > +    }
 >  
 >    *funname = NULL;
 >    *funlang = language_unknown;
 > @@ -1125,6 +1134,23 @@ find_frame_funname (struct frame_info *frame, char **funname,
 >      }
 >  }
 >  
 > +/* Fill unwinder-specific source location for the frame if available and
 > +   return 1. Otherwise return 0.  */

Two spaces after period.

 > +
 > +static int
 > +find_frame_source_location (struct frame_info *fi, const char **file,
 > +                            int *line)
 > +{
 > +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
 > +
 > +  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
 > +    return 0;
 > +
 > +  *file = frame_symbol->source_file;
 > +  *line = frame_symbol->source_line;
 > +  return 1;
 > +}
 > +
 >  static void
 >  print_frame (struct frame_info *frame, int print_level,
 >  	     enum print_what print_what, int print_args,
 > @@ -1140,6 +1166,8 @@ print_frame (struct frame_info *frame, int print_level,
 >    struct symbol *func;
 >    CORE_ADDR pc = 0;
 >    int pc_p;
 > +  const char *file;
 > +  int line;
 >  
 >    pc_p = get_frame_pc_if_available (frame, &pc);
 >  
 > @@ -1200,7 +1228,16 @@ print_frame (struct frame_info *frame, int print_level,
 >        args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
 >        TRY_CATCH (e, RETURN_MASK_ERROR)
 >  	{
 > -	  print_frame_args (func, frame, numargs, gdb_stdout);
 > +	  const struct frame_symbol_info *frame_symbol;
 > +	  frame_symbol = get_frame_symbol_info (frame);
 > +
 > +	  if (frame_symbol != NULL)
 > +	    {
 > +	      if (frame_symbol->arguments != NULL)
 > +		ui_out_text (uiout, frame_symbol->arguments);
 > +	    }
 > +	  else
 > +	    print_frame_args (func, frame, numargs, gdb_stdout);
 >  	}
 >        /* FIXME: ARGS must be a list.  If one argument is a string it
 >  	  will have " that will not be properly escaped.  */
 > @@ -1209,7 +1246,21 @@ print_frame (struct frame_info *frame, int print_level,
 >        QUIT;
 >      }
 >    ui_out_text (uiout, ")");
 > -  if (sal.symtab)
 > +
 > +  if (find_frame_source_location (frame, &file, &line))
 > +    {
 > +      annotate_frame_source_begin ();
 > +      ui_out_wrap_hint (uiout, "   ");
 > +      ui_out_text (uiout, " at ");
 > +      annotate_frame_source_file ();
 > +      ui_out_field_string (uiout, "file", file);
 > +      annotate_frame_source_file_end ();
 > +      ui_out_text (uiout, ":");
 > +      annotate_frame_source_line ();
 > +      ui_out_field_int (uiout, "line", line);
 > +      annotate_frame_source_end ();
 > +    }
 > +  else if (sal.symtab)
 >      {
 >        const char *filename_display;
 >        

That's all the comments I have for now.
I'd really like to hear other's opinions of this approach.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-01-13 18:46 ` Doug Evans
@ 2014-01-15  0:39   ` Alexander Smundak
  2014-02-11 22:26     ` Doug Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-01-15  0:39 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

I fixed the patch based on your comments, except for the one
about using LWP for thread identification.
Waiting for the opinions about the approach used in this RFC patch.

>  > +/* Returns LWP ID of the current thread or 0.  */
>  > +
>  > +typedef long (gdb_get_lwp) (void);
>
> The term "lwp" might be an insufficiently portable concept that we either
> need to pick a different name or use a different value here.
> It may be sufficient to stick with "lwp" but add further comments
> specifying how it's used so that one can translate it to whatever else
> is used on a non-lwp-using host.
> Alas I don't have a good answer.  Hopefully someone else here does.
...
>  > +long
>  > +jit_get_current_lwp (void)
>  > +{
>  > +  long lwp = ptid_get_lwp (inferior_ptid);
>  > +
>  > +  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
>  > +  // thread id then.
>  > +  // TODO(asmundak): perhaps use thread id always?
>  > +  return lwp ? lwp : ptid_get_tid (inferior_ptid);
>  > +}
>
> In what situation does ptid_get_lwp return 0 for remote debugging?
If inferior is run by the gdbserver, I see that inferior.ptid.lwp
is 0, while inferior_ptid.tid is LWP. If inferior runs locally, it's
the opposite:inferior_ptid.lwp is LWP while inferior_ptid.tid is 0.

> Also, getting back to whether "lwp" is sufficient portable,
> how does the external code use the value?
JVM maintains its own list of threads, using LWP as thread's unique
identifier.Java frame unwinder code checks that the frame belongs to
the thread it knows about.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-01-13 18:25 ` Alexander Smundak
@ 2014-02-07 21:54   ` Alexander Smundak
  0 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-02-07 21:54 UTC (permalink / raw)
  To: gdb-patches

Ping. Perhaps someone in addition to dje@ can comment on this proposal.
The problem it is addressing is real, debugging mixed Java/C applications
is no fun at all. The proposed changes to core GDB functionality are not
that large.


On Mon, Jan 13, 2014 at 10:25 AM, Alexander Smundak <asmundak@google.com> wrote:
> Ping.
>
> On Thu, Dec 26, 2013 at 10:36 AM, Sasha Smundak <asmundak@google.com> wrote:
>> The change proposed in this RFC is part of the effort to support
>> debugging applications containing Java code executed with the help of
>> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
>> the backtrace of an application being debugged by the GDB modified to
>> provide such support:
>>
>> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
>> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
>> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
>> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
>> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
>> #13 0x00007fffed000438 in <StubRoutines> ()
>>
>> I.e., Myclass.main() calls Myclass.method1() calls
>> Util.callObjectMethod(), which is implemented as a C function
>> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
>> Myclass.method1() is JIT-compiled.
>>
>> The implementation uses GDB's JIT API, and this RFC is the GDB patch
>> needed for that. The implementation of the actual debugging support
>> for Java (that is, the code that unwinds Java frames and is able to
>> provide symbol information for them) is not presented here; hopefully
>> it will make it into OpenJDK at some point, or will be distributed
>> standalone.
>>
>> GDB's current JIT API is sufficient to unwind the stack frames created
>> by the Java Virtual Machine (JVM). However, it needs to be extended in
>> order to be able to display correct symbol information, for two
>> reasons. First, there is no need to add and remove symbol information
>> as JIT compiles the code and disposes of it if JIT handler can provide
>> this information on demand. Second, when JVM runs Java code in the
>> interpreted mode, the code address in the frame points to the
>> interpreter code, which is not what should be displayed.
>>
>> The solution proposed here is to add a "symbol provider" function to
>> the unwinder interface and modify the code displaying frames to get
>> the symbol information associated with the frame (function name,
>> source location, arguments) from the frame's unwinder if it has
>> symbol provider and use the current logic as a fallback strategy.
>>
>> There are additional changes in this RFC exposing more GDB
>> functions to the JIT handler.
>>
>> 2013-12-19  Sasha Smundak  <asmundak@google.com>
>>
>>         * gdb/frame-unwind.h (frame_symbol_type): New function type.
>>         (struct frame_unwind): Add the pointer to the frame_symbol_type.
>>         * gdb/frame.c (get_frame_symbol_info): New function.
>>         * gdb/frame.h (struct frame_symbol_info): Declare the struct
>>         containing symbol information returned by the unwinder.
>>         (get_frame_symbol_info): Declare it.
>>         * gdb/jit-reader.in (gdb_unwind_stash): New function type.
>>         (gdb_find_symbol): Ditto.
>>         (gdb_get_lwp): Ditto.
>>         (gdb_enumerate_shared): Ditto.
>>         (gdb_unwind_debug_flag): Ditto.
>>         (struct gdb_unwind_callbacks): Add members pointing to
>>         gdb_unwind_stash, gdb_find_symbol, gdb_get_lwp and
>>         gdb_unwind_debug_flag functions.
>>         (enum jit_frame_symbol_attr): New enum.
>>         (enum jit_frame_language): Ditto.
>>         (gdb_get_symbol_attr): New function type.
>>         (struct gdb_reader_funcs): Add member pointing to
>>         gdb_get_symbol_attr function.
>>         * gdb/jit.c: Include solist.h
>>         Include ptid.h
>>         (jit_prepend_unwinder): Declare.
>>         (jit_find_symbol_address): Declare.
>>         (jit_get_current_lwp): Declare.
>>         (jit_enumerate_shared): Declare.
>>         (jit_unwind_debug_flag): Declare.
>>         (jit_unwind_debug): Declare.
>>         (show_jit_unwind_debug): New function.
>>         (jit_breakpoint_re_set_internal): Add the call to
>>         jit_prepend_unwinder.
>>         (struct jit_unwind_private): Add a member containing the
>>         pointer to JIT reader's private area.
>>         (jit_get_register_from_frame): New function returning a
>>         register from the given frame (refactored from
>>         jit_unwind_reg_get_impl).
>>         (jit_unwind_reg_get_impl): Now a wrapper around
>>         jit_get_register_from_frame.
>>         (jit_unwind_get_cpu_register_value): New function to get
>>         "live" registers.
>>         (jit_stash_impl): Allocate private data area for the JIT reader.
>>         (jit_dealloc_cache): Free JIT reader private area.
>>         (jit_set_unwind_callbacks_vtable): New function, refactored
>>         from two different places where callbacks were set up.
>>         (jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
>>         (jit_frame_language_to_language): New function converting
>>         JIT reader's language type to the GDB internal language type.
>>         (jit_symbol): New function returning symbol information supplied
>>         by the JIT reader.
>>         (jit_frame_unwind): Add the pointer to jit_symbol.
>>         (jit_find_symbol_address): New GDB callback for the JIT reader
>>         returning symbols's address.
>>         (jit_get_current_lwp): New GDB callback for the JIT reader returning
>>         the current LWP ID.
>>         (jit_enumerate_shared): New GDB callback for the JIT reader to
>>         traverse inferior's shared objects.
>>         (jit_unwind_debug_flag): New GDB callback returning JIT unwind
>>         debugging flags.
>>         (_initialize_jit): Register 'show/set debug jitunwind' subcommand.
>>         * gdb/stack.c (find_frame_funname): If symbol information from
>>         the unwindeer if available.
>>         (find_frame_source_location): New function to return symbol source
>>         location from the unwinder if available.
>>         (print_frame): Use arguments information and source location from
>>         the unwinder if available.
>>
>> diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
>> index 91ccf8c..ece6123 100644
>> --- a/gdb/frame-unwind.h
>> +++ b/gdb/frame-unwind.h
>> @@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct frame_info *self,
>>  typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
>>                                                  void **this_prologue_cache);
>>
>> +/* Return unwinder-specific symbol info or NULL.  */
>> +
>> +typedef const struct frame_symbol_info* (frame_symbol_ftype)
>> +    (struct frame_info *this_frame,
>> +     void **this_prologue_cache);
>> +
>>  struct frame_unwind
>>  {
>>    /* The frame's type.  Should this instead be a collection of
>> @@ -154,6 +160,9 @@ struct frame_unwind
>>    frame_sniffer_ftype *sniffer;
>>    frame_dealloc_cache_ftype *dealloc_cache;
>>    frame_prev_arch_ftype *prev_arch;
>> +  /* For the JIT interface to give external code a chance to supply
>> +     symbol information describing the frame.  */
>> +  frame_symbol_ftype *symbol_info;
>>  };
>>
>>  /* Register a frame unwinder, _prepending_ it to the front of the
>> diff --git a/gdb/frame.c b/gdb/frame.c
>> index ed31c9e..31ee739 100644
>> --- a/gdb/frame.c
>> +++ b/gdb/frame.c
>> @@ -2291,6 +2291,16 @@ find_frame_sal (struct frame_info *frame, struct symtab_and_line *sal)
>>    (*sal) = find_pc_line (pc, notcurrent);
>>  }
>>
>> +/* If frame-specific unwinder has symbol info, return it.  */
>> +
>> +const struct frame_symbol_info *
>> +get_frame_symbol_info (struct frame_info *fi)
>> +{
>> +  return ((fi->unwind->symbol_info == NULL)
>> +      ? NULL
>> +      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
>> +}
>> +
>>  /* Per "frame.h", return the ``address'' of the frame.  Code should
>>     really be using get_frame_id().  */
>>  CORE_ADDR
>> diff --git a/gdb/frame.h b/gdb/frame.h
>> index b03f212..579210c 100644
>> --- a/gdb/frame.h
>> +++ b/gdb/frame.h
>> @@ -483,6 +483,23 @@ enum unwind_stop_reason
>>  #undef FIRST_ERROR
>>    };
>>
>> +/* Unwinder-specific symbol information.  */
>> +
>> +struct frame_symbol_info
>> +{
>> +  const char *function;
>> +  const char *source_file;
>> +  const char *arguments;
>> +  enum language language;
>> +  int source_line;
>> +};
>> +
>> +/* Return symbol information supplied by the unwinder. If this frame's
>> +   unwinder implements frame_unwind.symbol_info method, calls it and
>> +   returns the result, otherwise returns NULL.  */
>> +
>> +const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
>> +
>>  /* Return the reason why we can't unwind past this frame.  */
>>
>>  enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
>> diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
>> index a42131b..6439e16 100644
>> --- a/gdb/jit-reader.in
>> +++ b/gdb/jit-reader.in
>> @@ -26,7 +26,7 @@ extern "C" {
>>
>>  /* Versioning information.  See gdb_reader_funcs.  */
>>
>> -#define GDB_READER_INTERFACE_VERSION 1
>> +#define GDB_READER_INTERFACE_VERSION 2
>>
>>  /* Readers must be released under a GPL compatible license.  To
>>     declare that the reader is indeed released under a GPL compatible
>> @@ -260,6 +260,32 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
>>  typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
>>                                     struct gdb_reg_value *val);
>>
>> +/* Provides access to the stashed data associated with the current frame.
>> +   On first call, allocates the memory and zeroes it. On subsequent calls
>> +   returns the pointer to the allocated memory.  */
>> +
>> +typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
>> +                                  unsigned size);
>> +
>> +/* Resolves the symbol and returns its address, or 0 if symbol is unknown.
>> +   The symbol is resolved as-is.  */
>> +
>> +typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol);
>> +
>> +/* Returns LWP ID of the current thread or 0.  */
>> +
>> +typedef long (gdb_get_lwp) (void);
>> +
>> +/* Iterates over shared objects present in the inferior, calling given
>> +   function for each shared object. Once the function returns non-zero,
>> +   stops iteration and returns shared object's path.  */
>> +
>> +typedef const char * (gdb_enumerate_shared) (int (*) (const char *, void *),
>> +                                            void *);
>> +
>> +/* Returns debug flags setting for the unwinding.  */
>> +typedef unsigned (gdb_unwind_debug_flag) (void);
>> +
>>  /* This struct is passed to unwind in gdb_reader_funcs, and is to be
>>     used to unwind the current frame (current being the frame whose
>>     registers can be read using reg_get) into the earlier frame.  The
>> @@ -270,6 +296,12 @@ struct gdb_unwind_callbacks
>>    gdb_unwind_reg_get *reg_get;
>>    gdb_unwind_reg_set *reg_set;
>>    gdb_target_read *target_read;
>> +  gdb_unwind_stash *stash;
>> +  gdb_unwind_reg_get *cpu_reg_get;
>> +  gdb_find_symbol *find_symbol;
>> +  gdb_get_lwp *get_lwp;
>> +  gdb_enumerate_shared *enumerate_shared;
>> +  gdb_unwind_debug_flag *debug_flag;
>>
>>    /* For internal use by GDB.  */
>>    void *priv_data;
>> @@ -306,6 +338,39 @@ typedef enum gdb_status (gdb_unwind_frame) (struct gdb_reader_funcs *self,
>>  typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
>>                                                  struct gdb_unwind_callbacks *c);
>>
>> +enum jit_frame_symbol_attr {
>> +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
>> +  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
>> +  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
>> +  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
>> +  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
>> +};
>> +
>> +enum jit_frame_language {
>> +  frame_language_unknown,           /* Language not known */
>> +  frame_language_auto,              /* Placeholder for automatic setting */
>> +  frame_language_c,                 /* C */
>> +  frame_language_cplus,             /* C++ */
>> +  frame_language_d,                 /* D */
>> +  frame_language_go,                /* Go */
>> +  frame_language_objc,              /* Objective-C */
>> +  frame_language_java,              /* Java */
>> +  frame_language_fortran,           /* Fortran */
>> +  frame_language_m2,                /* Modula-2 */
>> +  frame_language_asm,               /* Assembly language */
>> +  frame_language_pascal,            /* Pascal */
>> +  frame_language_ada,               /* Ada */
>> +  frame_language_opencl,            /* OpenCL */
>> +  frame_language_minimal,           /* All other languages, minimal support only */
>> +  frame_nr_languages
>> +};
>> +
>> +/* Return given attribute of a symbol associated with the current frame.  */
>> +
>> +typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self,
>> +                                      struct gdb_unwind_callbacks *c,
>> +                                      enum jit_frame_symbol_attr attr);
>> +
>>  /* Called when a reader is being unloaded.  This function should also
>>     free SELF, if required.  */
>>
>> @@ -336,6 +401,7 @@ struct gdb_reader_funcs
>>    gdb_read_debug_info *read;
>>    gdb_unwind_frame *unwind;
>>    gdb_get_frame_id *get_frame_id;
>> +  gdb_get_symbol_attr *get_symbol_attr;
>>    gdb_destroy_reader *destroy;
>>  };
>>
>> diff --git a/gdb/jit.c b/gdb/jit.c
>> index fde79e5..d6abd40 100644
>> --- a/gdb/jit.c
>> +++ b/gdb/jit.c
>> @@ -33,6 +33,7 @@
>>  #include "observer.h"
>>  #include "objfiles.h"
>>  #include "regcache.h"
>> +#include "solist.h"
>>  #include "symfile.h"
>>  #include "symtab.h"
>>  #include "target.h"
>> @@ -40,6 +41,7 @@
>>  #include <sys/stat.h>
>>  #include "exceptions.h"
>>  #include "gdb_bfd.h"
>> +#include "ptid.h"
>>
>>  static const char *jit_reader_dir = NULL;
>>
>> @@ -53,6 +55,18 @@ static const struct program_space_data *jit_program_space_data = NULL;
>>
>>  static void jit_inferior_init (struct gdbarch *gdbarch);
>>
>> +static void jit_prepend_unwinder (struct gdbarch *gdbarch);
>> +
>> +static CORE_ADDR jit_find_symbol_address (const char *);
>> +
>> +static long jit_get_current_lwp (void);
>> +
>> +static const char *jit_enumerate_shared (int (*callback) (const char *,
>> +                                                          void *),
>> +                                         void *);
>> +
>> +static unsigned int jit_unwind_debug_flag (void);
>> +
>>  /* An unwinder is registered for every gdbarch.  This key is used to
>>     remember if the unwinder has been registered for a particular
>>     gdbarch.  */
>> @@ -70,6 +84,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
>>    fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
>>  }
>>
>> +static unsigned int jit_unwind_debug;
>> +
>> +static void
>> +show_jit_unwind_debug (struct ui_file *file, int from_tty,
>> +                       struct cmd_list_element *c, const char *value)
>> +{
>> +  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
>> +}
>> +
>>  struct target_buffer
>>  {
>>    CORE_ADDR base;
>> @@ -1020,6 +1043,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>>    struct jit_objfile_data *objf_data;
>>    CORE_ADDR addr;
>>
>> +  jit_prepend_unwinder (gdbarch);
>> +
>>    if (ps_data->objfile == NULL)
>>      {
>>        /* Lookup the registration symbol.  If it is missing, then we
>> @@ -1076,6 +1101,12 @@ struct jit_unwind_private
>>
>>    /* The frame being unwound.  */
>>    struct frame_info *this_frame;
>> +
>> +  /* Symbol associated with this frame.  */
>> +  struct frame_symbol_info symbol_info;
>> +
>> +  /* Memory allocated on behalf of JIT handler.  */
>> +  void *stash;
>>  };
>>
>>  /* Sets the value of a particular register in this frame.  */
>> @@ -1110,29 +1141,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
>>    xfree (value);
>>  }
>>
>> -/* Get the value of register REGNUM in the previous frame.  */
>> +/* Get the value of register REGNUM in the given frame.  */
>>
>>  static struct gdb_reg_value *
>> -jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
>> +jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
>>  {
>> -  struct jit_unwind_private *priv;
>>    struct gdb_reg_value *value;
>>    int gdb_reg, size;
>>    struct gdbarch *frame_arch;
>>
>> -  priv = cb->priv_data;
>> -  frame_arch = get_frame_arch (priv->this_frame);
>> +  frame_arch = get_frame_arch (this_frame);
>>
>>    gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
>>    size = register_size (frame_arch, gdb_reg);
>>    value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
>> -  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
>> +  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
>>                                                    value->value);
>>    value->size = size;
>>    value->free = reg_value_free_impl;
>>    return value;
>>  }
>>
>> +/* Get the value of register REGNUM in the previous frame.  */
>> +
>> +static struct gdb_reg_value *
>> +jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
>> +{
>> +  return jit_get_register_from_frame (
>> +      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
>> +}
>> +
>> +/* Get the value of the register REGNUM in the topmost frame.  */
>> +
>> +static struct gdb_reg_value *
>> +jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
>> +{
>> +  struct frame_info *frame
>> +      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
>> +  struct frame_info *next_frame;
>> +
>> +  while ((next_frame = get_next_frame (frame)) != NULL)
>> +    frame = next_frame;
>> +  return jit_get_register_from_frame (frame, regnum);
>> +}
>> +
>> +/* Return memory area allocated for the JIT reader.  The area is allocated on
>> +   demand (the first time this function is called with non-zero size).
>> +   Calling this function with size == 0 will not allocate memory but will
>> +   return its address if it has been already allocated.  */
>> +
>> +static void *
>> +jit_stash_impl (struct gdb_unwind_callbacks *cb, unsigned size)
>> +{
>> +  struct jit_unwind_private *priv_data = cb->priv_data;
>> +
>> +  if (!priv_data->stash && size)
>> +    priv_data->stash = xcalloc (size, 1);
>> +  return priv_data->stash;
>> +}
>> +
>>  /* gdb_reg_value has a free function, which must be called on each
>>     saved register value.  */
>>
>> @@ -1151,9 +1218,25 @@ jit_dealloc_cache (struct frame_info *this_frame, void *cache)
>>        priv_data->registers[i]->free (priv_data->registers[i]);
>>
>>    xfree (priv_data->registers);
>> +  xfree (priv_data->stash);
>>    xfree (priv_data);
>>  }
>>
>> +static void
>> +jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
>> +                                 int allow_reg_set)
>> +{
>> +  callbacks->reg_get = jit_unwind_reg_get_impl;
>> +  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
>> +  callbacks->target_read = jit_target_read_impl;
>> +  callbacks->stash = jit_stash_impl;
>> +  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
>> +  callbacks->find_symbol = jit_find_symbol_address;
>> +  callbacks->get_lwp = jit_get_current_lwp;
>> +  callbacks->enumerate_shared = jit_enumerate_shared;
>> +  callbacks->debug_flag = jit_unwind_debug_flag;
>> +}
>> +
>>  /* The frame sniffer for the pseudo unwinder.
>>
>>     While this is nominally a frame sniffer, in the case where the JIT
>> @@ -1170,9 +1253,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
>>    struct gdb_unwind_callbacks callbacks;
>>    struct gdb_reader_funcs *funcs;
>>
>> -  callbacks.reg_get = jit_unwind_reg_get_impl;
>> -  callbacks.reg_set = jit_unwind_reg_set_impl;
>> -  callbacks.target_read = jit_target_read_impl;
>> +  jit_set_unwind_callbacks_vtable (&callbacks, 1);
>>
>>    if (loaded_jit_reader == NULL)
>>      return 0;
>> @@ -1226,9 +1307,7 @@ jit_frame_this_id (struct frame_info *this_frame, void **cache,
>>
>>    /* We don't expect the frame_id function to set any registers, so we
>>       set reg_set to NULL.  */
>> -  callbacks.reg_get = jit_unwind_reg_get_impl;
>> -  callbacks.reg_set = NULL;
>> -  callbacks.target_read = jit_target_read_impl;
>> +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
>>    callbacks.priv_data = &private;
>>
>>    gdb_assert (loaded_jit_reader);
>> @@ -1258,6 +1337,72 @@ jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg)
>>      return frame_unwind_got_optimized (this_frame, reg);
>>  }
>>
>> +/* Convert jit_frame_language enum to GDB's internal language enum.  */
>> +
>> +static enum language
>> +jit_frame_language_to_language (enum jit_frame_language frame_language)
>> +{
>> +#define LANG_CASE(x)  frame_language_##x: return language_##x
>> +  switch (frame_language)
>> +    {
>> +      LANG_CASE(auto);
>> +      LANG_CASE(c);
>> +      LANG_CASE(cplus);
>> +      LANG_CASE(d);
>> +      LANG_CASE(go);
>> +      LANG_CASE(objc);
>> +      LANG_CASE(java);
>> +      LANG_CASE(fortran);
>> +      LANG_CASE(m2);
>> +      LANG_CASE(asm);
>> +      LANG_CASE(pascal);
>> +      LANG_CASE(ada);
>> +      LANG_CASE(opencl);
>> +      LANG_CASE(minimal);
>> +      default:
>> +        return language_unknown;
>> +    }
>> +#undef LANG_CASE
>> +}
>> +
>> +/* Returns unwinder-specific symbol info.  */
>> +
>> +static const struct frame_symbol_info *
>> +jit_symbol (struct frame_info *this_frame, void **cache)
>> +{
>> +  struct gdb_reader_funcs *funcs;
>> +  struct gdb_unwind_callbacks callbacks;
>> +  struct jit_unwind_private *priv_data;
>> +
>> +  if (*cache == NULL)
>> +    return NULL;
>> +
>> +  jit_set_unwind_callbacks_vtable (&callbacks, 0);
>> +  priv_data = *cache;
>> +  callbacks.priv_data = priv_data;
>> +
>> +  gdb_assert (loaded_jit_reader);
>> +
>> +  funcs = loaded_jit_reader->functions;
>> +  priv_data->symbol_info.function
>> +      = funcs->get_symbol_attr (funcs, &callbacks,
>> +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
>> +  priv_data->symbol_info.language
>> +      = jit_frame_language_to_language (
>> +          (enum jit_frame_language)(funcs->get_symbol_attr (funcs, &callbacks,
>> +                                                            JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));
>> +  priv_data->symbol_info.source_file
>> +      = funcs->get_symbol_attr (funcs, &callbacks,
>> +                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
>> +  priv_data->symbol_info.source_line = (int)(uintptr_t)
>> +      funcs->get_symbol_attr (funcs, &callbacks,
>> +                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
>> +  priv_data->symbol_info.arguments
>> +      = funcs->get_symbol_attr (funcs, &callbacks,
>> +                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
>> +  return &priv_data->symbol_info;
>> +}
>> +
>>  /* Relay everything back to the unwinder registered by the JIT debug
>>     info reader.*/
>>
>> @@ -1267,9 +1412,11 @@ static const struct frame_unwind jit_frame_unwind =
>>    default_frame_unwind_stop_reason,
>>    jit_frame_this_id,
>>    jit_frame_prev_register,
>> -  NULL,
>> +  NULL,  /* frame_data */
>>    jit_frame_sniffer,
>> -  jit_dealloc_cache
>> +  jit_dealloc_cache,
>> +  NULL,  /* prev_arch */
>> +  jit_symbol,
>>  };
>>
>>
>> @@ -1310,8 +1457,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
>>    if (jit_debug)
>>      fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
>>
>> -  jit_prepend_unwinder (gdbarch);
>> -
>>    ps_data = get_jit_program_space_data ();
>>    if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
>>      return;
>> @@ -1443,6 +1590,52 @@ free_objfile_data (struct objfile *objfile, void *data)
>>    xfree (data);
>>  }
>>
>> +/* Locates exported symbol in the target and returns its address.  */
>> +
>> +CORE_ADDR
>> +jit_find_symbol_address (const char *symbol_name)
>> +{
>> +  struct minimal_symbol *min_symbol
>> +      = lookup_minimal_symbol (symbol_name, NULL, NULL);
>> +
>> +  return (min_symbol == NULL) ? 0 : SYMBOL_VALUE_ADDRESS (min_symbol);
>> +}
>> +
>> +/* Returns LWP of the inferior's current thread.  */
>> +
>> +long
>> +jit_get_current_lwp (void)
>> +{
>> +  long lwp = ptid_get_lwp (inferior_ptid);
>> +
>> +  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
>> +  // thread id then.
>> +  // TODO(asmundak): perhaps use thread id always?
>> +  return lwp ? lwp : ptid_get_tid (inferior_ptid);
>> +}
>> +
>> +/* Calls provided function for every shared object currently loaded into
>> +   the inferior until the function returns non zero.  */
>> +
>> +const char *
>> +jit_enumerate_shared (int (*callback) (const char *, void *), void *data)
>> +{
>> +  struct so_list *so;
>> +
>> +  for (so = master_so_list (); so; so = so->next)
>> +    {
>> +      if (callback (so->so_name, data))
>> +        return so->so_name;
>> +    }
>> +  return NULL;
>> +}
>> +
>> +static unsigned
>> +jit_unwind_debug_flag ()
>> +{
>> +  return jit_unwind_debug;
>> +}
>> +
>>  /* Initialize the jit_gdbarch_data slot with an instance of struct
>>     jit_gdbarch_data_type */
>>
>> @@ -1472,6 +1665,13 @@ _initialize_jit (void)
>>                              NULL,
>>                              show_jit_debug,
>>                              &setdebuglist, &showdebuglist);
>> +  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
>> +                             _("Set JIT frame unwinder debug."),
>> +                             _("Show JIT frame unwinder debug."),
>> +                             _("A collection of bit flags for debugging."),
>> +                             NULL,
>> +                             show_jit_unwind_debug,
>> +                             &setdebuglist, &showdebuglist);
>>
>>    observer_attach_inferior_exit (jit_inferior_exit_hook);
>>    observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
>> diff --git a/gdb/stack.c b/gdb/stack.c
>> index f45bb80..d14eced 100644
>> --- a/gdb/stack.c
>> +++ b/gdb/stack.c
>> @@ -1038,6 +1038,15 @@ find_frame_funname (struct frame_info *frame, char **funname,
>>                     enum language *funlang, struct symbol **funcp)
>>  {
>>    struct symbol *func;
>> +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
>> +
>> +  if (frame_symbol != NULL)
>> +    {
>> +      *funname = frame_symbol->function;
>> +      *funlang = frame_symbol->language;
>> +      *funcp = NULL;
>> +      return;
>> +    }
>>
>>    *funname = NULL;
>>    *funlang = language_unknown;
>> @@ -1125,6 +1134,23 @@ find_frame_funname (struct frame_info *frame, char **funname,
>>      }
>>  }
>>
>> +/* Fill unwinder-specific source location for the frame if available and
>> +   return 1. Otherwise return 0.  */
>> +
>> +static int
>> +find_frame_source_location (struct frame_info *fi, const char **file,
>> +                            int *line)
>> +{
>> +  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
>> +
>> +  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
>> +    return 0;
>> +
>> +  *file = frame_symbol->source_file;
>> +  *line = frame_symbol->source_line;
>> +  return 1;
>> +}
>> +
>>  static void
>>  print_frame (struct frame_info *frame, int print_level,
>>              enum print_what print_what, int print_args,
>> @@ -1140,6 +1166,8 @@ print_frame (struct frame_info *frame, int print_level,
>>    struct symbol *func;
>>    CORE_ADDR pc = 0;
>>    int pc_p;
>> +  const char *file;
>> +  int line;
>>
>>    pc_p = get_frame_pc_if_available (frame, &pc);
>>
>> @@ -1200,7 +1228,16 @@ print_frame (struct frame_info *frame, int print_level,
>>        args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
>>        TRY_CATCH (e, RETURN_MASK_ERROR)
>>         {
>> -         print_frame_args (func, frame, numargs, gdb_stdout);
>> +         const struct frame_symbol_info *frame_symbol;
>> +         frame_symbol = get_frame_symbol_info (frame);
>> +
>> +         if (frame_symbol != NULL)
>> +           {
>> +             if (frame_symbol->arguments != NULL)
>> +               ui_out_text (uiout, frame_symbol->arguments);
>> +           }
>> +         else
>> +           print_frame_args (func, frame, numargs, gdb_stdout);
>>         }
>>        /* FIXME: ARGS must be a list.  If one argument is a string it
>>           will have " that will not be properly escaped.  */
>> @@ -1209,7 +1246,21 @@ print_frame (struct frame_info *frame, int print_level,
>>        QUIT;
>>      }
>>    ui_out_text (uiout, ")");
>> -  if (sal.symtab)
>> +
>> +  if (find_frame_source_location (frame, &file, &line))
>> +    {
>> +      annotate_frame_source_begin ();
>> +      ui_out_wrap_hint (uiout, "   ");
>> +      ui_out_text (uiout, " at ");
>> +      annotate_frame_source_file ();
>> +      ui_out_field_string (uiout, "file", file);
>> +      annotate_frame_source_file_end ();
>> +      ui_out_text (uiout, ":");
>> +      annotate_frame_source_line ();
>> +      ui_out_field_int (uiout, "line", line);
>> +      annotate_frame_source_end ();
>> +    }
>> +  else if (sal.symtab)
>>      {
>>        const char *filename_display;
>>

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2013-12-26 18:36 [RFC][PATCH] Allow JIT unwinder provide symbol information Sasha Smundak
  2014-01-13 18:25 ` Alexander Smundak
  2014-01-13 18:46 ` Doug Evans
@ 2014-02-08  7:08 ` Yao Qi
  2014-02-10  2:16   ` Alexander Smundak
  2014-04-24 13:22 ` Pedro Alves
  3 siblings, 1 reply; 28+ messages in thread
From: Yao Qi @ 2014-02-08  7:08 UTC (permalink / raw)
  To: Sasha Smundak; +Cc: gdb-patches

On 12/27/2013 02:36 AM, Sasha Smundak wrote:
> The change proposed in this RFC is part of the effort to support
> debugging applications containing Java code executed with the help of
> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
> the backtrace of an application being debugged by the GDB modified to
> provide such support:
> 
> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
> #13 0x00007fffed000438 in <StubRoutines> ()
> 
> I.e., Myclass.main() calls Myclass.method1() calls
> Util.callObjectMethod(), which is implemented as a C function
> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
> Myclass.method1() is JIT-compiled.

IWBN to show how GDB behaves without this patch.

> 
> The implementation uses GDB's JIT API, and this RFC is the GDB patch
> needed for that. The implementation of the actual debugging support
> for Java (that is, the code that unwinds Java frames and is able to
> provide symbol information for them) is not presented here; hopefully
> it will make it into OpenJDK at some point, or will be distributed
> standalone.
> 
> GDB's current JIT API is sufficient to unwind the stack frames created
> by the Java Virtual Machine (JVM). However, it needs to be extended in
> order to be able to display correct symbol information, for two

I agree that JIT API can be extended to get correct symbol information.

> reasons. First, there is no need to add and remove symbol information
> as JIT compiles the code and disposes of it if JIT handler can provide
> this information on demand. Second, when JVM runs Java code in the
> interpreted mode, the code address in the frame points to the
> interpreter code, which is not what should be displayed.

That is a separate problem, IMO.  GDB JIT interface was designed to deal
with JIT'ed code.  In interpreted mode, it is reasonable to me
that GDB shows the interpreter code in backtrace, because JVM is just a
typical executable, which is being debugged via GDB.  I don't know how
does your patch help to this problem.

> 
> The solution proposed here is to add a "symbol provider" function to
> the unwinder interface and modify the code displaying frames to get
> the symbol information associated with the frame (function name,
> source location, arguments) from the frame's unwinder if it has
> symbol provider and use the current logic as a fallback strategy.

IMHO, it is not a good idea to add "symbol provider" to the unwinder, as
it is unrelated to unwinding.  Unwinder's job is to compute the 'next'
frame_info, given one frame_info.  Displaying the function name and
source file of a frame_info is out the scope of its responsibilities.

It is fine to register "symbol provider" to interpret frames of JIT'ed
code to function name, source file, and so forth, but I feel
uncomfortable to add "symbol provider" to the unwinder, at least to
'struct frame_unwind'.

-- 
Yao (齐尧)

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-08  7:08 ` Yao Qi
@ 2014-02-10  2:16   ` Alexander Smundak
  2014-02-11 22:00     ` Doug Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-02-10  2:16 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On Fri, Feb 7, 2014 at 11:06 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 12/27/2013 02:36 AM, Sasha Smundak wrote:
>> The change proposed in this RFC is part of the effort to support
>> debugging applications containing Java code executed with the help of
>> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
>> the backtrace of an application being debugged by the GDB modified to
>> provide such support:
>>
>> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
>> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
>> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
>> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
>> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
>> #13 0x00007fffed000438 in <StubRoutines> ()
>>
>> I.e., Myclass.main() calls Myclass.method1() calls
>> Util.callObjectMethod(), which is implemented as a C function
>> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
>> Myclass.method1() is JIT-compiled.
>
> IWBN to show how GDB behaves without this patch.

Here's what GDB shows without the plugin:
(gdb) where
#0  Java_gdbtest_Util_breakpoint (env=0x7ffff0009220,
myclass=0x7ffff649c258) at <...>/testjni.cc:24
#1  0x00007fffea013306 in ?? ()
#2  0x00007ffff649c468 in ?? ()
#3  0x00007ffff649c208 in ?? ()
#4  0x000000060569ef48 in ?? ()
#5  0x00007ffff649c260 in ?? ()
#6  0x000000060569f638 in ?? ()
#7  0x0000000000000000 in ?? ()

and this is what is shows with the plugin:
(gdb) jit-reader-load java.so
(gdb) where
#0  Java_gdbtest_Util_breakpoint (env=0x7ffff0009220,
myclass=0x7ffff649c258) at <...>/testjni.cc:24
#1  0x00007fffea013306 in gdbtest.Util.breakpoint#I () at Util.java:-1
#2  0x00007fffea0667f4 in gdbtest.Interpreted.inner+0x54[C] (this=?)
at Interpreted.java:31
#3  0x00007fffea0004e7 in <StubRoutines> ()
#4  0x00007ffff6bb6988 in JavaCalls::call_helper (result=<optimized
out>, m=<optimized out>, args=<optimized out>,
__the_thread__=<optimized out>)
    at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:422
#5  0x00007ffff6bb59d8 in JavaCalls::call (result=<optimized out>,
method=..., args=0x0, __the_thread__=0x0)
    at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:320
#6  0x00007ffff6bc579d in jni_invoke_nonstatic (env=<optimized out>,
result=0x7ffff649c620, receiver=<optimized out>, call_type=<optimized
out>,
    method_id=<optimized out>, args=0x7ffff649c5c0,
__the_thread__=0x7ffff0009000)
    at <...>/hotspot/src/share/vm/prims/jni.cpp:1414
#7  0x00007ffff6bd1586 in jni_CallVoidMethodV(JNIEnv *, jobject,
jmethodID, typedef __va_list_tag __va_list_tag *) (env=0x7ffff0009220,
    obj=<optimized out>, methodID=0x7ffff0079848, args=<optimized out>)
    at <...>/hotspot/src/share/vm/prims/jni.cpp:1958
#8  0x00007fffe181ab03 in JNIEnv_::CallVoidMethod
(this=0x7ffff0009220, obj=0x7ffff649c7e0, methodID=0x7ffff0079848)
    at <...>/jdk/include/jni.h:1054
#9  0x00007fffe181a9c2 in Java_gdbtest_Util_callObjectMethod
(env=0x7ffff0009220, myclass=0x7ffff649c810,
target_object=0x7ffff649c7e0,
    method_name=0x7ffff649c7e8) at <...>/testjni.cc:48
#10 0x00007fffea06043b in gdbtest.Util.callObjectMethod+0x15b[C]
(java.lang.Object=???) at Util.java:-1
#11 0x00007fffea066508 in gdbtest.Interpreted.method1+0x68[C] (this=?)
at Interpreted.java:18
#12 0x00007fffea006058 in gdbtest.Interpreted.main#I ([]=[...]) at
Interpreted.java:48
#13 0x00007fffea0004e7 in <StubRoutines> ()
#14 0x00007ffff6bb6988 in JavaCalls::call_helper (result=<optimized
out>, m=<optimized out>, args=<optimized out>,
__the_thread__=<optimized out>)
    at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:422
#15 0x00007ffff6bb59d8 in JavaCalls::call (result=<optimized out>,
method=..., args=0x0, __the_thread__=0x0)
    at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:320
#16 0x00007ffff6bc6501 in jni_invoke_static (env=<optimized out>,
result=0x7ffff649cc90, receiver=<optimized out>, call_type=<optimized
out>,
    method_id=<optimized out>, args=0x7ffff649cc10,
__the_thread__=0x7ffff0009000)
    at <...>/hotspot/src/share/vm/prims/jni.cpp:1338
#17 0x00007ffff6bd3008 in jni_CallStaticVoidMethod
(env=0x7ffff0009220, cls=<optimized out>, methodID=0x7ffff0079840)
    at <...>/hotspot/src/share/vm/prims/jni.cpp:2541
#18 0x00007ffff7fdc7a4 in JavaMain (_args=<optimized out>) at
../../../src/share/bin/java.c:522
#19 0x00007ffff7bc314e in start_thread () from
/usr/grte/v3/lib64/libpthread.so.0
#20 0x00007ffff763569d in clone () from /usr/grte/v3/lib64/libc.so.6
#21 0x0000000000000000 in ?? ()

(I've replaced absolute paths with <...>).
Note that unaided GDB is unable to unwind the stack correctly as JVM
does not always save the frame pointer at the expected place and
does not supply explicit unwind info.

>> ... Second, when JVM runs Java code in the
>> interpreted mode, the code address in the frame points to the
>> interpreter code, which is not what should be displayed.
>
> That is a separate problem, IMO.  GDB JIT interface was designed to deal
> with JIT'ed code.  In interpreted mode, it is reasonable to me
> that GDB shows the interpreter code in backtrace, because JVM is just a
> typical executable, which is being debugged via GDB.  I don't know how
> does your patch help to this problem.
Consider this traceback (obtained with the same code running in JVM
with JIT disabled):

#0  Java_gdbtest_Util_breakpoint ...
#1  0x00007fffea013306 in gdbtest.Util.breakpoint#I () at Util.java:-1
#2  0x00007fffea006058 in gdbtest.Interpreted.inner#I (this=@58cd65e0)
at Interpreted.java:31
#3  0x00007fffea0004e7 in <StubRoutines> ()
#4  0x00007ffff6bb6988 in JavaCalls::call_helper (...)
#5  0x00007ffff6bb59d8 in JavaCalls::call (...)
#6  0x00007ffff6bc579d in jni_invoke_nonstatic (...)
#7  0x00007ffff6bd1586 in jni_CallVoidMethodV(...)
#8  0x00007fffe181ab03 in JNIEnv_::CallVoidMethod (...)
#9  0x00007fffe181a9c2 in Java_gdbtest_Util_callObjectMethod (...)
#10 0x00007fffea013306 in gdbtest.Util.callObjectMethod#I ()
#11 0x00007fffea006058 in gdbtest.Interpreted.method1#I ()
#12 0x00007fffea006058 in gdbtest.Interpreted.main#I (...)
#13 0x00007fffea0004e7 in <StubRoutines> ()
...
Notice that the frames #2, #11, #12 have the same PC, which is the location of
the place in the interpreter where it executes 'call method' instruction.
The JIT plugin in this case additionally shows the location of the call in the
Java source code, which is more useful.

>>
>> The solution proposed here is to add a "symbol provider" function to
>> the unwinder interface and modify the code displaying frames to get
>> the symbol information associated with the frame (function name,
>> source location, arguments) from the frame's unwinder if it has
>> symbol provider and use the current logic as a fallback strategy.
>
> IMHO, it is not a good idea to add "symbol provider" to the unwinder, as
> it is unrelated to unwinding.  Unwinder's job is to compute the 'next'
> frame_info, given one frame_info.  Displaying the function name and
> source file of a frame_info is out the scope of its responsibilities.
>
> It is fine to register "symbol provider" to interpret frames of JIT'ed
> code to function name, source file, and so forth, but I feel
> uncomfortable to add "symbol provider" to the unwinder, at least to
> 'struct frame_unwind'.
I agree it would be more appropriate to rename 'frame_unwind'
to 'frame_handler'. However, it's just a refactoring that can be done
separately.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-10  2:16   ` Alexander Smundak
@ 2014-02-11 22:00     ` Doug Evans
  0 siblings, 0 replies; 28+ messages in thread
From: Doug Evans @ 2014-02-11 22:00 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: Yao Qi, gdb-patches

On Sun, Feb 9, 2014 at 6:16 PM, Alexander Smundak <asmundak@google.com> wrote:
> On Fri, Feb 7, 2014 at 11:06 PM, Yao Qi <yao@codesourcery.com> wrote:
>> On 12/27/2013 02:36 AM, Sasha Smundak wrote:
>>> The change proposed in this RFC is part of the effort to support
>>> debugging applications containing Java code executed with the help of
>>> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
>>> the backtrace of an application being debugged by the GDB modified to
>>> provide such support:
>>>
>>> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
>>> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
>>> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
>>> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
>>> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
>>> #13 0x00007fffed000438 in <StubRoutines> ()
>>>
>>> I.e., Myclass.main() calls Myclass.method1() calls
>>> Util.callObjectMethod(), which is implemented as a C function
>>> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
>>> Myclass.method1() is JIT-compiled.
>>
>> IWBN to show how GDB behaves without this patch.
>
> Here's what GDB shows without the plugin:
> (gdb) where
> #0  Java_gdbtest_Util_breakpoint (env=0x7ffff0009220,
> myclass=0x7ffff649c258) at <...>/testjni.cc:24
> #1  0x00007fffea013306 in ?? ()
> #2  0x00007ffff649c468 in ?? ()
> #3  0x00007ffff649c208 in ?? ()
> #4  0x000000060569ef48 in ?? ()
> #5  0x00007ffff649c260 in ?? ()
> #6  0x000000060569f638 in ?? ()
> #7  0x0000000000000000 in ?? ()
>
> and this is what is shows with the plugin:
> (gdb) jit-reader-load java.so
> (gdb) where
> #0  Java_gdbtest_Util_breakpoint (env=0x7ffff0009220,
> myclass=0x7ffff649c258) at <...>/testjni.cc:24
> #1  0x00007fffea013306 in gdbtest.Util.breakpoint#I () at Util.java:-1
> #2  0x00007fffea0667f4 in gdbtest.Interpreted.inner+0x54[C] (this=?)
> at Interpreted.java:31
> #3  0x00007fffea0004e7 in <StubRoutines> ()
> #4  0x00007ffff6bb6988 in JavaCalls::call_helper (result=<optimized
> out>, m=<optimized out>, args=<optimized out>,
> __the_thread__=<optimized out>)
>     at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:422
> #5  0x00007ffff6bb59d8 in JavaCalls::call (result=<optimized out>,
> method=..., args=0x0, __the_thread__=0x0)
>     at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:320
> #6  0x00007ffff6bc579d in jni_invoke_nonstatic (env=<optimized out>,
> result=0x7ffff649c620, receiver=<optimized out>, call_type=<optimized
> out>,
>     method_id=<optimized out>, args=0x7ffff649c5c0,
> __the_thread__=0x7ffff0009000)
>     at <...>/hotspot/src/share/vm/prims/jni.cpp:1414
> #7  0x00007ffff6bd1586 in jni_CallVoidMethodV(JNIEnv *, jobject,
> jmethodID, typedef __va_list_tag __va_list_tag *) (env=0x7ffff0009220,
>     obj=<optimized out>, methodID=0x7ffff0079848, args=<optimized out>)
>     at <...>/hotspot/src/share/vm/prims/jni.cpp:1958
> #8  0x00007fffe181ab03 in JNIEnv_::CallVoidMethod
> (this=0x7ffff0009220, obj=0x7ffff649c7e0, methodID=0x7ffff0079848)
>     at <...>/jdk/include/jni.h:1054
> #9  0x00007fffe181a9c2 in Java_gdbtest_Util_callObjectMethod
> (env=0x7ffff0009220, myclass=0x7ffff649c810,
> target_object=0x7ffff649c7e0,
>     method_name=0x7ffff649c7e8) at <...>/testjni.cc:48
> #10 0x00007fffea06043b in gdbtest.Util.callObjectMethod+0x15b[C]
> (java.lang.Object=???) at Util.java:-1
> #11 0x00007fffea066508 in gdbtest.Interpreted.method1+0x68[C] (this=?)
> at Interpreted.java:18
> #12 0x00007fffea006058 in gdbtest.Interpreted.main#I ([]=[...]) at
> Interpreted.java:48
> #13 0x00007fffea0004e7 in <StubRoutines> ()
> #14 0x00007ffff6bb6988 in JavaCalls::call_helper (result=<optimized
> out>, m=<optimized out>, args=<optimized out>,
> __the_thread__=<optimized out>)
>     at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:422
> #15 0x00007ffff6bb59d8 in JavaCalls::call (result=<optimized out>,
> method=..., args=0x0, __the_thread__=0x0)
>     at <...>/hotspot/src/share/vm/runtime/javaCalls.cpp:320
> #16 0x00007ffff6bc6501 in jni_invoke_static (env=<optimized out>,
> result=0x7ffff649cc90, receiver=<optimized out>, call_type=<optimized
> out>,
>     method_id=<optimized out>, args=0x7ffff649cc10,
> __the_thread__=0x7ffff0009000)
>     at <...>/hotspot/src/share/vm/prims/jni.cpp:1338
> #17 0x00007ffff6bd3008 in jni_CallStaticVoidMethod
> (env=0x7ffff0009220, cls=<optimized out>, methodID=0x7ffff0079840)
>     at <...>/hotspot/src/share/vm/prims/jni.cpp:2541
> #18 0x00007ffff7fdc7a4 in JavaMain (_args=<optimized out>) at
> ../../../src/share/bin/java.c:522
> #19 0x00007ffff7bc314e in start_thread () from
> /usr/grte/v3/lib64/libpthread.so.0
> #20 0x00007ffff763569d in clone () from /usr/grte/v3/lib64/libc.so.6
> #21 0x0000000000000000 in ?? ()
>
> (I've replaced absolute paths with <...>).
> Note that unaided GDB is unable to unwind the stack correctly as JVM
> does not always save the frame pointer at the expected place and
> does not supply explicit unwind info.
>
>>> ... Second, when JVM runs Java code in the
>>> interpreted mode, the code address in the frame points to the
>>> interpreter code, which is not what should be displayed.
>>
>> That is a separate problem, IMO.  GDB JIT interface was designed to deal
>> with JIT'ed code.  In interpreted mode, it is reasonable to me
>> that GDB shows the interpreter code in backtrace, because JVM is just a
>> typical executable, which is being debugged via GDB.  I don't know how
>> does your patch help to this problem.
> Consider this traceback (obtained with the same code running in JVM
> with JIT disabled):
>
> #0  Java_gdbtest_Util_breakpoint ...
> #1  0x00007fffea013306 in gdbtest.Util.breakpoint#I () at Util.java:-1
> #2  0x00007fffea006058 in gdbtest.Interpreted.inner#I (this=@58cd65e0)
> at Interpreted.java:31
> #3  0x00007fffea0004e7 in <StubRoutines> ()
> #4  0x00007ffff6bb6988 in JavaCalls::call_helper (...)
> #5  0x00007ffff6bb59d8 in JavaCalls::call (...)
> #6  0x00007ffff6bc579d in jni_invoke_nonstatic (...)
> #7  0x00007ffff6bd1586 in jni_CallVoidMethodV(...)
> #8  0x00007fffe181ab03 in JNIEnv_::CallVoidMethod (...)
> #9  0x00007fffe181a9c2 in Java_gdbtest_Util_callObjectMethod (...)
> #10 0x00007fffea013306 in gdbtest.Util.callObjectMethod#I ()
> #11 0x00007fffea006058 in gdbtest.Interpreted.method1#I ()
> #12 0x00007fffea006058 in gdbtest.Interpreted.main#I (...)
> #13 0x00007fffea0004e7 in <StubRoutines> ()
> ...
> Notice that the frames #2, #11, #12 have the same PC, which is the location of
> the place in the interpreter where it executes 'call method' instruction.
> The JIT plugin in this case additionally shows the location of the call in the
> Java source code, which is more useful.

Thanks for the examples!

>>>
>>> The solution proposed here is to add a "symbol provider" function to
>>> the unwinder interface and modify the code displaying frames to get
>>> the symbol information associated with the frame (function name,
>>> source location, arguments) from the frame's unwinder if it has
>>> symbol provider and use the current logic as a fallback strategy.
>>
>> IMHO, it is not a good idea to add "symbol provider" to the unwinder, as
>> it is unrelated to unwinding.  Unwinder's job is to compute the 'next'
>> frame_info, given one frame_info.  Displaying the function name and
>> source file of a frame_info is out the scope of its responsibilities.
>>
>> It is fine to register "symbol provider" to interpret frames of JIT'ed
>> code to function name, source file, and so forth, but I feel
>> uncomfortable to add "symbol provider" to the unwinder, at least to
>> 'struct frame_unwind'.
> I agree it would be more appropriate to rename 'frame_unwind'
> to 'frame_handler'. However, it's just a refactoring that can be done
> separately.

Agreed.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-01-15  0:39   ` Alexander Smundak
@ 2014-02-11 22:26     ` Doug Evans
  2014-02-12  7:50       ` Doug Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Doug Evans @ 2014-02-11 22:26 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches

On Tue, Jan 14, 2014 at 4:39 PM, Alexander Smundak <asmundak@google.com> wrote:
> I fixed the patch based on your comments, except for the one
> about using LWP for thread identification.
> Waiting for the opinions about the approach used in this RFC patch.
>
>>  > +/* Returns LWP ID of the current thread or 0.  */
>>  > +
>>  > +typedef long (gdb_get_lwp) (void);
>>
>> The term "lwp" might be an insufficiently portable concept that we either
>> need to pick a different name or use a different value here.
>> It may be sufficient to stick with "lwp" but add further comments
>> specifying how it's used so that one can translate it to whatever else
>> is used on a non-lwp-using host.
>> Alas I don't have a good answer.  Hopefully someone else here does.
> ...
>>  > +long
>>  > +jit_get_current_lwp (void)
>>  > +{
>>  > +  long lwp = ptid_get_lwp (inferior_ptid);
>>  > +
>>  > +  // ptid_get_lwp returns 0 in the remote debugging case. Try getting
>>  > +  // thread id then.
>>  > +  // TODO(asmundak): perhaps use thread id always?
>>  > +  return lwp ? lwp : ptid_get_tid (inferior_ptid);
>>  > +}
>>
>> In what situation does ptid_get_lwp return 0 for remote debugging?
> If inferior is run by the gdbserver, I see that inferior.ptid.lwp
> is 0, while inferior_ptid.tid is LWP. If inferior runs locally, it's
> the opposite:inferior_ptid.lwp is LWP while inferior_ptid.tid is 0.

Ah, righto.  Blech.

>> Also, getting back to whether "lwp" is sufficient portable,
>> how does the external code use the value?
> JVM maintains its own list of threads, using LWP as thread's unique
> identifier.Java frame unwinder code checks that the frame belongs to
> the thread it knows about.

This feels like a case where a level of indirection is sufficient, the
only hard part is picking a good name.
IOW, if we replaced "lwp" with "jti" [Jit Thread Id] then I think
that'd be sufficient (or even "jtid").
Java folks can even pretend the J is for Java and not Jit. :-)
As for whether jti{,d} is a reasonable name ... dunno.

If other's don't chime in within a week I'm going to make a Command
Decision :-) and say let's go with that.
And I'm happy to pick a different name.  I just feel uncomfortable
with using "lwp" since in the remote case while it is the lwp it comes
from ptid.tid, and other systems may not use the term "lwp".
I'm willing to be persuaded that using "lwp" here is just fine though.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-11 22:26     ` Doug Evans
@ 2014-02-12  7:50       ` Doug Evans
  2014-02-19  3:30         ` Alexander Smundak
  2014-02-25  1:19         ` Doug Evans
  0 siblings, 2 replies; 28+ messages in thread
From: Doug Evans @ 2014-02-12  7:50 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches, Pedro Alves

On Tue, Feb 11, 2014 at 2:25 PM, Doug Evans <dje@google.com> wrote:
> On Tue, Jan 14, 2014 at 4:39 PM, Alexander Smundak <asmundak@google.com> wrote:
>> I fixed the patch based on your comments, except for the one
>> about using LWP for thread identification.
>> Waiting for the opinions about the approach used in this RFC patch.
>>
>>>  > +/* Returns LWP ID of the current thread or 0.  */
>>>  > +
>>>  > +typedef long (gdb_get_lwp) (void);

Another issue that occurs to me is what if the loaded jit shared
library on some platform (not necessarily linux) wants to use
ptid.tid, even if both ptid.lwp and ptid.tid are available?

Does it make sense to provide routines that access each?

Pedro, the issue is what handle on a thread to export to the
jit-reader-load shared library.
Java for linux wants the lwp, and currently the patch will return
ptid.tid instead of ptid.lwp if  lwp == 0 to shield the shared lib
from gdb vs gdbserver thread ptid usage differences, on the assumption
that if lwp == 0 then tid is actually lwp.

On a separate note,
IIRC we still have to decide how to handle version 1 jit-reader-load
shared libs.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-12  7:50       ` Doug Evans
@ 2014-02-19  3:30         ` Alexander Smundak
  2014-02-19  3:50           ` Eli Zaretskii
                             ` (2 more replies)
  2014-02-25  1:19         ` Doug Evans
  1 sibling, 3 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-02-19  3:30 UTC (permalink / raw)
  To: gdb-patches

> On a separate note,
> IIRC we still have to decide how to handle version 1 jit-reader-load
> shared libs.The revised patch addressing the reviewer's comments is below.
Is it known if anybody is using this version?

The patch addressing review comments is below.

2013-12-19  Sasha Smundak  <asmundak@google.com>

* gdb/NEWS: Announcement.
* gdb/frame-unwind.h (frame_symbol_type): New function type.
(struct frame_unwind): Add the pointer to the frame_symbol_type.
* gdb/frame.c (get_frame_symbol_info): New function.
* gdb/frame.h (struct frame_symbol_info): Declare the struct
containing symbol information returned by the unwinder.
(get_frame_symbol_info): Declare it.
* gdb/jit-reader.in (GDB_READER_INTERFACE_VERSION): New version.
        (gdb_unwind_stash): New function type.
(gdb_find_symbol): Ditto.
(gdb_get_jtid): Ditto.
(gdb_enumerate_shared): Ditto.
(gdb_unwind_debug_flag): Ditto.
(gdb_architecture_name): Ditto.
(gdb_pointer_size): Ditto.
(struct gdb_unwind_callbacks): Add members pointing to
gdb_unwind_stash, unwind_reg_get, gdb_find_symbol, gdb_get_jtid,
gdb_enumerate_shared, gdb_unwind_debug_flag,
gdb_architecture_name and gdb_pointer_size functions.
(enum jit_frame_symbol_attr): New enum.
(enum jit_frame_language): Ditto.
(gdb_get_symbol_attr): New function type.
(struct gdb_reader_funcs): Add member pointing to
gdb_get_symbol_attr function.
* gdb/jit.c: Include arch-utils.h.
Include solist.h.
Include ptid.h.
(jit_prepend_unwinder): Declare.
(jit_find_symbol_address): Declare.
(jit_get_current_jtid): Declare.
(jit_enumerate_shared): Declare.
(jit_unwind_debug_flag): Declare.
(jit_unwind_architecture_name): Declare.
(jit_unwind_pointer_size): Declare.
(jit_unwind_debug): Declare.
(show_jit_unwind_debug): New function.
(jit_breakpoint_re_set_internal): Add the call to
jit_prepend_unwinder.
(struct jit_unwind_private): Add a member containing the
pointer to JIT reader's private area.
(jit_get_register_from_frame): New function returning a
register from the given frame (refactored from
jit_unwind_reg_get_impl).
(jit_unwind_reg_get_impl): Now a wrapper around
jit_get_register_from_frame.
(jit_unwind_get_cpu_register_value): New function to get
"live" registers.
(jit_stash_impl): Allocate private data area for the JIT reader.
(jit_dealloc_cache): Free JIT reader private area.
(jit_set_unwind_callbacks_vtable): New function, refactored
from two different places where callbacks were set up.
(jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
(jit_frame_language_to_language): New function converting
JIT reader's language type to the GDB internal language type.
(jit_symbol): New function returning symbol information supplied
by the JIT reader.
(jit_frame_unwind): Add the pointer to jit_symbol.
(jit_find_symbol_address): New GDB callback for the JIT reader
returning symbols's address.
(jit_get_current_jtid): New GDB callback for the JIT reader
returning the current thread ID.
(jit_enumerate_shared): New GDB callback for the JIT reader to
traverse inferior's shared objects.
(jit_unwind_debug_flag): New GDB callback returning JIT unwind
debugging flags.
(jit_unwind_architecture_name): New GDB callback returning
inferior's architecture name.
(jit_unwind_pointer_size): New GDB callback returning inferior's
pointer size.
(_initialize_jit): Register 'show/set debug jitunwind'
subcommand.
* gdb/stack.c (find_frame_funname): If symbol information from
the unwindeer if available.
(find_frame_source_location): New function to return symbol
source location from the unwinder if available.
(print_frame): Use arguments information and source location
from the unwinder if available.

diff --git a/gdb/NEWS b/gdb/NEWS
index b54a414..34b7dc6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@

 *** Changes since GDB 7.7

+* Revised JIT interface allows creating plugins that display
+  JIT_specific symbol information.
+
 * Guile scripting

   GDB now has support for scripting using Guile.  Whether this is
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 9507be2..407b882 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct
frame_info *self,
 typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
  void **this_prologue_cache);

+/* Return unwinder-specific symbol info or NULL.  */
+
+typedef const struct frame_symbol_info* (frame_symbol_ftype)
+    (struct frame_info *this_frame,
+     void **this_prologue_cache);
+
 struct frame_unwind
 {
   /* The frame's type.  Should this instead be a collection of
@@ -154,6 +160,9 @@ struct frame_unwind
   frame_sniffer_ftype *sniffer;
   frame_dealloc_cache_ftype *dealloc_cache;
   frame_prev_arch_ftype *prev_arch;
+  /* For the JIT interface to give external code a chance to supply
+     symbol information describing the frame.  */
+  frame_symbol_ftype *symbol_info;
 };

 /* Register a frame unwinder, _prepending_ it to the front of the
diff --git a/gdb/frame.c b/gdb/frame.c
index 8cd607b..72331ac 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2307,6 +2307,16 @@ find_frame_sal (struct frame_info *frame,
struct symtab_and_line *sal)
   (*sal) = find_pc_line (pc, notcurrent);
 }

+/* If frame-specific unwinder has symbol info, return it.  */
+
+const struct frame_symbol_info *
+get_frame_symbol_info (struct frame_info *fi)
+{
+  return ((fi->unwind->symbol_info == NULL)
+      ? NULL
+      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
+}
+
 /* Per "frame.h", return the ``address'' of the frame.  Code should
    really be using get_frame_id().  */
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index e451a93..c7365c7 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -491,6 +491,23 @@ enum unwind_stop_reason
 #undef FIRST_ERROR
   };

+/* Unwinder-specific symbol information.  */
+
+struct frame_symbol_info
+{
+  const char *function;
+  const char *source_file;
+  const char *arguments;
+  enum language language;
+  int source_line;
+};
+
+/* Return symbol information supplied by the unwinder. If this frame's
+   unwinder implements frame_unwind.symbol_info method, calls it and
+   returns the result, otherwise returns NULL.  */
+
+const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
+
 /* Return the reason why we can't unwind past this frame.  */

 enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
index 6e2ee64..a6a0ab1 100644
--- a/gdb/jit-reader.in
+++ b/gdb/jit-reader.in
@@ -26,7 +26,7 @@ extern "C" {

 /* Versioning information.  See gdb_reader_funcs.  */

-#define GDB_READER_INTERFACE_VERSION 1
+#define GDB_READER_INTERFACE_VERSION 2

 /* Readers must be released under a GPL compatible license.  To
    declare that the reader is indeed released under a GPL compatible
@@ -260,6 +260,43 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
 typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
                                    struct gdb_reg_value *val);

+/* Provides access to the stashed data associated with the current frame.
+   On first call, allocates the memory and zeroes it. On subsequent calls
+   returns the pointer to the allocated memory.  */
+
+typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
+                                   size_t size);
+
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches SYMBOL_NAME. Return symbol's
+   address, 0 if symbol is unknown.
+   If it is a C++ symbol, SYMBOL_NAME is in a mangled form.  */
+
+typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol_name);
+
+/* Returns thread ID of the current thread or 0.  */
+
+typedef long (gdb_get_jtid) (void);
+
+/* Iterate over shared objects present in the inferior, calling given
+   callback function for each shared object. The callback is passed
+   the shared object file name, expanded to something GDB can open,
+   and data pointer.  Once the callback function returns non-zero,
+   stop iteration and return shared object file name.  */
+
+typedef const char * (gdb_enumerate_shared) (int (*) (const char *so_name,
+                                                      void *data),
+                                             void *data);
+
+/* Returns debug flags setting for the unwinding.  */
+typedef unsigned int (gdb_unwind_debug_flag) (void);
+
+/* Returns inferior's architecture name.  */
+typedef const char * (gdb_architecture_name) (void);
+
+/* Returns inferior's pointer size in bytes.  */
+typedef size_t (gdb_pointer_size)(void);
+
 /* This struct is passed to unwind in gdb_reader_funcs, and is to be
    used to unwind the current frame (current being the frame whose
    registers can be read using reg_get) into the earlier frame.  The
@@ -270,7 +307,14 @@ struct gdb_unwind_callbacks
   gdb_unwind_reg_get *reg_get;
   gdb_unwind_reg_set *reg_set;
   gdb_target_read *target_read;
-
+  gdb_unwind_stash *stash;
+  gdb_unwind_reg_get *cpu_reg_get;
+  gdb_find_symbol *find_symbol;
+  gdb_get_jtid *get_jtid;
+  gdb_enumerate_shared *enumerate_shared;
+  gdb_unwind_debug_flag *debug_flag;
+  gdb_architecture_name *architecture_name;
+  gdb_pointer_size *pointer_size;
   /* For internal use by GDB.  */
   void *priv_data;
 };
@@ -306,6 +350,39 @@ typedef enum gdb_status (gdb_unwind_frame)
(struct gdb_reader_funcs *self,
 typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
                                                 struct
gdb_unwind_callbacks *c);

+enum jit_frame_symbol_attr {
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
+  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
+};
+
+enum jit_frame_language {
+  frame_language_unknown,           /* Language not known */
+  frame_language_auto,              /* Placeholder for automatic setting */
+  frame_language_c,                 /* C */
+  frame_language_cplus,             /* C++ */
+  frame_language_d,                 /* D */
+  frame_language_go,                /* Go */
+  frame_language_objc,              /* Objective-C */
+  frame_language_java,              /* Java */
+  frame_language_fortran,           /* Fortran */
+  frame_language_m2,                /* Modula-2 */
+  frame_language_asm,               /* Assembly language */
+  frame_language_pascal,            /* Pascal */
+  frame_language_ada,               /* Ada */
+  frame_language_opencl,            /* OpenCL */
+  frame_language_minimal,           /* All other languages, minimal
support only */
+  frame_nr_languages
+};
+
+/* Return given attribute of a symbol associated with the current frame.  */
+
+typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self,
+                                      struct gdb_unwind_callbacks *c,
+                                      enum jit_frame_symbol_attr attr);
+
 /* Called when a reader is being unloaded.  This function should also
    free SELF, if required.  */

@@ -336,6 +413,7 @@ struct gdb_reader_funcs
   gdb_read_debug_info *read;
   gdb_unwind_frame *unwind;
   gdb_get_frame_id *get_frame_id;
+  gdb_get_symbol_attr *get_symbol_attr;
   gdb_destroy_reader *destroy;
 };

diff --git a/gdb/jit.c b/gdb/jit.c
index 546b3b6..147ab52 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -21,6 +21,7 @@

 #include "jit.h"
 #include "jit-reader.h"
+#include "arch-utils.h"
 #include "block.h"
 #include "breakpoint.h"
 #include "command.h"
@@ -33,6 +34,7 @@
 #include "observer.h"
 #include "objfiles.h"
 #include "regcache.h"
+#include "solist.h"
 #include "symfile.h"
 #include "symtab.h"
 #include "target.h"
@@ -40,6 +42,7 @@
 #include <sys/stat.h>
 #include "exceptions.h"
 #include "gdb_bfd.h"
+#include "ptid.h"

 static const char *jit_reader_dir = NULL;

@@ -53,6 +56,22 @@ static const struct program_space_data
*jit_program_space_data = NULL;

 static void jit_inferior_init (struct gdbarch *gdbarch);

+static void jit_prepend_unwinder (struct gdbarch *gdbarch);
+
+static CORE_ADDR jit_unwind_find_symbol_address (const char *);
+
+static long jit_unwind_get_current_jtid (void);
+
+static const char *jit_unwind_enumerate_shared (int (*callback) (const char *,
+                                                          void *),
+                                         void *);
+
+static unsigned int jit_unwind_debug_flag (void);
+
+static const char *jit_unwind_architecture_name (void);
+
+static size_t jit_unwind_pointer_size (void);
+
 /* An unwinder is registered for every gdbarch.  This key is used to
    remember if the unwinder has been registered for a particular
    gdbarch.  */
@@ -70,6 +89,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
 }

+static unsigned jit_unwind_debug;
+
+static void
+show_jit_unwind_debug (struct ui_file *file, int from_tty,
+                       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
+}
+
 struct target_buffer
 {
   CORE_ADDR base;
@@ -1020,6 +1048,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;

+  jit_prepend_unwinder (gdbarch);
+
   if (ps_data->objfile == NULL)
     {
       /* Lookup the registration symbol.  If it is missing, then we
@@ -1076,6 +1106,12 @@ struct jit_unwind_private

   /* The frame being unwound.  */
   struct frame_info *this_frame;
+
+  /* Symbol associated with this frame.  */
+  struct frame_symbol_info symbol_info;
+
+  /* Memory allocated on behalf of JIT handler.  */
+  void *stash;
 };

 /* Sets the value of a particular register in this frame.  */
@@ -1110,29 +1146,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
   xfree (value);
 }

-/* Get the value of register REGNUM in the previous frame.  */
+/* Get the value of register REGNUM in the given frame.  */

 static struct gdb_reg_value *
-jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
 {
-  struct jit_unwind_private *priv;
   struct gdb_reg_value *value;
   int gdb_reg, size;
   struct gdbarch *frame_arch;

-  priv = cb->priv_data;
-  frame_arch = get_frame_arch (priv->this_frame);
+  frame_arch = get_frame_arch (this_frame);

   gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
   size = register_size (frame_arch, gdb_reg);
   value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
-  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
+  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
    value->value);
   value->size = size;
   value->free = reg_value_free_impl;
   return value;
 }

+/* Get the value of register REGNUM in the previous frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  return jit_get_register_from_frame (
+      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
+}
+
+/* Get the value of the register REGNUM in the innermost frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  struct frame_info *frame
+      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
+  struct frame_info *next_frame;
+
+  while ((next_frame = get_next_frame (frame)) != NULL)
+    frame = next_frame;
+  return jit_get_register_from_frame (frame, regnum);
+}
+
+/* Return memory area allocated for the JIT reader.  The area is allocated on
+   demand (the first time this function is called with non-zero size).
+   Calling this function with size == 0 will not allocate memory but will
+   return its address if it has been already allocated.  */
+
+static void *
+jit_unwind_stash_impl (struct gdb_unwind_callbacks *cb, size_t size)
+{
+  struct jit_unwind_private *priv_data = cb->priv_data;
+
+  if (!priv_data->stash && size)
+    priv_data->stash = xcalloc (size, 1);
+  return priv_data->stash;
+}
+
 /* gdb_reg_value has a free function, which must be called on each
    saved register value.  */

@@ -1151,9 +1223,27 @@ jit_dealloc_cache (struct frame_info
*this_frame, void *cache)
       priv_data->registers[i]->free (priv_data->registers[i]);

   xfree (priv_data->registers);
+  xfree (priv_data->stash);
   xfree (priv_data);
 }

+static void
+jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
+                                 int allow_reg_set)
+{
+  callbacks->reg_get = jit_unwind_reg_get_impl;
+  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
+  callbacks->target_read = jit_target_read_impl;
+  callbacks->stash = jit_unwind_stash_impl;
+  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
+  callbacks->find_symbol = jit_unwind_find_symbol_address;
+  callbacks->get_jtid = jit_unwind_get_current_jtid;
+  callbacks->enumerate_shared = jit_unwind_enumerate_shared;
+  callbacks->debug_flag = jit_unwind_debug_flag;
+  callbacks->architecture_name = jit_unwind_architecture_name;
+  callbacks->pointer_size = jit_unwind_pointer_size;
+}
+
 /* The frame sniffer for the pseudo unwinder.

    While this is nominally a frame sniffer, in the case where the JIT
@@ -1170,9 +1260,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
   struct gdb_unwind_callbacks callbacks;
   struct gdb_reader_funcs *funcs;

-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = jit_unwind_reg_set_impl;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 1);

   if (loaded_jit_reader == NULL)
     return 0;
@@ -1226,9 +1314,7 @@ jit_frame_this_id (struct frame_info
*this_frame, void **cache,

   /* We don't expect the frame_id function to set any registers, so we
      set reg_set to NULL.  */
-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = NULL;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
   callbacks.priv_data = &private;

   gdb_assert (loaded_jit_reader);
@@ -1258,6 +1344,73 @@ jit_frame_prev_register (struct frame_info
*this_frame, void **cache, int reg)
     return frame_unwind_got_optimized (this_frame, reg);
 }

+/* Convert jit_frame_language enum to GDB's internal language enum.  */
+
+static enum language
+jit_frame_language_to_language (enum jit_frame_language frame_language)
+{
+#define LANG_CASE(x)  frame_language_##x: return language_##x
+  switch (frame_language)
+    {
+      LANG_CASE (auto);
+      LANG_CASE (c);
+      LANG_CASE (cplus);
+      LANG_CASE (d);
+      LANG_CASE (go);
+      LANG_CASE (objc);
+      LANG_CASE (java);
+      LANG_CASE (fortran);
+      LANG_CASE (m2);
+      LANG_CASE (asm);
+      LANG_CASE (pascal);
+      LANG_CASE (ada);
+      LANG_CASE (opencl);
+      LANG_CASE (minimal);
+      default:
+        return language_unknown;
+    }
+#undef LANG_CASE
+}
+
+/* Returns unwinder-specific symbol info.  */
+
+static const struct frame_symbol_info *
+jit_symbol (struct frame_info *this_frame, void **cache)
+{
+  struct gdb_reader_funcs *funcs;
+  struct gdb_unwind_callbacks callbacks;
+  struct jit_unwind_private *priv_data;
+
+  if (*cache == NULL)
+    return NULL;
+
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
+  priv_data = *cache;
+  callbacks.priv_data = priv_data;
+
+  gdb_assert (loaded_jit_reader);
+
+  funcs = loaded_jit_reader->functions;
+  priv_data->symbol_info.function
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
+  priv_data->symbol_info.language
+      = jit_frame_language_to_language (
+          (enum jit_frame_language)
+          (funcs->get_symbol_attr (funcs, &callbacks,
+                                   JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));
+  priv_data->symbol_info.source_file
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
+  priv_data->symbol_info.source_line = (int)(uintptr_t)
+      funcs->get_symbol_attr (funcs, &callbacks,
+                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
+  priv_data->symbol_info.arguments
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
+  return &priv_data->symbol_info;
+}
+
 /* Relay everything back to the unwinder registered by the JIT debug
    info reader.*/

@@ -1267,9 +1420,11 @@ static const struct frame_unwind jit_frame_unwind =
   default_frame_unwind_stop_reason,
   jit_frame_this_id,
   jit_frame_prev_register,
-  NULL,
+  NULL,  /* frame_data */
   jit_frame_sniffer,
-  jit_dealloc_cache
+  jit_dealloc_cache,
+  NULL,  /* prev_arch */
+  jit_symbol,
 };


@@ -1310,8 +1465,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");

-  jit_prepend_unwinder (gdbarch);
-
   ps_data = get_jit_program_space_data ();
   if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
     return;
@@ -1443,6 +1596,67 @@ free_objfile_data (struct objfile *objfile, void *data)
   xfree (data);
 }

+/* Locates exported symbol in the target and returns its address.  */
+
+CORE_ADDR
+jit_unwind_find_symbol_address (const char *symbol_name)
+{
+  struct minimal_symbol *min_symbol
+      = lookup_minimal_symbol (symbol_name, NULL, NULL);
+
+  return (min_symbol == NULL) ? 0 : SYMBOL_VALUE_ADDRESS (min_symbol);
+}
+
+/* Returns thread ID of the inferior's current thread.  */
+
+long
+jit_unwind_get_current_jtid (void)
+{
+  long jtid = ptid_get_lwp (inferior_ptid);
+
+  /* ptid_get_lwp returns 0 if inferior is run via gdbserver.
+     However, ptid_get_tid returns 0 for the local inferior.  Ugly.  */
+  return jtid ? jtid : ptid_get_tid (inferior_ptid);
+}
+
+/* Calls provided function for every shared object currently loaded into
+   the inferior until the function returns non zero.  */
+
+const char *
+jit_unwind_enumerate_shared (int (*callback) (const char *so_name,
+                                              void *data),
+                             void *data)
+{
+  struct so_list *so;
+
+  for (so = master_so_list (); so; so = so->next)
+    {
+      if (callback (so->so_name, data))
+        return so->so_name;
+    }
+  return NULL;
+}
+
+static unsigned int
+jit_unwind_debug_flag (void)
+{
+  return jit_unwind_debug;
+}
+
+
+/* Return current architecture's string name.  */
+static const char *
+jit_unwind_architecture_name (void)
+{
+  return gdbarch_bfd_arch_info (target_gdbarch ())->printable_name;
+}
+
+static size_t
+jit_unwind_pointer_size (void)
+{
+  return gdbarch_ptr_bit (target_gdbarch()) / 8;
+}
+
 /* Initialize the jit_gdbarch_data slot with an instance of struct
    jit_gdbarch_data_type */

@@ -1472,6 +1686,13 @@ _initialize_jit (void)
      NULL,
      show_jit_debug,
      &setdebuglist, &showdebuglist);
+  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
+                             _("Set JIT frame unwinder debug."),
+                             _("Show JIT frame unwinder debug."),
+                             _("A collection of bit flags for debugging."),
+                             NULL,
+                             show_jit_unwind_debug,
+                             &setdebuglist, &showdebuglist);

   observer_attach_inferior_exit (jit_inferior_exit_hook);
   observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
diff --git a/gdb/stack.c b/gdb/stack.c
index 54553bc..3bcc54e 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1038,6 +1038,15 @@ find_frame_funname (struct frame_info *frame,
char **funname,
     enum language *funlang, struct symbol **funcp)
 {
   struct symbol *func;
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      *funname = xstrdup (frame_symbol->function);
+      *funlang = frame_symbol->language;
+      *funcp = NULL;
+      return;
+    }

   *funname = NULL;
   *funlang = language_unknown;
@@ -1125,6 +1134,23 @@ find_frame_funname (struct frame_info *frame,
char **funname,
     }
 }

+/* Fill unwinder-specific source location for the frame if available and
+   return 1.  Otherwise return 0.  */
+
+static int
+find_frame_source_location (struct frame_info *fi, const char **file,
+                            int *line)
+{
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
+
+  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
+    return 0;
+
+  *file = frame_symbol->source_file;
+  *line = frame_symbol->source_line;
+  return 1;
+}
+
 static void
 print_frame (struct frame_info *frame, int print_level,
      enum print_what print_what, int print_args,
@@ -1140,6 +1166,8 @@ print_frame (struct frame_info *frame, int print_level,
   struct symbol *func;
   CORE_ADDR pc = 0;
   int pc_p;
+  const char *file;
+  int line;

   pc_p = get_frame_pc_if_available (frame, &pc);

@@ -1200,7 +1228,16 @@ print_frame (struct frame_info *frame, int print_level,
       args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
       TRY_CATCH (e, RETURN_MASK_ERROR)
  {
-  print_frame_args (func, frame, numargs, gdb_stdout);
+  const struct frame_symbol_info *frame_symbol;
+  frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      if (frame_symbol->arguments != NULL)
+ ui_out_text (uiout, frame_symbol->arguments);
+    }
+  else
+    print_frame_args (func, frame, numargs, gdb_stdout);
  }
       /* FIXME: ARGS must be a list.  If one argument is a string it
   will have " that will not be properly escaped.  */
@@ -1209,7 +1246,21 @@ print_frame (struct frame_info *frame, int print_level,
       QUIT;
     }
   ui_out_text (uiout, ")");
-  if (sal.symtab)
+
+  if (find_frame_source_location (frame, &file, &line))
+    {
+      annotate_frame_source_begin ();
+      ui_out_wrap_hint (uiout, "   ");
+      ui_out_text (uiout, " at ");
+      annotate_frame_source_file ();
+      ui_out_field_string (uiout, "file", file);
+      annotate_frame_source_file_end ();
+      ui_out_text (uiout, ":");
+      annotate_frame_source_line ();
+      ui_out_field_int (uiout, "line", line);
+      annotate_frame_source_end ();
+    }
+  else if (sal.symtab)
     {
       const char *filename_display;


On Tue, Feb 11, 2014 at 11:50 PM, Doug Evans <dje@google.com> wrote:
> On Tue, Feb 11, 2014 at 2:25 PM, Doug Evans <dje@google.com> wrote:
>> On Tue, Jan 14, 2014 at 4:39 PM, Alexander Smundak <asmundak@google.com> wrote:
>>> I fixed the patch based on your comments, except for the one
>>> about using LWP for thread identification.
>>> Waiting for the opinions about the approach used in this RFC patch.
>>>
>>>>  > +/* Returns LWP ID of the current thread or 0.  */
>>>>  > +
>>>>  > +typedef long (gdb_get_lwp) (void);
>
> Another issue that occurs to me is what if the loaded jit shared
> library on some platform (not necessarily linux) wants to use
> ptid.tid, even if both ptid.lwp and ptid.tid are available?
>
> Does it make sense to provide routines that access each?
>
> Pedro, the issue is what handle on a thread to export to the
> jit-reader-load shared library.
> Java for linux wants the lwp, and currently the patch will return
> ptid.tid instead of ptid.lwp if  lwp == 0 to shield the shared lib
> from gdb vs gdbserver thread ptid usage differences, on the assumption
> that if lwp == 0 then tid is actually lwp.
>
> On a separate note,
> IIRC we still have to decide how to handle version 1 jit-reader-load
> shared libs.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-19  3:30         ` Alexander Smundak
@ 2014-02-19  3:50           ` Eli Zaretskii
  2014-02-19  5:23             ` Alexander Smundak
  2014-04-11 18:47           ` Doug Evans
  2014-04-11 18:58           ` Doug Evans
  2 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-02-19  3:50 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches

> Date: Tue, 18 Feb 2014 19:30:43 -0800
> From: Alexander Smundak <asmundak@google.com>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,9 @@
> 
>  *** Changes since GDB 7.7
> 
> +* Revised JIT interface allows creating plugins that display
> +  JIT_specific symbol information.
     ^^^^^^^^^^^^
Did you mean "JIT-specific" here?

Thanks.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-19  3:50           ` Eli Zaretskii
@ 2014-02-19  5:23             ` Alexander Smundak
  0 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-02-19  5:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Tue, Feb 18, 2014 at 7:50 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> +  JIT_specific symbol information.
>      ^^^^^^^^^^^^
> Did you mean "JIT-specific" here?
Yes. Sorry, will fix.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-12  7:50       ` Doug Evans
  2014-02-19  3:30         ` Alexander Smundak
@ 2014-02-25  1:19         ` Doug Evans
  2014-02-25  3:00           ` Alexander Smundak
  2014-03-11  1:46           ` Alexander Smundak
  1 sibling, 2 replies; 28+ messages in thread
From: Doug Evans @ 2014-02-25  1:19 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches, Pedro Alves

On Tue, Feb 11, 2014 at 11:50 PM, Doug Evans <dje@google.com> wrote:
> On Tue, Feb 11, 2014 at 2:25 PM, Doug Evans <dje@google.com> wrote:
>> On Tue, Jan 14, 2014 at 4:39 PM, Alexander Smundak <asmundak@google.com> wrote:
>>> I fixed the patch based on your comments, except for the one
>>> about using LWP for thread identification.
>>> Waiting for the opinions about the approach used in this RFC patch.
>>>
>>>>  > +/* Returns LWP ID of the current thread or 0.  */
>>>>  > +
>>>>  > +typedef long (gdb_get_lwp) (void);
>
> Another issue that occurs to me is what if the loaded jit shared
> library on some platform (not necessarily linux) wants to use
> ptid.tid, even if both ptid.lwp and ptid.tid are available?
>
> Does it make sense to provide routines that access each?
>
> Pedro, the issue is what handle on a thread to export to the
> jit-reader-load shared library.
> Java for linux wants the lwp, and currently the patch will return
> ptid.tid instead of ptid.lwp if  lwp == 0 to shield the shared lib
> from gdb vs gdbserver thread ptid usage differences, on the assumption
> that if lwp == 0 then tid is actually lwp.
>
> On a separate note,
> IIRC we still have to decide how to handle version 1 jit-reader-load
> shared libs.

Hi all.
In an attempt to keep this patch moving along here are my current thoughts.

The lwp vs tid issue has been resolved by cleaning up gdb's own
internal usage of the values so now a remote connection should provide
the user the same values as a local connection.

And given that there are two values, I'm less inclined to invent
something and think we should just go with gdb_get_lwp for now.  Later
we can add gdb_get_tid if a user comes along that needs it.  [I'm
happy to add it now of course if someone really wants to.]

Thus I think(!) the only remaining issues are:
- jit-reader-load version 1 support.
- update documentation
- testcase for new functionality
- testcase to verify version 1 API still works
We can't break jit readers that have been compiled with the version 1 API.
[Well, IWBN if we had a published mechanism to migrate users of
deprecated APIs to newer versions, but that's a separate discussion.]

Can you update the patch to handle the remaining TODOs?
I can do that if you want, just let me know.
Enough time has passed for comments that I think we can proceed with
the final details.
[I didn't audit your last patch/changelog for code style and other
nits.  I'm saving that for once all the main TODOs are done.]

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-25  1:19         ` Doug Evans
@ 2014-02-25  3:00           ` Alexander Smundak
  2014-03-11  1:46           ` Alexander Smundak
  1 sibling, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-02-25  3:00 UTC (permalink / raw)
  To: gdb-patches

I will address the remaining issues in two days and will post the new patch.

On Mon, Feb 24, 2014 at 5:19 PM, Doug Evans <dje@google.com> wrote:
> On Tue, Feb 11, 2014 at 11:50 PM, Doug Evans <dje@google.com> wrote:
>> On Tue, Feb 11, 2014 at 2:25 PM, Doug Evans <dje@google.com> wrote:
>>> On Tue, Jan 14, 2014 at 4:39 PM, Alexander Smundak <asmundak@google.com> wrote:
>>>> I fixed the patch based on your comments, except for the one
>>>> about using LWP for thread identification.
>>>> Waiting for the opinions about the approach used in this RFC patch.
>>>>
>>>>>  > +/* Returns LWP ID of the current thread or 0.  */
>>>>>  > +
>>>>>  > +typedef long (gdb_get_lwp) (void);
>>
>> Another issue that occurs to me is what if the loaded jit shared
>> library on some platform (not necessarily linux) wants to use
>> ptid.tid, even if both ptid.lwp and ptid.tid are available?
>>
>> Does it make sense to provide routines that access each?
>>
>> Pedro, the issue is what handle on a thread to export to the
>> jit-reader-load shared library.
>> Java for linux wants the lwp, and currently the patch will return
>> ptid.tid instead of ptid.lwp if  lwp == 0 to shield the shared lib
>> from gdb vs gdbserver thread ptid usage differences, on the assumption
>> that if lwp == 0 then tid is actually lwp.
>>
>> On a separate note,
>> IIRC we still have to decide how to handle version 1 jit-reader-load
>> shared libs.
>
> Hi all.
> In an attempt to keep this patch moving along here are my current thoughts.
>
> The lwp vs tid issue has been resolved by cleaning up gdb's own
> internal usage of the values so now a remote connection should provide
> the user the same values as a local connection.
>
> And given that there are two values, I'm less inclined to invent
> something and think we should just go with gdb_get_lwp for now.  Later
> we can add gdb_get_tid if a user comes along that needs it.  [I'm
> happy to add it now of course if someone really wants to.]
>
> Thus I think(!) the only remaining issues are:
> - jit-reader-load version 1 support.
> - update documentation
> - testcase for new functionality
> - testcase to verify version 1 API still works
> We can't break jit readers that have been compiled with the version 1 API.
> [Well, IWBN if we had a published mechanism to migrate users of
> deprecated APIs to newer versions, but that's a separate discussion.]
>
> Can you update the patch to handle the remaining TODOs?
> I can do that if you want, just let me know.
> Enough time has passed for comments that I think we can proceed with
> the final details.
> [I didn't audit your last patch/changelog for code style and other
> nits.  I'm saving that for once all the main TODOs are done.]

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-25  1:19         ` Doug Evans
  2014-02-25  3:00           ` Alexander Smundak
@ 2014-03-11  1:46           ` Alexander Smundak
  1 sibling, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-03-11  1:46 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches, Pedro Alves

I apologize for the delay.

On Mon, Feb 24, 2014 at 5:19 PM, Doug Evans <dje@google.com> wrote:
> Thus I think(!) the only remaining issues are:
> - jit-reader-load version 1 support.
> - update documentation
> - testcase for new functionality
> - testcase to verify version 1 API still works
> We can't break jit readers that have been compiled with the version 1 API.
I started with writing the testcase for this (attached below). The test checks
that a JIT reader can help GDB unwind the stack that could not be unwound
otherwise. That is, the test application contains a function that deliberately
corrupts the address of the outer frame when called (and then restores it).
The test sets the breakpoint at the location where the backtrace is broken
and verifies that the backtrace is shown correctly only in the presence of
a JIT reader (which happens to know where the correct outer frame was
saved).
This test fails with upstream GDB because JIT reader's unwinder is never
called.
Which IMHO is an indication that JIT reader in its current form is unlikely
to be used. And the absence of any tests that involve loading JIT reader
points in the same direction.

Sasha

diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader.c
b/gdb/testsuite/gdb.arch/amd64-jit-reader.c
new file mode 100644
index 0000000..2887abd
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader.c
@@ -0,0 +1,117 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include "jit-reader.h"
+
+GDB_DECLARE_GPL_COMPATIBLE_READER
+
+extern struct gdb_reader_funcs *gdb_init_reader (void);
+
+static enum gdb_status debug_info_provider (struct gdb_reader_funcs *self,
+                                            struct gdb_symbol_callbacks *cb,
+                                            void *memory, long memory_sz);
+static enum gdb_status frame_unwinder (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb);
+static struct gdb_frame_id frame_id_provider(struct gdb_reader_funcs *self,
+                                             struct gdb_unwind_callbacks *cb);
+static void destructor (struct gdb_reader_funcs *self);
+
+#define DATA_COOKIE 0xdeaffead
+static int jit_data = DATA_COOKIE;
+
+static struct gdb_reader_funcs jit_reader_funcs =
+{
+  1,  /* version */
+  (void *)&jit_data,
+  debug_info_provider,
+  frame_unwinder,
+  frame_id_provider,
+  destructor
+};
+
+struct gdb_reader_funcs *
+gdb_init_reader (void)
+{
+  fprintf (stderr, "Test JIT reader loaded.\n");
+  return &jit_reader_funcs;
+}
+
+enum gdb_status
+debug_info_provider (struct gdb_reader_funcs *self,
+                     struct gdb_symbol_callbacks *cb,
+                     void *memory, long memory_sz)
+{
+  if (*(int *)(self->priv_data) != DATA_COOKIE)
+    return GDB_FAIL;
+  fprintf (stderr, "Debug info provider called for %p..%p.\n",
+           memory, (void *)(memory_sz + (char *)memory));
+  return GDB_SUCCESS;
+}
+
+void
+destructor (struct gdb_reader_funcs *self)
+{
+  fprintf (stderr, "Test JIT reader unloaded.\n");
+  return;
+}
+
+static GDB_CORE_ADDR
+_get_frame_register (struct gdb_unwind_callbacks *cb, int reg_no)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  GDB_CORE_ADDR reg_value = *(GDB_CORE_ADDR *)reg->value;
+  reg->free (reg);
+  return reg_value;
+}
+
+static void
+_set_frame_register (struct gdb_unwind_callbacks *cb, int reg_no,
+                    GDB_CORE_ADDR value)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  *(GDB_CORE_ADDR *)(reg->value) = value;
+  reg->defined = 1;
+  (cb->reg_set)(cb, reg_no, reg);
+}
+
+#define REG_BP 6
+#define REG_PC 16
+#define REG_SP 7
+enum gdb_status
+frame_unwinder (struct gdb_reader_funcs *self,
+                struct gdb_unwind_callbacks *cb)
+{
+  GDB_CORE_ADDR bp = _get_frame_register (cb, REG_BP);
+  GDB_CORE_ADDR pc = _get_frame_register (cb, REG_PC);
+  GDB_CORE_ADDR sp = _get_frame_register (cb, REG_SP);
+  GDB_CORE_ADDR word;
+  cb->target_read(bp, &word, 8);
+  if (word != bp)
+    return GDB_FAIL;
+
+  /* We are at the frame that xxx-jit-unwind program altered for us.
+     We have:
+      BP-8   +--------------+
+             | Previous BP  |
+      BP     +--------------+
+             | BP (wrong!)  |
+      BP+8   +--------------+
+             | Previous PC  |
+             +--------------+
+    Save outer frame's BP, PC and SP.  */
+  cb->target_read (bp - 8, &word, 8);
+  _set_frame_register (cb, REG_BP, word);
+  cb->target_read (bp + 8, &word, 8);
+  _set_frame_register (cb, REG_PC, word);
+  _set_frame_register (cb, REG_SP, bp + 16);
+  return GDB_SUCCESS;
+}
+
+struct gdb_frame_id frame_id_provider (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb)
+{
+  struct gdb_frame_id frame_id =
+      { _get_frame_register (cb, REG_PC),
+        _get_frame_register (cb, REG_SP) };
+  fprintf (stderr, "frame_id_provider called.\n");
+  return frame_id;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
b/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
new file mode 100644
index 0000000..513e8b0
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
@@ -0,0 +1,60 @@
+if {[skip_shlib_tests]} {
+    untested "Shared libraries are not supported"
+    return -1
+}
+
+if {[get_compiler_info]} {
+    return -1
+}
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping amd64 JIT reader tests."
+    return
+}
+
+set testfile "amd64-jit-unwind"
+set srcfile ${testfile}.c
+set binfile [standard_output_file ${testfile}]
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable [list debug]] != "" } {
+    untested "could not compile test program"
+    return -1
+}
+
+set jit_reader "amd64-jit-reader"
+set jit_reader_source "${srcdir}/${subdir}/${jit_reader}.c"
+set jit_reader_binfile [standard_output_file ${jit_reader}.so]
+
+if { [gdb_compile_shlib ${jit_reader_source} ${jit_reader_binfile}
{additional_flags="-I.."}] != "" } {
+    untested "Cannot compile amd-jit-reader"
+    return  -1
+}
+
+# Check that GDB is unable to show the backtrace for jit-unwind without
+# JIT reader.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test "where" "Backtrace stopped: frame did not save the .*"
+gdb_test "continue" "Continuing\..*$inferior_exited_re.*"
+
+# Check that GDB shows backtrace with JIT reader present
+# and that JIT reader can be unloaded.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_test "jit-reader-load ${jit_reader_binfile}" "Test JIT reader loaded."
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test_sequence "where"  "Bad backtrace" {
+    "\[\r\n\]+#0 .* break_unwind_chain \\(\\) at "
+    "\[\r\n\]+#1 .* break_unwind_chain_1 \\(\\) at "
+    "\[\r\n\]+#2 .* main \\(.*\\) at"
+}
+gdb_test "continue"  "Continuing\..*$inferior_exited_re.*"
+gdb_test "jit-reader-unload" "Test JIT reader unloaded."
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
b/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
new file mode 100644
index 0000000..c2ae3de
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
@@ -0,0 +1,43 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is used to verify that a JIT handler can unwind custom
+   stack frames.  */
+
+#include <stdint.h>
+#include <stdio.h>
+static void break_unwind_chain()
+{
+  /* Save outer frame address, then corrupt the frame chain by setting
+     the outer frame address in it to self.  */
+  void *outer_fp = *(void **)__builtin_frame_address(0);
+  *(void  **)__builtin_frame_address(0) = __builtin_frame_address(0);
+  /* Now restore it so that we can return.  The test sets the
+     breakpoint just before this happens, and GDB will not be able to
+     show the backtrace without JIT reader.  */
+  *(void **)__builtin_frame_address(0) = outer_fp; /* break backtrace-broken */
+}
+
+static void break_unwind_chain_1()
+{
+  break_unwind_chain();
+}
+
+int main(int argc, char *argv[])
+{
+  break_unwind_chain_1();
+}

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-19  3:30         ` Alexander Smundak
  2014-02-19  3:50           ` Eli Zaretskii
@ 2014-04-11 18:47           ` Doug Evans
  2014-04-11 18:58           ` Doug Evans
  2 siblings, 0 replies; 28+ messages in thread
From: Doug Evans @ 2014-04-11 18:47 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches

Alexander Smundak writes:
 > diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
 > index 6e2ee64..a6a0ab1 100644
 > --- a/gdb/jit-reader.in
 > +++ b/gdb/jit-reader.in
 > @@ -336,6 +413,7 @@ struct gdb_reader_funcs
 >    gdb_read_debug_info *read;
 >    gdb_unwind_frame *unwind;
 >    gdb_get_frame_id *get_frame_id;
 > +  gdb_get_symbol_attr *get_symbol_attr;
 >    gdb_destroy_reader *destroy;
 >  };
 > 

Hi.

I think the easiest thing to do to preserve version 1 support would be to reorganize this to:

@@ -336,6 +413,7 @@ struct gdb_reader_funcs
   gdb_read_debug_info *read;
   gdb_unwind_frame *unwind;
   gdb_get_frame_id *get_frame_id;
   gdb_destroy_reader *destroy;
+  gdb_get_symbol_attr *get_symbol_attr;
 };


and then have the code that dereferences get_symbol_attr first check that the interface is version 2.

Can you do that?
And any needed doc updates.

And then with that I think(!) we can finally check this in.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-02-19  3:30         ` Alexander Smundak
  2014-02-19  3:50           ` Eli Zaretskii
  2014-04-11 18:47           ` Doug Evans
@ 2014-04-11 18:58           ` Doug Evans
  2014-04-21  1:35             ` Alexander Smundak
  2 siblings, 1 reply; 28+ messages in thread
From: Doug Evans @ 2014-04-11 18:58 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches

Alexander Smundak writes:
 >[...]
 > diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
 > index 6e2ee64..a6a0ab1 100644
 > --- a/gdb/jit-reader.in
 > +++ b/gdb/jit-reader.in
 > @@ -270,7 +307,14 @@ struct gdb_unwind_callbacks
 >    gdb_unwind_reg_get *reg_get;
 >    gdb_unwind_reg_set *reg_set;
 >    gdb_target_read *target_read;
 > -
 > +  gdb_unwind_stash *stash;
 > +  gdb_unwind_reg_get *cpu_reg_get;
 > +  gdb_find_symbol *find_symbol;
 > +  gdb_get_jtid *get_jtid;
 > +  gdb_enumerate_shared *enumerate_shared;
 > +  gdb_unwind_debug_flag *debug_flag;
 > +  gdb_architecture_name *architecture_name;
 > +  gdb_pointer_size *pointer_size;
 >    /* For internal use by GDB.  */
 >    void *priv_data;
 >  };

Oops, missed this one.

Move the new entries to the end.

@@ -270,7 +307,14 @@ struct gdb_unwind_callbacks
   gdb_unwind_reg_get *reg_get;
   gdb_unwind_reg_set *reg_set;
   gdb_target_read *target_read;

   /* For internal use by GDB.  */
   void *priv_data;
+
+  /* New entries for version 2 interface.  */
+  gdb_unwind_stash *stash;
+  gdb_unwind_reg_get *cpu_reg_get;
+  gdb_find_symbol *find_symbol;
+  gdb_get_jtid *get_jtid;
+  gdb_enumerate_shared *enumerate_shared;
+  gdb_unwind_debug_flag *debug_flag;
+  gdb_architecture_name *architecture_name;
+  gdb_pointer_size *pointer_size;
 };

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-11 18:58           ` Doug Evans
@ 2014-04-21  1:35             ` Alexander Smundak
  2014-04-21  7:14               ` Eli Zaretskii
  0 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-04-21  1:35 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

I've reshuffled the structs to make them v1-compatible, and added two
tests, one to verify that old JIT readers still work, and the other
for the current JIT reader.
The revised patch is below.

2013-12-19  Sasha Smundak  <asmundak@google.com>

* gdb/NEWS: Announcement.
* gdb/frame-unwind.h (frame_symbol_type): New function type.
(struct frame_unwind): Add the pointer to the frame_symbol_type.
* gdb/frame.c (get_frame_symbol_info): New function.
* gdb/frame.h (struct frame_symbol_info): Declare the struct
containing symbol information returned by the unwinder.
(get_frame_symbol_info): Declare it.
* gdb/jit-reader.in (GDB_READER_INTERFACE_VERSION): New version.
        (gdb_unwind_stash): New function type.
(gdb_find_symbol): Ditto.
(gdb_get_jtid): Ditto.
(gdb_enumerate_shared): Ditto.
(gdb_unwind_debug_flag): Ditto.
(gdb_architecture_name): Ditto.
(gdb_pointer_size): Ditto.
(struct gdb_unwind_callbacks): Add members pointing to
gdb_unwind_stash, unwind_reg_get, gdb_find_symbol, gdb_get_jtid,
gdb_enumerate_shared, gdb_unwind_debug_flag,
gdb_architecture_name and gdb_pointer_size functions.
(enum jit_frame_symbol_attr): New enum.
(enum jit_frame_language): Ditto.
(gdb_get_symbol_attr): New function type.
(struct gdb_reader_funcs): Add member pointing to
gdb_get_symbol_attr function.
* gdb/jit.c: Include arch-utils.h.
Include solist.h.
Include ptid.h.
(jit_prepend_unwinder): Declare.
(jit_find_symbol_address): Declare.
(jit_get_current_jtid): Declare.
(jit_enumerate_shared): Declare.
(jit_unwind_debug_flag): Declare.
(jit_unwind_architecture_name): Declare.
(jit_unwind_pointer_size): Declare.
(jit_unwind_debug): Declare.
(show_jit_unwind_debug): New function.
(jit_breakpoint_re_set_internal): Add the call to
jit_prepend_unwinder.
(struct jit_unwind_private): Add a member containing the
pointer to JIT reader's private area.
(jit_get_register_from_frame): New function returning a
register from the given frame (refactored from
jit_unwind_reg_get_impl).
(jit_unwind_reg_get_impl): Now a wrapper around
jit_get_register_from_frame.
(jit_unwind_get_cpu_register_value): New function to get
"live" registers.
(jit_stash_impl): Allocate private data area for the JIT reader.
(jit_dealloc_cache): Free JIT reader private area.
(jit_set_unwind_callbacks_vtable): New function, refactored
from two different places where callbacks were set up.
(jit_frame_sniffer): Use jit_set_unwind_callbacks_vtable.
(jit_frame_language_to_language): New function converting
JIT reader's language type to the GDB internal language type.
(jit_symbol): New function returning symbol information supplied
by the JIT reader.
(jit_frame_unwind): Add the pointer to jit_symbol.
(jit_find_symbol_address): New GDB callback for the JIT reader
returning symbols's address.
(jit_get_current_jtid): New GDB callback for the JIT reader
returning the current thread ID.
(jit_enumerate_shared): New GDB callback for the JIT reader to
traverse inferior's shared objects.
(jit_unwind_debug_flag): New GDB callback returning JIT unwind
debugging flags.
(jit_unwind_architecture_name): New GDB callback returning
inferior's architecture name.
(jit_unwind_pointer_size): New GDB callback returning inferior's
pointer size.
(_initialize_jit): Register 'show/set debug jitunwind'
subcommand.
* gdb/stack.c (find_frame_funname): If symbol information from
the unwindeer if available.
(find_frame_source_location): New function to return symbol
source location from the unwinder if available.
(print_frame): Use arguments information and source location
from the unwinder if available.
* gdb/testsuite/gdb.arch/amd64-jit-reader-v1.c: JIT reader that
implements JIT reader ABI v1.
* gdb/testsuite/gdb.arch/amd64-jit-reader-v1.exp: verifies that
the current JIT reader framework is backwards compatible.
* gdb/testsuite/gdb.arch/amd64-jit-reader.c: JIT reader
that implements the current JIT reader ABI.
* gdb/testsuite/gdb.arch/amd64-jit-reader.exp: verifies current
JIT reader framework functionality.
* gdb/testsuite/gdb.arch/amd64-jit-unwind.c: an application to
test JIT reader unwinding.
diff --git a/gdb/NEWS b/gdb/NEWS
index df233fc..df0f730 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@

 *** Changes since GDB 7.7

+* Revised JIT interface allows creating plugins that display
+  JIT_specific symbol information.
+
 * Guile scripting

   GDB now has support for scripting using Guile.  Whether this is
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 9507be2..407b882 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -140,6 +140,12 @@ typedef void (frame_dealloc_cache_ftype) (struct
frame_info *self,
 typedef struct gdbarch *(frame_prev_arch_ftype) (struct frame_info *this_frame,
  void **this_prologue_cache);

+/* Return unwinder-specific symbol info or NULL.  */
+
+typedef const struct frame_symbol_info* (frame_symbol_ftype)
+    (struct frame_info *this_frame,
+     void **this_prologue_cache);
+
 struct frame_unwind
 {
   /* The frame's type.  Should this instead be a collection of
@@ -154,6 +160,9 @@ struct frame_unwind
   frame_sniffer_ftype *sniffer;
   frame_dealloc_cache_ftype *dealloc_cache;
   frame_prev_arch_ftype *prev_arch;
+  /* For the JIT interface to give external code a chance to supply
+     symbol information describing the frame.  */
+  frame_symbol_ftype *symbol_info;
 };

 /* Register a frame unwinder, _prepending_ it to the front of the
diff --git a/gdb/frame.c b/gdb/frame.c
index 97d54e9..6a4a511 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2307,6 +2307,16 @@ find_frame_sal (struct frame_info *frame,
struct symtab_and_line *sal)
   (*sal) = find_pc_line (pc, notcurrent);
 }

+/* If frame-specific unwinder has symbol info, return it.  */
+
+const struct frame_symbol_info *
+get_frame_symbol_info (struct frame_info *fi)
+{
+  return ((fi->unwind->symbol_info == NULL)
+      ? NULL
+      : fi->unwind->symbol_info (fi, &fi->prologue_cache));
+}
+
 /* Per "frame.h", return the ``address'' of the frame.  Code should
    really be using get_frame_id().  */
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index e451a93..c7365c7 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -491,6 +491,23 @@ enum unwind_stop_reason
 #undef FIRST_ERROR
   };

+/* Unwinder-specific symbol information.  */
+
+struct frame_symbol_info
+{
+  const char *function;
+  const char *source_file;
+  const char *arguments;
+  enum language language;
+  int source_line;
+};
+
+/* Return symbol information supplied by the unwinder. If this frame's
+   unwinder implements frame_unwind.symbol_info method, calls it and
+   returns the result, otherwise returns NULL.  */
+
+const struct frame_symbol_info *get_frame_symbol_info (struct frame_info *);
+
 /* Return the reason why we can't unwind past this frame.  */

 enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
index 6e2ee64..543259a 100644
--- a/gdb/jit-reader.in
+++ b/gdb/jit-reader.in
@@ -26,7 +26,7 @@ extern "C" {

 /* Versioning information.  See gdb_reader_funcs.  */

-#define GDB_READER_INTERFACE_VERSION 1
+#define GDB_READER_INTERFACE_VERSION 2

 /* Readers must be released under a GPL compatible license.  To
    declare that the reader is indeed released under a GPL compatible
@@ -260,6 +260,43 @@ typedef struct gdb_reg_value *(gdb_unwind_reg_get)
 typedef void (gdb_unwind_reg_set) (struct gdb_unwind_callbacks *cb, int regnum,
                                    struct gdb_reg_value *val);

+/* Provides access to the stashed data associated with the current frame.
+   On first call, allocates the memory and zeroes it. On subsequent calls
+   returns the pointer to the allocated memory.  */
+
+typedef void * (gdb_unwind_stash) (struct gdb_unwind_callbacks *cb,
+                                   size_t size);
+
+/* Look through all the current minimal symbol tables and find the
+   first minimal symbol that matches SYMBOL_NAME. Return symbol's
+   address, 0 if symbol is unknown.
+   If it is a C++ symbol, SYMBOL_NAME is in a mangled form.  */
+
+typedef GDB_CORE_ADDR (gdb_find_symbol) (const char *symbol_name);
+
+/* Returns thread ID of the current thread or 0.  */
+
+typedef long (gdb_get_jtid) (void);
+
+/* Iterate over shared objects present in the inferior, calling given
+   callback function for each shared object. The callback is passed
+   the shared object file name, expanded to something GDB can open,
+   and data pointer.  Once the callback function returns non-zero,
+   stop iteration and return shared object file name.  */
+
+typedef const char * (gdb_enumerate_shared) (int (*) (const char *so_name,
+                                                      void *data),
+                                             void *data);
+
+/* Returns debug flags setting for the unwinding.  */
+typedef unsigned int (gdb_unwind_debug_flag) (void);
+
+/* Returns inferior's architecture name.  */
+typedef const char * (gdb_architecture_name) (void);
+
+/* Returns inferior's pointer size in bytes.  */
+typedef size_t (gdb_pointer_size)(void);
+
 /* This struct is passed to unwind in gdb_reader_funcs, and is to be
    used to unwind the current frame (current being the frame whose
    registers can be read using reg_get) into the earlier frame.  The
@@ -273,6 +310,15 @@ struct gdb_unwind_callbacks

   /* For internal use by GDB.  */
   void *priv_data;
+  /* These are provided by JIT API v2 and above.  */
+  gdb_unwind_stash *stash;
+  gdb_unwind_reg_get *cpu_reg_get;
+  gdb_find_symbol *find_symbol;
+  gdb_get_jtid *get_jtid;
+  gdb_enumerate_shared *enumerate_shared;
+  gdb_unwind_debug_flag *debug_flag;
+  gdb_architecture_name *architecture_name;
+  gdb_pointer_size *pointer_size;
 };

 /* Forward declaration.  */
@@ -306,6 +352,39 @@ typedef enum gdb_status (gdb_unwind_frame)
(struct gdb_reader_funcs *self,
 typedef struct gdb_frame_id (gdb_get_frame_id) (struct gdb_reader_funcs *self,
                                                 struct
gdb_unwind_callbacks *c);

+enum jit_frame_symbol_attr {
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
+  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,
+};
+
+enum jit_frame_language {
+  frame_language_unknown,           /* Language not known */
+  frame_language_auto,              /* Placeholder for automatic setting */
+  frame_language_c,                 /* C */
+  frame_language_cplus,             /* C++ */
+  frame_language_d,                 /* D */
+  frame_language_go,                /* Go */
+  frame_language_objc,              /* Objective-C */
+  frame_language_java,              /* Java */
+  frame_language_fortran,           /* Fortran */
+  frame_language_m2,                /* Modula-2 */
+  frame_language_asm,               /* Assembly language */
+  frame_language_pascal,            /* Pascal */
+  frame_language_ada,               /* Ada */
+  frame_language_opencl,            /* OpenCL */
+  frame_language_minimal,           /* All other languages, minimal
support only */
+  frame_nr_languages
+};
+
+/* Return given attribute of a symbol associated with the current frame.  */
+
+typedef void * (gdb_get_symbol_attr) (struct gdb_reader_funcs *self,
+                                      struct gdb_unwind_callbacks *c,
+                                      enum jit_frame_symbol_attr attr);
+
 /* Called when a reader is being unloaded.  This function should also
    free SELF, if required.  */

@@ -337,6 +416,8 @@ struct gdb_reader_funcs
   gdb_unwind_frame *unwind;
   gdb_get_frame_id *get_frame_id;
   gdb_destroy_reader *destroy;
+  /* Members below are supplied by JIT API v2 and above.  */
+  gdb_get_symbol_attr *get_symbol_attr;
 };

 #ifdef __cplusplus
diff --git a/gdb/jit.c b/gdb/jit.c
index db6c1b0..46f9dcc 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -21,6 +21,7 @@

 #include "jit.h"
 #include "jit-reader.h"
+#include "arch-utils.h"
 #include "block.h"
 #include "breakpoint.h"
 #include "command.h"
@@ -33,6 +34,7 @@
 #include "observer.h"
 #include "objfiles.h"
 #include "regcache.h"
+#include "solist.h"
 #include "symfile.h"
 #include "symtab.h"
 #include "target.h"
@@ -40,6 +42,7 @@
 #include <sys/stat.h>
 #include "exceptions.h"
 #include "gdb_bfd.h"
+#include "ptid.h"

 static const char *jit_reader_dir = NULL;

@@ -53,6 +56,22 @@ static const struct program_space_data
*jit_program_space_data = NULL;

 static void jit_inferior_init (struct gdbarch *gdbarch);

+static void jit_prepend_unwinder (struct gdbarch *gdbarch);
+
+static CORE_ADDR jit_unwind_find_symbol_address (const char *);
+
+static long jit_unwind_get_current_jtid (void);
+
+static const char *jit_unwind_enumerate_shared (int (*callback) (const char *,
+                                                          void *),
+                                         void *);
+
+static unsigned int jit_unwind_debug_flag (void);
+
+static const char *jit_unwind_architecture_name (void);
+
+static size_t jit_unwind_pointer_size (void);
+
 /* An unwinder is registered for every gdbarch.  This key is used to
    remember if the unwinder has been registered for a particular
    gdbarch.  */
@@ -70,6 +89,15 @@ show_jit_debug (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("JIT debugging is %s.\n"), value);
 }

+static unsigned jit_unwind_debug;
+
+static void
+show_jit_unwind_debug (struct ui_file *file, int from_tty,
+                       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("JIT unwinder debugging is %s.\n"), value);
+}
+
 struct target_buffer
 {
   CORE_ADDR base;
@@ -186,7 +214,7 @@ jit_reader_load (const char *file_name)
     error (_("Reader not GPL compatible."));

   funcs = init_fn ();
-  if (funcs->reader_version != GDB_READER_INTERFACE_VERSION)
+  if (funcs->reader_version > GDB_READER_INTERFACE_VERSION)
     error (_("Reader version does not match GDB version."));

   new_reader = XCNEW (struct jit_reader);
@@ -1022,6 +1050,8 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   struct jit_objfile_data *objf_data;
   CORE_ADDR addr;

+  jit_prepend_unwinder (gdbarch);
+
   if (ps_data->objfile == NULL)
     {
       /* Lookup the registration symbol.  If it is missing, then we
@@ -1079,6 +1109,12 @@ struct jit_unwind_private

   /* The frame being unwound.  */
   struct frame_info *this_frame;
+
+  /* Symbol associated with this frame.  */
+  struct frame_symbol_info symbol_info;
+
+  /* Memory allocated on behalf of JIT handler.  */
+  void *stash;
 };

 /* Sets the value of a particular register in this frame.  */
@@ -1113,29 +1149,65 @@ reg_value_free_impl (struct gdb_reg_value *value)
   xfree (value);
 }

-/* Get the value of register REGNUM in the previous frame.  */
+/* Get the value of register REGNUM in the given frame.  */

 static struct gdb_reg_value *
-jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+jit_get_register_from_frame (struct frame_info *this_frame, int regnum)
 {
-  struct jit_unwind_private *priv;
   struct gdb_reg_value *value;
   int gdb_reg, size;
   struct gdbarch *frame_arch;

-  priv = cb->priv_data;
-  frame_arch = get_frame_arch (priv->this_frame);
+  frame_arch = get_frame_arch (this_frame);

   gdb_reg = gdbarch_dwarf2_reg_to_regnum (frame_arch, regnum);
   size = register_size (frame_arch, gdb_reg);
   value = xmalloc (sizeof (struct gdb_reg_value) + size - 1);
-  value->defined = deprecated_frame_register_read (priv->this_frame, gdb_reg,
+  value->defined = deprecated_frame_register_read (this_frame, gdb_reg,
    value->value);
   value->size = size;
   value->free = reg_value_free_impl;
   return value;
 }

+/* Get the value of register REGNUM in the previous frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_reg_get_impl (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  return jit_get_register_from_frame (
+      ((struct jit_unwind_private *) cb->priv_data)->this_frame, regnum);
+}
+
+/* Get the value of the register REGNUM in the innermost frame.  */
+
+static struct gdb_reg_value *
+jit_unwind_get_cpu_register_value (struct gdb_unwind_callbacks *cb, int regnum)
+{
+  struct frame_info *frame
+      = ((struct jit_unwind_private *) cb->priv_data)->this_frame;
+  struct frame_info *next_frame;
+
+  while ((next_frame = get_next_frame (frame)) != NULL)
+    frame = next_frame;
+  return jit_get_register_from_frame (frame, regnum);
+}
+
+/* Return memory area allocated for the JIT reader.  The area is allocated on
+   demand (the first time this function is called with non-zero size).
+   Calling this function with size == 0 will not allocate memory but will
+   return its address if it has been already allocated.  */
+
+static void *
+jit_unwind_stash_impl (struct gdb_unwind_callbacks *cb, size_t size)
+{
+  struct jit_unwind_private *priv_data = cb->priv_data;
+
+  if (!priv_data->stash && size)
+    priv_data->stash = xcalloc (size, 1);
+  return priv_data->stash;
+}
+
 /* gdb_reg_value has a free function, which must be called on each
    saved register value.  */

@@ -1154,9 +1226,27 @@ jit_dealloc_cache (struct frame_info
*this_frame, void *cache)
       priv_data->registers[i]->free (priv_data->registers[i]);

   xfree (priv_data->registers);
+  xfree (priv_data->stash);
   xfree (priv_data);
 }

+static void
+jit_set_unwind_callbacks_vtable (struct gdb_unwind_callbacks *callbacks,
+                                 int allow_reg_set)
+{
+  callbacks->reg_get = jit_unwind_reg_get_impl;
+  callbacks->reg_set = allow_reg_set ? jit_unwind_reg_set_impl : NULL;
+  callbacks->target_read = jit_target_read_impl;
+  callbacks->stash = jit_unwind_stash_impl;
+  callbacks->cpu_reg_get = jit_unwind_get_cpu_register_value;
+  callbacks->find_symbol = jit_unwind_find_symbol_address;
+  callbacks->get_jtid = jit_unwind_get_current_jtid;
+  callbacks->enumerate_shared = jit_unwind_enumerate_shared;
+  callbacks->debug_flag = jit_unwind_debug_flag;
+  callbacks->architecture_name = jit_unwind_architecture_name;
+  callbacks->pointer_size = jit_unwind_pointer_size;
+}
+
 /* The frame sniffer for the pseudo unwinder.

    While this is nominally a frame sniffer, in the case where the JIT
@@ -1173,9 +1263,7 @@ jit_frame_sniffer (const struct frame_unwind *self,
   struct gdb_unwind_callbacks callbacks;
   struct gdb_reader_funcs *funcs;

-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = jit_unwind_reg_set_impl;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 1);

   if (loaded_jit_reader == NULL)
     return 0;
@@ -1229,9 +1317,7 @@ jit_frame_this_id (struct frame_info
*this_frame, void **cache,

   /* We don't expect the frame_id function to set any registers, so we
      set reg_set to NULL.  */
-  callbacks.reg_get = jit_unwind_reg_get_impl;
-  callbacks.reg_set = NULL;
-  callbacks.target_read = jit_target_read_impl;
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
   callbacks.priv_data = &private;

   gdb_assert (loaded_jit_reader);
@@ -1261,6 +1347,74 @@ jit_frame_prev_register (struct frame_info
*this_frame, void **cache, int reg)
     return frame_unwind_got_optimized (this_frame, reg);
 }

+/* Convert jit_frame_language enum to GDB's internal language enum.  */
+
+static enum language
+jit_frame_language_to_language (enum jit_frame_language frame_language)
+{
+#define LANG_CASE(x)  frame_language_##x: return language_##x
+  switch (frame_language)
+    {
+      LANG_CASE (auto);
+      LANG_CASE (c);
+      LANG_CASE (cplus);
+      LANG_CASE (d);
+      LANG_CASE (go);
+      LANG_CASE (objc);
+      LANG_CASE (java);
+      LANG_CASE (fortran);
+      LANG_CASE (m2);
+      LANG_CASE (asm);
+      LANG_CASE (pascal);
+      LANG_CASE (ada);
+      LANG_CASE (opencl);
+      LANG_CASE (minimal);
+      default:
+        return language_unknown;
+    }
+#undef LANG_CASE
+}
+
+/* Returns unwinder-specific symbol info.  */
+
+static const struct frame_symbol_info *
+jit_symbol (struct frame_info *this_frame, void **cache)
+{
+  struct gdb_reader_funcs *funcs;
+  struct gdb_unwind_callbacks callbacks;
+  struct jit_unwind_private *priv_data;
+
+  /* Backwards compatibility: JIT reader v1 does not support this.  */
+  if (*cache == NULL || loaded_jit_reader->functions->reader_version == 1)
+    return NULL;
+
+  jit_set_unwind_callbacks_vtable (&callbacks, 0);
+  priv_data = *cache;
+  callbacks.priv_data = priv_data;
+
+  gdb_assert (loaded_jit_reader);
+
+  funcs = loaded_jit_reader->functions;
+  priv_data->symbol_info.function
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME);
+  priv_data->symbol_info.language
+      = jit_frame_language_to_language (
+          (enum jit_frame_language)
+          (funcs->get_symbol_attr (funcs, &callbacks,
+                                   JIT_FRAME_SYMBOL_ATTR_LANGUAGE)));
+  priv_data->symbol_info.source_file
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE);
+  priv_data->symbol_info.source_line = (int)(uintptr_t)
+      funcs->get_symbol_attr (funcs, &callbacks,
+                              JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE);
+  priv_data->symbol_info.arguments
+      = funcs->get_symbol_attr (funcs, &callbacks,
+                                JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS);
+  return &priv_data->symbol_info;
+}
+
 /* Relay everything back to the unwinder registered by the JIT debug
    info reader.*/

@@ -1270,9 +1424,11 @@ static const struct frame_unwind jit_frame_unwind =
   default_frame_unwind_stop_reason,
   jit_frame_this_id,
   jit_frame_prev_register,
-  NULL,
+  NULL,  /* frame_data */
   jit_frame_sniffer,
-  jit_dealloc_cache
+  jit_dealloc_cache,
+  NULL,  /* prev_arch */
+  jit_symbol,
 };


@@ -1313,8 +1469,6 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");

-  jit_prepend_unwinder (gdbarch);
-
   ps_data = get_jit_program_space_data ();
   if (jit_breakpoint_re_set_internal (gdbarch, ps_data) != 0)
     return;
@@ -1446,6 +1600,68 @@ free_objfile_data (struct objfile *objfile, void *data)
   xfree (data);
 }

+/* Locates exported symbol in the target and returns its address.  */
+
+CORE_ADDR
+jit_unwind_find_symbol_address (const char *symbol_name)
+{
+  struct bound_minimal_symbol bound_min_symbol
+      = lookup_bound_minimal_symbol (symbol_name);
+
+  return (bound_min_symbol.minsym == NULL)
+      ? 0 : BMSYMBOL_VALUE_ADDRESS (bound_min_symbol);
+}
+
+/* Returns thread ID of the inferior's current thread.  */
+
+long
+jit_unwind_get_current_jtid (void)
+{
+  long jtid = ptid_get_lwp (inferior_ptid);
+
+  /* ptid_get_lwp returns 0 if inferior is run via gdbserver.
+     However, ptid_get_tid returns 0 for the local inferior.  Ugly.  */
+  return jtid ? jtid : ptid_get_tid (inferior_ptid);
+}
+
+/* Calls provided function for every shared object currently loaded into
+   the inferior until the function returns non zero.  */
+
+const char *
+jit_unwind_enumerate_shared (int (*callback) (const char *so_name,
+                                              void *data),
+                             void *data)
+{
+  struct so_list *so;
+
+  for (so = master_so_list (); so; so = so->next)
+    {
+      if (callback (so->so_name, data))
+        return so->so_name;
+    }
+  return NULL;
+}
+
+static unsigned int
+jit_unwind_debug_flag (void)
+{
+  return jit_unwind_debug;
+}
+
+
+/* Return current architecture's string name.  */
+static const char *
+jit_unwind_architecture_name (void)
+{
+  return gdbarch_bfd_arch_info (target_gdbarch ())->printable_name;
+}
+
+static size_t
+jit_unwind_pointer_size (void)
+{
+  return gdbarch_ptr_bit (target_gdbarch()) / 8;
+}
+
 /* Initialize the jit_gdbarch_data slot with an instance of struct
    jit_gdbarch_data_type */

@@ -1475,6 +1691,13 @@ _initialize_jit (void)
      NULL,
      show_jit_debug,
      &setdebuglist, &showdebuglist);
+  add_setshow_zuinteger_cmd ("jitunwind", class_maintenance, &jit_unwind_debug,
+                             _("Set JIT frame unwinder debug."),
+                             _("Show JIT frame unwinder debug."),
+                             _("A collection of bit flags for debugging."),
+                             NULL,
+                             show_jit_unwind_debug,
+                             &setdebuglist, &showdebuglist);

   observer_attach_inferior_exit (jit_inferior_exit_hook);
   observer_attach_breakpoint_deleted (jit_breakpoint_deleted);
diff --git a/gdb/stack.c b/gdb/stack.c
index da7d977..68f4b68 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1039,6 +1039,15 @@ find_frame_funname (struct frame_info *frame,
char **funname,
     enum language *funlang, struct symbol **funcp)
 {
   struct symbol *func;
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      *funname = xstrdup (frame_symbol->function);
+      *funlang = frame_symbol->language;
+      *funcp = NULL;
+      return;
+    }

   *funname = NULL;
   *funlang = language_unknown;
@@ -1126,6 +1135,23 @@ find_frame_funname (struct frame_info *frame,
char **funname,
     }
 }

+/* Fill unwinder-specific source location for the frame if available and
+   return 1.  Otherwise return 0.  */
+
+static int
+find_frame_source_location (struct frame_info *fi, const char **file,
+                            int *line)
+{
+  const struct frame_symbol_info *frame_symbol = get_frame_symbol_info (fi);
+
+  if (frame_symbol == NULL || frame_symbol->source_file == NULL)
+    return 0;
+
+  *file = frame_symbol->source_file;
+  *line = frame_symbol->source_line;
+  return 1;
+}
+
 static void
 print_frame (struct frame_info *frame, int print_level,
      enum print_what print_what, int print_args,
@@ -1141,6 +1167,8 @@ print_frame (struct frame_info *frame, int print_level,
   struct symbol *func;
   CORE_ADDR pc = 0;
   int pc_p;
+  const char *file;
+  int line;

   pc_p = get_frame_pc_if_available (frame, &pc);

@@ -1201,7 +1229,16 @@ print_frame (struct frame_info *frame, int print_level,
       args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
       TRY_CATCH (e, RETURN_MASK_ERROR)
  {
-  print_frame_args (func, frame, numargs, gdb_stdout);
+  const struct frame_symbol_info *frame_symbol;
+  frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      if (frame_symbol->arguments != NULL)
+ ui_out_text (uiout, frame_symbol->arguments);
+    }
+  else
+    print_frame_args (func, frame, numargs, gdb_stdout);
  }
       /* FIXME: ARGS must be a list.  If one argument is a string it
   will have " that will not be properly escaped.  */
@@ -1210,7 +1247,21 @@ print_frame (struct frame_info *frame, int print_level,
       QUIT;
     }
   ui_out_text (uiout, ")");
-  if (sal.symtab)
+
+  if (find_frame_source_location (frame, &file, &line))
+    {
+      annotate_frame_source_begin ();
+      ui_out_wrap_hint (uiout, "   ");
+      ui_out_text (uiout, " at ");
+      annotate_frame_source_file ();
+      ui_out_field_string (uiout, "file", file);
+      annotate_frame_source_file_end ();
+      ui_out_text (uiout, ":");
+      annotate_frame_source_line ();
+      ui_out_field_int (uiout, "line", line);
+      annotate_frame_source_end ();
+    }
+  else if (sal.symtab)
     {
       const char *filename_display;

diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.c
b/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.c
new file mode 100644
index 0000000..f75052e
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.c
@@ -0,0 +1,135 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include "jit-reader.h"
+
+GDB_DECLARE_GPL_COMPATIBLE_READER
+
+extern struct gdb_reader_funcs *gdb_init_reader (void);
+
+static enum gdb_status debug_info_provider (struct gdb_reader_funcs *self,
+                                            struct gdb_symbol_callbacks *cb,
+                                            void *memory, long memory_sz);
+static enum gdb_status frame_unwinder (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb);
+static struct gdb_frame_id frame_id_provider(struct gdb_reader_funcs *self,
+                                             struct gdb_unwind_callbacks *cb);
+static void destructor (struct gdb_reader_funcs *self);
+
+#define DATA_COOKIE 0xdeaffead
+static int jit_data = DATA_COOKIE;
+
+static struct
+{
+  int reader_version;
+  void *priv_data;
+  gdb_read_debug_info *read;
+  gdb_unwind_frame *unwind;
+  gdb_get_frame_id *get_frame_id;
+  gdb_destroy_reader *destroy;
+} gdb_reader_funcs_v1 =
+{
+  1,  /* version */
+  (void *)&jit_data,
+  debug_info_provider,
+  frame_unwinder,
+  frame_id_provider,
+  destructor
+};
+
+struct gdb_reader_funcs *
+gdb_init_reader (void)
+{
+  fprintf (stderr, "Test JIT reader loaded.\n");
+  return (struct gdb_reader_funcs *)&gdb_reader_funcs_v1;
+}
+
+enum gdb_status
+debug_info_provider (struct gdb_reader_funcs *self,
+                     struct gdb_symbol_callbacks *cb,
+                     void *memory, long memory_sz)
+{
+  if (*(int *)(self->priv_data) != DATA_COOKIE)
+    return GDB_FAIL;
+  fprintf (stderr, "Debug info provider called for %p..%p.\n",
+           memory, (void *)(memory_sz + (char *)memory));
+  return GDB_SUCCESS;
+}
+
+void
+destructor (struct gdb_reader_funcs *self)
+{
+  fprintf (stderr, "Test JIT reader unloaded.\n");
+  return;
+}
+
+static GDB_CORE_ADDR
+_get_frame_register (struct gdb_unwind_callbacks *cb, int reg_no)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  GDB_CORE_ADDR reg_value = *(GDB_CORE_ADDR *)reg->value;
+  reg->free (reg);
+  return reg_value;
+}
+
+static void
+_set_frame_register (struct gdb_unwind_callbacks *cb, int reg_no,
+                     GDB_CORE_ADDR value)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  *(GDB_CORE_ADDR *)(reg->value) = value;
+  reg->defined = 1;
+  (cb->reg_set)(cb, reg_no, reg);
+}
+
+#define REG_BP 6
+#define REG_PC 16
+#define REG_SP 7
+enum gdb_status
+frame_unwinder (struct gdb_reader_funcs *self,
+                struct gdb_unwind_callbacks *cb)
+{
+  GDB_CORE_ADDR bp = _get_frame_register (cb, REG_BP);
+  GDB_CORE_ADDR pc = _get_frame_register (cb, REG_PC);
+  GDB_CORE_ADDR sp = _get_frame_register (cb, REG_SP);
+  GDB_CORE_ADDR word;
+  cb->target_read(bp, &word, 8);
+  fprintf (stderr, "frame_unwinder called for pc=%p, bp=%p,
*bp=%p.\n", (void *)pc, (void *)bp, (void *)word);
+  if (word != bp)
+    return GDB_FAIL;
+  /* Found a frame that the test program altered for us.  Tell GDB what
+     the previous frame is.  The test program saved the correct outer
+     frame's FP is one word above.  */
+  cb->target_read (bp - 8, &word, 8);
+  _set_frame_register (cb, REG_BP, word);
+  cb->target_read (bp + 8, &word, 8);
+  _set_frame_register (cb, REG_PC, word);
+  _set_frame_register (cb, REG_SP, bp + 16);
+  return GDB_SUCCESS;
+}
+
+struct gdb_frame_id frame_id_provider (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb)
+{
+  struct gdb_frame_id frame_id =
+      { _get_frame_register (cb, REG_PC),
+        _get_frame_register (cb, REG_SP) };
+  fprintf (stderr, "frame_id_provider called.\n");
+  return frame_id;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.exp
b/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.exp
new file mode 100644
index 0000000..c52a7c9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader-v1.exp
@@ -0,0 +1,81 @@
+# Copyright 2010-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite. It tests that the the current
+# JIT reader framework is backwards compatible with old (v1) JIT readers.
+
+if {[skip_shlib_tests]} {
+    untested "Shared libraries are not supported"
+    return -1
+}
+
+if {[get_compiler_info]} {
+    return -1
+}
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping amd64 JIT reader tests."
+    return
+}
+
+set testfile "amd64-jit-unwind"
+set srcfile ${testfile}.c
+set binfile [standard_output_file ${testfile}]
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable [list debug]] != "" } {
+    untested "could not compile test program"
+    return -1
+}
+
+set jit_reader "amd64-jit-reader-v1"
+set jit_reader_source "${srcdir}/${subdir}/${jit_reader}.c"
+set jit_reader_binfile [standard_output_file ${jit_reader}.so]
+
+if { [gdb_compile_shlib ${jit_reader_source} ${jit_reader_binfile}
{additional_flags="-I.."}] != "" } {
+    untested "Cannot compile amd-jit-reader"
+    return  -1
+}
+
+# Check that GDB is unable to show the backtrace for jit-unwind without
+# JIT reader.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test "where" "Backtrace stopped: frame did not save the .*"
+gdb_test "continue" "Continuing\..*$inferior_exited_re.*"
+
+# Check that GDB shows backtrace with JIT reader present
+# and that JIT reader can be unloaded.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_test "jit-reader-load ${jit_reader_binfile}" "Test JIT reader loaded."
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test_sequence "where"  "Bad backtrace" {
+    "\[\r\n\]+#0 .* break_backtrace \\(\\) at "
+    "\[\r\n\]+#1 .* break_backtrace_caller \\(\\) at "
+    "\[\r\n\]+#2 .* main \\(.*\\) at"
+}
+gdb_test "continue"  "Continuing\..*$inferior_exited_re.*"
+gdb_test "jit-reader-unload" "Test JIT reader unloaded."
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader.c
b/gdb/testsuite/gdb.arch/amd64-jit-reader.c
new file mode 100644
index 0000000..b40c52b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader.c
@@ -0,0 +1,152 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include "jit-reader.h"
+
+GDB_DECLARE_GPL_COMPATIBLE_READER
+
+extern struct gdb_reader_funcs *gdb_init_reader (void);
+
+static enum gdb_status debug_info_provider (struct gdb_reader_funcs *self,
+                                            struct gdb_symbol_callbacks *cb,
+                                            void *memory, long memory_sz);
+static enum gdb_status frame_unwinder (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb);
+static struct gdb_frame_id frame_id_provider(struct gdb_reader_funcs *self,
+                                             struct gdb_unwind_callbacks *cb);
+static void destructor (struct gdb_reader_funcs *self);
+static void * get_symbol_attr (struct gdb_reader_funcs *self,
+                               struct gdb_unwind_callbacks *c,
+                               enum jit_frame_symbol_attr attr);
+
+#define DATA_COOKIE 0xdeaffead
+static int jit_data = DATA_COOKIE;
+
+static struct gdb_reader_funcs gdb_reader_funcs =
+{
+  GDB_READER_INTERFACE_VERSION,  /* version */
+  (void *)&jit_data,
+  debug_info_provider,
+  frame_unwinder,
+  frame_id_provider,
+  destructor,
+  get_symbol_attr
+};
+
+struct gdb_reader_funcs *
+gdb_init_reader (void)
+{
+  fprintf (stderr, "Test JIT reader loaded.\n");
+  return (struct gdb_reader_funcs *)&gdb_reader_funcs;
+}
+
+enum gdb_status
+debug_info_provider (struct gdb_reader_funcs *self,
+                     struct gdb_symbol_callbacks *cb,
+                     void *memory, long memory_sz)
+{
+  if (*(int *)(self->priv_data) != DATA_COOKIE)
+    return GDB_FAIL;
+  fprintf (stderr, "Debug info provider called for %p..%p.\n",
+           memory, (void *)(memory_sz + (char *)memory));
+  return GDB_SUCCESS;
+}
+
+void
+destructor (struct gdb_reader_funcs *self)
+{
+  fprintf (stderr, "Test JIT reader unloaded.\n");
+  return;
+}
+
+static GDB_CORE_ADDR
+_get_frame_register (struct gdb_unwind_callbacks *cb, int reg_no)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  GDB_CORE_ADDR reg_value = *(GDB_CORE_ADDR *)reg->value;
+  reg->free (reg);
+  return reg_value;
+}
+
+static void
+_set_frame_register (struct gdb_unwind_callbacks *cb, int reg_no,
+                     GDB_CORE_ADDR value)
+{
+  struct gdb_reg_value *reg = cb->reg_get (cb, reg_no);
+  *(GDB_CORE_ADDR *) (reg->value) = value;
+  reg->defined = 1;
+  (cb->reg_set) (cb, reg_no, reg);
+}
+
+#define REG_BP 6
+#define REG_PC 16
+#define REG_SP 7
+enum gdb_status
+frame_unwinder (struct gdb_reader_funcs *self,
+                struct gdb_unwind_callbacks *cb)
+{
+  GDB_CORE_ADDR bp = _get_frame_register (cb, REG_BP);
+  GDB_CORE_ADDR pc = _get_frame_register (cb, REG_PC);
+  GDB_CORE_ADDR sp = _get_frame_register (cb, REG_SP);
+  GDB_CORE_ADDR word;
+  cb->target_read(bp, &word, 8);
+  fprintf (stderr, "frame_unwinder called for pc=%p, bp=%p,
*bp=%p.\n", (void *)pc, (void *)bp, (void *)word);
+  if (word != bp)
+    return GDB_FAIL;
+  /* Found a frame that the test program altered for us.  Tell GDB what
+     the previous frame is.  The test program saved the correct outer
+     frame's FP one word above.  */
+  cb->target_read (bp - 8, &word, 8);
+  _set_frame_register (cb, REG_BP, word);
+  cb->target_read (bp + 8, &word, 8);
+  _set_frame_register (cb, REG_PC, word);
+  _set_frame_register (cb, REG_SP, bp + 16);
+  return GDB_SUCCESS;
+}
+
+struct gdb_frame_id frame_id_provider (struct gdb_reader_funcs *self,
+                                       struct gdb_unwind_callbacks *cb)
+{
+  struct gdb_frame_id frame_id =
+      { _get_frame_register (cb, REG_PC),
+        _get_frame_register (cb, REG_SP) };
+  fprintf (stderr, "frame_id_provider called.\n");
+  return frame_id;
+}
+
+void *
+get_symbol_attr (struct gdb_reader_funcs *self, struct gdb_unwind_callbacks *c,
+                 enum jit_frame_symbol_attr attr)
+{
+  switch (attr) {
+    case JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME:
+      return "restored_break_backtrace";
+    case JIT_FRAME_SYMBOL_ATTR_LANGUAGE:
+      return (void *) frame_language_c;
+    case JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE:
+      return "";
+    case JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE:
+      return (void *) 42;
+    case JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS:
+      return "";
+    default:
+      fprintf (stderr, "Bad enum value: %d\n", (int)attr);
+      return NULL;
+  }
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
b/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
new file mode 100644
index 0000000..1b57e14
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-reader.exp
@@ -0,0 +1,80 @@
+# Copyright 2010-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if {[skip_shlib_tests]} {
+    untested "Shared libraries are not supported"
+    return -1
+}
+
+if {[get_compiler_info]} {
+    return -1
+}
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping amd64 JIT reader tests."
+    return
+}
+
+set testfile "amd64-jit-unwind"
+set srcfile ${testfile}.c
+set binfile [standard_output_file ${testfile}]
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable [list debug]] != "" } {
+    untested "could not compile test program"
+    return -1
+}
+
+set jit_reader "amd64-jit-reader"
+set jit_reader_source "${srcdir}/${subdir}/${jit_reader}.c"
+set jit_reader_binfile [standard_output_file ${jit_reader}.so]
+
+if { [gdb_compile_shlib ${jit_reader_source} ${jit_reader_binfile}
{additional_flags="-I.."}] != "" } {
+    untested "Cannot compile amd-jit-reader"
+    return  -1
+}
+
+# Check that GDB is unable to show the backtrace for jit-unwind without
+# JIT reader.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test "where" "Backtrace stopped: frame did not save the .*"
+gdb_test "continue" "Continuing\..*$inferior_exited_re.*"
+
+# Check that GDB shows backtrace with JIT reader present
+# and that JIT reader can be unloaded.
+clean_restart $testfile
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+gdb_test "jit-reader-load ${jit_reader_binfile}" "Test JIT reader loaded."
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken" ]
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test_sequence "where"  "Bad backtrace" {
+    "\[\r\n\]+#0 .* restored_break_backtrace \\(\\) at :42"
+    "\[\r\n\]+#1 .* break_backtrace_caller \\(\\) at "
+    "\[\r\n\]+#2 .* main \\(.*\\) at"
+}
+gdb_test "continue"  "Continuing\..*$inferior_exited_re.*"
+gdb_test "jit-reader-unload" "Test JIT reader unloaded."
diff --git a/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
b/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
new file mode 100644
index 0000000..20c1e22
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-jit-unwind.c
@@ -0,0 +1,68 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011-2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is used to verify that a JIT handler can unwind custom
+   stack frames.  */
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void *
+swap_value (void **location, void *new_value)
+{
+  void *old_value = *location;
+  *location = new_value;
+  return old_value;
+}
+
+#define MY_FRAME (__builtin_frame_address (0))
+static void
+break_backtrace ()
+{
+  /* Save outer frame address, then corrupt the unwind chain by
+     setting the outer frame address in it to self.  This is
+     ABI-specific: the first word of the frame contains previous frame
+     address in amd64.  */
+  void *outer_fp = swap_value ((void **)MY_FRAME, MY_FRAME);
+
+  /* Verify the compiler allocates the first local variable one word
+     below frame.  This is where test JIT reader expects to find the
+     correct outer frame address.  */
+  if (&outer_fp + 1 != (void **)MY_FRAME)
+    {
+      fprintf (stderr, "First variable should be allocated one word below "
+               "the frame, got variable's address %p, frame at %p instead\n",
+               &outer_fp, MY_FRAME);
+      abort();
+    }
+
+  /* Now restore it so that we can return.  The test sets the
+     breakpoint just before this happens, and GDB will not be able to
+     show the backtrace without JIT reader.  */
+  swap_value (MY_FRAME, outer_fp); /* break backtrace-broken */
+}
+
+static void break_backtrace_caller ()
+{
+  break_backtrace ();
+}
+
+int main (int argc, char *argv[])
+{
+  break_backtrace_caller ();
+}

On Fri, Apr 11, 2014 at 11:58 AM, Doug Evans <dje@google.com> wrote:
> Alexander Smundak writes:
>  >[...]
>  > diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
>  > index 6e2ee64..a6a0ab1 100644
>  > --- a/gdb/jit-reader.in
>  > +++ b/gdb/jit-reader.in
>  > @@ -270,7 +307,14 @@ struct gdb_unwind_callbacks
>  >    gdb_unwind_reg_get *reg_get;
>  >    gdb_unwind_reg_set *reg_set;
>  >    gdb_target_read *target_read;
>  > -
>  > +  gdb_unwind_stash *stash;
>  > +  gdb_unwind_reg_get *cpu_reg_get;
>  > +  gdb_find_symbol *find_symbol;
>  > +  gdb_get_jtid *get_jtid;
>  > +  gdb_enumerate_shared *enumerate_shared;
>  > +  gdb_unwind_debug_flag *debug_flag;
>  > +  gdb_architecture_name *architecture_name;
>  > +  gdb_pointer_size *pointer_size;
>  >    /* For internal use by GDB.  */
>  >    void *priv_data;
>  >  };
>
> Oops, missed this one.
>
> Move the new entries to the end.
>
> @@ -270,7 +307,14 @@ struct gdb_unwind_callbacks
>    gdb_unwind_reg_get *reg_get;
>    gdb_unwind_reg_set *reg_set;
>    gdb_target_read *target_read;
>
>    /* For internal use by GDB.  */
>    void *priv_data;
> +
> +  /* New entries for version 2 interface.  */
> +  gdb_unwind_stash *stash;
> +  gdb_unwind_reg_get *cpu_reg_get;
> +  gdb_find_symbol *find_symbol;
> +  gdb_get_jtid *get_jtid;
> +  gdb_enumerate_shared *enumerate_shared;
> +  gdb_unwind_debug_flag *debug_flag;
> +  gdb_architecture_name *architecture_name;
> +  gdb_pointer_size *pointer_size;
>  };

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-21  1:35             ` Alexander Smundak
@ 2014-04-21  7:14               ` Eli Zaretskii
  2014-04-21 16:43                 ` Alexander Smundak
  0 siblings, 1 reply; 28+ messages in thread
From: Eli Zaretskii @ 2014-04-21  7:14 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: dje, gdb-patches

> Date: Sun, 20 Apr 2014 18:35:01 -0700
> From: Alexander Smundak <asmundak@google.com>
> Cc: gdb-patches <gdb-patches@sourceware.org>
> 
> I've reshuffled the structs to make them v1-compatible, and added two
> tests, one to verify that old JIT readers still work, and the other
> for the current JIT reader.
> The revised patch is below.

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,9 @@
> 
>  *** Changes since GDB 7.7
> 
> +* Revised JIT interface allows creating plugins that display
> +  JIT_specific symbol information.
     ^^^^^^^^^^^^
You mean "JIT-specific", with a dash, right?

The NEWS part is OK with this fixed.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-21  7:14               ` Eli Zaretskii
@ 2014-04-21 16:43                 ` Alexander Smundak
  0 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-04-21 16:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Doug Evans, gdb-patches

On Mon, Apr 21, 2014 at 12:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> +* Revised JIT interface allows creating plugins that display
>> +  JIT_specific symbol information.
>      ^^^^^^^^^^^^
> You mean "JIT-specific", with a dash, right?

Sorry again. Fixed.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2013-12-26 18:36 [RFC][PATCH] Allow JIT unwinder provide symbol information Sasha Smundak
                   ` (2 preceding siblings ...)
  2014-02-08  7:08 ` Yao Qi
@ 2014-04-24 13:22 ` Pedro Alves
  2014-04-25 23:40   ` Alexander Smundak
  3 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2014-04-24 13:22 UTC (permalink / raw)
  To: Sasha Smundak; +Cc: gdb-patches

On 12/26/2013 06:36 PM, Sasha Smundak wrote:
> The change proposed in this RFC is part of the effort to support
> debugging applications containing Java code executed with the help of
> Java interpreter/ just-in-time (JIT) compiler. Here's the fragment of
> the backtrace of an application being debugged by the GDB modified to
> provide such support:
> 
> #8 0x00007fffea743b03 in JNIEnv_::CallVoidMethod (this=0x7ffff001f220, obj=0x7ffff665d810, methodID=0x7ffff019d6d0) at <...>/jdk/include/jni.h:1054
> #9 0x00007fffea7439c2 in Java_Util_callObjectMethod (env=0x7ffff001f220, myclass=0x7ffff665d840, target_object=0x7ffff665d810, method_name=0x7ffff665d818) at <...>/testjni.cc:48
> #10 0x00007fffed05ef7b in Util.callObjectMethod+0x15b[C] (java.lang.Object=???) at Util.java:-1
> #11 0x00007fffed061188 in Myclass.method1+0x68[C] (this=@f665d2a8) at Myclass.java:18
> #12 0x00007fffed005f98 in Myclass.main#I ([]=[...]) at Myclass.java:48
> #13 0x00007fffed000438 in <StubRoutines> ()
> 
> I.e., Myclass.main() calls Myclass.method1() calls
> Util.callObjectMethod(), which is implemented as a C function
> Java_Util_callObjectMethod(). Myclass.main() is interpreted, while
> Myclass.method1() is JIT-compiled.
> 
> The implementation uses GDB's JIT API, and this RFC is the GDB patch
> needed for that. The implementation of the actual debugging support
> for Java (that is, the code that unwinds Java frames and is able to
> provide symbol information for them) is not presented here; hopefully
> it will make it into OpenJDK at some point, or will be distributed
> standalone.
> 
> GDB's current JIT API is sufficient to unwind the stack frames created
> by the Java Virtual Machine (JVM). However, it needs to be extended in
> order to be able to display correct symbol information, for two
> reasons. First, there is no need to add and remove symbol information
> as JIT compiles the code and disposes of it if JIT handler can provide
> this information on demand. Second, when JVM runs Java code in the
> interpreted mode, the code address in the frame points to the
> interpreter code, which is not what should be displayed.
> 
> The solution proposed here is to add a "symbol provider" function to
> the unwinder interface and modify the code displaying frames to get
> the symbol information associated with the frame (function name,
> source location, arguments) from the frame's unwinder if it has
> symbol provider and use the current logic as a fallback strategy.
> 
> There are additional changes in this RFC exposing more GDB
> functions to the JIT handler.

Apologies for only now taking a look at what this is used for.

AFAICS, this only allows providing these.

+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_NAME,
+  JIT_FRAME_SYMBOL_ATTR_LANGUAGE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_FILE,
+  JIT_FRAME_SYMBOL_ATTR_SOURCE_LINE,
+  JIT_FRAME_SYMBOL_ATTR_FUNCTION_ARGS,

It seems to me that what you want to do here is what Python
frame filters were invented for.  IOW, I think you should
be able to write a frame filter that interacts with the Java
JIT to expose the same info to the user your API extension
is proposing.  But I think that direction would be more
flexible, even.  You could for example, hide functions
if you want (say, the JIT innards), and probably give more
detail of the args (this solution seems to provide their names
as strings only).

-- 
Pedro Alves

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-24 13:22 ` Pedro Alves
@ 2014-04-25 23:40   ` Alexander Smundak
  2014-04-29 15:22     ` Pedro Alves
  0 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-04-25 23:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Thu, Apr 24, 2014 at 6:22 AM, Pedro Alves <palves@redhat.com> wrote:
> It seems to me that what you want to do here is what Python
> frame filters were invented for.  IOW, I think you should
> be able to write a frame filter that interacts with the Java
> JIT to expose the same info to the user your API extension
> is proposing.
The patch allows JIT readers to provide the symbol information to GDB.
I am not sure how this can be achieved with Python frame filters. IMHO
they have different purpose.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-25 23:40   ` Alexander Smundak
@ 2014-04-29 15:22     ` Pedro Alves
  2014-05-02 16:58       ` Alexander Smundak
                         ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Pedro Alves @ 2014-04-29 15:22 UTC (permalink / raw)
  To: Alexander Smundak; +Cc: gdb-patches

On 04/26/2014 12:39 AM, Alexander Smundak wrote:
> On Thu, Apr 24, 2014 at 6:22 AM, Pedro Alves <palves@redhat.com> wrote:
>> It seems to me that what you want to do here is what Python
>> frame filters were invented for.  IOW, I think you should
>> be able to write a frame filter that interacts with the Java
>> JIT to expose the same info to the user your API extension
>> is proposing.
> The patch allows JIT readers to provide the symbol information to GDB.
> I am not sure how this can be achieved with Python frame filters. IMHO
> they have different purpose.

But it doesn't actually provide symbol information.  Not in the sense
that it hooks with GDB's symbol lookup mechanisms.

E.g., the patch does this:

@@ -1201,7 +1229,16 @@ print_frame (struct frame_info *frame, int print_level,
       args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
       TRY_CATCH (e, RETURN_MASK_ERROR)
  {
-  print_frame_args (func, frame, numargs, gdb_stdout);
+  const struct frame_symbol_info *frame_symbol;
+  frame_symbol = get_frame_symbol_info (frame);
+
+  if (frame_symbol != NULL)
+    {
+      if (frame_symbol->arguments != NULL)
+ ui_out_text (uiout, frame_symbol->arguments);
+    }
+  else
+    print_frame_args (func, frame, numargs, gdb_stdout);

This is just wrong.  It's printing the frame arguments as
a single, and simple string.  This simply doesn't work correctly
with MI frontends.  Or what happens if the user selects
one of those JIT frames and tries to print one of the arguments?

I really think the way this patch is hooking a bespoke
function/line/args mechanism into the frame machinery is
quite hacky as is, sorry.  :-/

-- 
Pedro Alves

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-29 15:22     ` Pedro Alves
@ 2014-05-02 16:58       ` Alexander Smundak
  2014-05-19 21:30         ` Alexander Smundak
  2014-05-29  1:07       ` Alexander Smundak
  2014-06-02  1:15       ` Alexander Smundak
  2 siblings, 1 reply; 28+ messages in thread
From: Alexander Smundak @ 2014-05-02 16:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Tue, Apr 29, 2014 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
> But it doesn't actually provide symbol information.  Not in the sense
> that it hooks with GDB's symbol lookup mechanisms.
I cannot find a better way to hook into GDB symbols lookup machinery.
It looks to me that GDB is operating on assumption that symbol information
is static (except for shared objects loading), and it not frame-specific.
The code emitted by JIT does not conform to these assumptions.
IMHO what I am proposing has the beginnings of the approach to
overcome this difficulty by allowing unwinder to have its own symbol handler.
It this approach looks reasonable, obviously more changes are needed,
but I'd like to have a confirmation that the approach is reasonable.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-05-02 16:58       ` Alexander Smundak
@ 2014-05-19 21:30         ` Alexander Smundak
  0 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-05-19 21:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Ping.

On Fri, May 2, 2014 at 9:58 AM, Alexander Smundak <asmundak@google.com> wrote:
> On Tue, Apr 29, 2014 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
>> But it doesn't actually provide symbol information.  Not in the sense
>> that it hooks with GDB's symbol lookup mechanisms.
> I cannot find a better way to hook into GDB symbols lookup machinery.
> It looks to me that GDB is operating on assumption that symbol information
> is static (except for shared objects loading), and it not frame-specific.
> The code emitted by JIT does not conform to these assumptions.
> IMHO what I am proposing has the beginnings of the approach to
> overcome this difficulty by allowing unwinder to have its own symbol handler.
> It this approach looks reasonable, obviously more changes are needed,
> but I'd like to have a confirmation that the approach is reasonable.

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-29 15:22     ` Pedro Alves
  2014-05-02 16:58       ` Alexander Smundak
@ 2014-05-29  1:07       ` Alexander Smundak
  2014-06-02  1:15       ` Alexander Smundak
  2 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-05-29  1:07 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Ping.

On Tue, Apr 29, 2014 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/26/2014 12:39 AM, Alexander Smundak wrote:
>> On Thu, Apr 24, 2014 at 6:22 AM, Pedro Alves <palves@redhat.com> wrote:
>>> It seems to me that what you want to do here is what Python
>>> frame filters were invented for.  IOW, I think you should
>>> be able to write a frame filter that interacts with the Java
>>> JIT to expose the same info to the user your API extension
>>> is proposing.
>> The patch allows JIT readers to provide the symbol information to GDB.
>> I am not sure how this can be achieved with Python frame filters. IMHO
>> they have different purpose.
>
> But it doesn't actually provide symbol information.  Not in the sense
> that it hooks with GDB's symbol lookup mechanisms.
>
> E.g., the patch does this:
>
> @@ -1201,7 +1229,16 @@ print_frame (struct frame_info *frame, int print_level,
>        args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
>        TRY_CATCH (e, RETURN_MASK_ERROR)
>   {
> -  print_frame_args (func, frame, numargs, gdb_stdout);
> +  const struct frame_symbol_info *frame_symbol;
> +  frame_symbol = get_frame_symbol_info (frame);
> +
> +  if (frame_symbol != NULL)
> +    {
> +      if (frame_symbol->arguments != NULL)
> + ui_out_text (uiout, frame_symbol->arguments);
> +    }
> +  else
> +    print_frame_args (func, frame, numargs, gdb_stdout);
>
> This is just wrong.  It's printing the frame arguments as
> a single, and simple string.  This simply doesn't work correctly
> with MI frontends.  Or what happens if the user selects
> one of those JIT frames and tries to print one of the arguments?
>
> I really think the way this patch is hooking a bespoke
> function/line/args mechanism into the frame machinery is
> quite hacky as is, sorry.  :-/
>
> --
> Pedro Alves
>

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

* Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
  2014-04-29 15:22     ` Pedro Alves
  2014-05-02 16:58       ` Alexander Smundak
  2014-05-29  1:07       ` Alexander Smundak
@ 2014-06-02  1:15       ` Alexander Smundak
  2 siblings, 0 replies; 28+ messages in thread
From: Alexander Smundak @ 2014-06-02  1:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

I have looked again at the possibility of using Frame Filters.
They can be utilized as follows:
1. Use existing JIT reader API to unwind JIT frames.
2. Add methods to the Python Frame object to access frame's
stack pointer and frame pointer (or perhaps provide a method
to access any available register?)
3. Implement FrameDecorator that uses PC/FP/SP to retrieve
frame information (from JVM or whatever). Implement Frame
Filter that returns this FrameDecorator iterator.
4. Decide how to coordinate JIT reader and corresponding
Frame Filter loading.

Better?

On Tue, Apr 29, 2014 at 8:22 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/26/2014 12:39 AM, Alexander Smundak wrote:
>> On Thu, Apr 24, 2014 at 6:22 AM, Pedro Alves <palves@redhat.com> wrote:
>>> It seems to me that what you want to do here is what Python
>>> frame filters were invented for.  IOW, I think you should
>>> be able to write a frame filter that interacts with the Java
>>> JIT to expose the same info to the user your API extension
>>> is proposing.
>> The patch allows JIT readers to provide the symbol information to GDB.
>> I am not sure how this can be achieved with Python frame filters. IMHO
>> they have different purpose.
>
> But it doesn't actually provide symbol information.  Not in the sense
> that it hooks with GDB's symbol lookup mechanisms.
>
> E.g., the patch does this:
>
> @@ -1201,7 +1229,16 @@ print_frame (struct frame_info *frame, int print_level,
>        args_list_chain = make_cleanup_ui_out_list_begin_end (uiout, "args");
>        TRY_CATCH (e, RETURN_MASK_ERROR)
>   {
> -  print_frame_args (func, frame, numargs, gdb_stdout);
> +  const struct frame_symbol_info *frame_symbol;
> +  frame_symbol = get_frame_symbol_info (frame);
> +
> +  if (frame_symbol != NULL)
> +    {
> +      if (frame_symbol->arguments != NULL)
> + ui_out_text (uiout, frame_symbol->arguments);
> +    }
> +  else
> +    print_frame_args (func, frame, numargs, gdb_stdout);
>
> This is just wrong.  It's printing the frame arguments as
> a single, and simple string.  This simply doesn't work correctly
> with MI frontends.  Or what happens if the user selects
> one of those JIT frames and tries to print one of the arguments?
>
> I really think the way this patch is hooking a bespoke
> function/line/args mechanism into the frame machinery is
> quite hacky as is, sorry.  :-/
>
> --
> Pedro Alves
>

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

end of thread, other threads:[~2014-06-02  1:15 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-26 18:36 [RFC][PATCH] Allow JIT unwinder provide symbol information Sasha Smundak
2014-01-13 18:25 ` Alexander Smundak
2014-02-07 21:54   ` Alexander Smundak
2014-01-13 18:46 ` Doug Evans
2014-01-15  0:39   ` Alexander Smundak
2014-02-11 22:26     ` Doug Evans
2014-02-12  7:50       ` Doug Evans
2014-02-19  3:30         ` Alexander Smundak
2014-02-19  3:50           ` Eli Zaretskii
2014-02-19  5:23             ` Alexander Smundak
2014-04-11 18:47           ` Doug Evans
2014-04-11 18:58           ` Doug Evans
2014-04-21  1:35             ` Alexander Smundak
2014-04-21  7:14               ` Eli Zaretskii
2014-04-21 16:43                 ` Alexander Smundak
2014-02-25  1:19         ` Doug Evans
2014-02-25  3:00           ` Alexander Smundak
2014-03-11  1:46           ` Alexander Smundak
2014-02-08  7:08 ` Yao Qi
2014-02-10  2:16   ` Alexander Smundak
2014-02-11 22:00     ` Doug Evans
2014-04-24 13:22 ` Pedro Alves
2014-04-25 23:40   ` Alexander Smundak
2014-04-29 15:22     ` Pedro Alves
2014-05-02 16:58       ` Alexander Smundak
2014-05-19 21:30         ` Alexander Smundak
2014-05-29  1:07       ` Alexander Smundak
2014-06-02  1:15       ` Alexander Smundak

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).