[9fans] sysrfork fp bug?

[9fans] sysrfork fp bug?

[erik quanstrom]

sysrfork() does *not* do a procsave before forking.  thus the
floating point registers in the new process are just going to
be a copy of whatever was last saved, and perhaps nothing.

on my atom system (but not some others), this attached
program does fault.  i wrote a bit of it in assembler to control
what was in F0.  the c compiler wasn't keeping things in a
register.

have i missed something or do we need a procsave() in fork?

- erik

---
sed 's/.//' >fp.c <<'//GO.SYSIN DD fp.c'
-#include <u.h>
-#include <libc.h>
-
-char *theformat = "%g\n";
-
-void
-main(void)
-{
-	extern void dofp(void);
-
-	dofp();
-}
//GO.SYSIN DD fp.c
echo myfp.s
sed 's/.//' >myfp.s <<'//GO.SYSIN DD myfp.s'
-TEXT	dofp(SB), $0
-	SUBL		$12, SP
-	FLD1
-	CALL		fork(SB)
-
-	MOVL		$2000, AX
-	MOVL		AX, 0x0(SP)
-	CALL		sleep(SB)
-
-	MOVL		theformat(SB), AX
-	MOVL		AX, 0x0(SP)
-	FMOVDP	F0,0x4(SP)
-	CALL		print(SB)
-	ADDL		$12, SP
-	RET
//GO.SYSIN DD myfp.s

Re: [9fans] sysrfork fp bug?

[Charles Forsyth]

registers are dead on entry to a function, so there's no point in saving
particular values, because they won't be used. on an amd64, however,
the kernel should
reset the FP-used flag, to cause the preset values to be set in the high FP
registers if the fork'd process does touch the FP.

On 21 November 2011 07:08, erik quanstrom <quanstro@quanstro.net> wrote:
> sysrfork() does *not* do a procsave before forking.  thus the
> floating point registers in the new process are just going to
> be a copy of whatever was last saved, and perhaps nothing.

Re: [9fans] sysrfork fp bug?

[erik quanstrom]

On Mon Nov 21 10:34:15 EST 2011, charles.forsyth@gmail.com wrote:
> registers are dead on entry to a function, so there's no point in saving
> particular values, because they won't be used. on an amd64, however,
> the kernel should
> reset the FP-used flag, to cause the preset values to be set in the high FP
> registers if the fork'd process does touch the FP.
>
> On 21 November 2011 07:08, erik quanstrom <quanstro@quanstro.net> wrote:
> > sysrfork() does *not* do a procsave before forking.  thus the
> > floating point registers in the new process are just going to
> > be a copy of whatever was last saved, and perhaps nothing.

so you're suggesting that the forked process always pretend that it
has never used floating point?

anyway, the real bug i'm chasing is on the intel atom i get exactly the same
symptom from stats.  about once a day, i get this sort of diagnostic:
(obviously this isn't from stats)

ladd; 8.fp
8.fp 233351: suicide: sys: fp: stack underflow fppc=0x104c status=0x80c1 pc=0x4922
1

perhaps it's some other bug, but i think i've exhausted all the straightforward
answers.

- erik

Re: [9fans] sysrfork fp bug?

[erik quanstrom]

	/* don't penalize the child, it hasn't done FP in a note handler. */
	p->fpstate = up->fpstate & ~FPillegal;
[...]
	ready(p);
	sched();
	return pid;

we do know that fp->state is FPinactive (because of rfork), but it seems
like this isn't doing what was intended, and if there are any values on the
x87 stack, they could well ... stack, which could lead to eventual fp stack
overflow.

given this discussion, and some prior cleanup i'm currently using this
incantation called from sysrfork in the pc arch dependent code.
fp is a FPArch* to accomidate sse or x87.  it's a little gross, but i did
need some sse instructions at one point on a 386 kernel.  eventually
the x87 stuff should be killed.

/* called from newproc() since newproc() doesn't know about fpstates */
void
procfpinit(Proc *p)
{
	p->fpstate = FPinit;
	p->fpusave = (FPsave*)((uintptr)p->fxsave + 15 & ~15);
}

/*
 *  set up floating point unit before running new process; that is
 *  turn floating point off and allow the coprocessor not avail.
 *  trap to initialize the x87/sse on an as-needed basis.
 */
void
procsetup(Proc *p)
{
	fp->off();
}

/*
 * "clone" the fpu.  assume called from rfork() [sic], assume c api (regs dead
 * on function call return) [sic, maybe?] so we can get away with discarding the old
 * fp state by setting the fpstate to FPinit.
 */
void
clonefpu(PFPU *t, PFPU *s)
{
	/* child doesn't inherit fcr, etc.?  man page not conclusive */
	t->fpstate = FPinit;
	USED(s);
}

- erik

Re: [9fans] sysrfork fp bug?

[erik quanstrom]

also, the getfcr(2) man page doesn't say if either the fcr or fsr
is inherited across fork.

- erik

Re: [9fans] sysrfork fp bug?

[erik quanstrom]

On Mon Nov 21 10:34:15 EST 2011, charles.forsyth@gmail.com wrote:
> registers are dead on entry to a function, so there's no point in saving
> particular values, because they won't be used. on an amd64, however,
> the kernel should
> reset the FP-used flag, to cause the preset values to be set in the high FP
> registers if the fork'd process does touch the FP.
>

why does pc/trap.c:/^syscall take pains to save the fp registers, then?

- erik

Re: [9fans] sysrfork fp bug?

[Charles Forsyth]

it only needs to save the non-register state (ie, the control register
you'd wondered about),
but it's free to do more if that's somehow easier, even if the values
will never be used.
on BG/P, the equivalent function does:
	 * If there isn't a pending exception, just save the
	 * initial state and current status, because registers
	 * were killed by the system call.

Re: [9fans] sysrfork fp bug?

[Charles Forsyth]

a common cause for a value being left on the fp stack
(although your diagnostic said "stack underflow" so i didn't suggest
it originally)
is that a function returns float or double but isn't declared as such
at the call site. (i had that problem once with dot or dotty.)
the function will leave the return value on the stack, and the call
site won't pop it, since it's not declared (or declared incorrectly).
the -T external type checking system should detect that, but
not for imported programs that aren't or can't be compiled that way
(eg, because there are other type errors, or -B must be used with pcc)

Re: [9fans] sysrfork fp bug?

[erik quanstrom]

On Mon Nov 21 18:14:43 EST 2011, charles.forsyth@gmail.com wrote:
> it only needs to save the non-register state (ie, the control register
> you'd wondered about),
> but it's free to do more if that's somehow easier, even if the values
> will never be used.
> on BG/P, the equivalent function does:
> 	 * If there isn't a pending exception, just save the
> 	 * initial state and current status, because registers
> 	 * were killed by the system call.

but not the statuses, correct?  this is what i'm thinking for
a minimal clone which assumes it will be called from rfork,
but doesn't assume anything about the up using x87 or sse.

static void
sseclonestate(PFPU *t, PFPU *s)
{
	Fxsave *r;

	memmove(t->fpusave, s->fpusave, sizeof(FPsave));
	r = (Fxsave*)t;
	r->ftw = 0;			/* all x87 registers invalid (clear stack) */
	r->fsw = 0;			/* clear x87 status */
	r->mxcsr &= ~SSEflags;		/* clear sse status; preserve settings */
}

- erik