Title: [283332] trunk
Revision
283332
Author
sbar...@apple.com
Date
2021-09-30 13:37:55 -0700 (Thu, 30 Sep 2021)

Log Message

The DFG/FTL need to be aware that Proxy's can produce "function" for typeof and might be callable
https://bugs.webkit.org/show_bug.cgi?id=230804
<rdar://problem/83543951>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/ai-typeof-needs-to-be-aware-of-proxy-2.js: Added.
(assert):
(builtin.vm.createBuiltin):
(builtin2.vm.createBuiltin):
(let.p.new.Proxy):
* stress/ai-typeof-needs-to-be-aware-of-proxy.js: Added.
(assert):
(builtin.vm.createBuiltin):
(let.p.new.Proxy):
* stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js: Added.
(main):

Source/_javascript_Core:

This patch fixes a couple bugs:
- We were constant folding typeof on ProxyObject to "object"
  even when ProxyObject might produce a callable Proxy, and hence,
  should produce "function". This was a bug in AI.
- This also fixes a similar bug in IsCallable's implementation in
  the FTL where we assumed that ProxyObject's type can't be callable.

* bytecode/SpeculatedType.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (283331 => 283332)


--- trunk/JSTests/ChangeLog	2021-09-30 20:21:45 UTC (rev 283331)
+++ trunk/JSTests/ChangeLog	2021-09-30 20:37:55 UTC (rev 283332)
@@ -1,3 +1,23 @@
+2021-09-30  Saam Barati  <sbar...@apple.com>
+
+        The DFG/FTL need to be aware that Proxy's can produce "function" for typeof and might be callable
+        https://bugs.webkit.org/show_bug.cgi?id=230804
+        <rdar://problem/83543951>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/ai-typeof-needs-to-be-aware-of-proxy-2.js: Added.
+        (assert):
+        (builtin.vm.createBuiltin):
+        (builtin2.vm.createBuiltin):
+        (let.p.new.Proxy):
+        * stress/ai-typeof-needs-to-be-aware-of-proxy.js: Added.
+        (assert):
+        (builtin.vm.createBuiltin):
+        (let.p.new.Proxy):
+        * stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js: Added.
+        (main):
+
 2021-09-29  Mark Lam  <mark....@apple.com>
 
         DFG strength reduction on % operator should handle an INT_MIN divisor.

Added: trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy-2.js (0 => 283332)


--- trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy-2.js	                        (rev 0)
+++ trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy-2.js	2021-09-30 20:37:55 UTC (rev 283332)
@@ -0,0 +1,29 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+var builtin = $vm.createBuiltin(`(function (a) {
+    if (@isProxyObject(a)) {
+        if (typeof a === "object")
+            return false;
+    }
+    return true;
+})`);
+
+noInline(builtin);
+
+var builtin2 = $vm.createBuiltin(`(function (a) {
+    if (@isProxyObject(a)) {
+        if (typeof a === "function")
+            return true;
+    }
+    return false;
+})`);
+noInline(builtin2);
+
+let p = new Proxy(function(){}, {});
+for (let i = 0; i < 10000; ++i) {
+    assert(builtin(p) === true);
+    assert(builtin2(p) === true);
+}

Added: trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy.js (0 => 283332)


--- trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy.js	                        (rev 0)
+++ trunk/JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy.js	2021-09-30 20:37:55 UTC (rev 283332)
@@ -0,0 +1,15 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+var builtin = $vm.createBuiltin(`(function (a) {
+    if (@isProxyObject(a))
+        return typeof a;
+})`);
+
+noInline(builtin);
+
+let p = new Proxy(function(){}, {});
+for (let i = 0; i < 10000; ++i)
+    assert(builtin(p) === "function");

Added: trunk/JSTests/stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js (0 => 283332)


--- trunk/JSTests/stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js	                        (rev 0)
+++ trunk/JSTests/stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js	2021-09-30 20:37:55 UTC (rev 283332)
@@ -0,0 +1,16 @@
+//@ runDefault("--validateOptions=true --useConcurrentJIT=false", "--useConcurrentGC=false", "--thresholdForJITSoon=10", "--thresholdForJITAfterWarmUp=10", "--thresholdForOptimizeAfterWarmUp=100", "--thresholdForOptimizeAfterLongWarmUp=100", "--thresholdForOptimizeSoon=100", "--thresholdForFTLOptimizeAfterWarmUp=1000", "--thresholdForFTLOptimizeSoon=1000", "--validateBCE=true", "--useFTLJIT=true")
+
+function main() {
+    let v162;
+    const v25 = {__proto__:"name"};
+    
+    for (let v113 = 0; v113 < 255; v113++) {
+        const v141 = new Proxy(Object,v25);
+        const v145 = v141["bind"]();
+        // when running with FTL, the previous line raises a JS exception:
+        // TypeError: |this| is not a function inside Function.prototype.bind
+        // without FTL or in v8 this doesn't throw.
+
+    }   
+}
+main();

