Alright! I'll test that new patch and submit it. Thanks, - Andrew
On Fri, Aug 12, 2016 at 4:01 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 10 August 2016 at 04:35, Andrew Dutcher <andrewrdutc...@gmail.com> 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