RE: FW: Remaining Itanium exception, static linking patches

["Boehm, Hans" <hans_boehm at hp dot com>]

Andrew -

After reconsidering this yet again, and in spite of David's objection, I
would like to check in your Java signal/exception handling patch after all.
Is your approval still good?

I agree that this is a bit ugly as it stands.  Basically our conventions for
the unwinder are inconsistent with the exception lookup mechanism.  The
unwider lookup is based on return addresses.  The exception table lookup is
based on the call instruction address.

But here's my argument that it's correct:

- The signal handler only adjusts the ip when it was pointing to a faulting
load or store.  Such an instruction doesn't affect the unwind state.  Thus
incrementing it is technically wrong for the unwinder, but it can't matter.
We can't omit the increment, because it's essentila for the exception lookup
mechanism.

- The personality function decrements the ip when ideally it shouldn't.  But
the exception tables are generated on the assumption that it does.  Hence
this leads to correct results.  My understanding is that this personality
function only applies to gcj generated code, so there are no ABI compliance
issues?  The exception lookup mechanism is independent of the unwinder, and
I don't think this interacts with the ip lookup in the unwinder, which
directly uses the return pointer to look up unwind information.

Doing this more cleanly seems hard, in that it involves changing the
exception tables generated by gcc.  That may introduce new inconsistencies
between platforms, and thus introduce a different kind of ugliness.  It
would also involve recompilation of all gcj or g++ generated code. 

Just to remind you, here's the current patch, which I believe is identical
to what we were considering all along.  Only the justification changed.
Again.

With this and yet another unwinder patch, which I will post separately, I'm
down to zero exception failures in both the libjava test suite and SPECjbb.

Hans

Index: libjava/configure
===================================================================
RCS file: /cvs/gcc/gcc/libjava/configure,v
retrieving revision 1.126.2.8
diff -u -r1.126.2.8 configure
--- configure	2002/03/18 06:30:17	1.126.2.8
+++ configure	2002/03/23 01:42:50
@@ -6387,6 +6387,9 @@
 #    SYSDEP_SOURCES=sysdep/ia64.c
 #    test -d sysdep || mkdir sysdep
 #    ;;
+ ia64-*-linux*)
+    SIGNAL_HANDLER=include/dwarf2-signal.h
+    ;;
  powerpc-*-linux*)
     SIGNAL_HANDLER=include/dwarf2-signal.h
     ;;
Index: libjava/configure.in
===================================================================
RCS file: /cvs/gcc/gcc/libjava/configure.in,v
retrieving revision 1.114.2.7
diff -u -r1.114.2.7 configure.in
--- configure.in	2002/03/18 06:30:17	1.114.2.7
+++ configure.in	2002/03/23 01:42:50
@@ -850,6 +850,9 @@
 #    SYSDEP_SOURCES=sysdep/ia64.c
 #    test -d sysdep || mkdir sysdep
 #    ;;
+ ia64-*-linux*)
+    SIGNAL_HANDLER=include/dwarf2-signal.h
+    ;;
  powerpc-*-linux*)
     SIGNAL_HANDLER=include/dwarf2-signal.h
     ;;
Index: libjava/include/dwarf2-signal.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/dwarf2-signal.h,v
retrieving revision 1.3
diff -u -r1.3 dwarf2-signal.h
--- dwarf2-signal.h	2001/05/29 17:50:50	1.3
+++ dwarf2-signal.h	2002/03/23 01:42:50
@@ -41,7 +41,22 @@
   _sc->sc_pc += 4;							\
 }									\
 while (0)
+
+#elif defined(__ia64__)
+
+#define MAKE_THROW_FRAME(_exception)					\
+do									\
+{									\
+  /* IA-64 either leaves PC pointing at a faulting instruction or the	\
+   following instruction, depending on the signal.  SEGV always does	\
+   the former, so we adjust the saved PC to point to the following	\
+   instruction; this is what the handler in libgcc expects.  */
\
+  struct sigcontext *_sc = (struct sigcontext *)_p;			\
+  _sc->sc_ip++;	 /* fprintf (stderr, "%p si:%p sc:%p\n",
_sc->sc_ip, si, _p); */						\
+}									\
+while (0)
 #else
+#error
 #define MAKE_THROW_FRAME(_exception)		\
 do						\
 {						\
@@ -50,6 +65,10 @@
 while (0)
 #endif
 
+extern "C" int __libc_sigaction (int __sig, 
+		      __const struct sigaction *__restrict __act,
+                      struct sigaction *__restrict __oact) throw ();
+
 #define INIT_SEGV						\
 do								\
   {								\
@@ -58,7 +77,7 @@
     act.sa_sigaction = _Jv_catch_segv;      			\
     sigemptyset (&act.sa_mask);					\
     act.sa_flags = SA_SIGINFO;	       				\
-    syscall (SYS_sigaction, SIGSEGV, &act, NULL);		\
+    __libc_sigaction (SIGSEGV, &act, NULL);		\
   }								\
 while (0)  
 
@@ -71,7 +90,7 @@
     act.sa_sigaction = _Jv_catch_fpe;				\
     sigemptyset (&act.sa_mask);					\
     act.sa_flags = SA_SIGINFO;		       			\
-    syscall (SYS_sigaction, SIGFPE, &act, NULL);		\
+    __libc_sigaction (SIGFPE, &act, NULL);		\
   }								\
 while (0)  

> -----Original Message-----
> From: Andrew Haley [mailto:aph@cambridge.redhat.com]
> Sent: Monday, March 18, 2002 2:26 AM
> To: davidm@hpl.hp.com
> Cc: Boehm, Hans
> Subject: Re: FW: Remaining Itanium exception, static linking patches
> 
> 
> David Mosberger writes:
>  > >>>>> On Sat, 16 Mar 2002 10:17:54 +0000 (GMT), Andrew 
> Haley <aph@cambridge.redhat.com> said:
>  > 
>  >   Andrew> However, I cannot see any similar logic in the IA-64
>  >   Andrew> unwinder.  I don't know why this is.  Does not the return
>  >   Andrew> address of a function point to the instruction 
> to return to?
>  > 
>  > See above.  We have gone through this once already with Richard
>  > Henderson and Jim Wilson.  I thought all the places that subtracted
>  > one from the PC had been fixed at that time (for ia64).  Apparently
>  > that is not the case?
> 
> No, not at all: the patch we're looking at is quite old and has never
> been checked in.  We should remove the unnecessary PC adjustment.
> 
> Andrew.
>