Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Support for x86 on-stack trampolines
@ 2011-05-04  0:21 Jerome Guitton
  2011-05-04  8:55 ` Pedro Alves
  2011-05-04 10:20 ` Mark Kettenis
  0 siblings, 2 replies; 9+ messages in thread
From: Jerome Guitton @ 2011-05-04  0:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jerome Guitton

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?

gdb/ChangeLog:

	* i386-tdep.c (i386_in_stack_tramp_p, i386_stack_tramp_frame_sniffer):
	New functions.
	(i386_stack_tramp_frame_unwind): New static global.
	(i386_match_pattern): New function, extracted from i386_match_insn.
	(i386_find_insn): Renaming of i386_match_insn. Use i386_match_pattern.
	(i386_match_insn_block): New function.
	(i386_analyze_frame_setup): Sync with renaming of i386_match_insn.
	(i386_tramp_chain_in_reg_insns)
	(i386_insn i386_tramp_chain_on_stack_insns): New static variables.
	(i386_gdbarch_init): Add i386_stack_tramp_frame_unwind to list
	of unwinders.
---
 gdb/i386-tdep.c |  183 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 5f4089b..13cbc5b 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1126,47 +1126,96 @@ struct i386_insn
   gdb_byte mask[I386_MAX_MATCHED_INSN_LEN];
 };
 
-/* Search for the instruction at PC in the list SKIP_INSNS.  Return
-   the first instruction description that matches.  Otherwise, return
-   NULL.  */
+/* Return whether instruction at PC matches PATTERN.  */
 
