Re: [rfc] Improve NetBSD/i386 signal trampoline detection

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> From: Nick Hudson <nick.hudson@dsl.pipex.com>
> Date: Tue, 2 Oct 2007 12:07:15 +0100
> 
> Hi,
> 
> I put together this patch to improve NetBSD/i386 signal trampoline
> detection.  I believe tramp_frame is a better and preferred method.

I've been hesitant using tramp_frame on i386, since it doesn't have
fixed-length instructions.  This means that there is a chance of false
matches, because you're forced to set the instruction length to 1 byte
(like you do in your diff).

That said, the sequences in your diff are probably long enough to make
the chance of getting a false positive pretty slim.

> I think I need to add the instructions for when the
> setcontext/sigreturn fails as well. Is this correct?

Probably a good idea, since it makes the false positives even less
likely.

> 
> Any/all comments appreciated.

By not calling i386bsd_init_abi(), tdep->jb_pc_offset is no longer
set.  You could initialize tdep->jb_pc_offset in i386nbsd_init_abi(),
but I'd prefer leaving in the i386bsd_init_abi() call, because it
shows the inheritance structure a bit more clearly.

It'd also be nice to have some documentation about what versions of
NetBSD use which sigtramp code (best to stick to official releases
here).  Perhaps that information could be used to give somewhat more
meaningfull names to the tramp_frame varaibles.

