RESOLVED FIXED 68166
Unsigned bit shift fails under certain conditions in 32 bit builds
https://bugs.webkit.org/show_bug.cgi?id=68166
Summary Unsigned bit shift fails under certain conditions in 32 bit builds
keith.thorne
Reported 2011-09-15 08:54:11 PDT
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.
Attachments
test case (285 bytes, text/html)
2011-09-16 00:43 PDT, Alexey Proskuryakov
no flags
Fix (6.94 KB, patch)
2011-09-16 14:28 PDT, Gavin Barraclough
ggaren: review+
ggaren: commit-queue-
Alexey Proskuryakov
Comment 1 2011-09-15 16:00:56 PDT
> 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
Comment 2 2011-09-15 23:06:04 PDT
(-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?
keith.thorne
Comment 3 2011-09-16 00:09:56 PDT
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.
keith.thorne
Comment 4 2011-09-16 00:11:00 PDT
And a quick addendum...this is only on Windows. Works fine on the Mac
Alexey Proskuryakov
Comment 5 2011-09-16 00:43:03 PDT
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
Comment 6 2011-09-16 13:58:02 PDT
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
Comment 7 2011-09-16 14:28:33 PDT
Gavin Barraclough
Comment 8 2011-09-16 14:32:53 PDT
I should probably add a test case for this. :-(
Geoffrey Garen
Comment 9 2011-09-16 15:35:06 PDT
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
Comment 10 2011-09-16 17:19:32 PDT
Fixed in r95338
Note You need to log in before you can comment on or make changes to this bug.