Re: [i386newframe/PATCH] New i386newframe branch

[Andrew Cagney <ac131313@redhat.com>]

(thanks for eliminating the code cache!).

> +static int
> +i386_frameless_function_invocation (struct frame_info *frame)
>  {

I'd consider deleting it.

For non legacy architectures, apart from appending "(FRAMELESS)" to the 
output of "info frame", that architecture method does nothing useful 
(and even this is marginal :-).  I think a per frame 
PRINT_EXTRA_FRAME_INFO like function would be more useful. 
Alternatively, a per-frame ``frameless'' attribute could be added.

Have you tried adding just this:

> +/* Signal trampolines.  */
> +
> +static struct i386_frame_cache *
> +i386_sigtramp_frame_cache (struct frame_info *next_frame, void **this_cache)
> +{
> +  struct i386_frame_cache *cache;
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
> +  CORE_ADDR addr;
> +  char buf[4];
> +
> +  if (*this_cache)
> +    return *this_cache;
> +
> +  cache = i386_alloc_frame_cache ();
> +
> +  frame_unwind_register (next_frame, I386_ESP_REGNUM, buf);
> +  cache->base = extract_address (buf, 4) - 4;
> +
> +  addr = tdep->sigcontext_addr (next_frame);
> +  cache->saved_regs[I386_EIP_REGNUM] = addr + tdep->sc_pc_offset;
> +  cache->saved_regs[I386_ESP_REGNUM] = addr + tdep->sc_sp_offset;
> +
> +  *this_cache = cache;
> +  return cache;
>  }
>  
>  static void
> -i386_pop_frame (void)
> +i386_sigtramp_frame_this_id (struct frame_info *next_frame, void **this_cache,
> +			     struct frame_id *this_id)
>  {
> -  generic_pop_current_frame (i386_do_pop_frame);
> +  struct i386_frame_cache *cache =
> +    i386_sigtramp_frame_cache (next_frame, this_cache);
> +
> +  (*this_id) = frame_id_build (cache->base, frame_pc_unwind (next_frame));
> +}
> +
> +static void
> +i386_sigtramp_frame_prev_register (struct frame_info *next_frame,
> +				   void **this_cache,
> +				   int regnum, int *optimizedp,
> +				   enum lval_type *lvalp, CORE_ADDR *addrp,
> +				   int *realnump, void *valuep)
> +{
> +  /* Make sure we've initialized the cache.  */
> +  i386_sigtramp_frame_cache (next_frame, this_cache);
> +
> +  i386_frame_prev_register (next_frame, this_cache, regnum,
> +			    optimizedp, lvalp, addrp, realnump, valuep);
> +}
> +
> +static const struct frame_unwind i386_sigtramp_frame_unwind =
> +{
> +  SIGTRAMP_FRAME,
> +  i386_sigtramp_frame_this_id,
> +  i386_sigtramp_frame_prev_register
> +};
> +\f
> +
> +const struct frame_unwind *
> +i386_frame_p (CORE_ADDR pc)
> +{
> +  char *name;
> +
> +  find_pc_partial_function (pc, &name, NULL, NULL);
> +  if (PC_IN_SIGTRAMP (pc, name))
> +    return &i386_sigtramp_frame_unwind;

The intent was for there to be one predicate function per unwinder.  So 
the below would be in a separate unwinder, registered separatly.

> +  return &i386_frame_unwind;
> +}

Have you tried adding just the sigtramp unwinder?  I should get just 
that addition debugged regardless - it should make migrating other ISAs 
easier.

Andrew