Re: Bogus signal handler causes kernel panic (5.2.1-p8/i386)

On Sat, 19 Jun 2004, Simon Barner wrote:

> I'll call that one patch3.
>
> > Index: machdep.c
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/i386/i386/machdep.c,v
> > retrieving revision 1.590
> > diff -u -2 -r1.590 machdep.c
> > --- machdep.c	11 Jun 2004 11:16:22 -0000	1.590
> > +++ machdep.c	19 Jun 2004 05:27:18 -0000
> > _at__at_ -1134,4 +1134,7 _at__at_
> >          }
> >
> > +	/* XXX drop the FP state correctly, unlike in the next 3 statements. */
> > +	fpstate_drop(td);
> > +
> >  	/*
> >  	 * Initialize the math emulator (if any) for the current process.
> > %%%
>
> I was not sure whether to back out patch2 (against npx.c) before
> applying patch3, so I tried both combinations.

You needed this with patch2.  A cleaner version of it has now been committed.

> Unfortunately, I have to refer you to the attached stack traces once
> again :(

Another try:

%%%
Index: npx.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/isa/npx.c,v
retrieving revision 1.151
diff -u -2 -r1.151 npx.c
--- npx.c	18 Jun 2004 02:10:55 -0000	1.151
+++ npx.c	19 Jun 2004 13:44:15 -0000
_at__at_ -945,4 +945,8 _at__at_
 	s = intr_disable();
 	if (td == PCPU_GET(fpcurthread)) {
+#ifdef CPU_ENABLE_SSE
+		if (!cpu_fxsr)
+#endif
+			fnclex();	/* As in npxdrop(). */
 		fpurstor(addr);
 		intr_restore(s);
%%%

I tried too hard to keep the fnclex() out of fpurstor().  It should work
there (patch1), but it it is fairly expensive and fpurstor() is in the
main path for FPU context switching.  The above is in npxsetegs(), which
is called mainly for signal handling (on return).  It needs to set the
state, and it doesn't know anything about the current state except that it
is active, so it (the function) needs to defuse the current state
unconditionally.  Most signal handlers don't use FP, so this is not in a
fast path like FPU context switching.

I will let you test it before I commit it.

Bruce