[patch] sigaltstack fixes for hppa-linux

[patch] sigaltstack fixes for hppa-linux

[Randolph Chung]

This patch fixes sigaltstack handling on hppa. It requires one arch-indep 
change in frame.c: on hppa (stack-grows-up), the relationship of the 
sigaltstack can be that it is not "inner" to the signal frame, if the 
sigaltstack resides in a caller's stack frame (as is true in gdb's testcase.)

ok to apply?

2004-05-03  Randolph Chung  <tausq@debian.org>

	* frame.c (get_prev_frame_1): Relax frame_id_inner check for 
	SIGTRAMP_FRAME.
	* hppa-linux-tdep.c (hppa_linux_sigtramp_find_sigcontext): Pass in pc
	instead of sp, handle sigaltstack case.
	(hppa_linux_sigtramp_frame_unwind_cache)
	(hppa_linux_sigtramp_unwind_sniffer): Adjust calls to
	hppa_linux_sigtramp_find_sigcontext.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.173
diff -u -p -r1.173 frame.c
--- frame.c	26 Apr 2004 09:49:35 -0000	1.173
+++ frame.c	4 May 2004 14:43:32 -0000
@@ -1794,6 +1794,7 @@ get_prev_frame_1 (struct frame_info *thi
      Exclude signal trampolines (due to sigaltstack the frame ID can
      go backwards) and sentinel frames (the test is meaningless).  */
   if (this_frame->next->level >= 0
+      && this_frame->type != SIGTRAMP_FRAME
       && this_frame->next->type != SIGTRAMP_FRAME
       && frame_id_inner (get_frame_id (this_frame),
 			 get_frame_id (this_frame->next)))
Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.1
diff -u -p -r1.1 hppa-linux-tdep.c
--- hppa-linux-tdep.c	29 Apr 2004 03:36:49 -0000	1.1
+++ hppa-linux-tdep.c	4 May 2004 14:43:32 -0000
@@ -282,7 +282,7 @@ hppa_linux_skip_trampoline_code (CORE_AD
    Note that with a 2.4 64-bit kernel, the signal context is not properly
    passed back to userspace so the unwind will not work correctly.  */
 static CORE_ADDR
-hppa_linux_sigtramp_find_sigcontext (CORE_ADDR sp)
+hppa_linux_sigtramp_find_sigcontext (CORE_ADDR pc)
 {
   unsigned int dummy[HPPA_MAX_INSN_PATTERN_LEN];
   int offs = 0;
@@ -291,6 +291,12 @@ hppa_linux_sigtramp_find_sigcontext (COR
   static int pcoffs[] = { 0, 4*4, 5*4 };
   /* offsets to the rt_sigframe structure */
   static int sfoffs[] = { 4*4, 10*4, 10*4 };
+  CORE_ADDR sp;
+
+  /* Most of the time, this will be correct.  The one case when this will
+     fail is if the user defined an alternate stack, in which case the
+     beginning of the stack will not be pc & 63.  */
+  sp = (pc & ~63);
 
   /* rt_sigreturn trampoline:
      3419000x ldi 0, %r25 or ldi 1, %r25   (x = 0 or 2)
@@ -308,7 +314,18 @@ hppa_linux_sigtramp_find_sigcontext (COR
     }
 
   if (offs == 0)
-    return 0;
+    {
+      if (insns_match_pattern (pc, hppa_sigtramp, dummy))
+	{
+	  /* sigaltstack case: we have no way of knowing which offset to 
+	     use in this case; default to new kernel handling. If this is
+	     wrong the unwinding will fail.  */
+	  try = 2;
+	  sp = pc - pcoffs[try];
+	}
+      else
+        return 0;
+    }
 
   /* sp + sfoffs[try] points to a struct rt_sigframe, which contains
      a struct siginfo and a struct ucontext.  struct ucontext contains
@@ -331,7 +348,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
 {
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
   struct hppa_linux_sigtramp_unwind_cache *info;
-  CORE_ADDR sp, pc, scptr;
+  CORE_ADDR pc, scptr;
   int i;
 
   if (*this_cache)
@@ -342,8 +359,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
   info->saved_regs = trad_frame_alloc_saved_regs (next_frame);
 
   pc = frame_pc_unwind (next_frame);
-  sp = (pc & ~63);
-  scptr = hppa_linux_sigtramp_find_sigcontext (sp);
+  scptr = hppa_linux_sigtramp_find_sigcontext (pc);
 
   /* structure of struct sigcontext:
    
@@ -448,14 +464,13 @@ static const struct frame_unwind *
 hppa_linux_sigtramp_unwind_sniffer (struct frame_info *next_frame)
 {
   CORE_ADDR pc = frame_pc_unwind (next_frame);
-  CORE_ADDR sp = (pc & ~63);
 
-  if (hppa_linux_sigtramp_find_sigcontext (sp))
+  if (hppa_linux_sigtramp_find_sigcontext (pc))
     return &hppa_linux_sigtramp_frame_unwind;
 
   return NULL;
 }
 
 /* Forward declarations.  */
 extern initialize_file_ftype _initialize_hppa_linux_tdep;
 
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] sigaltstack fixes for hppa-linux

[Andrew Cagney]

> This patch fixes sigaltstack handling on hppa. It requires one arch-indep 
> change in frame.c: on hppa (stack-grows-up), the relationship of the 
> sigaltstack can be that it is not "inner" to the signal frame, if the 
> sigaltstack resides in a caller's stack frame (as is true in gdb's testcase.)