-static struct i386_insn *
-i386_match_insn (CORE_ADDR pc, struct i386_insn *skip_insns)
+static int
+i386_match_pattern (CORE_ADDR pc, struct i386_insn pattern)
 {
-  struct i386_insn *insn;
   gdb_byte op;
 
   if (target_read_memory (pc, &op, 1))
-    return NULL;
+    return 0;
 
-  for (insn = skip_insns; insn->len > 0; insn++)
+  if ((op & pattern.mask[0]) == pattern.insn[0])
     {
-      if ((op & insn->mask[0]) == insn->insn[0])
-	{
-	  gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
-	  int insn_matched = 1;
-	  size_t i;
+      gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
+      int insn_matched = 1;
+      size_t i;
 
-	  gdb_assert (insn->len > 1);
-	  gdb_assert (insn->len <= I386_MAX_MATCHED_INSN_LEN);
+      gdb_assert (pattern.len > 1);
+      gdb_assert (pattern.len <= I386_MAX_MATCHED_INSN_LEN);
 
-	  if (target_read_memory (pc + 1, buf, insn->len - 1))
-	    return NULL;
-
-	  for (i = 1; i < insn->len; i++)
-	    {
-	      if ((buf[i - 1] & insn->mask[i]) != insn->insn[i])
-		insn_matched = 0;
-	    }
+      if (target_read_memory (pc + 1, buf, pattern.len - 1))
+	return 0;
 
-	  if (insn_matched)
-	    return insn;
+      for (i = 1; i < pattern.len; i++)
+	{
+	  if ((buf[i - 1] & pattern.mask[i]) != pattern.insn[i])
+	    insn_matched = 0;
 	}
+      return insn_matched;
     }
+  return 0;
+}
+
+/* Search for the instruction at PC in the list INSN_PATTERNS.  Return
+   the first instruction description that matches.  Otherwise, return
+   NULL.  If a non-null value is returned, IX points to the matching
+   entry.  */
 
+static struct i386_insn *
+i386_find_insn (CORE_ADDR pc, struct i386_insn *insn_patterns, int *ix)
+{
+  struct i386_insn *pattern;
+
+  for (pattern = insn_patterns, *ix = 0;
+       pattern->len > 0;
+       pattern++, *ix = *ix + 1)
+    {
+      if (i386_match_pattern (pc, *pattern))
+	return pattern;
+    }
+
+  ix = 0;
   return NULL;
 }
 
+/* Return whether PC points inside a sequence of instructions that
+   matches INSN_PATTERNS.  */
+
+static int
+i386_match_insn_block (CORE_ADDR pc, struct i386_insn *insn_patterns)
+{
+  CORE_ADDR current_pc;
+  int ix, i;
+  gdb_byte op;
+  struct i386_insn *insn;
+
+  insn = i386_find_insn (pc, insn_patterns, &ix);
+  if (insn == NULL)
+    return 0;
+
+  current_pc = pc - insn->len;
+  for (i = ix - 1; i >= 0; i--)
+    {
+      if (!i386_match_pattern (current_pc, insn_patterns[i]))
+	return 0;
+
+      current_pc -= insn_patterns[i].len;
+    }
+
+  current_pc = pc + insn->len;
+  for (insn = insn_patterns + ix + 1; insn->len > 0; insn++)
+    {
+      if (!i386_match_pattern (current_pc, *insn))
+	return 0;
+
+      current_pc += insn->len;
+    }
+
+  return 1;
+}
+
 /* Some special instructions that might be migrated by GCC into the
    part of the prologue that sets up the new stack frame.  Because the
    stack frame hasn't been setup yet, no registers have been saved
@@ -1287,6 +1336,7 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
   struct i386_insn *insn;
   gdb_byte op;
   int skip = 0;
+  int ix;
 
   if (limit <= pc)
     return limit;
@@ -1316,7 +1366,7 @@ i386_analyze_frame_setup (struct gdbarch *gdbarch,
 	 `movl %esp, %ebp' that actually sets up the frame.  */
       while (pc + skip < limit)
 	{
-	  insn = i386_match_insn (pc + skip, i386_frame_setup_skip_insns);
+	  insn = i386_find_insn (pc + skip, i386_frame_setup_skip_insns, &ix);
 	  if (insn == NULL)
 	    break;
 
@@ -1938,6 +1988,86 @@ static const struct frame_unwind i386_epilogue_frame_unwind =
 };
 \f
 
+/* 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.
+*/
+
+/* Static chain passed in register */
+
+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) */
+
+struct i386_insn i386_tramp_chain_on_stack_insns[] =
+{
+  /* push   <imm> */
+  {5, {0x68}, {0xff}},
+
+  /* jmp   <imm> */
+  {5, {0xe9}, {0xff}},
+
+  {0}
+};
+
+/* 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;
+}
+
+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);
 
-- 
1.7.0.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04  0:21 [RFA] Support for x86 on-stack trampolines Jerome Guitton
@ 2011-05-04  8:55 ` Pedro Alves
  2011-05-04 14:52   ` Jerome Guitton
  2011-05-04 10:20 ` Mark Kettenis
  1 sibling, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-05-04  8:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jerome Guitton

One think I noted on a quick skim over the patch:

On Wednesday 04 May 2011 01:20:24, Jerome Guitton wrote:
> +static const struct frame_unwind i386_stack_tramp_frame_unwind =
> +{
> +  NORMAL_FRAME,
> +  default_frame_unwind_stop_reason,

default_frame_unwind_stop_reason is only used by 
archs/targets that don't support tracepoints at the moment.
It shouldn't be used in any x86-specific unwinder.

If you reuse the this _frame_this_id method of the
epilogue unwinder, you should reuse the _unwind_stop_reason
method of the same unwinder, since you're effectively
also reusing i386_epilogue_frame_cache.

> +  i386_epilogue_frame_this_id,
> +  i386_frame_prev_register,
> +  NULL, 
> +  i386_stack_tramp_frame_sniffer
> +};
> +\f

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04  0:21 [RFA] Support for x86 on-stack trampolines Jerome Guitton
  2011-05-04  8:55 ` Pedro Alves
@ 2011-05-04 10:20 ` Mark Kettenis
  2011-05-04 15:17   ` Jerome Guitton
  1 sibling, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2011-05-04 10:20 UTC (permalink / raw)
  To: guitton; +Cc: gdb-patches, guitton

> 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);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04  8:55 ` Pedro Alves
@ 2011-05-04 14:52   ` Jerome Guitton
  0 siblings, 0 replies; 9+ messages in thread
From: Jerome Guitton @ 2011-05-04 14:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves (pedro@codesourcery.com):

> default_frame_unwind_stop_reason is only used by 
> archs/targets that don't support tracepoints at the moment.
> It shouldn't be used in any x86-specific unwinder.
> 
> If you reuse the this _frame_this_id method of the
> epilogue unwinder, you should reuse the _unwind_stop_reason
> method of the same unwinder, since you're effectively
> also reusing i386_epilogue_frame_cache.

Definitely. Thank you for catching that.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04 10:20 ` Mark Kettenis
@ 2011-05-04 15:17   ` Jerome Guitton
  2011-05-04 15:31     ` Mark Kettenis
  0 siblings, 1 reply; 9+ messages in thread
From: Jerome Guitton @ 2011-05-04 15:17 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

Mark Kettenis (mark.kettenis@xs4all.nl):

> 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.

OK, I don't mind pointer arithmetics. I'm not sure about the name; I haven't
much imagination for names, I must say. Any suggestion?

The thing that I would like to make clear is that this new function is
different from i386_find_insn: it only checks one instruction
pattern. It is used by both i386_find_insn (which tries to match one
instruction against any pattern in a set) and by i386_match_insn_block
(which checks that a given PC points inside a block of instruction
matching an ordered list of patterns).


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

I guess that it depends (big symbol tables vs low connection to
target).  In any case, to be consistent with the other sniffers, I
should probably check the name first.

Otherwise, I have taken the rest of your comments into account; and I
will send an updated patch as soon as we have some proper name for
i386_match_insn. Thank you for your review!



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04 15:17   ` Jerome Guitton
@ 2011-05-04 15:31     ` Mark Kettenis
  2011-05-05 15:10       ` Jerome Guitton
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2011-05-04 15:31 UTC (permalink / raw)
  To: guitton; +Cc: mark.kettenis, gdb-patches

> Date: Wed, 4 May 2011 17:17:23 +0200
> From: Jerome Guitton <guitton@adacore.com>
> 
> Mark Kettenis (mark.kettenis@xs4all.nl):
> 
> > 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.
> 
> OK, I don't mind pointer arithmetics. I'm not sure about the name; I haven't
> much imagination for names, I must say. Any suggestion?
> 
> The thing that I would like to make clear is that this new function is
> different from i386_find_insn: it only checks one instruction
> pattern. It is used by both i386_find_insn (which tries to match one
> instruction against any pattern in a set) and by i386_match_insn_block
> (which checks that a given PC points inside a block of instruction
> matching an ordered list of patterns).

Ah, it looks like I created some confusement.  It is i386_find_insn()
as a name that I object to; simply name that funcion i386_match_insn()
and give it the old comment for i386_match_insn(), and I'm happy.
That way you don't have to adjust any of its callers.

i386_match_pattern() is fine as the name for the broken out code that
matches only a single pattern.

> > Is checking the instructions before checking the name the most
> > efficient way of doing this?
> 
> I guess that it depends (big symbol tables vs low connection to
> target).  In any case, to be consistent with the other sniffers, I
> should probably check the name first.

Yeah, it's not clear what's more efficient.  But consistency is always
good.

Cheers,

Mark


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-04 15:31     ` Mark Kettenis
@ 2011-05-05 15:10       ` Jerome Guitton
  2011-05-05 15:18         ` Mark Kettenis
  0 siblings, 1 reply; 9+ messages in thread
From: Jerome Guitton @ 2011-05-05 15:10 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

Mark Kettenis (mark.kettenis@xs4all.nl):

> Ah, it looks like I created some confusement.  It is i386_find_insn()
> as a name that I object to; simply name that funcion i386_match_insn()
> and give it the old comment for i386_match_insn(), and I'm happy.
> That way you don't have to adjust any of its callers.
> 
> i386_match_pattern() is fine as the name for the broken out code that
> matches only a single pattern.


Ah, OK, I was confused indeed. Patch updated! OK to apply?


gdb/ChangeLog:

	* i386-tdep.c (i386_in_stack_tramp_p, i386_stack_tramp_frame_sniffer):
	New functions.
	(i386_stack_tramp_frame_unwind): New static global.
	(i386_match_pattern): New function, extracted from i386_match_insn.
	(i386_match_insn): Use i386_match_pattern.
	(i386_match_insn_block): New function.
	(i386_tramp_chain_in_reg_insns)
	(i386_tramp_chain_on_stack_insns): New static variables.
	(i386_gdbarch_init): Add i386_stack_tramp_frame_unwind to list
	of unwinders.

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 5f4089b..b2b7412 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1126,47 +1126,93 @@ struct i386_insn
   gdb_byte mask[I386_MAX_MATCHED_INSN_LEN];
 };
 
-/* Search for the instruction at PC in the list SKIP_INSNS.  Return
-   the first instruction description that matches.  Otherwise, return
-   NULL.  */
+/* Return whether instruction at PC matches PATTERN.  */
 
