Port-arm archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

possible bug in arm's cpu_setmcontext()?



Hi,

I just ported Golang (http://golang.org) to NetBSD/arm platform, and I
used both RPI
and the recent merged INTEGRATOR_CP config.

In the course of that, I was hit by two problems: the first is that
gcc generates bad code
for 64-bit shifts (this is definitely a bug, i will propose fix in
another thread), and the other
is that setcontext(2) returns EINVAL with no reason (the Go runtime
uses its own signal
return trampoline).

My signal handler doesn't modify uc_flags, so it remains what the
kernel provides:
0xd000d (_UC_TLSBASE|_UC_CLRSTACK|_UC_ARM_VFP|_UC_FPU|_UC_CPU|_UC_SIGMASK),
but the kernel rejects it (see below) in setcontext(2).

I added some debugging prints to the kernel, and found it is this if
statement in
function cpu_setmcontext() in sys/arch/arm/arm/sig_machdep.c that returns EINVAL
to me:
#ifdef FPU_VFP
        if ((flags & _UC_FPU)
            && (curcpu()->ci_vfp_id || (flags & _UC_ARM_VFP) == 0)) {
                return EINVAL;
        }
#endif

I figured that the if condition could be wrong, as it's unreasonable
to give error whenever
flag _UC_FPU is set and curcpu()->ci_vfp_id is non-zero as the user
couldn't control the
latter while the kernel automatically set the former.

I think the correct condition should really be:

#ifdef FPU_VFP
        if ((flags & _UC_FPU)
            && (curcpu()->ci_vfp_id && (flags & _UC_ARM_VFP) == 0)) {
                return EINVAL;
        }
#endif

That is, switch || to &&, and the meaning is obvious: if the cpu has
VFP and user is asking
to set FPU context (by setting _UC_FPU), but didn't provide a VFP
context (_UC_ARM_VFP)
then return EINVAL.

I've attached the proposed patch.

What do you think?
Thank you.

PS: 2 days ago, I sent the same mail to the list, but somehow it never
turned up in the
web archive, and I'm not sure why. Thus I apology if anyone is
receiving duplicated
mail of this topic.

Cheers,
minux

Attachment: arm_setmcontext.patch
Description: Binary data



Home | Main Index | Thread Index | Old Index