The current code handles the transition:

stk 1: caller (this_frame)
stk 2: trampoline (this_frame->next)
stk 2: handler

are you saying that the transition:

stk 1: caller
stk 1: trampoline (this_frame)
stk 2: handler (this_frame->next)

is also possible?

Perhaphs the test should be just:

    if (this_frame->type == NORMAL_FRAME
        && this_frame->next->type == NORMAL_FRAME
        && frame_id_inner (get_frame_id (this_frame),
  			 get_frame_id (this_frame->next)))

Andrew

> ok to apply?

Re: [patch] sigaltstack fixes for hppa-linux

[Randolph Chung]

> are you saying that the transition:
> 
> stk 1: caller
> stk 1: trampoline (this_frame)
> stk 2: handler (this_frame->next)
> 
> is also possible?

well, only because the hppa sigtramp unwinder is slightly broken :-) for
the sigtramp frame it current uses the sp stored in the sigcontext as
the frame base. instead it should probably use the handler's stack.

will send out an updated patch in a little bit.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] sigaltstack fixes for hppa-linux

[Randolph Chung]

> well, only because the hppa sigtramp unwinder is slightly broken :-) for
> the sigtramp frame it current uses the sp stored in the sigcontext as
> the frame base. instead it should probably use the handler's stack.

here's a fixed version. No frame.c modifications needed :)

one question related to this: i noticed that some of the tests are
failing when:
- a breakpoint is placed in the signal trampoline (e.g. "finish" from 
  the signal handler)
- we try to unwind from the signal trampoline frame

they fail because breakpoints are not yet disabled when the unwinding
starts (get_frame_id() is called early on in handle_inferior_event, 
which runs through the unwinders)

Are the unwinders that do code matching supposed to deal with the case
where there are breakpoints inserted in the code stream? or does this
point to something else that is broken?

thanks
randolph


