If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

IonMonkey: incorrect result with nan uint comparison

RESOLVED FIXED in Firefox 25

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: h4writer)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
mozilla25
All
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox24 affected, firefox25 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
function f()
{
    return (4 >>> 0) > ((0 % (1 == 2)) >>> 0);
}
for (var i = 0; i < 5; ++i) {
    assertEq(f(), true);
}

With --ion-eager: got false, expected true

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/ce43d28276e4
user:        Brian Hackett
date:        Fri Jun 14 05:58:28 2013 -0600
summary:     Bug 678037 - Enable lazy JS parsing and fix various bugs, r=waldo,evilpie,nobody.
Looks like the problem is not related to the function but to the loop.

If you call just once assertEq(f(), true), it will work.
If you call it several times in a row by hand, it should work too.

for (var i = 0; i < 1; ++i) { assertEq(f(), true); } // works

for (var i = 0; i < 2; ++i) { assertEq(f(), true); } // asserts
(Reporter)

Comment 2

4 years ago
You don't need a loop to trigger the bug.  Two calls do it too:

function f()
{
    return (4 >>> 0) > ((0 % (1 == 2)) >>> 0);
}
assertEq(f(), true);
assertEq(f(), true);
(Assignee)

Updated

4 years ago
Assignee: general → hv1989
(Assignee)

Updated

4 years ago
Blocks: 871002
status-firefox24: --- → affected
status-firefox25: --- → affected
Hardware: x86_64 → All
(Assignee)

Comment 3

4 years ago
Created attachment 773517 [details] [diff] [review]
Patch

@Jesse, sorry about the delay. Totally forgot you said fixing this bug would help you fuzzing.

EliminateDeadResumePointOperands removes the first "4 >>> 0", since it only has uses in resumepoints. The actual use in the "<" has been removed by the UINT32 'hack' in MCompare::infer and there we don't mark the instruction as folded !
Attachment #773517 - Flags: review?(bhackett1024)
Attachment #773517 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 4

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/52627f6818c4
Flags: in-testsuite+

Comment 5

4 years ago
https://hg.mozilla.org/mozilla-central/rev/52627f6818c4
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

4 years ago
status-firefox25: affected → fixed
You need to log in before you can comment on or make changes to this bug.