Re: [RFA 1/2] mips: Switch inferior function calls to ON_STACK method.

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Fri, 4 May 2012 13:58:18 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> >  Understood, but I'd be happier if the comment you're removing or a 
> > similar stayed in place.  If by trap you mean SIGTRAP, then I think this 
> > is not going to be the case.
> 
> I think you refer to the comment from Andrew Cagney? I've put it back
> as is.

I really don't think that comment is helpful anymore.  Almost ten
years have gone by without anyone feeling the need to implement the
command Andrew is alluding to here,

> OK to commit, modulo the possible rename above?

Seems you missed my comment about the the mips_frame_align() call.
It's unecessary since the frame is already properly aligned by the
calles of push_dummy_code().

> commit b78a75e1442a349531a017036a02f43c4df71427
> Author: Joel Brobecker <brobecker@adacore.com>
> Date:   Wed May 2 20:39:57 2012 -0400
> 
>     mips: Switch inferior function calls to ON_STACK method.
>     
>     This patch switches the mips code to use the ON_STACK method
>     for function calls instead of AT_SYMBOL, which we want to remove.
>     
>     gdb/ChangeLog:
>     
>             * mips-tdep.c (mips_push_dummy_code): New function.
>             (mips_gdbarch_init): Set the gdbarch call_dummy_location to
>             ON_STACK and install mips_push_dummy_code as our gdbarch
>             push_dummy_code routine.
> 
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 9a3c7fb..5e9a6ed 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -3009,6 +3009,37 @@ mips_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
>    return align_down (addr, 16);
>  }
>  
> +/* Implement the push_dummy_code gdbarch method for mips targets.  */

Perhaps change that comment into:

  /* Implement the "push_dummy_call" gdbarch method.  */

such that it is consistent with the style of comments in rl78-tdep.c
and rx-tdep.c and moxie-tdep.c?  The "for mips targets" isn't really
adding any information (and might end up accidentally being copied).

> +
> +static CORE_ADDR
> +mips_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
> +		      CORE_ADDR funaddr, struct value **args,
> +		      int nargs, struct type *value_type,
> +		      CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
> +		      struct regcache *regcache)
> +{
> +  int bp_len;
> +  gdb_byte null_insn[4] = { 0 };
> +
> +  *bp_addr = mips_frame_align (gdbarch, sp);
> +  gdbarch_breakpoint_from_pc (gdbarch, bp_addr, &bp_len);
> +
> +  /* The breakpoint layer automatically adjusts the address of
> +     breakpoints inserted in a branch delay slot.  With enough
> +     bad luck, the 4 bytes located just before our breakpoint
> +     instruction could look like a branch instruction, and thus
> +     trigger the adjustement, and break the function call entirely.
> +     So, we reserve those 4 bytes and write a null instruction
> +     to prevent that from happening.  */
> +  write_memory (*bp_addr - bp_len, null_insn, sizeof (null_insn));
> +  sp = mips_frame_align (gdbarch, *bp_addr - 2 * bp_len);
> +
> +  /* Inferior resumes at the function entry point.  */
> +  *real_pc = funaddr;
> +
> +  return sp;
> +}
> +
>  static CORE_ADDR
>  mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  			   struct regcache *regcache, CORE_ADDR bp_addr,
> @@ -6909,7 +6940,8 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    /* NOTE: cagney/2003-08-05: Eventually call dummy location will be
>       replaced by a command, and all targets will default to on stack
>       (regardless of the stack's execute status).  */
> -  set_gdbarch_call_dummy_location (gdbarch, AT_SYMBOL);
> +  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
> +  set_gdbarch_push_dummy_code (gdbarch, mips_push_dummy_code);
>    set_gdbarch_frame_align (gdbarch, mips_frame_align);
>  
>    set_gdbarch_convert_register_p (gdbarch, mips_convert_register_p);
> 
> --fXStkuK2IQBfcDe+--
>