Unsigned bit shift fails under certain conditions in 32 bit builds

• Component: JavaScriptCore
• Status: RESOLVED FIXED

A code snippet will explain it best:

        var i = 0,
            value = -1698987653.3475556;

        var j = (value >>> (i * 8)) % 256;

(I know that the value is a weird number, but it is what I had in this case).

So, in the above code snippet on Webkit for windows, the formula will return -2 as the answer, which is wrong, as it should return 254 (as it does on all other browsers).
If, however, you do this:

       var value = -1698987653.3475556;

       var j = (value >>> (0 * 8)) % 256;

Note that I have just replaced the i with a 0, the returned value is correct. If the initial value is positive, then the first code snippet works as expected, so the point is, with a negative value and an assigned variable instead of a hard coded integer, the unsigned right shift will fail.

NOTE: the modular is simply there because I had it in my code, it does not really make a difference to the bug itself
NOTE 2: I did not try this with an unsigned left shift
NOTE 3: A signed right shift works as expected.

[Alexey Proskuryakov]

> So, in the above code snippet on Webkit for windows, the formula will return -2 as the answer, 
> which is wrong, as it should return 254 (as it does on all other browsers).

Are you seeing this with a WebKit nightly, or with shipping Safari/WebKit 5.1?

On a Mac, I'm getting 123 everywhere, including Firefox.

[Gavin Barraclough]

(-1698987653.3475556 >>> 0) % 256

I believe evaluates as:

((uint32_t)(int32_t)-1698987653.3475556) % 256
((uint32_t)-1698987653) % 256
0x9ABB817Bu % 256
0x7Bu
123

So as Alexey says, 123 is the correct result.
Are you sure you were seeing -2 or 254 from this test case?

Okay, my bad, I should have double checked the snippet in the console before posting. This here snippet breaks as it should, in the console as well:

var encodeNumber = function(target, bytes, value) {
    var i = 0;

    for( ; i < bytes; i++ ) {
        target.push((value >>> (i * 8)) % 256);
    }
}

var target = [];
var bytes = 4;
var value = -1699021058.648747;
encodeNumber(target, bytes, value);
console.log(target);

(This mirrors more the code that I am working with). The first value of the array (-2) is the problem value.

And a quick addendum...this is only on Windows. Works fine on the Mac

[Alexey Proskuryakov]

Created attachment 107616 [details]
test case

Attaching same test for easier verification.

I can reproduce this in Safari 5.0.5 on Mac OS X when running in 32-bit mode. Haven't tested ToT.

[Gavin Barraclough]

Gottcha.  Hmmm, this repros with just:

var value = -1699021058.1;
var i = 0;
print(value >>> i);

(prints -1699021058, the result of unsigned right shift should never be negative).

[Gavin Barraclough]

Created attachment 107722 [details]
Fix

[Gavin Barraclough]

I should probably add a test case for this. :-(

[Geoffrey Garen]

Comment on attachment 107722 [details]
Fix

r=me, but please check in with a test, and be careful when conflict-resolving with r95324.

[Gavin Barraclough]

Fixed in r95338