[Bug 30465] New: BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Bug #: 30465
           Summary: BridgeCentral: Invalid floating point operation in
                    mozjs.dll (fldcw in JSDOUBLE_IS_INT32)
           Product: Wine
           Version: 1.4
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: mshtml
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
    Classification: Unclassified

When BridgeCentral 2.2.3 is logged in to a club database on the Internet, it
will be fatally hit by an "Invalid floating point operation" dialog box after
at most (and often exactly) 9 minutes of operation, i.e. 9 minutes after
opening of the main window displaying the very simple web page
http://bridge.dk/bridgecentral/nyheder/Welcome/Welcome.html .  When you press
OK to the dialog box, the application is either completely confused and
unusable, or it immediately crashes with 2 more dialog boxes about Access
violation in mozjs.dll for read of an address, and the general wine "Serious
problem" box.  The crash happens completely reproducibly if you just let the
application wait at the Welcome window after login and don't touch it.

"Unfortunately" (from a debugging point of view) the bug does _not_ appear when
BridgeCentral 2.2.3 uses only a local database, without any Firebird TCP
connection.  So although anyone can download BridgeCentral from
http://bridgecentral.dk/ , you cannot easily reproduce the bug without being a
privileged member of a Danish DBf bridge club.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #1 from Ulrik Dickow  2012-04-17 05:35:01 CDT ---
Created attachment 39832
  --> http://bugs.winehq.org/attachment.cgi?id=39832
Wine 1.4 backtrace, floating point regs and disassembly (short)

To begin with my simplest debug session, this first attachment is a rather
short one made with wine 1.4, i.e. the latest STABLE version.  It shows that an
invalid floating point operation exception was triggered by the latter of the
two fldcw instructions in this sequence (AT&T syntax):

fldcw  0xfffffff4(%ebp)   # Load FP Control Word 0x0c72 (c = truncate)
    fistps 0xfffffff0(%ebp)   # Convert ST0 to 32 bit int => pending exception
    fldcw  0xfffffff6(%ebp)   # Load old CW (0x1372) => triggers the exception

The floating point number to be converted is ST0 = d = 1332603567267000.0 = the
number of microseconds from Jan 1 1970 (00:00) to the time of the crash, Mar 24
16:39:27 2012.  You will see in a later attachment that this number comes from
the Gecko history expiration timer event in nsPlacesExpiration.js.

The number is of course too large for an int32.  That should'nt be a problem,
since the whole point of JSDOUBLE_IS_INT32 is to see whether or not the double
fits into an int32.  Indeed normally and by default the "Invalid operation"
exception is masked out by bit 0 being set in the Floating point Control Word
(FLCW), i.e. uneven CW.  But for some unknown reason, here we have entered
JSDOUBLE_IS_INT32 with an even CW (0x1372), so that the exception generated by
the invalid conversion to int32 is triggered immediately at the next floating
point instruction (second fldcw).

More comments follow on the next attachments.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #2 from Ulrik Dickow  2012-04-17 05:40:42 CDT ---
Created attachment 39833
  --> http://bugs.winehq.org/attachment.cgi?id=39833
Wine 1.5.0 WINEDEBUG=+seh,+relay output (tail -30) right before crash

