Re: [PATCH 16/18] Add tdep files for x86 and powerpc.

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

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>
> 
> This patch adds a new GDB_OSABI_VXWORKS for VxWorks targets, and
> introduces new VxWorks tdep files for x86 and powerpc.  They will
> be included in the build by the next patch.
> 
> gdb/ChangeLog:
> 
>         * defs.h (enum gdb_osabi): Add GDB_OSABI_VXWORKS.
>         * osabi.c (gdb_osabi_names): Add entry for GDB_OSABI_VXWORKS.
>         * i386-vxworks-tdep.c, rs6000-vxworks-tdep.c: New files.
> ---
>  gdb/defs.h                |    1 +
>  gdb/i386-vxworks-tdep.c   |   53 +++++++++++++++++++++++
>  gdb/osabi.c               |    1 +
>  gdb/rs6000-vxworks-tdep.c |  105 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 160 insertions(+), 0 deletions(-)
>  create mode 100644 gdb/i386-vxworks-tdep.c
>  create mode 100644 gdb/rs6000-vxworks-tdep.c

The i386 bits have my blessing.  The powerpc bits look a bit weird though:

> +static CORE_ADDR
> +rs6000_vxworks_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)
> +{
> +  /* Something here to findout the size of a breakpoint and then
> +     allocate space for it on the stack.  */
> +  int bplen;
> +  /* This code assumes frame align.  */
> +  gdb_assert (gdbarch_frame_align_p (gdbarch));
> +  /* Force the stack's alignment.  The intent is to ensure that the SP
> +     is aligned to at least a breakpoint instruction's boundary.  */
> +  sp = gdbarch_frame_align (gdbarch, sp);
> +  /* Allocate space for, and then position the breakpoint on the stack.  */
> +  if (gdbarch_inner_than (gdbarch, 1, 2))
> +    {
> +      CORE_ADDR bppc = sp;
> +      gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen);
> +      sp = gdbarch_frame_align (gdbarch, sp - bplen);
> +      (*bp_addr) = sp;
> +      /* Should the breakpoint size/location be re-computed here?  */
> +    }
> +  else
> +    {
> +      (*bp_addr) = sp;
> +      gdbarch_breakpoint_from_pc (gdbarch, bp_addr, &bplen);
> +      sp = gdbarch_frame_align (gdbarch, sp + bplen);
> +    }

This bit of code looks like it is trying to support both stacks
growing up and stack growing down.  Does that really happen on
VxWorks?  Your code doesn't call set_gdbarch_inner_than(), so at least
one of these branches must be dead code.