This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] DWARF-2 unwinder off-by-one problem with signal frames


Richard Henderson wrote:
> On Thu, Jul 01, 2004 at 12:13:13AM +0200, Ulrich Weigand wrote:
> > However, if we assume that the platform *has* the information 
> > required to decide whether the RA points to the begin or end of
> > the trapping insn, one possible implementation would be a new
> > flag in _Unwind_FrameState that MD_FALLBACK_FRAME_STATE_FOR
> > could set to indicate the former case.  Common unwind-dw.c code
> > would then simply need to consider that flag.
> 
> Is that actually going to be useful?  Many Linux targets are not
> actually using MD_FALLBACK_FRAME_STATE_FOR anymore.  The unwind
> information is coming directly from libc or the kernel.

Well, we're probably not going to do that on s390 as we don't
share any address space between kernel and user.

In any case, the problem still remains: how do I express within
the CFI itself that the return address should be incremented?
While I can use a REG_SAVED_EXP, even that allows me only to
compute the *address* of the return address save slot via a
DWARF expression, not using the expression to change its value.
If I knew that, I could just construct that CFI from within my
MD_FALLBACK_FRAME_STATE_FOR ...

However, I have now found a way to do that, which may be somewhat
of a hack, but appears to work fine without any modification to
common unwinder code.  The code below checks whether we have a
SIGSEGV or SIGBUS frame, and increments the RA if so.  Other
signal frames are left unchanged.  Note that this works only for
SIGINFO-style frames for current kernel (since for regular frames
we don't store the signal number on the stack), but we're going
to change the kernel to add this information.  (This should be
both completely ABI compatible and reliably detectable by the
unwinder code ...)

That change allows me to remove the hacks from the libjava 
signal handlers; I've also changed them to SIGINFO-style signals
so that they continue to work with old kernels (but this is good
anyway as it'll allow to handle divide fault as well).

Unless you have any objections or suggestions how to implement
this in a cleaner way in common code, I'll commit the s390-
specific patch appended below (once we've pushed the corresponding
kernel change).

Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux,
fixes the cleanup-8.c test case failures.

Bye,
Ulrich


ChangeLog:

        * config/s390/linux.h (MD_FALLBACK_FRAME_STATE_FOR): For SIGSEGV and 
	SIGBUS signal frames, the PSW address points *to* the faulting
	instruction, not after it.

libjava/ChangeLog:

	* include/s390-signal.c (SIGNAL_HANDLER): Use SIGINFO-style prototype.
	(struct old_s390_kernel_sigaction): Likewise for k_sa_handler.
	(MAKE_THROW_FRAME): Do not modify PSW address.
	(INIT_SEGV): Install SIGINFO-style signal handler.
	(INIT_FPE): Likewise.

Index: gcc/config/s390/linux.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/linux.h,v
retrieving revision 1.34
diff -c -p -r1.34 linux.h
*** gcc/config/s390/linux.h	29 Nov 2003 03:08:12 -0000	1.34
--- gcc/config/s390/linux.h	7 Jul 2004 00:08:34 -0000
*************** Software Foundation, 59 Temple Place - S
*** 113,118 ****
--- 113,119 ----
        } __attribute__ ((__aligned__ (8))) sigregs_;			\
  									\
      sigregs_ *regs_;							\
+     int *signo_ = NULL;							\
  									\
      /* svc $__NR_sigreturn or svc $__NR_rt_sigreturn  */		\
      if (pc_[0] != 0x0a || (pc_[1] != 119 && pc_[1] != 173))		\
*************** Software Foundation, 59 Temple Place - S
*** 133,138 ****
--- 134,140 ----
  	  } *uc_ = (CONTEXT)->cfa + 8 + 128;				\
  									\
  	regs_ = &uc_->uc_mcontext;					\
+ 	signo_ = (CONTEXT)->cfa + sizeof(long);				\
        }									\
  									\
      /* Old-style RT frame and all non-RT frames:			\
*************** Software Foundation, 59 Temple Place - S
*** 141,146 ****
--- 143,154 ----
      else								\
        {									\
  	regs_ = *(sigregs_ **)((CONTEXT)->cfa + 8);			\
+ 									\
+ 	/* Recent kernels store the signal number immediately after	\
+ 	   the sigregs; old kernels have the return trampoline at	\
+ 	   this location.  */						\
+ 	if ((void *)(regs_ + 1) != (CONTEXT)->ra)			\
+ 	  signo_ = (int *)(regs_ + 1);					\
        }									\
        									\
      new_cfa_ = regs_->gprs[15] + 16*sizeof(long) + 32;			\
*************** Software Foundation, 59 Temple Place - S
*** 163,172 ****
--- 171,201 ----
        }									\
  									\
      /* Load return addr from PSW into dummy register 32.  */		\
