Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: tromey@redhat.com
Cc: jan.kratochvil@redhat.com, gdb-patches@sourceware.org
Subject: Re: RFC: one approach to fixing PR 14100
Date: Thu, 09 Aug 2012 09:42:00 -0000	[thread overview]
Message-ID: <201208090942.q799gSti019924@glazunov.sibelius.xs4all.nl> (raw)
In-Reply-To: <87vcgwjavg.fsf@fleche.redhat.com> (message from Tom Tromey on	Mon, 06 Aug 2012 09:30:59 -0600)

> From: Tom Tromey <tromey@redhat.com>
> Date: Mon, 06 Aug 2012 09:30:59 -0600
> 
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> Then we need a cleanup instead.
> Tom> I'll look at it on Monday.
> 
> Here it is.
> I regression-tested it on x86-64 Fedora 16.
> 
> Tom
> 
>     	* dwarf2-frame.c (clear_pointer_cleanup): New function.
>     	(dwarf2_frame_cache): Use it.
>     	* frame-unwind.h (frame_sniffer_ftype): Document prologue
>     	cache initialization constraint.

Sorry, but I really think you're working around a problem in the
tailcall sniffer here.  The tailcall sniffer seems to violate several
of the design principles of the frame unwinder framework.  It should
be fixed instead.

> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index 741a103..986aaea 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -994,10 +994,20 @@ struct dwarf2_frame_cache
>    void *tailcall_cache;
>  };
>  
> +/* A cleanup that sets a pointer to NULL.  */
> +
> +static void
> +clear_pointer_cleanup (void *arg)
> +{
> +  void **ptr = arg;
> +
> +  *ptr = NULL;
> +}
> +
>  static struct dwarf2_frame_cache *
>  dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>  {
> -  struct cleanup *old_chain;
> +  struct cleanup *reset_cache_cleanup, *old_chain;
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>    const int num_regs = gdbarch_num_regs (gdbarch)
>  		       + gdbarch_num_pseudo_regs (gdbarch);
> @@ -1017,6 +1027,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>    cache = FRAME_OBSTACK_ZALLOC (struct dwarf2_frame_cache);
>    cache->reg = FRAME_OBSTACK_CALLOC (num_regs, struct dwarf2_frame_state_reg);
>    *this_cache = cache;
> +  reset_cache_cleanup = make_cleanup (clear_pointer_cleanup, this_cache);
>  
>    /* Allocate and initialize the frame state.  */
>    fs = XZALLOC (struct dwarf2_frame_state);
> @@ -1111,6 +1122,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>  	{
>  	  cache->unavailable_retaddr = 1;
>  	  do_cleanups (old_chain);
> +	  discard_cleanups (reset_cache_cleanup);
>  	  return cache;
>  	}
>  
> @@ -1226,6 +1238,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
>  				 (entry_cfa_sp_offset_p
>  				  ? &entry_cfa_sp_offset : NULL));
>  
> +  discard_cleanups (reset_cache_cleanup);
>    return cache;
>  }
>  
> diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
> index f82d763..aa58640 100644
> --- a/gdb/frame-unwind.h
> +++ b/gdb/frame-unwind.h
> @@ -44,7 +44,9 @@ struct value;
>  
>  /* Given THIS frame, take a whiff of its registers (namely
>     the PC and attributes) and if SELF is the applicable unwinder,
> -   return non-zero.  Possibly also initialize THIS_PROLOGUE_CACHE.  */
> +   return non-zero.  Possibly also initialize THIS_PROLOGUE_CACHE; but
> +   only if returning 1.  Initializing THIS_PROLOGUE_CACHE in other
> +   cases (0 return, or exception) is invalid.  */
>  
>  typedef int (frame_sniffer_ftype) (const struct frame_unwind *self,
>  				   struct frame_info *this_frame,
> 


  parent reply	other threads:[~2012-08-09  9:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-02 16:12 Tom Tromey
2012-08-03 16:03 ` Tom Tromey
2012-08-03 21:02   ` Jan Kratochvil
2012-08-04  1:58     ` Tom Tromey
2012-08-04  6:26       ` Jan Kratochvil
2012-08-06 15:31       ` Tom Tromey
2012-08-06 15:45         ` Jan Kratochvil
2012-08-09  9:42         ` Mark Kettenis [this message]
2012-08-09  9:46           ` Jan Kratochvil
2012-08-09 20:12             ` Tom Tromey
2012-08-08 18:27 ` Mark Kettenis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201208090942.q799gSti019924@glazunov.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox