Closed Bug 911369 Opened 11 years ago Closed 11 years ago

Differential Testing: Different output message involving valueOf

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: gkw, Assigned: jandem)

References

(Depends on 1 open bug)

Details

(Keywords: testcase)

Attachments

(1 file)

a1 = b1 = new ArrayBuffer();
for (x = 0; x < 9; ++x) {
    Array.prototype.unshift.call(a1, b1)
}
a1[0] = 7
Array.prototype.reverse.call(a1).valueOf = (function () {
    print("foo")
})
__proto__ = []
f0 = (function (a, a1) {
    var r = 6 - a1
});
Array, reduceRight.call(a1, f0, 2)

prints "foo" 8 times in a js opt 64-bit deterministic threadsafe shell on m-c changeset c7459bc8e449 without any CLI arguments but prints "foo" only 2 times with --ion-eager.

My configure flags are:

-target=x86_64-apple-darwin11.4.0 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-threadsafe --with-ccache <other NSPR flags>

This bug has probably existed for some time. Setting needinfo from jandem, as this is another bug that blocks differential testing.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Problem is that the "6 - a" expression is specialized as int32 based on baseline feedback. DCE then eliminates the ToInt32 and Sub instructions and we don't invoke the valueOf method.

The same issue came up a long time ago with the bitwise operators, fix then was to never specialize these if one operand may be an object. This patch does the same for the arithmetic operators.

I also noticed that we call HasOperationOverflowed and pass a "bool overflowed" around but no longer do anything with it, so the patch removes that as well.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #798487 - Flags: review?(bhackett1024)
Flags: needinfo?(jdemooij)
Severity: critical → major
Summary: Differential Testing: Different output message involving ArrayBuffer → Differential Testing: Different output message involving valueOf
Comment on attachment 798487 [details] [diff] [review]
Patch

Review of attachment 798487 [details] [diff] [review]:
-----------------------------------------------------------------

This looks OK, but it seems weird that we need to do it this way and I wonder if this patch is masking a deeper problem.  int32/double instructions require int32/double inputs, and it is the ToInt32 on the object where the side effect happens.  If DCE is eliminating that instruction then it is not being considered effectful and we could get correctness problems from the ToInt32 being consolidated or moved across other instructions.
Attachment #798487 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/e2977628a9d5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Depends on: 959655
No longer depends on: 959655
Depends on: 969765
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: