Title: [264781] trunk
Revision
264781
Author
ysuz...@apple.com
Date
2020-07-23 11:51:49 -0700 (Thu, 23 Jul 2020)

Log Message

[JSC] BigInt can be `false` in boolean context in DFG AI
https://bugs.webkit.org/show_bug.cgi?id=214678
<rdar://problem/65894751>

Reviewed by Mark Lam.

JSTests:

* stress/bigint-can-be-false-in-boolean-context.js: Added.
(foo.bar):
(foo):

Source/_javascript_Core:

DFG::AbstractInterpreter::booleanResult returns wrong result if finite structure includes HeapBigInt structure
since HeapBigInt 0n can be evaluated `false` in boolean context while this function does not care it. This patch
fixes it and cleans up code by using WTF/TriState.

* dfg/DFGAbstractInterpreter.h:
(): Deleted.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::booleanResult):
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (264780 => 264781)


--- trunk/JSTests/ChangeLog	2020-07-23 18:45:58 UTC (rev 264780)
+++ trunk/JSTests/ChangeLog	2020-07-23 18:51:49 UTC (rev 264781)
@@ -1,3 +1,15 @@
+2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] BigInt can be `false` in boolean context in DFG AI
+        https://bugs.webkit.org/show_bug.cgi?id=214678
+        <rdar://problem/65894751>
+
+        Reviewed by Mark Lam.
+
+        * stress/bigint-can-be-false-in-boolean-context.js: Added.
+        (foo.bar):
+        (foo):
+
 2020-07-23  Angelos Oikonomopoulos  <ange...@igalia.com>
 
         Fix logic error in r264703

Added: trunk/JSTests/stress/bigint-can-be-false-in-boolean-context.js (0 => 264781)


--- trunk/JSTests/stress/bigint-can-be-false-in-boolean-context.js	                        (rev 0)
+++ trunk/JSTests/stress/bigint-can-be-false-in-boolean-context.js	2020-07-23 18:51:49 UTC (rev 264781)
@@ -0,0 +1,14 @@
+function foo() {
+    let b0 = 0n.valueOf();
+    function bar() {
+        let b1 = b0;
+        if (!b0) {
+            b1[Symbol.for('a')];
+        }
+    }
+    bar();
+}
+
+for (let i=0; i<10000; i++) {
+    foo();
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (264780 => 264781)


--- trunk/Source/_javascript_Core/ChangeLog	2020-07-23 18:45:58 UTC (rev 264780)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-07-23 18:51:49 UTC (rev 264781)
@@ -1,3 +1,21 @@
+2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] BigInt can be `false` in boolean context in DFG AI
+        https://bugs.webkit.org/show_bug.cgi?id=214678
+        <rdar://problem/65894751>
+
+        Reviewed by Mark Lam.
+
+        DFG::AbstractInterpreter::booleanResult returns wrong result if finite structure includes HeapBigInt structure
+        since HeapBigInt 0n can be evaluated `false` in boolean context while this function does not care it. This patch
+        fixes it and cleans up code by using WTF/TriState.
+
+        * dfg/DFGAbstractInterpreter.h:
+        (): Deleted.
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::booleanResult):
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+
 2020-07-23  Caio Lima  <ticaiol...@gmail.com>
 
         [32-bits] Fixing the return of doVMEntry into LowLevelInterpreter32_64.asm

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h (264780 => 264781)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h	2020-07-23 18:45:58 UTC (rev 264780)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreter.h	2020-07-23 18:51:49 UTC (rev 264781)
@@ -32,6 +32,7 @@
 #include "DFGNode.h"
 #include "DFGNodeFlowProjection.h"
 #include "DFGPhiChildren.h"
+#include <wtf/TriState.h>
 
 namespace JSC { namespace DFG {
 
@@ -232,12 +233,7 @@
     void observeTransitions(unsigned indexInBlock, const TransitionVector&);
 private:
     
-    enum BooleanResult {
-        UnknownBooleanResult,
-        DefinitelyFalse,
-        DefinitelyTrue
-    };
-    BooleanResult booleanResult(Node*, AbstractValue&);
+    TriState booleanResult(Node*, AbstractValue&);
     
     void setBuiltInConstant(Node* node, FrozenValue value)
     {

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (264780 => 264781)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2020-07-23 18:45:58 UTC (rev 264780)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2020-07-23 18:51:49 UTC (rev 264781)
@@ -72,15 +72,13 @@
 }
 
 template<typename AbstractStateType>
-typename AbstractInterpreter<AbstractStateType>::BooleanResult
-AbstractInterpreter<AbstractStateType>::booleanResult(
-    Node* node, AbstractValue& value)
+TriState AbstractInterpreter<AbstractStateType>::booleanResult(Node* node, AbstractValue& value)
 {
     JSValue childConst = value.value();
     if (childConst) {
         if (childConst.toBoolean(m_codeBlock->globalObjectFor(node->origin.semantic)))
-            return DefinitelyTrue;
-        return DefinitelyFalse;
+            return TriState::True;
+        return TriState::False;
     }
 
     // Next check if we can fold because we know that the source is an object or string and does not equal undefined.
@@ -88,17 +86,16 @@
         bool allTrue = true;
         for (unsigned i = value.m_structure.size(); i--;) {
             RegisteredStructure structure = value.m_structure[i];
-            if (structure->masqueradesAsUndefined(m_codeBlock->globalObjectFor(node->origin.semantic))
-                || structure->typeInfo().type() == StringType) {
+            if (structure->masqueradesAsUndefined(m_codeBlock->globalObjectFor(node->origin.semantic)) || structure->typeInfo().type() == StringType || structure->typeInfo().type() == HeapBigIntType) {
                 allTrue = false;
                 break;
             }
         }
         if (allTrue)
-            return DefinitelyTrue;
+            return TriState::True;
     }
     
-    return UnknownBooleanResult;
+    return TriState::Indeterminate;
 }
 
 template<typename AbstractStateType>
@@ -1350,13 +1347,13 @@
             
     case LogicalNot: {
         switch (booleanResult(node, forNode(node->child1()))) {
-        case DefinitelyTrue:
+        case TriState::True:
             setConstant(node, jsBoolean(false));
             break;
-        case DefinitelyFalse:
+        case TriState::False:
             setConstant(node, jsBoolean(true));
             break;
-        default:
+        case TriState::Indeterminate:
             setNonCellTypeForNode(node, SpecBoolean);
             break;
         }
@@ -2604,20 +2601,21 @@
             
     case Branch: {
         Node* child = node->child1().node();
-        BooleanResult result = booleanResult(node, forNode(child));
-        if (result == DefinitelyTrue) {
+        switch (booleanResult(node, forNode(child))) {
+        case TriState::True:
             m_state.setBranchDirection(TakeTrue);
             break;
-        }
-        if (result == DefinitelyFalse) {
+        case TriState::False:
             m_state.setBranchDirection(TakeFalse);
             break;
+        case TriState::Indeterminate:
+            // FIXME: The above handles the trivial cases of sparse conditional
+            // constant propagation, but we can do better:
+            // We can specialize the source variable's value on each direction of
+            // the branch.
+            m_state.setBranchDirection(TakeBoth);
+            break;
         }
-        // FIXME: The above handles the trivial cases of sparse conditional
-        // constant propagation, but we can do better:
-        // We can specialize the source variable's value on each direction of
-        // the branch.
-        m_state.setBranchDirection(TakeBoth);
         break;
     }
         
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to