[PATCH] i386nbsd_pc_in_sigtramp robustness

[PATCH] i386nbsd_pc_in_sigtramp robustness

[Jason R Thorpe]

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

The strategy of comparing against hard-coded addresses to find the
signal trampoline is a bad one; it is possible for the user to change
the VM layout, and thus the location of the signal trampoline, with a
kernel option.

The following patch changes the i386-netbsd* targets to disassemble to
find the trampoline, as is done for other NetBSD targets.

Committed to trunk.

	* i386nbsd-tdep.c (i386nbsd_sigtramp_offset): New function.
	(i386nbsd_pc_in_sigtramp): Rewrite to use i386nbsd_sigtramp_offset.
	(i386nbsd_init_abi): Don't initialize tdep->sigtramp_start or
	tdep->sigtramp_end.
	(i386nbsd_sigtramp_start, i386nbsd_sigtramp_end): Remove.
	* config/i386/tm-nbsd.h (SIGTRAMP_START, SIGTRAMP_END)
	(i386bsd_sigtramp_start, i386bsd_sigtramp_end): Remove.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: i386nbsd-sig-patch --]
[-- Type: text/plain, Size: 4920 bytes --]

Index: i386nbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386nbsd-tdep.c,v
retrieving revision 1.7
diff -c -r1.7 i386nbsd-tdep.c
*** i386nbsd-tdep.c	1 Sep 2002 23:24:19 -0000	1.7
--- i386nbsd-tdep.c	2 Sep 2002 16:26:23 -0000
***************
*** 140,162 ****
    NULL					/* next */
  };
  
! static int
! i386nbsd_pc_in_sigtramp (CORE_ADDR pc, char *name)
  {
!   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
  
!   /* Check for libc-provided signal trampoline.  */
!   if (nbsd_pc_in_sigtramp (pc, name))
!     return 1;
  
!   /* FIXME: sigtramp_start/sigtramp_end need to go away; we should
!      not be assuming the location of the kernel-provided trampoline!  */
  
!   return (pc >= tdep->sigtramp_start && pc < tdep->sigtramp_end);
  }
  
! CORE_ADDR i386nbsd_sigtramp_start = 0xbfbfdf20;
! CORE_ADDR i386nbsd_sigtramp_end = 0xbfbfdff0;
  
  /* From <machine/signal.h>.  */
  int i386nbsd_sc_pc_offset = 44;
--- 140,240 ----
    NULL					/* next */
  };
  
! /* 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 (CORE_ADDR pc)
! {
!   unsigned char ret[sizeof(sigtramp_retcode)], insn;
!   LONGEST off;
!   int i;
! 
!   if (read_memory_nobpt (pc, &insn, 1) != 0)
!     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 (read_memory_nobpt (pc + 1, &insn, 1) != 0)
! 	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 (read_memory_nobpt (pc, (char *) ret, sizeof (ret)) != 0)
!     return -1;
  
!   if (memcmp (ret, sigtramp_retcode, sizeof (ret)) == 0)
!     return off;
  
!   return -1;
  }
  
! static int
! i386nbsd_pc_in_sigtramp (CORE_ADDR pc, char *name)
! {
! 
!   return (nbsd_pc_in_sigtramp (pc, name)
! 	  || i386nbsd_sigtramp_offset (pc) >= 0);
! }
  
  /* From <machine/signal.h>.  */
  int i386nbsd_sc_pc_offset = 44;
***************
*** 175,184 ****
  
    /* NetBSD uses -freg-struct-return by default.  */
    tdep->struct_return = reg_struct_return;
- 
-   /* NetBSD uses a different memory layout.  */
-   tdep->sigtramp_start = i386nbsd_sigtramp_start;
-   tdep->sigtramp_end = i386nbsd_sigtramp_end;
  
    /* NetBSD has a `struct sigcontext' that's different from the
       origional 4.3 BSD.  */
--- 253,258 ----
Index: config/i386/tm-nbsd.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/tm-nbsd.h,v
retrieving revision 1.13
diff -c -r1.13 tm-nbsd.h
*** config/i386/tm-nbsd.h	30 Jun 2002 17:22:57 -0000	1.13
--- config/i386/tm-nbsd.h	2 Sep 2002 16:26:26 -0000
***************
*** 23,38 ****
  
  #include "i386/tm-i386.h"
  
- /* These defines allow the recognition of sigtramps as a function name
-    <sigtramp>.
- 
-    FIXME: kettenis/2002-05-12: Of course these defines will have to go
-    if we go truly "multi-arch", but I don't know yet how to get rid of
-    them.  */
- 
- #define SIGTRAMP_START(pc) i386bsd_sigtramp_start (pc)
- #define SIGTRAMP_END(pc) i386bsd_sigtramp_end (pc)
- extern CORE_ADDR i386bsd_sigtramp_start (CORE_ADDR pc);
- extern CORE_ADDR i386bsd_sigtramp_end (CORE_ADDR pc);
- 
  #endif /* TM_NBSD_H */
--- 23,26 ----

Re: [PATCH] i386nbsd_pc_in_sigtramp robustness

[Mark Kettenis]

Jason R Thorpe <thorpej@wasabisystems.com> writes:

> --3xoW37o/FfUZJwQG
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> The strategy of comparing against hard-coded addresses to find the
> signal trampoline is a bad one; it is possible for the user to change
> the VM layout, and thus the location of the signal trampoline, with a
> kernel option.
> 
> The following patch changes the i386-netbsd* targets to disassemble to
> find the trampoline, as is done for other NetBSD targets.

Just a few thoughts:

 * Doesn't this break things for NetBSD before 1.4?  Did you consider
   falling back on the old method?

 * Did you notice any effect on performance?  Reading from a target's
   memory can be time-consuming for remote targets.

Mark

Re: [PATCH] i386nbsd_pc_in_sigtramp robustness

[Jason R Thorpe]

On Mon, Sep 02, 2002 at 11:39:05PM +0200, Mark Kettenis wrote:

 > Just a few thoughts:
 > 
 >  * Doesn't this break things for NetBSD before 1.4?  Did you consider
 >    falling back on the old method?

The old method didn't work on 1.3 anyway, because the VM layout was
different then, i.e the sigtramp start/end were different.  I could
add a disassemble-using version for 1.3, but it hardly seems worth
it at this point.

Heh, just checked, and FWIW, the old method didn't work for 1.4, either;
it used the same VM layout as 1.3.  The VM layout was changed between
1.4 and 1.5; the new VM layout also was in 1.4.2 and 1.4.3.

1.4 did, however, have a slightly different signal trampoline (which
changed based on a kernel option).  I'm going to file a PR to remind
me to audit all of the signal trampoline stuff back to NetBSD 1.4.

 >  * Did you notice any effect on performance?  Reading from a target's
 >    memory can be time-consuming for remote targets.

It certainly didn't seem any slower on my Athlon :-)  Yes, I know reading
memory is time-consuming for remote targets, however, there isn't really
any other sane way to do it, since the VM layout is totally arbitrary and
can be changed by the user.  I'll note that i386-linux also uses the
disassemble method.

Also note that the use of the disassemble-method is going to fade over
time, since post-1.6 NetBSD now uses symbol names to find the trampoline.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>