Re: [patch 04/12] entryval: Virtual tail call frames

[Pedro Alves <pedro at codesourcery dot com>]

This (and gcc/dwarf's side of the) work is really impressive.  Kudos!

I'll shoot a couple of comments below on a couple of hunks
I noticed on a first skim of this patch.

Sorry for the quick comments.  This definitely deserves a thorougher read.

On Monday 18 July 2011 21:16:58, Jan Kratochvil wrote:

> +static void
> +tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
> +                       struct frame_id *this_id)
> +{
> +  struct tailcall_cache *cache = *this_cache;
> +  struct frame_info *next_frame;
> +  CORE_ADDR this_frame_base;
> +
> +  /* Tail call does not make sense for a sentinel frame.  */
> +  next_frame = get_next_frame (this_frame);
> +  gdb_assert (next_frame != NULL);
> +
> +  /* SP does not change during tail calls.  */
> +  this_frame_base = get_frame_base (next_frame);

Should probably preserve frame_id->special_addr as well, for ia64.

> +
> +  (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));
> +  (*this_id).inline_depth = (cache->chain_levels
> +                            - existing_next_levels (this_frame, cache));
> +  gdb_assert ((*this_id).inline_depth > 0);
> +}

> @@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
>    CORE_ADDR addr;
>    int realnum;
>  
> +  /* Virtual tail call frames report different values only for PC.  Non-bottom
> +     frames of a virtual tail call frames chain use
> +     dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
> +     them.  */
> +  if (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
> +    return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
> +                                                      &cache->tailcall_cache,
> +                                                      regnum);
> +

This looked strange.  Isn't it breaking some abstraction?
I would have expected dwarf2_tailcall_frame_unwind.prev_register
to be called _first_ (automatically by frame_prev_register) and then
have that defer to dwarf2_frame_prev_register when necessary.

-- 
Pedro Alves