Modified: trunk/Source/_javascript_Core/ChangeLog (283331 => 283332)


--- trunk/Source/_javascript_Core/ChangeLog	2021-09-30 20:21:45 UTC (rev 283331)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-09-30 20:37:55 UTC (rev 283332)
@@ -1,3 +1,24 @@
+2021-09-30  Saam Barati  <sbar...@apple.com>
+
+        The DFG/FTL need to be aware that Proxy's can produce "function" for typeof and might be callable
+        https://bugs.webkit.org/show_bug.cgi?id=230804
+        <rdar://problem/83543951>
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch fixes a couple bugs:
+        - We were constant folding typeof on ProxyObject to "object"
+          even when ProxyObject might produce a callable Proxy, and hence,
+          should produce "function". This was a bug in AI.
+        - This also fixes a similar bug in IsCallable's implementation in
+          the FTL where we assumed that ProxyObject's type can't be callable.
+
+        * bytecode/SpeculatedType.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+
 2021-09-30  Fujii Hironori  <hironori.fu...@sony.com>
 
         Python 3 fails to run run-builtins-generator-tests : ModuleNotFoundError: No module named 'builtins_model'

Modified: trunk/Source/_javascript_Core/bytecode/SpeculatedType.h (283331 => 283332)


--- trunk/Source/_javascript_Core/bytecode/SpeculatedType.h	2021-09-30 20:21:45 UTC (rev 283331)
+++ trunk/Source/_javascript_Core/bytecode/SpeculatedType.h	2021-09-30 20:37:55 UTC (rev 283332)
@@ -116,6 +116,8 @@
 static constexpr SpeculatedType SpecBytecodeTop                       = SpecHeapTop | SpecEmpty; // It can be any of the above, except for SpecInt52Only and SpecDoubleImpureNaN. Corresponds to what could be found in a bytecode local.
 static constexpr SpeculatedType SpecFullTop                           = SpecBytecodeTop | SpecFullNumber; // It can be anything that bytecode could see plus exotic encodings of numbers.
 
+static constexpr SpeculatedType SpecTypeofMightBeFunction             = SpecFunction | SpecObjectOther | SpecProxyObject; // If you don't see these types, you can't be callable, and you can't have typeof produce "function". Inverse is not true, however. If you only see these types, you may produce more things than "function" in typeof, and you might not be callable.
+
 // SpecCellCheck is the type set representing the values that can flow through a cell check.
 // On 64-bit platforms, the empty value passes a cell check. Also, ~SpecCellCheck is the type
 // set that representing the values that flow through when testing that something is not a cell.

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (283331 => 283332)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2021-09-30 20:21:45 UTC (rev 283331)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2021-09-30 20:37:55 UTC (rev 283332)
@@ -1619,7 +1619,7 @@
             // Is the child's type an object that isn't an other-object (i.e. object that could
             // have masquaredes-as-undefined traps) and isn't a function? Then: we should fold
             // this to true.
-            if (!(child.m_type & ~(SpecObject - SpecObjectOther - SpecFunction))) {
+            if (!(child.m_type & ~(SpecObject - SpecTypeofMightBeFunction))) {
                 setConstant(node, jsBoolean(true));
                 constantWasSet = true;
                 break;
@@ -1732,7 +1732,7 @@
                 break;
             }
             
-            if (!(child.m_type & (SpecFunction | SpecObjectOther | SpecProxyObject))) {
+            if (!(child.m_type & SpecTypeofMightBeFunction)) {
                 setConstant(node, jsBoolean(false));
                 constantWasSet = true;
                 break;
@@ -1811,7 +1811,7 @@
 
         // FIXME: We could use the masquerades-as-undefined watchpoint here.
         // https://bugs.webkit.org/show_bug.cgi?id=144456
-        if (!(abstractChild.m_type & ~(SpecObject - SpecObjectOther - SpecFunction))) {
+        if (!(abstractChild.m_type & ~(SpecObject - SpecTypeofMightBeFunction))) {
             setConstant(node, *m_graph.freeze(m_vm.smallStrings.objectString()));
             break;
         }

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (283331 => 283332)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-09-30 20:21:45 UTC (rev 283331)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-09-30 20:37:55 UTC (rev 283332)
@@ -19541,7 +19541,7 @@
             
     LValue isExoticForTypeof(LValue cell, SpeculatedType type = SpecFullTop)
     {
-        if (!(type & SpecObjectOther))
+        if (!(type & (SpecObjectOther | SpecProxyObject)))
             return m_out.booleanFalse;
         return m_out.testNonZero32(
             m_out.load8ZeroExt32(cell, m_heaps.JSCell_typeInfoFlags),
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to