Division by zero in svghandler.cpp

[Dmitry A. Kuminov]

The SVG module has a function, pathArc() defined in source:/trunk/src/svg/qsvghandler.cpp​ that uses its arguments as divisors w/o checking if they are zero or not.

This function is a reason why e.g. mp3diags traps. First of all, mp3diags somehow supplies a wrong SVG file to QIcon which contains these invalid zero arguments (this needs to be investigated, I suspect it's an OS/2 specific bug). Then, this function causes SIGFPE (which is originally XCPT_FLOAT_INVALID_OPERATION) because of division by zero.

The obvious fix is to replace

    rx = qAbs(rx);
    ry = qAbs(ry);

at around #1285 in svghandler.cpp to

    rx = qAbs(rx);
    ry = qAbs(ry);
    if (rx == 0 || ry == 0)
        return; 

I didn't commit this fix though. This code is in the Qt code base for so long that it would definitely be fixed if it made problems on other platforms (it's very easy to create an invalid .svg to cause zero arguments to be passed in there). But it is still not fixed (see ​http://qt.gitorious.org/qt/qt/blobs/master/src/svg/qsvghandler.cpp) which means that for some reason it doesn't make any problems anywhere except OS/2.

[Dmitry A. Kuminov]

I checked division by zero on Windows and it seems that it doesn't cause SIGFPE -- it returns infinity (which is kind of OK since according to the C standard, see ​http://en.wikipedia.org/wiki/Division_by_zero#In_computer_arithmetic, it's entirely up to the compiler to decide what to do in this case.

I suspect that it's either kLIBC that instructs the CPU to trigger this exception on OS/2 or it's just the default behavior on OS/2. If someone has any additional info, you are welcome.

[Dmitry A. Kuminov]

Note that this only applies to floating point arithmetics, in case of integers most (all?) environments will trow an exception.

[Silvan Scherrer]

this is what Knut tells on Irc:
_diver BirdWrk: could you eventually comment this: ​http://svn.netlabs.org/qt4/ticket/105

BirdWrk _diver: somebody (a video driver, PM, GRADD, xxx) screws up the FPU status word, that's a well known issue on OS/2.

BirdWrk _diver: if you search os2ddprog and other os/2 programming dev lists, you'll find this topic comming up every so often.

BirdWrk _diver: it is *not* a bug in kLIBC.

[Silvan Scherrer]

Doodle _diver: I also ran into this FPU problem with Cairo,

Doodle _diver: see: ​http://cgit.freedesktop.org/cairo/tree/src/cairo-os2-surface.c?id=4fc7bdaed651a5c19eb89dddd88808468e0e7eb8
Doodle _diver: (search for "DisableFPUException")

[Dmitry A. Kuminov]

Thanks for this information. This explains it. Though, here even this simple program

#include <stdio.h>

typedef float real;
#define FMT "%f"

int main()
{
	real a = 1.;
	real b = 0.;
	printf("a=" FMT "b=" FMT "\n", a, b);
	real c = a/b;
	printf("c=" FMT "\n", c);
	return 0;
}

triggers it here (g++ 3.3.5 CSD3).

Maybe it's due to the VirtualBox GRADD driver (as I run eCS under VirtualBox).

Silvan, can you run this test on your real hardware?

[Dmitry A. Kuminov]

Applying a fix similar to DisableFPUException but with the EM_ZERODIVIDE flag (masking off the division by zero) helps indeed. I will try to incorporate this in Qt.

[Dmitry A. Kuminov]

Okay, placing the FPU CW restore code to the initializer is not enough: Qt apparently calls some Win/Gpi API that effectively breaks the CW again. Finding this requires some time, so I'm postponing it.

[Dmitry A. Kuminov]

I think it makes sense to do this for GA as it may cause a lot of potential problems in custom applications that expect that 1.0/0.0 won't cause an exception. Silvan?

The best solution is to simply create wrappers for all Win* calls which will take a day I suppose.

[Silvan Scherrer]

probably we really need to do it. even i know only one case till now.

[Dmitry A. Kuminov]

Okay, good, the wrapper approach worked. In r430, I committed the header that contains wrappers for all known Win* and Gpi* functions. This header is included from qt_os2.h (which is included in every source that uses OS/2 APIs) so the wrappers are always in action.

I tried to make the wrappers as tiny as possible. They are all inlined (no extra calls and returns), and the Control Word is restored using the assembler directly, so that the resulting overhead in the release build (with the inlines turned on) is just these several 5 asm instructions (~nothing).

I'm also attaching the AWK script that I used to create these wrappers (there are about 400 APIs in total).

With the wrappers, both my tests and MP3Diags with the broken icons work as expected (no SIGFPE). Pol, please check it on your side. Note that it will take quite a while to rebuild Qt after the checkout.

[Dmitry A. Kuminov]

AWK script to create wrappers for Win* and Gpi* calls that restore the FPU Control Word

[Dmitry A. Kuminov]

Elbert reported that mp3diags works now w/o any patch. This task may be considered as done.