Re: [Qemu-devel] Hang bug in 80-bit float square root implementation

[Peter Maydell]

On 10 August 2016 at 04:35, Andrew Dutcher <address@hidden> wrote:
> Hello!
>
> I ran into an issue where qemu (specifically, the unicorn engine)
> would hang forever in the middle of the emulated square root
> instruction under certain circumstances. I eventually tracked the
> issue down to the square root of an "unnormal" long double, one
> without the integer part bit set.
>
> Test case:
>
> cat > hang.s <<EOF
> .intel_syntax noprefix
>
> .global _start
> _start:
>   xor eax,eax
>   inc eax
>   push eax
>   push eax
>   push eax
>   fld tbyte ptr [esp]
>   fsqrt
>   int 0x80
> EOF
> as --32 hang.s -o hang.o
> ld -melf_i386 hang.o -o hang
> qemu-i386 ./hang
>
> qemu will hang! Assuming it jits the fsqrt as the soft float
> implementation and doesn't just use a native fsqrt instruction. I have
> no idea if qemu can do this, but it doesn't hurt to cover that base.

Hi; thanks for this patch and test case. You're right that QEMU
definitely shouldn't hang here, but I don't think this patch
is the correct fix for it. As you say, there's been variation
in how unnormals (80-bit-precision values with non-zero exponent
field and the 'integer' bit zero) should be handled, but the
Intel 64 and IA-32 architectures software developer's manual says
that "The Intel 387 match coprocessor and later IA-32 processors
generate an invalid-operation exception when [pseudo-NaNs,
pseudo-infinities and un-normal numbers] are encountered as operands"
(page 8-20 in the version of the doc I have). I checked on my
x86 box (a Core i7-3770) and it does indeed do this. Since QEMU
doesn't try to emulate the 287 we can stick to emulating just
the "invalid-operation" behaviour and don't need to also try
to emulate what the 287 did.

I think that would look something like:

    if (floatx80_invalid_encoding(x)) {
        float_raise(float_flag_invalid, status);
        return floatx80_default_nan(status);
    }

at the start of the relevant floatx80 functions (which is definitely
more than just the sqrt function; would need to check the Intel
docs for exactly which ones). floatx80_invalid_encoding()
would be a new function that identifies the pseudo-NaN,
pseudo-infinity and un-normal formats.

(The other class of odd encoding is pseudo-denormal, but those
are different because they are still supposed to be handled as
inputs, they just aren't generated as outputs.)

> I've never submitted a patch here before, so I hope this is the right
> way to do it!

It's pretty close. We have some documentation on patch formats
and so on here: http://wiki.qemu.org/Contribute/SubmitAPatch

The most important thing that you need to do is remember to
add a "Signed-off-by:" line; we can fix up most other minor
problems, but without the signed-off-by we can't accept the
patch at all.

thanks
-- PMM