This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [draft patch] <unavailable> unwinder for btrace [Re: [rfc 3/5] record: make it build again]


> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Monday, February 11, 2013 6:13 PM

Thanks, Jan,

I'll look at it once the "record disassembly" and "record list" commands are working for record-btrace.

Did you commit this to some archer branch, as well?

Thanks,
Markus.


> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org; markus.t.metzger@gmail.com
> Subject: [draft patch] <unavailable> unwinder for btrace [Re: [rfc 3/5] record: make it build again]
> 
> On Mon, 11 Feb 2013 15:14:51 +0100, Jan Kratochvil wrote:
> > FYI I have some draft <unavailable>-limitation unwinder for btrace here today,
> > to post it later.
> 
> ./gdb -q ./gdb -ex start -ex record -ex stepi -ex reverse-stepi -ex bt -ex q
> 
> No more reverse-execution history.
> main (argc=<unavailable>, argv=<unavailable>) at gdb.c:29
> 29    memset (&args, 0, sizeof args);
> #0  main (argc=<unavailable>, argv=<unavailable>) at gdb.c:29
> Backtrace stopped: not enough registers or memory available to unwind further
> 
> 
> That get_frame_unwind_stop_reason "optimization" to avoid calling an unwinder
> was present already in the initial post:
> 	[rfc, frame] Add backtrace stop reasons
> 	http://sourceware.org/ml/gdb-patches/2006-08/msg00129.html
> 	Message-ID: <20060819154646.GA25238@nevyn.them.org>
> I don't know its real reason but it is needed for this patch otherwise:
> main (argc=<error reading variable: can't compute CFA for this frame>, argv=<error reading variable: can't compute CFA for this
> frame>) at gdb.c:29
> 
> 
> For btrace you will need ptid_t of the unwound thread as the RECORD_IS_REPLAY
> flag is apparently irrelevant there.  I find inferior_ptid to use there
> acceptable despite it is discouraged now, passing there ptid_t from the
> unwinder framework needs some larger changes.
> 
> No real regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu and in
> gdbserver mode, with the patch as is the various *reverse* tests apparently
> FAIL due to the <unavailable> values (both displayed and implicitly missing).
> 
> There should be a testcase but that will be dependent upon the btrace
> integration.
> 
> 
> Regards,
> Jan
> 
> 
> gdb/
> 2013-02-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	New record unwinder reporting <unavailable>.
> 	* dwarf2-frame.c (dwarf2_frame_cfa): Move UNWIND_UNAVAILABLE check
> 	earlier.
> 	* frame-unwind.c: Include target.h.
> 	(frame_unwind_try_unwinder): New function with code from ...
> 	(frame_unwind_find_by_frame): ... here.  New variable
> 	unwinder_from_target, call also target_get_unwinder and
> 	frame_unwind_try_unwinder for it.
> 	* frame.c (get_frame_unwind_stop_reason): Unconditionally call
> 	get_prev_frame_1.
> 	* record.c: Include frame-unwind.h.
> 	(record_frame_unwind_stop_reason, record_frame_this_id)
> 	(record_frame_prev_register, record_frame_sniffer, record_frame_unwind):
> 	New.
> 	(init_record_ops, init_record_core_ops): Install it.
> 	* target.c (target_get_unwinder): New.
> 	* target.h (struct target_ops): New field to_get_unwinder.
> 	(target_get_unwinder): New declaration.
> 
> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index ec4edfa..45be107 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -1497,16 +1497,16 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
>  {
>    while (get_frame_type (this_frame) == INLINE_FRAME)
>      this_frame = get_prev_frame (this_frame);
> +  if (get_frame_unwind_stop_reason (this_frame) == UNWIND_UNAVAILABLE)
> +    throw_error (NOT_AVAILABLE_ERROR,
> +		 _("can't compute CFA for this frame: "
> +		   "required registers or memory are unavailable"));
>    /* This restriction could be lifted if other unwinders are known to
>       compute the frame base in a way compatible with the DWARF
>       unwinder.  */
>    if (!frame_unwinder_is (this_frame, &dwarf2_frame_unwind)
>        && !frame_unwinder_is (this_frame, &dwarf2_tailcall_frame_unwind))
>      error (_("can't compute CFA for this frame"));
> -  if (get_frame_unwind_stop_reason (this_frame) == UNWIND_UNAVAILABLE)
> -    throw_error (NOT_AVAILABLE_ERROR,
> -		 _("can't compute CFA for this frame: "
> -		   "required registers or memory are unavailable"));
>    return get_frame_base (this_frame);
>  }
>  

> diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
> index b66febf..cf33147 100644
> --- a/gdb/frame-unwind.c
> +++ b/gdb/frame-unwind.c
> @@ -27,6 +27,7 @@
>  #include "exceptions.h"
>  #include "gdb_assert.h"
>  #include "gdb_obstack.h"
> +#include "target.h"
> 
>  static struct gdbarch_data *frame_unwind_data;
> 
> @@ -88,6 +89,48 @@ frame_unwind_append_unwinder (struct gdbarch *gdbarch,
>    (*ip)->unwinder = unwinder;
>  }
> 
> +/* Call SNIFFER from UNWINDER.  If it succeeded set UNWINDER for
> +   THIS_FRAME and return 1.  Otherwise the function keeps THIS_FRAME
> +   unchanged and returns 0.  */
> +
> +static int
> +frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
> +			   const struct frame_unwind *unwinder)
> +{
> +  struct cleanup *old_cleanup;
> +  volatile struct gdb_exception ex;
> +  int res = 0;
> +
> +  old_cleanup = frame_prepare_for_sniffer (this_frame, unwinder);
> +
> +  TRY_CATCH (ex, RETURN_MASK_ERROR)
> +    {
> +      res = unwinder->sniffer (unwinder, this_frame, this_cache);
> +    }
> +  if (ex.reason < 0 && ex.error == NOT_AVAILABLE_ERROR)
> +    {
> +      /* This usually means that not even the PC is available,
> +	 thus most unwinders aren't able to determine if they're
> +	 the best fit.  Keep trying.  Fallback prologue unwinders
> +	 should always accept the frame.  */
> +      do_cleanups (old_cleanup);
> +      return 0;
> +    }
> +  else if (ex.reason < 0)
> +    throw_exception (ex);
> +  else if (res)
> +    {
> +      discard_cleanups (old_cleanup);
> +      return 1;
> +    }
> +  else
> +    {
> +      do_cleanups (old_cleanup);
> +      return 0;
> +    }
> +  gdb_assert_not_reached ("frame_unwind_try_unwinder");
> +}
> +
>  /* Iterate through sniffers for THIS_FRAME frame until one returns with an
>     unwinder implementation.  THIS_FRAME->UNWIND must be NULL, it will get set
>     by this function.  Possibly initialize THIS_CACHE.  */
> @@ -98,37 +141,18 @@ frame_unwind_find_by_frame (struct frame_info *this_frame, void **this_cache)
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>    struct frame_unwind_table *table = gdbarch_data (gdbarch, frame_unwind_data);
>    struct frame_unwind_table_entry *entry;
> +  const struct frame_unwind *unwinder_from_target;
> +
> +  unwinder_from_target = target_get_unwinder ();
> +  if (unwinder_from_target != NULL
> +      && frame_unwind_try_unwinder (this_frame, this_cache,
> +				    unwinder_from_target))
> +    return;
> 
>    for (entry = table->list; entry != NULL; entry = entry->next)
> -    {
> -      struct cleanup *old_cleanup;
> -      volatile struct gdb_exception ex;
> -      int res = 0;
> -
> -      old_cleanup = frame_prepare_for_sniffer (this_frame, entry->unwinder);
> -
> -      TRY_CATCH (ex, RETURN_MASK_ERROR)
> -	{
> -	  res = entry->unwinder->sniffer (entry->unwinder, this_frame,
> -					  this_cache);
> -	}
> -      if (ex.reason < 0 && ex.error == NOT_AVAILABLE_ERROR)
> -	{
> -	  /* This usually means that not even the PC is available,
> -	     thus most unwinders aren't able to determine if they're
> -	     the best fit.  Keep trying.  Fallback prologue unwinders
> -	     should always accept the frame.  */
> -	}
> -      else if (ex.reason < 0)
> -	throw_exception (ex);
> -      else if (res)
> -        {
> -          discard_cleanups (old_cleanup);
> -          return;
> -        }
> +    if (frame_unwind_try_unwinder (this_frame, this_cache, entry->unwinder))
> +      return;
> 
> -      do_cleanups (old_cleanup);
> -    }
>    internal_error (__FILE__, __LINE__, _("frame_unwind_find_by_frame failed"));
>  }
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 0821b6e..8fc1ca9 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2357,13 +2357,10 @@ get_frame_sp (struct frame_info *this_frame)
>  enum unwind_stop_reason
>  get_frame_unwind_stop_reason (struct frame_info *frame)
>  {
> -  /* If we haven't tried to unwind past this point yet, then assume
> -     that unwinding would succeed.  */
> -  if (frame->prev_p == 0)
> -    return UNWIND_NO_REASON;
> +  /* Fill-in STOP_REASON.  */
> +  get_prev_frame_1 (frame);
> +  gdb_assert (frame->prev_p);
> 
> -  /* Otherwise, we set a reason when we succeeded (or failed) to
> -     unwind.  */
>    return frame->stop_reason;
>  }
> 
> diff --git a/gdb/record.c b/gdb/record.c
> index 1a68738..dffce98 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -34,6 +34,7 @@
>  #include "inf-loop.h"
>  #include "gdb_bfd.h"
>  #include "observer.h"
> +#include "frame-unwind.h"
> 
>  #include <signal.h>
> 
> @@ -1953,6 +1954,59 @@ record_execution_direction (void)
>    return record_execution_dir;
>  }
> 
> +/* Implement stop_reason method for record_frame_unwind.  */
> +
> +static enum unwind_stop_reason
> +record_frame_unwind_stop_reason (struct frame_info *this_frame,
> +				 void **this_cache)
> +{
> +  return UNWIND_UNAVAILABLE;
> +}
> +
> +/* Implement this_id method for record_frame_unwind.  */
> +
> +static void
> +record_frame_this_id (struct frame_info *this_frame, void **this_cache,
> +		      struct frame_id *this_id)
> +{
> +  /* Leave there the outer_frame_id value.  */
> +}
> +
> +/* Implement prev_register method for record_frame_unwind.  */
> +
> +static struct value *
> +record_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> +			    int regnum)
> +{
> +  throw_error (NOT_AVAILABLE_ERROR,
> +	       _("Registers are not available in btrace record history"));
> +}
> +
> +/* Implement sniffer method for record_frame_unwind.  */
> +
> +static int
> +record_frame_sniffer (const struct frame_unwind *self,
> +		      struct frame_info *this_frame, void **this_cache)
> +{
> +  return RECORD_IS_REPLAY;
> +}
> +
> +/* btrace recording does not store previous memory content, neither the stack
> +   frames content.  Any unwinding would return errorneous results as the stack
> +   contents no longer matches the changed PC value restored from history.
> +   Therefore this unwinder reports any possibly unwound registers as
> +   <unavailable>.  */
> +
> +static const struct frame_unwind record_frame_unwind =
> +{
> +  NORMAL_FRAME,
> +  record_frame_unwind_stop_reason,
> +  record_frame_this_id,
> +  record_frame_prev_register,
> +  NULL,
> +  record_frame_sniffer
> +};
> +
>  static void
>  init_record_ops (void)
>  {
> @@ -1984,6 +2038,7 @@ init_record_ops (void)
>    record_ops.to_can_async_p = record_can_async_p;
>    record_ops.to_is_async_p = record_is_async_p;
>    record_ops.to_execution_direction = record_execution_direction;
> +  record_ops.to_get_unwinder = &record_frame_unwind;
>    record_ops.to_magic = OPS_MAGIC;
>  }
> 
> @@ -2209,6 +2264,7 @@ init_record_core_ops (void)
>    record_core_ops.to_can_async_p = record_can_async_p;
>    record_core_ops.to_is_async_p = record_is_async_p;
>    record_core_ops.to_execution_direction = record_execution_direction;
> +  record_core_ops.to_get_unwinder = &record_frame_unwind;
>    record_core_ops.to_magic = OPS_MAGIC;
>  }
> 
> diff --git a/gdb/target.c b/gdb/target.c
> index 9d8bf6e..28f00af 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -4147,6 +4147,20 @@ target_ranged_break_num_registers (void)
>    return -1;
>  }
> 
> +/* See target.h.  */
> +
> +const struct frame_unwind *
> +target_get_unwinder (void)
> +{
> +  struct target_ops *t;
> +
> +  for (t = current_target.beneath; t != NULL; t = t->beneath)
> +    if (t->to_get_unwinder != NULL)
> +      return t->to_get_unwinder;
> +
> +  return NULL;
> +}
> +
>  static void
>  debug_to_prepare_to_store (struct regcache *regcache)
>  {
> diff --git a/gdb/target.h b/gdb/target.h
> index 1971265..95bfb47 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -857,6 +857,10 @@ struct target_ops
>      /* Is the target able to use agent in current state?  */
>      int (*to_can_use_agent) (void);
> 
> +    /* This unwinder is tried before any other arch unwinders.  Use NULL if it
> +       is not used.  */
> +    const struct frame_unwind *to_get_unwinder;
> +
>      int to_magic;
>      /* Need sub-structure for target machine related rather than comm related?
>       */
> @@ -1735,6 +1739,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
> 
>  extern int target_core_of_thread (ptid_t ptid);
> 
> +/* See to_get_unwinder in struct target_ops.  */
> +extern const struct frame_unwind *target_get_unwinder (void);
> +
>  /* Verify that the memory in the [MEMADDR, MEMADDR+SIZE) range matches
>     the contents of [DATA,DATA+SIZE).  Returns 1 if there's a match, 0
>     if there's a mismatch, and -1 if an error is encountered while
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]