Re: [patch/rfc] signal trampoline frames

[Andrew Cagney <cagney@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]


>>>> >tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
>>>> >also.
>>
>>> 
>>> changed to ->insn[0]
> 
> 
> I was more concerned about the CORE_ADDR.

Ah, "both".

>>>>>> >>>+	  if (tramp->insn[i] == 0)
>>>>>> >>>+	    return func;
>>>
>>>> >
>>>> >
>>>> >So zeros in tramp->insn mark the end of the sequence?  Should document
>>>> >that, zeros are valid instructions and some bizarre architecture might
>>>> >use one as a syscall trap.
>>
>>> 
>>> Added TRAMP_SENTINEL_INSN, it _isn't_ zero.
> 
> 
> The comment in the attached still says it's zero.  Zero and -1 are just
> about equally likely/unlikely, so I don't think it matters what the
> value is; I like having TRAMP_SENTINEL_INSN though.

(LONGEST)-1 is typically 0xffffffffffffffffLL so unless an ISA has a 
64-bit insn there won't be a clash.

>>> I don't know.
> 
> 
> Doesn't much matter then.

I committed the attached,

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3008 bytes --]

2004-03-24  Andrew Cagney  <cagney@redhat.com>

	* tramp-frame.h (TRAMP_SENTINEL_INSN): Define, document.
	* tramp-frame.c: Include "gdb_assert.h".
	(tramp_frame_start): Use TRAMP_SENTINEL_INSN.  Use ULONGEST and
	correct sizeof.
	(tramp_frame_append): Validate the tramp frame's instructions.
	* Makefile.in (tramp-frame.o): Update dependencies.

Index: tramp-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.c,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.c
--- tramp-frame.c	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.c	24 Mar 2004 23:13:23 -0000
@@ -28,6 +28,7 @@
 #include "target.h"
 #include "trad-frame.h"
 #include "frame-base.h"
+#include "gdb_assert.h"
 
 struct frame_data
 {
@@ -89,15 +90,15 @@
   int ti;
   /* Search through the trampoline for one that matches the
      instruction sequence around PC.  */
-  for (ti = 0; tramp->insn[ti] != 0; ti++)
+  for (ti = 0; tramp->insn[ti] != TRAMP_SENTINEL_INSN; ti++)
     {
       CORE_ADDR func = pc - tramp->insn_size * ti;
       int i;
       for (i = 0; 1; i++)
 	{
-	  bfd_byte buf[sizeof (LONGEST)];
-	  CORE_ADDR insn;
-	  if (tramp->insn[i] == 0)
+	  bfd_byte buf[sizeof (tramp->insn[0])];
+	  ULONGEST insn;
+	  if (tramp->insn[i] == TRAMP_SENTINEL_INSN)
 	    return func;
 	  if (target_read_memory (func + i * tramp->insn_size, buf,
 				  tramp->insn_size) != 0)
@@ -148,6 +149,16 @@
 {
   struct frame_data *data;
   struct frame_unwind *unwinder;
+  int i;
+
+  /* Check that the instruction sequence contains a sentinel.  */
+  for (i = 0; i < ARRAY_SIZE (tramp_frame->insn); i++)
+    {
+      if (tramp_frame->insn[i] == TRAMP_SENTINEL_INSN)
+	break;
+    }
+  gdb_assert (i < ARRAY_SIZE (tramp_frame->insn));
+  gdb_assert (tramp_frame->insn_size <= sizeof (tramp_frame->insn[0]));
 
   data = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_data);
   unwinder = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct frame_unwind);
Index: tramp-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/tramp-frame.h,v
retrieving revision 1.2
diff -u -r1.2 tramp-frame.h
--- tramp-frame.h	23 Mar 2004 14:12:30 -0000	1.2
+++ tramp-frame.h	24 Mar 2004 23:13:23 -0000
@@ -39,6 +39,10 @@
 
 /* A trampoline descriptor.  */
 
+/* Magic instruction that to mark the end of the signal trampoline
+   instruction sequence.  */
+#define TRAMP_SENTINEL_INSN ((LONGEST) -1)
+
 struct tramp_frame
 {
   /* The trampoline's entire instruction sequence.  Search for this in
@@ -47,7 +51,8 @@
      one INSN_SIZE instruction.  It is also assumed that TRAMP[0]
      contains the first instruction of the trampoline and hence the
      address of the instruction matching TRAMP[0] is the trampoline's
-     "func" address.  */
+     "func" address.  The instruction sequence shall be terminated by
+     TRAMP_SENTINEL_INSN.  */
   int insn_size;
   ULONGEST insn[8];
   /* Initialize a trad-frame cache corresponding to the tramp-frame.