Backtraces show that the crashes happen in the same place whether you use wine
1.5.1, 1.5.0 or 1.4 (or wine 1.3.10+ with earlier versions of BridgeCentral;
wine 1.2.0 and 1.1.38 didn't have the bug).

Here is the last 30 lines of the 264 MB of output of running with +seh,+relay.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #3 from Ulrik Dickow  2012-04-17 05:51:03 CDT ---
Created attachment 39834
  --> http://bugs.winehq.org/attachment.cgi?id=39834
Wine 1.5.1 detailed debug session with Gecko source

Here Gecko source has been installed (wine-gecko-1.5 tag in git).

We see that the JavaScript triggering the bug is the history expiration code in
nsPlacesExpiration.js.  It explains why the crash usually happens after 9
minutes, since this is the time before the idle timer will trigger the
expiration logic.

However, there's nothing wrong with that JavaScript (related to the bug).  The
bug is either that JSDOUBLE_IS_INT32 is too careless about generating invalid
operation exceptions, or that it isn't properly ensured that these exceptions
are masked out when it runs -- and cleared before anyone unmasks the exception.
 Well, there are many ways to fix or avoid the problem.  More on that in my
next attachments.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #4 from Ulrik Dickow  2012-04-17 06:43:25 CDT ---
Created attachment 39836
  --> http://bugs.winehq.org/attachment.cgi?id=39836
Small stand-alone C(++) test program fiddling with the floating point CW

This small C/C++ test program demonstrates the effect of the floating point
control word on the x86 platform using old-fashioned 387 code (the default on
32-bit compiles).  It shows that if the CW, as in the wine crash, is 0x1372
when we reach the cast

(int32_t) d

then that cast will indeed trigger an Invalid floating point operation
exception IF d is too large to fit in an int32_t -- but not if it just has a
value at or between two valid 32 bit integers.

So a simple and portable way of avoiding the problem, modifying only Gecko C++
source code, would be to replace

if (JSDOUBLE_IS_NEGZERO(d))
with
    if (JSDOUBLE_IS_NEGZERO(d) || d < INT32_MIN || d > INT32_MAX)

in JSDOUBLE_IS_INT32 in wine-gecko/js/src/jsval.h.

Oops, now I see that only a few days ago in the main git repo for wine-gecko,
it was replaced with MOZ_DOUBLE_IS_INT32 -- but the same implementation, so the
same fix can apply.  It will make the code slower, though.

The test program also proves that if you compile with -mfpmath=sse -msse2, it
will not crash.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Ulrik Dickow  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download, regression
                URL|                            |http://appdb.winehq.org/obj
                   |                            |ectManager.php?sClass=appli
                   |                            |cation&iId=14064
           See Also|                            |https://bugzilla.mozilla.or
                   |                            |g/show_bug.cgi?id=744965

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #5 from Ulrik Dickow  2012-04-17 06:55:24 CDT ---
Created attachment 39837
  --> http://bugs.winehq.org/attachment.cgi?id=39837
Perl script replacing 387 code with SSE2 equivalent in mozjs.dll for Gecko
1.4+1.5

This Perl script is a useful quick-fix for all wine users hit by "Invalid
floating point operation" caused by invalid double-to-int32 conversions in
JSDOUBLE_IS_INT32 in either Gecko 1.4 or 1.5.  It replaces the old, slow and
exception-prone 387 instructions with the equivalent SSE2 instructions.  It
won't work on very old cpus without the SSE2 instruction set.

The script does a rather trivial binary substitution.  You must apply it to the
DEBUG version of mozjs.dll in order for it to work.

With a mozjs.dll fixed by this script, BridgeCentral is completely stable,
without any crash, during hours of network connection.  Not surprising, in view
of the small test program previously submitted.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Ulrik Dickow  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://appdb.winehq.org/obj |http://appdb.winehq.org/obj
                   |ectManager.php?sClass=appli |ectManager.php?sClass=appli
                   |cation&iId=14064
           |cation&iId=14064
           See Also|                            |http://bugs.winehq.org/show
                   |                            |_bug.cgi?id=17256

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Ulrik Dickow  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugzilla.mozilla.or
                   |                            |g/show_bug.cgi?id=61898

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Ulrik Dickow  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugzilla.mozilla.or
                   |                            |g/show_bug.cgi?id=530896

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Ulrik Dickow  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |http://bugs.winehq.org/show
                   |                            |_bug.cgi?id=26648

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #6 from Ulrik Dickow  2012-04-17 12:33:16 CDT ---
Created attachment 39842
  --> http://bugs.winehq.org/attachment.cgi?id=39842
Proposed tiny patch to MOZ_DOUBLE_IS_INT32 (only tested stand-alone)

Proposed patch to wine-gecko Master branch in git as of today, fixing
mozilla.org

Bug 744965 - MOZ_DOUBLE_IS_INT32 shouldn't rely on undefined behavior

Note: I haven't set up a full wine-gecko build environment, so I've only tested
this stand-alone in a small stand-alone program like attachment 39836.

With this patch, MOZ_DOUBLE_IS_INT32 also works when Gecko is compiled on x86
using the default 387 code (not SSE2) AND the application has chosen to enable
Invalid operation floating point exceptions (e.g. set the FPU Control Word to
0x1372).  The application may have good reason to do so for its own code. 
Gecko shouldn't, on its own, generate exceptions that spill out to the
application.

Before putting the patch into production, someone should test that it doesn't
slow down time critical JavaScript code by unreasonable amounts.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Jacek Caban  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |[e-mail]
     Ever Confirmed|0                           |1

--- Comment #7 from Jacek Caban  2012-04-18 05:15:32 CDT ---
Ulrik, nice investigation, thanks. It's highly preferred to fix bugs upstream.
Please attach your patch to Mozilla's bugzilla and request feedback. I suspect
additional checks may be too much if this function is used in hot paths, but
guys at Mozilla will know better and I'm sure there is a solution that will be
acceptable for them.

I will attach a build based on Wine Gecko 1.5 sources with your proposed
changes, so you may verify it's really fixed.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #8 from Jacek Caban  2012-04-18 05:18:39 CDT ---
Created attachment 39850
  --> http://bugs.winehq.org/attachment.cgi?id=39850
mozjs.dll

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #9 from Ulrik Dickow  2012-04-19 11:37:27 CDT ---
(In reply to comment #7)
> Ulrik, nice investigation, thanks. It's highly preferred to fix bugs upstream.
> Please attach your patch to Mozilla's bugzilla and request feedback.

Ok, I'll look into that.
> I suspect additional checks may be too much if this function is used in hot
> paths, but guys at Mozilla will know better and I'm sure there is a solution
> that will be acceptable for them.

I'll probably benchmark a bit in iexplore in Wine before I remake the patch
with hg(1) and submit it to Mozilla bugzilla.
> I will attach a build based on Wine Gecko 1.5 sources with your proposed
> changes, so you may verify it's really fixed.

Thanks a lot.  It really fixed it.  I used it today with Wine 1.5.1 for a
BridgeCentral network database connection for 3½ hours without any crash. 
Without a fixed dll, it would have crashed within 9 minutes.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #10 from Jacek Caban  2012-04-20 07:58:59 CDT ---
(In reply to comment #9)
> I'll probably benchmark a bit in iexplore in Wine before I remake the patch
> with hg(1) and submit it to Mozilla bugzilla.

Wine builds are not really good for benchmarking. I've pushed your patch to
Mozilla's automated benchmarks/tests. You may watch its results here, once they
are available:

https://tbpl.mozilla.org/?tree=Try&rev=08528a2f799d

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #11 from Anastasius Focht  2012-05-01 07:22:16 CDT ---
*** Bug 30563 has been marked as a duplicate of this bug. ***

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |30563

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jeroen.de.meerleer@telenet.
                   |                            |be

--- Comment #12 from Anastasius Focht  2012-05-02 07:54:53 CDT ---
*** Bug 26648 has been marked as a duplicate of this bug. ***

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Jacek Caban  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|http://bugs.winehq.org/show |
                   |_bug.cgi?id=26648           |

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #13 from Jacek Caban  2012-05-04 13:04:49 CDT ---
I've committed the patch to wine-gecko's git:

http://wine.git.sourceforge.net/git/gitweb.cgi?p=wine/wine-gecko;a=commitdif...

and backported to wine-gecko-1.6 branch (master branch is on schedule for 1.7
now):

http://wine.git.sourceforge.net/git/gitweb.cgi?p=wine/wine-gecko;a=commitdif...

So it's in 1.6-beta1 build now:

http://www.winehq.org/pipermail/wine-devel/2012-May/095424.html

Please confirm that the bug is fixed in beta.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Jerome Leclanche  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
          Component|mshtml                      |wine-gecko-unknown
            Version|1.4                         |unspecified
         AssignedTo|[e-mail]        |[e-mail]
            Product|Wine                        |Wine-gecko

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

--- Comment #14 from Ulrik Dickow  2012-05-06 09:10:10 CDT ---
(In reply to comment #13)
> I've committed the patch to wine-gecko's git:
> [...]
> So it's in 1.6-beta1 build now:
>
> http://www.winehq.org/pipermail/wine-devel/2012-May/095424.html
>
> Please confirm that the bug is fixed in beta.

Thanks a lot.  Yes, I can confirm that.  BridgeCentral survived more than half
an hour of network connection today without any error, using your beta
Gecko-build and my own 32-bit build of Wine from git today, with your
Gecko-version patch added as glue:

[ukd@quadtux wine]$ /opt/wine-1.5.3-git1/bin/wine --version
   wine-1.5.3-165-g10a7dc2

[ukd@quadtux wine]$ git show-branch --sha1-name
   ! [master] amstream: Add the corresponding pin to every media stream added
to the media stream filter + add tests.
    * [wine-ukd-test-gecko-1.6-beta1] Test using Wine Gecko 1.6-beta1 instead
of 1.5
   --
    * [10a7dc2] Test using Wine Gecko 1.6-beta1 instead of 1.5
   +* [dec3d50] amstream: Add the corresponding pin to every media stream added
to the media stream filter + add tests.

[ukd@quadtux wine]$ sha1sum /usr/share/wine/gecko/wine_gecko-1.6-beta1-x86.msi
e5a51e484d2b7b5c15d5f6f7a69bfccf057ff3a3 
/usr/share/wine/gecko/wine_gecko-1.6-beta1-x86.msi

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Jerome Leclanche  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #15 from Jerome Leclanche  2012-05-06 09:31:49 CDT ---
Fixed then

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Jacek Caban  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |9fa443dcd9d16265a2d2b41441c
                   |                            |ea17c3c07e782
          Component|wine-gecko-unknown          |mshtml
         AssignedTo|[e-mail]       |[e-mail]
            Product|Wine-gecko                  |Wine

--- Comment #16 from Jacek Caban  2012-06-13 13:20:10 CDT ---
New Gecko is in Wine. I still hope to see the fix upstreamed.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll (fldcw in JSDOUBLE_IS_INT32)

http://bugs.winehq.org/show_bug.cgi?id=30465

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #17 from Alexandre Julliard  2012-06-22 13:29:30 CDT ---
Closing bugs fixed in 1.5.7.

[Bug 30465] BridgeCentral: Invalid floating point operation in mozjs.dll ( fldcw in JSDOUBLE_IS_INT32)

https://bugs.winehq.org/show_bug.cgi?id=30465

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unspecified                 |1.4
                 CC|                            |[e-mail]