[Bug libgcj/14767] gcjlib under FreeBSD does not forward SIGSEGV to SA_SIGINFO handler

["andrew dot gray at anu dot edu dot au" <gcc-bugzilla at gcc dot gnu dot org>]

------- Additional Comments From andrew dot gray at anu dot edu dot au  2004-03-30 07:40 -------
Loren,

I am not planning to do more work on the apparent problem in
GC_write_fault_handler (unless it turns out to be a problem for
the project am working on, like this bug is).

However, if there is more work I could do that would increase the
chances of some form of my patch being accepted into GCC please let me
know.

So, to explain what is left to be done:

The apparent problem in GC_write_fault_handler stems from the fact that
FreeBSD (in versions >= 4) supports two protypes for signal handling:
the traditional BSD style:

void handler(int, int code, struct sigcontext *scp);

and the POSIX SA_SIGINFO style:

void handler(int, siginfo_t *info, ucontext_t *uap);

See the sigaction man page for more information:

http://www.freebsd.org/cgi/man.cgi?query=sigaction&sektion=2&apropos=0&manpath=FreeBSD+5.2-RELEASE+and+Ports

When specifying a signal handler, the caller sets the SA_SIGINFO bit
in the sa_flags field if their handler is the POSIX SA_SIGINFO style.

The code in GC_write_fault_handler appears to forward signals it is
handling to the handler recorded in GC_old_segv_handler (or
GC_old_bus_handler) when the fault is not in memory allocated by the
GC code.  Under FreeBSD the current code always calls the old handler
using the tradational BSD style.  If the old handler actually uses the
POSIX SA_SIGINFO style then it will not receive the arguments it
expects.

With the my patch it will always use the POSIX SA_SIGINO style.  In
this case, if the old handler actually uses the traditional BSD style
then it will not receive the arguments it expects.

I think the right thing for GC_write_fault_handler to do would be to
use the style that the old handler was specified with.  So the code
that currently reads:

#		if defined (SUNOS5SIGS)
		    (*(REAL_SIG_PF)old_handler) (sig, scp, context);
		    return;
#		endif

would look something like this (assuming my patch is applied):

#		if defined (SUNOS5SIGS)
                    if (old_handler_flags & SA_SIGINFO) {
		      (*(REAL_SIG_PF)old_handler) (sig, scp, context);
                    } else {
                      (*old_handler) (sig, scp->si_code,
                                      (struct sigcontext *)context);
		    return;
#		endif

This code raises at least two questions: Firstly, were does
old_handler_flags get set?  This would involve extra code in
GC_dirty_init, maybe setting a new GC_old_segv_handler_flags variable.
Secondly, can the third argument received by GC_write_fault_handler,
which is a "ucontext_t *" (assuming my patch is applied) simply be
cast to "struct sigcontext *"?  The following comment in
/usr/include/ucontext.h in the definition of struct ucontext_t
suggests this might be OK:

	 * Keep the order of the first two fields. Also,
	 * keep them the first two fields in the structure.
	 * This way we can have a union with struct
	 * sigcontext and ucontext_t. This allows us to
	 * support them both at the same time.
	 * note: the union is not defined, though.

In terms of what needs to be done to address this apparent problem:

- Confirm that it actually is a bug.  The code does not look
  right to me, but the next step would be to write a test case that
  demonstrates a problem.  I do not know enough about the code to know
  when the GC_write_fault_handler code comes into play, so I'm not
  sure what the test would look like.

- If a bug is confirmed, code a fix.  What I have outlined above
  should be a starting point.

- Test the fix.

Andrew



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14767