JM: jsop_length should use urshift32 instead of rshiftPtr

RESOLVED FIXED

Status

()

Product:
Core
Component:
JavaScript Engine
Type:
defect
Status:
RESOLVED FIXED
Opened:
9 years ago
Updated:
9 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

({regression})

Version:
unspecified
Platform:
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

Fix
1.13 KB, patch
sstangl
: review+
Details | Diff | Splinter Review 
Consider this test case:
---
function f() {
    var s1 = Array(15000).join('-');
    var s2 = Array(9000).join(s1);
    s2 = "" + s2;
    assertEq(s2.length, 134976001);
}
f();
---
This fails in JM:
test.js:6: Error: Assertion failed: got -133459455, expected 134976001

The problem is that mjit::Compiler::jsop_length uses rshiftPtr instead of urshift32.
Summary: JM: wrong result for jsop_length → JM: jsop_length should use urshift32 instead of rshiftPtr
Posted patch Fix — — Splinter Review 
This uses urshift32 like we do in generateStringLengthStub (PolyIC.cpp). I did not add the test case because it uses a lot of memory...
Attachment #499825 - Flags: review?(sstangl)
Keywords: regression
blocking2.0: --- → betaN+
Comment on attachment 499825 [details] [diff] [review]
Fix

Great find.
Attachment #499825 - Flags: review?(sstangl) → review+
Keywords: checkin-needed
Anybody willing to land this patch? It's a one line change and really safe.
http://hg.mozilla.org/mozilla-central/rev/96482f2ef48e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.