Closed Bug 465133 Opened 11 years ago Closed 11 years ago

TM: JIT affects object inequality

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: gal)

References

Details

(Keywords: testcase, verified1.9.1)

Attachments

(1 file, 1 obsolete file)

js> for(i=0;i<5;++i) print("" + ({} < {}));
false
false
true
true
true
No imacros here, this is a straight-up bug in TraceRecorder::cmp.

/be
Assignee: general → gal
Attached patch patch (obsolete) — Splinter Review
Attachment #348398 - Flags: review?(brendan)
Comment on attachment 348398 [details] [diff] [review]
patch

Breaks this kind of test:

js> var o={}, a=[]
js> for(var i=0;i<5;i++) a[i]=(o<=o);
true
js> uneval(a)
[true, true, true, true, true]

/be
Attachment #348398 - Flags: review?(brendan) → review-
Attachment #348398 - Attachment is obsolete: true
Attachment #348405 - Flags: review?(brendan)
Comment on attachment 348405 [details] [diff] [review]
v2, with imacro magic


>@@ -4482,18 +4528,17 @@ TraceRecorder::cmp(LOpcode op, int flags
>         // the right thing.
>         cond = evalCmp(op, l, r);
>         // For ==, !=, ===, and !=== the result is magically correct even if undefined (2) is
>         // involved. For the relational operations we need some additional cmov magic to make
>         // the result always false (since undefined becomes NaN per ECMA and that doesn't
>         // compare to anything, even itself). The code for this is emitted a few lines down.
>     } else if (JSVAL_IS_OBJECT(l) && JSVAL_IS_OBJECT(r)) {
>         if (op != LIR_feq) {
>-            negate = !(op == LIR_fle || op == LIR_fge);
>-            op = LIR_feq;
>+            JS_NOT_REACHED("we should have converted to numbers already");
>         }

Single-line then doesn't need braces -- but we really should return false after JS_NOT_REACHED, since it compiles to nothing on NDEBUG builds. r=me with that fix.

/be
Attachment #348405 - Flags: review?(brendan) → review+
Flags: blocking1.9.1?
Fixed on TM.

http://hg.mozilla.org/tracemonkey/rev/0cda5f044adb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 465236
Blocks: arithfuzz
test already in js1_8_1/trace/trace-test.js
Flags: in-testsuite+
Flags: in-litmus-
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.