Bug 136803 - REGRESSION: DataView result wrong when optimized
Summary: REGRESSION: DataView result wrong when optimized
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.9
: P1 Normal
Assignee: Nobody
URL: http://bertfreudenberg.github.io/Sque...
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2014-09-13 08:52 PDT by Vanessa Freudenberg
Modified: 2022-02-11 14:10 PST (History)
5 users (show)

See Also:


Attachments
Screenshot showing buggy behavior (147.67 KB, image/png)
2014-09-14 03:06 PDT, Vanessa Freudenberg
no flags Details
Screenshot showing intended behavior (196.91 KB, image/png)
2014-09-14 03:07 PDT, Vanessa Freudenberg
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vanessa Freudenberg 2014-09-13 08:52:57 PDT
I'm reading about 30,000 floats interspersed with other stuff from a large ArrayBuffer. They're regular IEEE-754 doubles, but stored with the upper and lower 32 bits swapped:

        var data = new DataView(theBits.buffer, theBits.byteOffset),
            buffer = new ArrayBuffer(8),
            swapped = new DataView(buffer);
        swapped.setUint32(0, data.getUint32(4));
        swapped.setUint32(4, data.getUint32(0));
        return swapped.getFloat64(0, true);

This works fine in Safari and Chrome and Firefox and IE11.

However, it stops working in Safari after about 25,000 executions. From a certain point forward, it always answers 1.3797216632888e-310, no matter what the actual bits are.

If I run it in Safari with the error console open, it reads all 30,000 floats correctly.

This sounded like an optimization error, so I added a workaround:

        var data = new DataView(theBits.buffer, theBits.byteOffset),
            buffer = new ArrayBuffer(8),
            swapped = new DataView(buffer);
        (function() {
            swapped.setUint32(0, data.getUint32(4));
            swapped.setUint32(4, data.getUint32(0));
        })();
        return swapped.getFloat64(0, true);

With this workaround, all 30,000 floats are read correctly even if the error console is not open.

How to reproduce:
I was unable to reproduce the problem in a small snippet. It happens on this webpage, the symptom being that the canvas rendering is incomplete:
http://bertfreudenberg.github.io/SqueakJS/etoys/#noFloatDecodeWorkaround

With my workaround not disabled, it renders correctly:
http://bertfreudenberg.github.io/SqueakJS/etoys/
(If you get an alert about errors, reload. They are not reproducible like the bug I report here, although I believe them to be caused by wrong optimization, too)

Here is the commit where I added a workaround, preventing optimization of my decodeFloat method:
https://github.com/bertfreudenberg/SqueakJS/commit/e7e5a33b24cebcfbc55b7c251fcdec836ea4f27b

Please let me know if I can be of any further assistance on this bug. The FTL JIT beats all other JS engines on the Mac hands-down in speed for this Smalltalk virtual machine project, except it gets it wrong, sometimes.
Comment 1 Alexey Proskuryakov 2014-09-13 13:09:53 PDT
What version of Safari are you seeing this with? I don't think that I can reproduce with Safari 7.0.6 (I didn't try with a nightly build).

Is the rendering problem expected to happen as soon as I open the page, or do I need to do something else?
Comment 2 Vanessa Freudenberg 2014-09-14 03:03:36 PDT
(In reply to comment #1)
> What version of Safari are you seeing this with? I don't think that I can reproduce with Safari 7.0.6 (I didn't try with a nightly build).

Ah, you're right, it currently happens only on the nightly. I just tried Nightly Version 7.0.6 (9537.78.2, r173567).

In the released Version 7.0.6 (9537.78.2) it works correctly. I'm almost sure an earlier release exhibited the problem, though.

> Is the rendering problem expected to happen as soon as I open the page, or do I need to do something else?

You don't need to do anything, just wait until the little red car starts animating.

I will attach a screenshot of the problematic behavior.
Comment 3 Vanessa Freudenberg 2014-09-14 03:06:12 PDT
Created attachment 238092 [details]
Screenshot showing buggy behavior
Comment 4 Vanessa Freudenberg 2014-09-14 03:07:04 PDT
Created attachment 238093 [details]
Screenshot showing intended behavior
Comment 5 Vanessa Freudenberg 2014-09-14 03:08:39 PDT
Just want to clarify that it is not the canvas rendering that is problematic, it is just indicative of a problem that happened much earlier.
Comment 6 Radar WebKit Bug Importer 2014-09-14 22:07:07 PDT
<rdar://problem/18333833>
Comment 7 Geoffrey Garen 2015-03-30 14:19:20 PDT
http://bertfreudenberg.github.io/SqueakJS/etoys/#noFloatDecodeWorkaround does not work in Firefox or Chrome, either.

Are you sure this isn't a content bug?
Comment 8 Vanessa Freudenberg 2015-03-31 04:56:16 PDT
(In reply to comment #7)
> http://bertfreudenberg.github.io/SqueakJS/etoys/#noFloatDecodeWorkaround
> does not work in Firefox or Chrome, either.
> 
> Are you sure this isn't a content bug?

In the meantime my arg parsing changed. Try this:

No workaround: http://bertfreudenberg.github.io/SqueakJS/etoys/#noFloatDecodeWorkaround=foo

With workaround: http://bertfreudenberg.github.io/SqueakJS/etoys/

Be sure to test in WebKit nightly, the released version still suffers from https://bugs.webkit.org/show_bug.cgi?id=139398

And it looks like there are more bugs even it WebKit that I have not isolated yet (e.g. the "system error handling failed" overlay). But it's still clear that the problem reported here is still present. Thank you for testing!