Changeset 90401 in webkit for trunk/Source/JavaScriptCore/runtime/Operations.h

Timestamp:

Jul 5, 2011, 12:01:41 PM (14 years ago)

Author:

[email protected]

Message:

​https://bugs.webkit.org/show_bug.cgi?id=63880
Evaluation order of conversions of operands to >, >= incorrect.

Reviewed by Sam Weinig.

Source/JavaScriptCore:

Add 'leftFirst' parameter to jsLess, jsLessEq matching that described in the ES5
spec. This allows these methods to be reused to perform >, >= relational compares
with correct ordering of type conversions.

• dfg/DFGOperations.cpp:
• interpreter/Interpreter.cpp:

(JSC::Interpreter::privateExecute):

• jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

• runtime/Operations.h:

(JSC::jsLess):
(JSC::jsLessEq):

LayoutTests:

Updated layout test results. Two of these tests now pass, however for
the third we now need to check in failing results, since the test is
incorrect!

The problem if that the test author has made the mistake of thinking
that the evaluation order for the operands to '>' is RHS then LHS.
This is due to a quirk in the way the spec is written. The greater
than opeator is defined to call the abstract relational comparison
algorithm with 'leftFirst' set to false, and as such conversion is
performed on the second operand ('y') first (see 11.8.5). However
the abstract relational comparison algorith is performing a less
than comaprison, and the greater than operator calls this algorithm
with the operands to the greater than operator reversed (see 11.8.2).
As such, the second operand to the abstract comaparison is the LHS
of the greater than. This bug also affects the corresponding less
than or equals test, where we already we have failing results checked
in, and again it is the test that is wrong (for the same reason).

• fast/js/exception-sequencing-binops2-expected.txt:
• sputnik/Conformance/11_Expressions/11.8_Relational_Operators/11.8.2_The_Greater_than_Operator/S11.8.2_A2.3_T1-expected.txt:
• sputnik/Conformance/11_Expressions/11.8_Relational_Operators/11.8.4_The_Grater_than_or_equal_Operator/S11.8.4_A2.3_T1-expected.txt:

File:

• trunk/Source/JavaScriptCore/runtime/Operations.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/runtime/Operations.h

@@ -335 +335 @@
     }
 
+    // See ES5 11.8.1/11.8.2/11.8.5 for definition of leftFirst, this value ensures correct
+    // evaluation ordering for argument conversions for '<' and '>'. For '<' pass the value
+    // true, for leftFirst, for '>' pass the value false (and reverse operand order).
+    template<bool leftFirst>
     ALWAYS_INLINE bool jsLess(CallFrame* callFrame, JSValue v1, JSValue v2)
     {
@@ -351 +355 @@
         JSValue p1;
         JSValue p2;
-        bool wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
-        bool wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+        bool wasNotString1;
+        bool wasNotString2;
+        if (leftFirst) {
+            wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
+            wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+        } else {
+            wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+            wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
+        }
 
         if (wasNotString1 | wasNotString2)
             return n1 < n2;
-
         return asString(p1)->value(callFrame) < asString(p2)->value(callFrame);
     }
 
-    inline bool jsLessEq(CallFrame* callFrame, JSValue v1, JSValue v2)
+    // See ES5 11.8.3/11.8.4/11.8.5 for definition of leftFirst, this value ensures correct
+    // evaluation ordering for argument conversions for '<=' and '=>'. For '<=' pass the
+    // value true, for leftFirst, for '=>' pass the value false (and reverse operand order).
+    template<bool leftFirst>
+    ALWAYS_INLINE bool jsLessEq(CallFrame* callFrame, JSValue v1, JSValue v2)
     {
         if (v1.isInt32() && v2.isInt32())
@@ -376 +390 @@
         JSValue p1;
         JSValue p2;
-        bool wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
-        bool wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+        bool wasNotString1;
+        bool wasNotString2;
+        if (leftFirst) {
+            wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
+            wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+        } else {
+            wasNotString2 = v2.getPrimitiveNumber(callFrame, n2, p2);
+            wasNotString1 = v1.getPrimitiveNumber(callFrame, n1, p1);
+        }
 
         if (wasNotString1 | wasNotString2)
             return n1 <= n2;
-
         return !(asString(p2)->value(callFrame) < asString(p1)->value(callFrame));
     }