Subject: Re: delivering faulted-upon address in trap frame to userland
To: Christos Zoulas <christos@astron.com>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: port-i386
Date: 10/17/2006 22:55:39
This is a multipart MIME message.

--==_Exmh_10615160462580
Content-Type: text/plain; charset=us-ascii


[which signal to send on SSE2 alignment violations]
christos@astron.com said:
> M.Drochner@fz-juelich.de said:
> > There might be bad effects in case programs catch SIGSEGV but get
> > a SIGILL now, but I didn't find one so far.

I've actually found one, which made me reconsider these things.
It is mono which tries to find out the limits of its stack by probing
the addresses. It does so as a last resort; other OSes use constants
or sysctls. This could be changed, but the fact remains that a user
program shouldn't notice if a kernel uses some <insert your favorite
curse here> segmentation techniques instead of a plain paging model.
So we should issue a SIGSEGV in any case where an unmapped address
is accessed. And since that poor i386 doesn't tell anything on that
catch-all GP fault which would help finding out the real reason we
don't have much choice than to issue that SIGSEGV on everything causing
a GPF.

> I think that sending SIGILL is wrong (because this is not an illegal
> instruction).

Reading the siginfo manpage I'd say that a SIGILL leaves much room
for interpretation. It can be ILLOPC or ILLOPN, and a lot more. It
is actually the most generic among the signals in question as I see it.

> Perhaps it is better to just put 0
> as the address instead of cr2?

I wouldn't use 0, or -1, or 0xdeadbeef, to avoid confusion with
traps generated by real user program errors.
Perhaps 0x12345678 (but I wouldn't seriously object 0x87654321:-).

> What do other OS's do?

I've checked Linux and FreeBSD; both aren't correct according to
SUSv3: FreeBSD sends a SIGBUS without a correct address, and Linux
sends a SIGSEGV, also without a correct address, iirc.
It seems that SUSv3's siginfo specification didn't consider the
sad i386 reality... anyway, user code dealing with that is very
OS dependant anyway, and SUSvX isn't going to change that. So I'd
say for us it is OK to remove the wrong use of cr2, and keep the
code as simple as possible, and stay compatible, would be the
most reasonable thing to do.

What do you think of the appended patch?

best regards
Matthias



--==_Exmh_10615160462580
Content-Type: text/plain ; name="i386trap.txt"; charset=us-ascii
Content-Description: i386trap.txt
Content-Disposition: attachment; filename="i386trap.txt"

Index: trap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/trap.c,v
retrieving revision 1.214
diff -u -p -r1.214 trap.c
--- trap.c	29 Sep 2006 14:48:15 -0000	1.214
+++ trap.c	17 Oct 2006 20:05:26 -0000
@@ -449,9 +449,24 @@ copyfault:
 		    &l->l_addr->u_pcb)) {
 			goto out;
 		}
+
+		/*
+		 * We get a GPF on all kinds of user program misbehaviour,
+		 * as privilege, alignment and segmentation problems.
+		 * We'd like to send SIGILL/SIGBUS/SIGSEGV appropriately,
+		 * but the processor doesn't tell enough details.
+		 * For historic reasons, send SIGSEGV (in particular for
+		 * user programs which shouldn't notice the difference
+		 * between segment limits and unmapped pages).
+		 */
 		KSI_INIT_TRAP(&ksi);
 		ksi.ksi_signo = SIGSEGV;
-		ksi.ksi_addr = (void *)rcr2();
+		/*
+		 * SUSv3 wants us to provide the address of the faulting
+		 * memory reference on SIGSEGV, but we don't know.
+		 * Fill in something which might help debugging.
+		 */
+		ksi.ksi_addr = (void *)0x12345678;
 		ksi.ksi_code = SEGV_ACCERR;
 		goto trapsignal;
 
@@ -460,32 +475,13 @@ copyfault:
 	case T_STKFLT|T_USER:
 	case T_ALIGNFLT|T_USER:
 	case T_NMI|T_USER:
-		KSI_INIT_TRAP(&ksi);
-		ksi.ksi_signo = SIGBUS;
-		ksi.ksi_addr = (void *)rcr2();
-		switch (type) {
-		case T_SEGNPFLT|T_USER:
-		case T_STKFLT|T_USER:
-			ksi.ksi_code = BUS_ADRERR;
-			break;
-		case T_TSSFLT|T_USER:
-		case T_NMI|T_USER:
-			ksi.ksi_code = BUS_OBJERR;
-			break;
-		case T_ALIGNFLT|T_USER:
-			ksi.ksi_code = BUS_ADRALN;
-			break;
-		default:
-			KASSERT(1);
-			break;
-		}
-		goto trapsignal;
-
+		/* "shouldn't happen", report generic error */
+		/* FALLTHROUGH */
 	case T_PRIVINFLT|T_USER:	/* privileged instruction fault */
 	case T_FPOPFLT|T_USER:		/* coprocessor operand fault */
 		KSI_INIT_TRAP(&ksi);
 		ksi.ksi_signo = SIGILL;
-		ksi.ksi_addr = (void *)rcr2();
+		ksi.ksi_addr = (void *)frame->tf_eip;
 		switch (type) {
 		case T_PRIVINFLT|T_USER:
 			ksi.ksi_code = ILL_PRVOPC;
@@ -494,7 +490,7 @@ copyfault:
 			ksi.ksi_code = ILL_COPROC;
 			break;
 		default:
-			ksi.ksi_code = 0;
+			ksi.ksi_code = ILL_ILLOPN; /* ??? */
 			break;
 		}
 		goto trapsignal;

--==_Exmh_10615160462580--