> Index: gdb/i386nbsd-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386nbsd-tdep.c,v
> retrieving revision 1.33
> diff -u -p -u -r1.33 i386nbsd-tdep.c
> --- gdb/i386nbsd-tdep.c	23 Aug 2007 18:08:34 -0000	1.33
> +++ gdb/i386nbsd-tdep.c	2 Oct 2007 10:39:21 -0000
> @@ -26,6 +26,8 @@
>  #include "regset.h"
>  #include "osabi.h"
>  #include "symtab.h"
> +#include "trad-frame.h"
> +#include "tramp-frame.h"
>  
>  #include "gdb_assert.h"
>  #include "gdb_string.h"
> @@ -91,109 +93,6 @@ i386nbsd_aout_regset_from_core_section (
>    return NULL;
>  }
>  
> -/* Under NetBSD/i386, signal handler invocations can be identified by the
> -   designated code sequence that is used to return from a signal handler.
> -   In particular, the return address of a signal handler points to the
> -   following code sequence:
> -
> -	leal	0x10(%esp), %eax
> -	pushl	%eax
> -	pushl	%eax
> -	movl	$0x127, %eax		# __sigreturn14
> -	int	$0x80
> -
> -   Each instruction has a unique encoding, so we simply attempt to match
> -   the instruction the PC is pointing to with any of the above instructions.
> -   If there is a hit, we know the offset to the start of the designated
> -   sequence and can then check whether we really are executing in the
> -   signal trampoline.  If not, -1 is returned, otherwise the offset from the
> -   start of the return sequence is returned.  */
> -#define RETCODE_INSN1		0x8d
> -#define RETCODE_INSN2		0x50
> -#define RETCODE_INSN3		0x50
> -#define RETCODE_INSN4		0xb8
> -#define RETCODE_INSN5		0xcd
> -
> -#define RETCODE_INSN2_OFF	4
> -#define RETCODE_INSN3_OFF	5
> -#define RETCODE_INSN4_OFF	6
> -#define RETCODE_INSN5_OFF	11
> -
> -static const unsigned char sigtramp_retcode[] =
> -{
> -  RETCODE_INSN1, 0x44, 0x24, 0x10,
> -  RETCODE_INSN2,
> -  RETCODE_INSN3,
> -  RETCODE_INSN4, 0x27, 0x01, 0x00, 0x00,
> -  RETCODE_INSN5, 0x80,
> -};
> -
> -static LONGEST
> -i386nbsd_sigtramp_offset (struct frame_info *next_frame)
> -{
> -  CORE_ADDR pc = frame_pc_unwind (next_frame);
> -  unsigned char ret[sizeof(sigtramp_retcode)], insn;
> -  LONGEST off;
> -  int i;
> -
> -  if (!safe_frame_unwind_memory (next_frame, pc, &insn, 1))
> -    return -1;
> -
> -  switch (insn)
> -    {
> -    case RETCODE_INSN1:
> -      off = 0;
> -      break;
> -
> -    case RETCODE_INSN2:
> -      /* INSN2 and INSN3 are the same.  Read at the location of PC+1
> -	 to determine if we're actually looking at INSN2 or INSN3.  */
> -      if (!safe_frame_unwind_memory (next_frame, pc + 1, &insn, 1))
> -	return -1;
> -
> -      if (insn == RETCODE_INSN3)
> -	off = RETCODE_INSN2_OFF;
> -      else
> -	off = RETCODE_INSN3_OFF;
> -      break;
> -
> -    case RETCODE_INSN4:
> -      off = RETCODE_INSN4_OFF;
> -      break;
> -
> -    case RETCODE_INSN5:
> -      off = RETCODE_INSN5_OFF;
> -      break;
> -
> -    default:
> -      return -1;
> -    }
> -
> -  pc -= off;
> -
> -  if (!safe_frame_unwind_memory (next_frame, pc, ret, sizeof (ret)))
> -    return -1;
> -
> -  if (memcmp (ret, sigtramp_retcode, sizeof (ret)) == 0)
> -    return off;
> -
> -  return -1;
> -}
> -
> -/* Return whether the frame preceding NEXT_FRAME corresponds to a
> -   NetBSD sigtramp routine.  */
> -
> -static int
> -i386nbsd_sigtramp_p (struct frame_info *next_frame)
> -{
> -  CORE_ADDR pc = frame_pc_unwind (next_frame);
> -  char *name;
> -
> -  find_pc_partial_function (pc, &name, NULL, NULL);
> -  return (nbsd_pc_in_sigtramp (pc, name)
> -	  || i386nbsd_sigtramp_offset (next_frame) >= 0);
> -}
> -
>  /* From <machine/signal.h>.  */
>  int i386nbsd_sc_reg_offset[] =
>  {
> @@ -215,31 +114,176 @@ int i386nbsd_sc_reg_offset[] =
>    0 * 4				/* %gs */
>  };
>  
> +/* From <machine/mcontext.h>.  */
> +int i386nbsd_mc_reg_offset[] =
> +{
> +  11 * 4,			/* %eax */
> +  10 * 4,			/* %ecx */
> +  9 * 4,			/* %edx */
> +  8 * 4,			/* %ebx */
> +  7 * 4,			/* %esp */
> +  6 * 4,			/* %ebp */
> +  5 * 4,			/* %esi */
> +  4 * 4,			/* %edi */
> +  14 * 4,			/* %eip */
> +  16 * 4,			/* %eflags */
> +  15 * 4,			/* %cs */
> +  18 * 4,			/* %ss */
> +  3 * 4,			/* %ds */
> +  2 * 4,			/* %es */
> +  1 * 4,			/* %fs */
> +  0 * 4				/* %gs */
> +};
> +
> +
> +static void
> +i386nbsd_sigtramp_cache_init (const struct tramp_frame *,
> +			     struct frame_info *,
> +			     struct trad_frame_cache *,
> +			     CORE_ADDR);
> +
> +static const struct tramp_frame i386nbsd_sigtramp1 =
> +{
> +  SIGTRAMP_FRAME,
> +  1,
> +  {
> +    { 0x8d, -1 }, { 0x44, -1 }, { 0x24, -1 }, { 0x10, -1 },
> +			/* leal  0x10(%esp), %eax */
> +    { 0x50, -1 },	/* pushl %eax */
> +    { 0x50, -1 },	/* pushl %eax */
> +    { 0xb8, -1 }, { 0x27, -1 }, {0x01, -1 }, {0x00, -1 }, {0x00, -1 },
> +			/* movl  $0x127, %eax		# __sigreturn14 */
> +    { 0xcd, -1 }, { 0x80, -1},
> +			/* int   $0x80 */
> +    { TRAMP_SENTINEL_INSN, -1 }
> +  },
> +  i386nbsd_sigtramp_cache_init
> +};
> +
> +static const struct tramp_frame i386nbsd_sigtramp2 =
> +{
> +  SIGTRAMP_FRAME,
> +  1,
> +  {
> +    { 0x8d, -1 }, { 0x44, -1 }, { 0x24, -1 }, { 0x0c, -1 },
> +			/* leal  0x0c(%esp), %eax */
> +    { 0x89, -1 }, { 0x44, -1 }, { 0x24, -1 }, { 0x04, -1 },
> +			/* movl  %eax, 0x4(%esp) */
> +    { 0xb8, -1 }, { 0x27, -1 }, {0x01, -1 }, {0x00, -1 }, {0x00, -1 },
> +			/* movl  $0x127, %eax		# __sigreturn14 */
> +    { 0xcd, -1 }, { 0x80, -1},
> +			/* int   $0x80 */
> +    { TRAMP_SENTINEL_INSN, -1 }
> +  },
> +  i386nbsd_sigtramp_cache_init
> +};
> +
> +static const struct tramp_frame i386nbsd_sigtramp3 =
> +{
> +  SIGTRAMP_FRAME,
> +  1,
> +  {
> +    { 0x8b, -1}, { 0x44, -1}, { 0x24, -1}, { 0x08, -1},
> +			/* movl  8(%esp),%eax */
> +    { 0x89, -1 }, { 0x44, -1 }, { 0x24, -1 }, { 0x04, -1 },
> +			/* movl  %eax, 0x4(%esp) */
> +    { 0xb8, -1 }, { 0x34, -1 }, { 0x01, -1 }, { 0x00, -1 }, { 0x00, -1 },
> +			/* movl  $0x134, %eax            # setcontext */
> +    { 0xcd, -1 }, { 0x80, -1},
> +			/* int   $0x80 */
> +    /* more... */
> +    { TRAMP_SENTINEL_INSN, -1 }
> +  },
> +  i386nbsd_sigtramp_cache_init
> +};
> +
> +
> +static const struct tramp_frame i386nbsd_sigtramp4 =
> +{
> +  SIGTRAMP_FRAME,
> +  1,
> +  {
> +    { 0x8d, -1 }, { 0x84, -1 }, { 0x24, -1 },
> +        { 0x8c, -1 }, { 0x00, -1 }, { 0x00, -1 }, { 0x00, -1 },
> +			/* leal  0x8c(%esp), %eax */
> +    { 0x89, -1 }, { 0x44, -1 }, { 0x24, -1 }, { 0x04, -1 },
> +			/* movl  %eax, 0x4(%esp) */
> +    { 0xb8, -1 }, { 0x34, -1 }, { 0x01, -1 }, { 0x00, -1 }, { 0x00, -1 },
> +			/* movl  $0x134, %eax            # setcontext */
> +    { 0xcd, -1 }, { 0x80, -1},
> +			/* int   $0x80 */
> +    /* more... */
> +    { TRAMP_SENTINEL_INSN, -1 }
> +  },
> +  i386nbsd_sigtramp_cache_init
> +};
> +
> +static void
> +i386nbsd_sigtramp_cache_init (const struct tramp_frame *self,
> +			     struct frame_info *next_frame,
> +			     struct trad_frame_cache *this_cache,
> +			     CORE_ADDR func)
> +{
> +  struct gdbarch *gdbarch = get_frame_arch (next_frame);
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +  CORE_ADDR sp = frame_unwind_register_unsigned (next_frame, I386_ESP_REGNUM);
> +  CORE_ADDR base;
> +  int *reg_offset;
> +  int num_regs;
> +  int i;
> +
> +  if (self == &i386nbsd_sigtramp1 || self == &i386nbsd_sigtramp2)
> +    {
> +      reg_offset = i386nbsd_sc_reg_offset;
> +      num_regs = ARRAY_SIZE (i386nbsd_sc_reg_offset);
> +
> +      /* Read in the sigcontext address */
> +      base = read_memory_unsigned_integer (sp + 8, 4);
> +    }
> +  else
> +    {
> +      reg_offset = i386nbsd_mc_reg_offset;
> +      num_regs = ARRAY_SIZE (i386nbsd_mc_reg_offset);
> +
> +      /* Read in the ucontext address */
> +      base = read_memory_unsigned_integer (sp + 8, 4);
> +      /* offsetof(ucontext_t, uc_mcontext) == 36 */
> +      base += 36;
> +    }
> +
> +  for (i = 0; i < num_regs; i++)
> +    if (reg_offset[i] != -1)
> +      trad_frame_set_reg_addr (this_cache, i, base + reg_offset[i]);
> +
> +  /* Construct the frame ID using the function start.  */
> +  trad_frame_set_id (this_cache, frame_id_build (sp, func));
> +}
> +
>  static void 
>  i386nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>  
> -  /* Obviously NetBSD is BSD-based.  */
> -  i386bsd_init_abi (info, gdbarch);
> -
>    /* NetBSD has a different `struct reg'.  */
>    tdep->gregset_reg_offset = i386nbsd_r_reg_offset;
>    tdep->gregset_num_regs = ARRAY_SIZE (i386nbsd_r_reg_offset);
>    tdep->sizeof_gregset = 16 * 4;
>  
> -  /* NetBSD has different signal trampoline conventions.  */
> -  tdep->sigtramp_start = 0;
> -  tdep->sigtramp_end = 0;
> -  tdep->sigtramp_p = i386nbsd_sigtramp_p;
> -
>    /* NetBSD uses -freg-struct-return by default.  */
>    tdep->struct_return = reg_struct_return;
>  
> -  /* NetBSD has a `struct sigcontext' that's different from the
> -     original 4.3 BSD.  */
> -  tdep->sc_reg_offset = i386nbsd_sc_reg_offset;
> -  tdep->sc_num_regs = ARRAY_SIZE (i386nbsd_sc_reg_offset);
> +  /* NetBSD uses tramp_frame sniffers for signal trampolines. */
> +  tdep->sigcontext_addr = 0;
> +  tdep->sigtramp_start = 0;
> +  tdep->sigtramp_end = 0;
> +  tdep->sigtramp_p = 0;
> +  tdep->sc_reg_offset = 0;
> +  tdep->sc_num_regs = 0;
> +
> +  tramp_frame_prepend_unwinder (gdbarch, &i386nbsd_sigtramp1);
> +  tramp_frame_prepend_unwinder (gdbarch, &i386nbsd_sigtramp2);
> +  tramp_frame_prepend_unwinder (gdbarch, &i386nbsd_sigtramp3);
> +  tramp_frame_prepend_unwinder (gdbarch, &i386nbsd_sigtramp4);
>  }
>  
>  /* NetBSD a.out.  */
> Index: gdb/tramp-frame.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/tramp-frame.h,v
> retrieving revision 1.9
> diff -u -p -u -r1.9 tramp-frame.h
> --- gdb/tramp-frame.h	23 Aug 2007 18:08:46 -0000	1.9
> +++ gdb/tramp-frame.h	2 Oct 2007 10:39:23 -0000
> @@ -62,7 +62,7 @@ struct tramp_frame
>    {
>      ULONGEST bytes;
>      ULONGEST mask;
> -  } insn[16];
> +  } insn[32];
>    /* Initialize a trad-frame cache corresponding to the tramp-frame.
>       FUNC is the address of the instruction TRAMP[0] in memory.  */
>    void (*init) (const struct tramp_frame *self,
> 
> --Boundary-00=_kZiAH4vl3ZRjugd--
>