JM: Function returns -65535 instead of 1

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
9 years ago
4 years ago

People

(Reporter: duncan.loveday, Assigned: dvander)

Tracking

({regression, testcase})

Trunk
Points:
---

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: fixed-in-tracemonkey, [sg:critical?])

Attachments

(2 attachments, 1 obsolete attachment)

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101017 Firefox/4.0b8pre
Build Identifier: 

The function below should return 1, however I on the Fx 4 trunk I see it returning -65535. Strangely, commenting out the second line "var dummy=1 * n;" makes it work correctly even though the value of dummy is not used.

function f() {
    var n = Math.pow(10, 0);
    var dummy=1 * n;
    return n/1;
}


Reproducible: Always

Steps to Reproduce:
1. Load the test case, click the button
2.
3.
Actual Results:  
The alert I see reads "function f returned -65535" when it should read "function f returned 1"
Posted file test case (obsolete) —
Keywords: regression, testcase
Posted file Test case v2
Test case v2. In fact Math.pow() is not necessary because when I try to work around this bug using multiplication in a loop I get the same thing.
Attachment #483838 - Attachment is obsolete: true
Summary: Function returns -65535 instead of 1 when using Math.pow in an expression → Function returns -65535 instead of 1
This only happens when method JIT is on
Summary: Function returns -65535 instead of 1 → JM: Function returns -65535 instead of 1
Confirmed on Linux using latest Minefield and latest Tracemonkey branch.

Removal or commenting out of the dummy line does produce the correct result.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
OS: Windows XP → All
Version: unspecified → Trunk
This appears to be x86-specific; x64 correctly returns 1.
I can work around this bug by changing "return n/1;" to "var ret=n/1; return ret;"

In the example below, removing the loop also produces the correct result even though the loop is never entered at run time


function g() {
    var n=1; for (var m=0;m<0;m++) n*=10;
    var dummy=1 * n;
    return n/1;
}
blocking2.0: ? → beta8+
This is register allocation bug in FrameState::loadTo(), tempRegFor*() are called in succession without pinning, and the first register is clobbered. There's a trivial fix but it results in an extra store, I'd like to try and make that go away.
Assignee: general → dvander
Status: NEW → ASSIGNED
This should block b7. It's more than a correctness issue. It could cause random crashes anywhere in the method JIT, and in an evil way since it happens in a method that is just about to return.

If we're going to ship with this though, it should be security sensitive.
Group: core-security
blocking2.0: beta8+ → ?
Hardware: x86 → All
blocking2.0: ? → beta7+
dvander, eta here?
Patch in a few minutes.
Posted patch fixSplinter Review
Thanks for the great test case, Duncan. This patch removes register evictions from the return path. Instead, it first pins registers if they are available, and then figures out the optimal way to either load from memory or shuffle the right registers into place.
Attachment #485411 - Flags: review?(dmandelin)
Attachment #485411 - Flags: review?(dmandelin) → review+
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey, [sg:critical?]
http://hg.mozilla.org/mozilla-central/rev/e9b6e2adba29
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This is working fine for me now (using current trunk nightly) so marking as VERIFIED
Status: RESOLVED → VERIFIED
Group: core-security
You need to log in before you can comment on or make changes to this bug.