+ 									\
      (FS)->regs.reg[32].how = REG_SAVED_OFFSET;				\
      (FS)->regs.reg[32].loc.offset = (long)&regs_->psw_addr - new_cfa_;	\
      (FS)->retaddr_column = 32;						\
  									\
+     /* If we got a SIGSEGV or a SIGBUS, the PSW address points *to*	\
+        the faulting instruction, not after it.  This causes the logic	\
+        in unwind-dw2.c that decrements the RA to determine the correct	\
+        CFI region to get confused.  To fix that, we *increment* the RA	\
+        here in that case.  Note that we cannot modify the RA in place,	\
+        and the frame state wants a *pointer*, not a value; thus we put	\
+        the modified RA value into the unused register 33 slot of FS and	\
+        have the register 32 save address point to that slot.		\
+ 									\
+        Unfortunately, for regular signals on old kernels, we don't know	\
+        the signal number.  We default to not fiddling with the RA; 	\
+        that can fail in rare cases.  Upgrade your kernel.  */		\
+ 									\
+     if (signo_ && (*signo_ == 11 || *signo_ == 7))			\
+       {									\
+ 	(FS)->regs.reg[33].loc.exp = regs_->psw_addr + 1;		\
+ 	(FS)->regs.reg[32].loc.offset = 				\
+ 		(long)&(FS)->regs.reg[33].loc.exp - new_cfa_;		\
+       }									\
+ 									\
      goto SUCCESS;							\
    } while (0)
  
Index: libjava/include/s390-signal.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/s390-signal.h,v
retrieving revision 1.2
diff -c -p -r1.2 s390-signal.h
*** libjava/include/s390-signal.h	13 Jun 2003 12:20:45 -0000	1.2
--- libjava/include/s390-signal.h	7 Jul 2004 00:08:38 -0000
*************** details.  */
*** 20,37 ****
  #undef HANDLE_FPE
  
  #define SIGNAL_HANDLER(_name)	\
! static void _name (int /* _signal */, struct sigcontext _sc)
  
! #define MAKE_THROW_FRAME(_exception)					\
! do									\
! {									\
!   /* Advance the program counter so that it is after the start of the	\
!      instruction:  the s390 exception handler expects the PSW to point 	\
!      to the instruction after a call. */				\
!   _sc.sregs->regs.psw.addr += 2;					\
! 									\
! }									\
! while (0)
  
  
  /* For an explanation why we cannot simply use sigaction to
--- 20,30 ----
  #undef HANDLE_FPE
  
  #define SIGNAL_HANDLER(_name)	\
! static void _name (int, siginfo_t *, void *)
  
! /* We no longer need to fiddle with the PSW address in the signal handler;
!    this is now all handled correctly in MD_FALLBACK_FRAME_STATE_FOR.  */
! #define MAKE_THROW_FRAME(_exception)
  
  
  /* For an explanation why we cannot simply use sigaction to
*************** while (0)
*** 43,49 ****
     visible to us in a header file so we define it here.  */
  
  struct old_s390_kernel_sigaction {
! 	void (*k_sa_handler) (int, struct sigcontext);
  	unsigned long k_sa_mask;
  	unsigned long k_sa_flags;
  	void (*sa_restorer) (void);
--- 36,42 ----
     visible to us in a header file so we define it here.  */
  
  struct old_s390_kernel_sigaction {
! 	void (*k_sa_handler) (int, siginfo_t *, void *);
  	unsigned long k_sa_mask;
  	unsigned long k_sa_flags;
  	void (*sa_restorer) (void);
*************** do							\
*** 55,61 ****
      struct old_s390_kernel_sigaction kact;		\
      kact.k_sa_handler = catch_segv;			\
      kact.k_sa_mask = 0;					\
!     kact.k_sa_flags = 0;				\
      syscall (SYS_sigaction, SIGSEGV, &kact, NULL);	\
    }							\
  while (0)  
--- 48,54 ----
      struct old_s390_kernel_sigaction kact;		\
      kact.k_sa_handler = catch_segv;			\
      kact.k_sa_mask = 0;					\
!     kact.k_sa_flags = SA_SIGINFO;			\
      syscall (SYS_sigaction, SIGSEGV, &kact, NULL);	\
    }							\
  while (0)  
*************** do								\
*** 66,72 ****
      struct old_s390_kernel_sigaction kact;			\
      kact.k_sa_handler = catch_fpe;				\
      kact.k_sa_mask = 0;						\
!     kact.k_sa_flags = 0;					\
      syscall (SYS_sigaction, SIGFPE, &kact, NULL);		\
    }								\
  while (0)  
--- 59,65 ----
      struct old_s390_kernel_sigaction kact;			\
      kact.k_sa_handler = catch_fpe;				\
      kact.k_sa_mask = 0;						\
!     kact.k_sa_flags = SA_SIGINFO;				\
      syscall (SYS_sigaction, SIGFPE, &kact, NULL);		\
    }								\
  while (0)  

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]