Changeset 283332 in webkit

Timestamp:

Sep 30, 2021, 1:37:55 PM (5 years ago)

Author:

sbarati@apple.com

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/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy-2.js (added)
• JSTests/stress/ai-typeof-needs-to-be-aware-of-proxy.js (added)
• JSTests/stress/is-callable-in-ftl-needs-to-be-aware-of-proxy.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecode/SpeculatedType.h (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (modified) (3 diffs)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-09-30  Saam Barati  <sbarati@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.lam@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-09-30  Saam Barati  <sbarati@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.Fujii@sony.com>
 

trunk/Source/JavaScriptCore/bytecode/SpeculatedType.h

@@ -117 +117 @@
 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

trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -1620 +1620 @@
             // 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;
@@ -1733 +1733 @@
             }
             
-            if (!(child.m_type & (SpecFunction | SpecObjectOther | SpecProxyObject))) {
+            if (!(child.m_type & SpecTypeofMightBeFunction)) {
                 setConstant(node, jsBoolean(false));
                 constantWasSet = true;
@@ -1812 +1812 @@
         // 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;

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -19542 +19542 @@
     LValue isExoticForTypeof(LValue cell, SpeculatedType type = SpecFullTop)
     {
-        if (!(type & SpecObjectOther))
+        if (!(type & (SpecObjectOther | SpecProxyObject)))
             return m_out.booleanFalse;
         return m_out.testNonZero32(