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