From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24333 invoked by alias); 20 Aug 2011 17:59:03 -0000 Received: (qmail 24318 invoked by uid 22791); 20 Aug 2011 17:59:00 -0000 X-SWARE-Spam-Status: No, hits=-5.5 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ,TW_DB,TW_WH X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 20 Aug 2011 17:58:42 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7KHwfCC009204 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 20 Aug 2011 13:58:42 -0400 Received: from host1.jankratochvil.net (ovpn-116-42.ams2.redhat.com [10.36.116.42]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p7KHwd6I008864 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sat, 20 Aug 2011 13:58:41 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p7KHwdAo003769; Sat, 20 Aug 2011 19:58:39 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p7KHwcCS003765; Sat, 20 Aug 2011 19:58:38 +0200 Date: Sat, 20 Aug 2011 17:59:00 -0000 From: Jan Kratochvil To: Sanjoy Das Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 5/6] New JIT unwinder. Message-ID: <20110820175838.GC26447@host1.jankratochvil.net> References: <1313821635-22137-1-git-send-email-sanjoy@playingwithpointers.com> <1313821635-22137-6-git-send-email-sanjoy@playingwithpointers.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1313821635-22137-6-git-send-email-sanjoy@playingwithpointers.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00399.txt.bz2 On Sat, 20 Aug 2011 08:27:14 +0200, Sanjoy Das wrote: > Introduce a "proxy unwinder", whcih will pass down all calls to the > functions the JIT reader provides. > > gdb/ChangeLog > > * jit.c (jut_unwind_reg_set_impl, free_reg_value_impl) > (jit_unwind_reg_get_impl, jit_frame_sniffer) > (jit_frame_unwind_stop_reason, jit_frame_this_id) > (jit_frame_prev_register, jit_dealloc_cache) > (jit_gdbarch_data_init): New functions > (_initialize_jit): Register new gdbarch data slot > jit_gdbarch_data. > --- > gdb/ChangeLog | 10 ++ > gdb/jit.c | 260 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 270 insertions(+), 0 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 70c280e..92d645b 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,5 +1,15 @@ > 2011-08-20 Sanjoy Das > > + * jit.c (jut_unwind_reg_set_impl, free_reg_value_impl) > + (jit_unwind_reg_get_impl, jit_frame_sniffer) > + (jit_frame_unwind_stop_reason, jit_frame_this_id) > + (jit_frame_prev_register, jit_dealloc_cache) > + (jit_gdbarch_data_init): New functions > + (_initialize_jit): Register new gdbarch data slot > + jit_gdbarch_data. > + > +2011-08-20 Sanjoy Das > + > * jit.c (add_objfile_entry, jit_target_read_impl) > (jit_object_open_impl, jit_symtab_open_impl, compare_block) > (jit_block_open_impl, jit_block_open_impl) > diff --git a/gdb/jit.c b/gdb/jit.c > index d4d7ddb..da39525 100644 > --- a/gdb/jit.c > +++ b/gdb/jit.c > @@ -31,6 +31,7 @@ > #include "inferior.h" > #include "observer.h" > #include "objfiles.h" > +#include "regcache.h" > #include "symfile.h" > #include "symtab.h" > #include "target.h" > @@ -49,6 +50,12 @@ static const struct inferior_data *jit_inferior_data = NULL; > > static void jit_inferior_init (struct gdbarch *gdbarch); > > +/* An unwinder is registered for every gdbarch. This key is used to > + remember if the unwinder has been registered for a particular > + gdbarch. */ > + > +static struct gdbarch_data *jit_gdbarch_data; > + > /* Non-zero if we want to see trace of jit level stuff. */ > > static int jit_debug = 0; > @@ -907,6 +914,244 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, > return 0; > } > > +/* The private data passed around in the frame unwind callback > + functions. */ > + > +struct jit_unwind_private > +{ > + /* Cached register values. See jit_frame_sniffer to see how this > + works. */ > + > + struct gdb_reg_value **registers; > + > + /* The frame being unwound. */ > + > + struct frame_info *this_frame; > +}; > + > +/* Sets the value of a particular register in this frame. */ > + > +static void > +jit_unwind_reg_set_impl (struct gdb_unwind_callbacks *cb, int regnum, > + struct gdb_reg_value *value) Please rename / comment REGNUM it is DWARF register. > +{ > + struct jit_unwind_private *priv; > + int gdb_reg; > + > + gdb_reg = gdbarch_dwarf2_reg_to_regnum (target_gdbarch, regnum); gdb_assert (gdb_reg != -1); but maybe it should be rather some non-fatal error. > + priv = cb->priv_data; > + gdb_assert (priv->registers); > + priv->registers[gdb_reg] = value; > +} > + > +/* Passed in the `free' field of a gdb_reg_value. */ > + > +static void > +free_reg_value_impl (struct gdb_reg_value *reg_value) > +{ > + xfree (reg_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) > +{ > + struct jit_unwind_private *priv; > + struct gdb_reg_value *value; > + int gdb_reg, size; > + > + priv = cb->priv_data; > + gdb_reg = gdbarch_dwarf2_reg_to_regnum (target_gdbarch, regnum); > + size = register_size (target_gdbarch, gdb_reg); > + value = xmalloc (sizeof (struct gdb_reg_value) + size - 1); > + value->defined = frame_register_read (priv->this_frame, gdb_reg, > + value->value); > + value->size = size; > + value->free = free_reg_value_impl; > + return value; > +} > + > +/* The frame sniffer for the pseudo unwinder. > + > + While this is nominally a frame sniffer, in the case where the JIT > + reader actually recognizes the frame, it does a lot more work -- it > + unwinds the frame and saves the corresponding register values in > + the cache. jit_frame_prev_register simply returns the saved > + register values. */ > + > +static int > +jit_frame_sniffer (const struct frame_unwind *self, > + struct frame_info *this_frame, void **cache) > +{ > + struct jit_inferior_data *inf_data; > + struct jit_unwind_private *priv_data; > + struct jit_dbg_reader *iter; > + struct gdb_unwind_callbacks callbacks; > + struct gdb_reader_funcs *funcs; const struct gdb_reader_funcs *funcs; > + > + inf_data = get_jit_inferior_data (); > + > + callbacks.reg_get = jit_unwind_reg_get_impl; > + callbacks.reg_set = jit_unwind_reg_set_impl; > + callbacks.target_read = jit_target_read_impl; Couldn't GDB just export these functions to the plugin without having to use the callbacks vector? Python does so, libthread_db does so (see proc-service.list). > + > + if (loaded_jit_reader == NULL) > + return 0; > + > + funcs = loaded_jit_reader->functions; > + > + if (!*cache) > + { > + *cache = XZALLOC (struct jit_unwind_private); > + priv_data = *cache; > + priv_data->registers = XCALLOC (gdbarch_num_regs (target_gdbarch), > + struct gdb_reg_value *); Registers for this frame can be different from target_gdbarch, you should use get_frame_arch. Please read: Re: [00/03] per-aspace target_gdbarch (+local gdbarch obsoletion?) http://sourceware.org/ml/gdb-patches/2010-01/msg00497.html > + priv_data->this_frame = this_frame; > + } > + else When can be *cache != NULL? IMO never. Otherwise I think there would be some reference counting problems or so. > + { > + priv_data = *cache; > + priv_data->this_frame = this_frame; > + } > + > + callbacks.priv_data = priv_data; > + > + /* Try to coax the provided unwinder to unwind the stack */ > + if (funcs->unwind (funcs, &callbacks) == GDB_SUCCESS) > + { > + if (jit_debug) > + fprintf_unfiltered (gdb_stdlog, "Successfully unwound frame using " > + "JIT reader.\n"); Missing _(...) localization. > + return 1; > + } > + if (jit_debug) > + fprintf_unfiltered (gdb_stdlog, "Could not unwind frame using " > + "JIT reader.\n"); Missing _(...) localization. > + > + xfree (priv_data->registers); > + xfree (priv_data); Could you reuse jit_dealloc_cache instead? > + *cache = NULL; > + > + return 0; > +} > + > +/* Also for the pseudo unwinder. */ > + > +static enum unwind_stop_reason > +jit_frame_unwind_stop_reason (struct frame_info *this_frame, void **cache) > +{ > + return UNWIND_NO_REASON; > +} Just use default_frame_unwind_stop_reason. > + > +/* The frame_id function for the pseudo unwinder. Relays the call to > + the loaded plugin. */ > + > +static void > +jit_frame_this_id (struct frame_info *this_frame, void **cache, > + struct frame_id *this_id) > +{ > + struct jit_unwind_private private; > + struct gdb_frame_id frame_id; > + struct gdb_reader_funcs *funcs; > + struct gdb_unwind_callbacks callbacks; > + > + private.registers = NULL; > + private.this_frame = this_frame; > + > + /* 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; > + callbacks.priv_data = &private; > + > + gdb_assert (loaded_jit_reader); > + funcs = loaded_jit_reader->functions; > + > + frame_id = funcs->get_frame_id (funcs, &callbacks); > + *this_id = frame_id_build (frame_id.stack_address, frame_id.code_address); Why to call frame_id_build at all and not just to use frame_id? > +} > + > +/* Pseudo unwinder function. Reads the previously fetched value for > + the register from the cache. */ > + > +static struct value * > +jit_frame_prev_register (struct frame_info *this_frame, void **cache, int reg) > +{ > + struct jit_unwind_private *priv = *cache; > + struct gdb_reg_value *value; > + > + if (priv == NULL) > + return frame_unwind_got_optimized (this_frame, reg); > + > + gdb_assert (priv->registers); > + value = priv->registers[reg]; > + if (value && value->defined) > + return frame_unwind_got_bytes (this_frame, reg, value->value); > + else > + return frame_unwind_got_optimized (this_frame, reg); > +} > + > +/* gdb_reg_value has a free function, which must be called on each > + saved register value. */ > + > +static void > +jit_dealloc_cache (struct frame_info *this_frame, void *cache) > +{ > + struct jit_unwind_private *priv_data = cache; > + int i; > + > + gdb_assert (priv_data->registers); > + > + for (i = 0; i < gdbarch_num_regs (target_gdbarch); i++) > + if (priv_data->registers[i] && priv_data->registers[i]->free) > + priv_data->registers[i]->free (priv_data->registers[i]); > + > + xfree (priv_data->registers); > + xfree (priv_data); > +} > + > +/* Relay everything back to the unwinder registered by the JIT debug > + info reader.*/ > + > +static const struct frame_unwind jit_frame_unwind = > +{ > + NORMAL_FRAME, > + jit_frame_unwind_stop_reason, > + jit_frame_this_id, > + jit_frame_prev_register, > + NULL, > + jit_frame_sniffer, > + jit_dealloc_cache > +}; > + > + > +/* This is the information that is stored at jit_gdbarch_data for each > + architecture. */ > + > +struct jit_gdbarch_data_type { struct jit_gdbarch_data_type { > + > + /* Has the (pseudo) unwinder been prepended? */ > + > + int unwinder_registered; > +}; > + > +/* Check GDBARCH and prepend the pseudo JIT unwinder if needed. */ > + > +static void > +jit_prepend_unwinder (struct gdbarch *gdbarch) > +{ > + struct jit_gdbarch_data_type *data; > + > + data = gdbarch_data (gdbarch, jit_gdbarch_data); > + if (!data->unwinder_registered) > + { > + frame_unwind_prepend_unwinder (gdbarch, &jit_frame_unwind); > + data->unwinder_registered = 1; > + } > +} > + > /* Register any already created translations. */ > > static void > @@ -920,6 +1165,8 @@ jit_inferior_init (struct gdbarch *gdbarch) > if (jit_debug) > fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n"); > > + jit_prepend_unwinder (gdbarch); > + > inf_data = get_jit_inferior_data (); > if (jit_breakpoint_re_set_internal (gdbarch, inf_data) != 0) > return; > @@ -1082,6 +1329,18 @@ free_objfile_data (struct objfile *objfile, void *data) > xfree (data); > } > > +/* Initialize the jit_gdbarch_data slot with an instance of struct > + jit_gdbarch_data_type */ > + > +static void * > +jit_gdbarch_data_init (struct obstack *obstack) > +{ > + struct jit_gdbarch_data_type *data = > + obstack_alloc (obstack, sizeof (struct jit_gdbarch_data_type)); Empty line from declarations. > + data->unwinder_registered = 0; > + return data; > +} > + > /* Provide a prototype to silence -Wmissing-prototypes. */ > > extern void _initialize_jit (void); > @@ -1104,6 +1363,7 @@ _initialize_jit (void) > register_objfile_data_with_cleanup (NULL,free_objfile_data); > jit_inferior_data = > register_inferior_data_with_cleanup (jit_inferior_data_cleanup); > + jit_gdbarch_data = gdbarch_data_register_pre_init (jit_gdbarch_data_init); > add_com ("load-jit-reader", no_class, load_jit_reader_command, _("\ > Try to load file FILE as a debug info reader (and unwinder) for\n\ > JIT compiled code from " LIBDIR "/gdb\n\ > -- > 1.7.5.4 Thanks, Jan