Re: [RFA] Support for x86 on-stack trampolines

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

> From: Jerome Guitton <guitton@adacore.com>
> Date: Wed,  4 May 2011 02:20:24 +0200
> 
> On x86 cross targets, the debugger has trouble stepping over the following
> line of Ada code (initialization of a class-wide object):
> 
>     S: Shape'Class := R;
> 
> Here is what happens:
>    (gdb) n
>    0x60c49e8a in ?? ()
> 
> The program jumps to this code location, which is a trampoline
> located on the stack (there is an implicit call to a routine internally
> generated by the Ada expander). As it is on the stack, GDB is naturally
> unable to find the bounds of the current function, or any debugging
> information, and is thus unable to continue.
> 
> This patch adds support for this sort of trampoline. It re-uses some
> code from prologue scan (insn pattern matching). Tested on x86-linux,
> no new regression. OK to apply?

Hmm, I think the new name for i386_match_insn is confusing.  Also, it
isn't really necessary to change its prototype.  It returns a pointer
to the matched pattern, so some trivial pointer arithmetic will give
you the index into the array of patterns.

Some further comments inline below.

> +/* Stack-based trampolines.  */
> +
> +/* These trampolines are used on cross x86 targets, when taking the
> +   address of a nested function.  When executing these trampolines,
> +   no stack frame is set up, so we are in a similar situation as in
> +   epilogues and i386_epilogue_frame_this_id can be re-used.
> +*/

That final */ should be on the previous line.

> +
> +/* Static chain passed in register */

Missing full stop at the end of the comment.

> +
> +struct i386_insn i386_tramp_chain_in_reg_insns[] =
> +{
> +  /* mov   <imm>, %ecx (or %eax, in case of fastcall) */
> +  {5, {0xb8}, {0xfe}},
> +
> +  /* jmp   <imm> */
> +  {5, {0xe9}, {0xff}},
> +
> +  {0}
> +};
> +
> +/* Static chain passed on stack (when regparm=3) */

Same here.

> +
> +struct i386_insn i386_tramp_chain_on_stack_insns[] =
> +{
> +  /* push   <imm> */
> +  {5, {0x68}, {0xff}},
> +
> +  /* jmp   <imm> */
> +  {5, {0xe9}, {0xff}},
> +
> +  {0}
> +};

Could you use the same style for these as the existing instruction
patterns (both for the way you describe the instructions in the
comments and the spacing for the actual data)?

> +
> +/* Return whether PC points inside a stack trampoline.   */
> +
> +static int
> +i386_in_stack_tramp_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  gdb_byte insn;
> +  char *name;
> +
> +  if (target_read_memory (pc, &insn, 1))
> +    return 0;
> +
> +  /* A stack trampoline is detected if no name is associated
> +    to the current pc and if it points inside a trampoline
> +    sequence.  */
> +
> +  if (!i386_match_insn_block (pc, i386_tramp_chain_in_reg_insns)
> +      && !i386_match_insn_block (pc, i386_tramp_chain_on_stack_insns))
> +    return 0;
> +
> +  find_pc_partial_function (pc, &name, NULL, NULL);
> +  return !name;
> +}

Is checking the instructions before checking the name the most
efficient way of doing this?

> +
> +static int
> +i386_stack_tramp_frame_sniffer (const struct frame_unwind *self,
> +			     struct frame_info *this_frame,
> +			     void **this_prologue_cache)
> +{
> +  if (frame_relative_level (this_frame) == 0)
> +    return i386_in_stack_tramp_p (get_frame_arch (this_frame),
> +				  get_frame_pc (this_frame));
> +  else
> +    return 0;
> +}
> +
> +static const struct frame_unwind i386_stack_tramp_frame_unwind =
> +{
> +  NORMAL_FRAME,
> +  default_frame_unwind_stop_reason,
> +  i386_epilogue_frame_this_id,
> +  i386_frame_prev_register,
> +  NULL, 
> +  i386_stack_tramp_frame_sniffer
> +};
> +\f
> +
>  /* Signal trampolines.  */
>  
>  static struct i386_frame_cache *
> @@ -7295,6 +7425,7 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>      tdep->mm0_regnum = -1;
>  
>    /* Hook in the legacy prologue-based unwinders last (fallback).  */
> +  frame_unwind_append_unwinder (gdbarch, &i386_stack_tramp_frame_unwind);
>    frame_unwind_append_unwinder (gdbarch, &i386_sigtramp_frame_unwind);
>    frame_unwind_append_unwinder (gdbarch, &i386_frame_unwind);