Re: RFC: one approach to fixing PR 14100

Mark Kettenis
2012-08-09T09:42:48Z

> 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,
>