2004-05-06  Randolph Chung  <tausq@debian.org>

	* hppa-linux-tdep.c (hppa_linux_sigtramp_find_sigcontext): Pass in pc
	instead of sp, handle sigaltstack case.
	(hppa_linux_sigtramp_frame_unwind_cache): Adjust calls to 
	hppa_linux_sigtramp_find_sigcontext, and set base to the frame of the
	signal handler and not that of the caller.
	(hppa_linux_sigtramp_unwind_sniffer): Adjust calls to
	hppa_linux_sigtramp_find_sigcontext.

Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.2
diff -u -p -r1.2 hppa-linux-tdep.c
--- hppa-linux-tdep.c	7 May 2004 05:48:49 -0000	1.2
+++ hppa-linux-tdep.c	7 May 2004 06:24:07 -0000
@@ -282,7 +284,7 @@ hppa_linux_skip_trampoline_code (CORE_AD
    Note that with a 2.4 64-bit kernel, the signal context is not properly
    passed back to userspace so the unwind will not work correctly.  */
 static CORE_ADDR
-hppa_linux_sigtramp_find_sigcontext (CORE_ADDR sp)
+hppa_linux_sigtramp_find_sigcontext (CORE_ADDR pc)
 {
   unsigned int dummy[HPPA_MAX_INSN_PATTERN_LEN];
   int offs = 0;
@@ -291,6 +293,12 @@ hppa_linux_sigtramp_find_sigcontext (COR
   static int pcoffs[] = { 0, 4*4, 5*4 };
   /* offsets to the rt_sigframe structure */
   static int sfoffs[] = { 4*4, 10*4, 10*4 };
+  CORE_ADDR sp;
+
+  /* Most of the time, this will be correct.  The one case when this will
+     fail is if the user defined an alternate stack, in which case the
+     beginning of the stack will not be pc & 63.  */
+  sp = (pc & ~63);
 
   /* rt_sigreturn trampoline:
      3419000x ldi 0, %r25 or ldi 1, %r25   (x = 0 or 2)
@@ -308,7 +316,20 @@ hppa_linux_sigtramp_find_sigcontext (COR
     }
 
   if (offs == 0)
-    return 0;
+    {
+      if (insns_match_pattern (pc, hppa_sigtramp, dummy))
+	{
+	  /* sigaltstack case: we have no way of knowing which offset to 
+	     use in this case; default to new kernel handling. If this is
+	     wrong the unwinding will fail.  */
+	  try = 2;
+	  sp = pc - pcoffs[try];
+	}
+      else
+      {
+        return 0;
+      }
+    }
 
   /* sp + sfoffs[try] points to a struct rt_sigframe, which contains
      a struct siginfo and a struct ucontext.  struct ucontext contains
@@ -331,7 +352,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
 {
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
   struct hppa_linux_sigtramp_unwind_cache *info;
-  CORE_ADDR sp, pc, scptr;
+  CORE_ADDR pc, scptr;
   int i;
 
   if (*this_cache)
@@ -342,8 +363,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
   info->saved_regs = trad_frame_alloc_saved_regs (next_frame);
 
   pc = frame_pc_unwind (next_frame);
-  sp = (pc & ~63);
-  scptr = hppa_linux_sigtramp_find_sigcontext (sp);
+  scptr = hppa_linux_sigtramp_find_sigcontext (pc);
 
   /* structure of struct sigcontext:
    
@@ -393,8 +413,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
   info->saved_regs[HPPA_PCOQ_TAIL_REGNUM].addr = scptr;
   scptr += 4;
 
-  info->base = read_memory_unsigned_integer (
-		  info->saved_regs[HPPA_SP_REGNUM].addr, 4);
+  info->base = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
 
   return info;
 }
@@ -448,14 +458,84 @@ static const struct frame_unwind *
 hppa_linux_sigtramp_unwind_sniffer (struct frame_info *next_frame)
 {
   CORE_ADDR pc = frame_pc_unwind (next_frame);
-  CORE_ADDR sp = (pc & ~63);
 
-  if (hppa_linux_sigtramp_find_sigcontext (sp))
+  if (hppa_linux_sigtramp_find_sigcontext (pc))
     return &hppa_linux_sigtramp_frame_unwind;
 
   return NULL;
 }
 
 /* Forward declarations.  */
 extern initialize_file_ftype _initialize_hppa_linux_tdep;
 

Re: [patch] sigaltstack fixes for hppa-linux

[Andrew Cagney]

>>> well, only because the hppa sigtramp unwinder is slightly broken :-) for
>>> the sigtramp frame it current uses the sp stored in the sigcontext as
>>> the frame base. instead it should probably use the handler's stack.
> 
> 
> here's a fixed version. No frame.c modifications needed :)
> 
> one question related to this: i noticed that some of the tests are
> failing when:
> - a breakpoint is placed in the signal trampoline (e.g. "finish" from 
>   the signal handler)
> - we try to unwind from the signal trampoline frame
> 
> they fail because breakpoints are not yet disabled when the unwinding
> starts (get_frame_id() is called early on in handle_inferior_event, 
> which runs through the unwinders)
> 
> Are the unwinders that do code matching supposed to deal with the case
> where there are breakpoints inserted in the code stream? or does this
> point to something else that is broken?

Yes, use frame*memory* and it will work (see tramp-frame.c: 
tramp_frame_start()) (and yes, having those memory routines possibly 
include breakpoints is just weird).

Feel free to fix any other HP unwinders with a follow-on commit.

Some suggested tweaks before committing follow:

> 2004-05-06  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-linux-tdep.c (hppa_linux_sigtramp_find_sigcontext): Pass in pc
> 	instead of sp, handle sigaltstack case.
> 	(hppa_linux_sigtramp_frame_unwind_cache): Adjust calls to 
> 	hppa_linux_sigtramp_find_sigcontext, and set base to the frame of the
> 	signal handler and not that of the caller.
> 	(hppa_linux_sigtramp_unwind_sniffer): Adjust calls to
> 	hppa_linux_sigtramp_find_sigcontext.
> 
> Index: hppa-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 hppa-linux-tdep.c
> --- hppa-linux-tdep.c	7 May 2004 05:48:49 -0000	1.2
> +++ hppa-linux-tdep.c	7 May 2004 06:24:07 -0000
> @@ -282,7 +284,7 @@ hppa_linux_skip_trampoline_code (CORE_AD
>     Note that with a 2.4 64-bit kernel, the signal context is not properly
>     passed back to userspace so the unwind will not work correctly.  */
>  static CORE_ADDR
> -hppa_linux_sigtramp_find_sigcontext (CORE_ADDR sp)
> +hppa_linux_sigtramp_find_sigcontext (CORE_ADDR pc)
>  {
>    unsigned int dummy[HPPA_MAX_INSN_PATTERN_LEN];
>    int offs = 0;
> @@ -291,6 +293,12 @@ hppa_linux_sigtramp_find_sigcontext (COR
>    static int pcoffs[] = { 0, 4*4, 5*4 };
>    /* offsets to the rt_sigframe structure */
>    static int sfoffs[] = { 4*4, 10*4, 10*4 };
> +  CORE_ADDR sp;
> +
> +  /* Most of the time, this will be correct.  The one case when this will
> +     fail is if the user defined an alternate stack, in which case the
> +     beginning of the stack will not be pc & 63.  */
> +  sp = (pc & ~63);

Use align_down() (I never trust C's sign extension rules).

Andrew