-static struct i386_insn *
-i386_match_insn (CORE_ADDR pc, struct i386_insn *skip_insns)
+static int
+i386_match_pattern (CORE_ADDR pc, struct i386_insn pattern)
 {
-  struct i386_insn *insn;
   gdb_byte op;
 
   if (target_read_memory (pc, &op, 1))
-    return NULL;
+    return 0;
 
-  for (insn = skip_insns; insn->len > 0; insn++)
+  if ((op & pattern.mask[0]) == pattern.insn[0])
     {
-      if ((op & insn->mask[0]) == insn->insn[0])
-	{
-	  gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
-	  int insn_matched = 1;
-	  size_t i;
-
-	  gdb_assert (insn->len > 1);
-	  gdb_assert (insn->len <= I386_MAX_MATCHED_INSN_LEN);
+      gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
+      int insn_matched = 1;
+      size_t i;
 
-	  if (target_read_memory (pc + 1, buf, insn->len - 1))
-	    return NULL;
+      gdb_assert (pattern.len > 1);
+      gdb_assert (pattern.len <= I386_MAX_MATCHED_INSN_LEN);
 
-	  for (i = 1; i < insn->len; i++)
-	    {
-	      if ((buf[i - 1] & insn->mask[i]) != insn->insn[i])
-		insn_matched = 0;
-	    }
+      if (target_read_memory (pc + 1, buf, pattern.len - 1))
+	return 0;
 
-	  if (insn_matched)
-	    return insn;
+      for (i = 1; i < pattern.len; i++)
+	{
+	  if ((buf[i - 1] & pattern.mask[i]) != pattern.insn[i])
+	    insn_matched = 0;
 	}
+      return insn_matched;
+    }
+  return 0;
+}
+
+/* Search for the instruction at PC in the list INSN_PATTERNS.  Return
+   the first instruction description that matches.  Otherwise, return
+   NULL.  */
+
+static struct i386_insn *
+i386_match_insn (CORE_ADDR pc, struct i386_insn *insn_patterns)
+{
+  struct i386_insn *pattern;
+
+  for (pattern = insn_patterns; pattern->len > 0; pattern++)
+    {
+      if (i386_match_pattern (pc, *pattern))
+	return pattern;
     }
 
   return NULL;
 }
 
+/* Return whether PC points inside a sequence of instructions that
+   matches INSN_PATTERNS.  */
+
+static int
+i386_match_insn_block (CORE_ADDR pc, struct i386_insn *insn_patterns)
+{
+  CORE_ADDR current_pc;
+  int ix, i;
+  gdb_byte op;
+  struct i386_insn *insn;
+
+  insn = i386_match_insn (pc, insn_patterns);
+  if (insn == NULL)
+    return 0;
+
+  current_pc = pc - insn->len;
+  ix = insn - insn_patterns;
+  for (i = ix - 1; i >= 0; i--)
+    {
+      if (!i386_match_pattern (current_pc, insn_patterns[i]))
+	return 0;
+
+      current_pc -= insn_patterns[i].len;
+    }
+
+  current_pc = pc + insn->len;
+  for (insn = insn_patterns + ix + 1; insn->len > 0; insn++)
+    {
+      if (!i386_match_pattern (current_pc, *insn))
+	return 0;
+
+      current_pc += insn->len;
+    }
+
+  return 1;
+}
+
 /* Some special instructions that might be migrated by GCC into the
    part of the prologue that sets up the new stack frame.  Because the
    stack frame hasn't been setup yet, no registers have been saved
@@ -1938,6 +1984,88 @@ static const struct frame_unwind i386_epilogue_frame_unwind =
 };
 \f
 
+/* 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.  */
+
+/* Static chain passed in register.  */
+
+struct i386_insn i386_tramp_chain_in_reg_insns[] =
+{
+  /* `movl imm32, %eax' and `movl imm32, %ecx' */
+  { 5, { 0xb8 }, { 0xfe } },
+
+  /* `jmp imm32' */
+  { 5, { 0xe9 }, { 0xff } },
+
+  {0}
+};
+
+/* Static chain passed on stack (when regparm=3).  */
+
+struct i386_insn i386_tramp_chain_on_stack_insns[] =
+{
+  /* `push imm32' */
+  { 5, { 0x68 }, { 0xff } },
+
+  /* `jmp imm32' */
+  { 5, { 0xe9 }, { 0xff } },
+
+  {0}
+};
+
+/* 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;
+
+  /* A stack trampoline is detected if no name is associated
+    to the current pc and if it points inside a trampoline
+    sequence.  */
+
+  find_pc_partial_function (pc, &name, NULL, NULL);
+  if (name)
+    return 0;
+
+  if (target_read_memory (pc, &insn, 1))
+    return 0;
+
+  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;
+
+  return 1;
+}
+
+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,
+  i386_epilogue_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 +7423,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);
 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-05 15:10       ` Jerome Guitton
@ 2011-05-05 15:18         ` Mark Kettenis
  2011-05-05 16:03           ` Jerome Guitton
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2011-05-05 15:18 UTC (permalink / raw)
  To: guitton; +Cc: gdb-patches

> Date: Thu, 5 May 2011 17:10:14 +0200
> From: Jerome Guitton <guitton@adacore.com>
> 
> Mark Kettenis (mark.kettenis@xs4all.nl):
> 
> > Ah, it looks like I created some confusement.  It is i386_find_insn()
> > as a name that I object to; simply name that funcion i386_match_insn()
> > and give it the old comment for i386_match_insn(), and I'm happy.
> > That way you don't have to adjust any of its callers.
> > 
> > i386_match_pattern() is fine as the name for the broken out code that
> > matches only a single pattern.
> 
> 
> Ah, OK, I was confused indeed. Patch updated! OK to apply?

Yes, thanks!

> gdb/ChangeLog:
> 
> 	* i386-tdep.c (i386_in_stack_tramp_p, i386_stack_tramp_frame_sniffer):
> 	New functions.
> 	(i386_stack_tramp_frame_unwind): New static global.
> 	(i386_match_pattern): New function, extracted from i386_match_insn.
> 	(i386_match_insn): Use i386_match_pattern.
> 	(i386_match_insn_block): New function.
> 	(i386_tramp_chain_in_reg_insns)
> 	(i386_tramp_chain_on_stack_insns): New static variables.
> 	(i386_gdbarch_init): Add i386_stack_tramp_frame_unwind to list
> 	of unwinders.
> 
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 5f4089b..b2b7412 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -1126,47 +1126,93 @@ struct i386_insn
>    gdb_byte mask[I386_MAX_MATCHED_INSN_LEN];
>  };
>  
> -/* Search for the instruction at PC in the list SKIP_INSNS.  Return
> -   the first instruction description that matches.  Otherwise, return
> -   NULL.  */
> +/* Return whether instruction at PC matches PATTERN.  */
>  
> -static struct i386_insn *
> -i386_match_insn (CORE_ADDR pc, struct i386_insn *skip_insns)
> +static int
> +i386_match_pattern (CORE_ADDR pc, struct i386_insn pattern)
>  {
> -  struct i386_insn *insn;
>    gdb_byte op;
>  
>    if (target_read_memory (pc, &op, 1))
> -    return NULL;
> +    return 0;
>  
> -  for (insn = skip_insns; insn->len > 0; insn++)
> +  if ((op & pattern.mask[0]) == pattern.insn[0])
>      {
> -      if ((op & insn->mask[0]) == insn->insn[0])
> -	{
> -	  gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
> -	  int insn_matched = 1;
> -	  size_t i;
> -
> -	  gdb_assert (insn->len > 1);
> -	  gdb_assert (insn->len <= I386_MAX_MATCHED_INSN_LEN);
> +      gdb_byte buf[I386_MAX_MATCHED_INSN_LEN - 1];
> +      int insn_matched = 1;
> +      size_t i;
>  
> -	  if (target_read_memory (pc + 1, buf, insn->len - 1))
> -	    return NULL;
> +      gdb_assert (pattern.len > 1);
> +      gdb_assert (pattern.len <= I386_MAX_MATCHED_INSN_LEN);
>  
> -	  for (i = 1; i < insn->len; i++)
> -	    {
> -	      if ((buf[i - 1] & insn->mask[i]) != insn->insn[i])
> -		insn_matched = 0;
> -	    }
> +      if (target_read_memory (pc + 1, buf, pattern.len - 1))
> +	return 0;
>  
> -	  if (insn_matched)
> -	    return insn;
> +      for (i = 1; i < pattern.len; i++)
> +	{
> +	  if ((buf[i - 1] & pattern.mask[i]) != pattern.insn[i])
> +	    insn_matched = 0;
>  	}
> +      return insn_matched;
> +    }
> +  return 0;
> +}
> +
> +/* Search for the instruction at PC in the list INSN_PATTERNS.  Return
> +   the first instruction description that matches.  Otherwise, return
> +   NULL.  */
> +
> +static struct i386_insn *
> +i386_match_insn (CORE_ADDR pc, struct i386_insn *insn_patterns)
> +{
> +  struct i386_insn *pattern;
> +
> +  for (pattern = insn_patterns; pattern->len > 0; pattern++)
> +    {
> +      if (i386_match_pattern (pc, *pattern))
> +	return pattern;
>      }
>  
>    return NULL;
>  }
>  
> +/* Return whether PC points inside a sequence of instructions that
> +   matches INSN_PATTERNS.  */
> +
> +static int
> +i386_match_insn_block (CORE_ADDR pc, struct i386_insn *insn_patterns)
> +{
> +  CORE_ADDR current_pc;
> +  int ix, i;
> +  gdb_byte op;
> +  struct i386_insn *insn;
> +
> +  insn = i386_match_insn (pc, insn_patterns);
> +  if (insn == NULL)
> +    return 0;
> +
> +  current_pc = pc - insn->len;
> +  ix = insn - insn_patterns;
> +  for (i = ix - 1; i >= 0; i--)
> +    {
> +      if (!i386_match_pattern (current_pc, insn_patterns[i]))
> +	return 0;
> +
> +      current_pc -= insn_patterns[i].len;
> +    }
> +
> +  current_pc = pc + insn->len;
> +  for (insn = insn_patterns + ix + 1; insn->len > 0; insn++)
> +    {
> +      if (!i386_match_pattern (current_pc, *insn))
> +	return 0;
> +
> +      current_pc += insn->len;
> +    }
> +
> +  return 1;
> +}
> +
>  /* Some special instructions that might be migrated by GCC into the
>     part of the prologue that sets up the new stack frame.  Because the
>     stack frame hasn't been setup yet, no registers have been saved
> @@ -1938,6 +1984,88 @@ static const struct frame_unwind i386_epilogue_frame_unwind =
>  };
>  \f
>  
> +/* 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.  */
> +
> +/* Static chain passed in register.  */
> +
> +struct i386_insn i386_tramp_chain_in_reg_insns[] =
> +{
> +  /* `movl imm32, %eax' and `movl imm32, %ecx' */
> +  { 5, { 0xb8 }, { 0xfe } },
> +
> +  /* `jmp imm32' */
> +  { 5, { 0xe9 }, { 0xff } },
> +
> +  {0}
> +};
> +
> +/* Static chain passed on stack (when regparm=3).  */
> +
> +struct i386_insn i386_tramp_chain_on_stack_insns[] =
> +{
> +  /* `push imm32' */
> +  { 5, { 0x68 }, { 0xff } },
> +
> +  /* `jmp imm32' */
> +  { 5, { 0xe9 }, { 0xff } },
> +
> +  {0}
> +};
> +
> +/* 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;
> +
> +  /* A stack trampoline is detected if no name is associated
> +    to the current pc and if it points inside a trampoline
> +    sequence.  */
> +
> +  find_pc_partial_function (pc, &name, NULL, NULL);
> +  if (name)
> +    return 0;
> +
> +  if (target_read_memory (pc, &insn, 1))
> +    return 0;
> +
> +  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;
> +
> +  return 1;
> +}
> +
> +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,
> +  i386_epilogue_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 +7423,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);
>  
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Support for x86 on-stack trampolines
  2011-05-05 15:18         ` Mark Kettenis
@ 2011-05-05 16:03           ` Jerome Guitton
  0 siblings, 0 replies; 9+ messages in thread
From: Jerome Guitton @ 2011-05-05 16:03 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

Mark Kettenis (mark.kettenis@xs4all.nl):

> > Ah, OK, I was confused indeed. Patch updated! OK to apply?
> 
> Yes, thanks!

Now committed. Thank you for the review!


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-05-05 16:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-04  0:21 [RFA] Support for x86 on-stack trampolines Jerome Guitton
2011-05-04  8:55 ` Pedro Alves
2011-05-04 14:52   ` Jerome Guitton
2011-05-04 10:20 ` Mark Kettenis
2011-05-04 15:17   ` Jerome Guitton
2011-05-04 15:31     ` Mark Kettenis
2011-05-05 15:10       ` Jerome Guitton
2011-05-05 15:18         ` Mark Kettenis
2011-05-05 16:03           ` Jerome Guitton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox