This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
- From: Doug Evans <dje at google dot com>
- To: Sasha Smundak <asmundak at google dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 13 Jan 2014 10:46:00 -0800
- Subject: Re: [RFC][PATCH] Allow JIT unwinder provide symbol information
- Authentication-results: sourceware.org; auth=none
- References: <20131226183618 dot D264CA18A0 at sasha2 dot mtv dot corp dot google dot com>
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.