JSTests:
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Aug 2017 01:10:01 +0000 (01:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Aug 2017 01:10:01 +0000 (01:10 +0000)
Support the 'with' keyword in FTL.
https://bugs.webkit.org/show_bug.cgi?id=175585

Patch by Robin Morisset <rmorisset@apple.com> on 2017-08-15
Reviewed by Saam Barati.

Also improve the JSTest/stress/with.js file to test
what happens when non-objects are passed to with.

* stress/with.js:
(foo):
(i.catch):
(i.with): Deleted.

Source/JavaScriptCore:
Support the 'with' keyword in FTL
https://bugs.webkit.org/show_bug.cgi?id=175585

Patch by Robin Morisset <rmorisset@apple.com> on 2017-08-15
Reviewed by Saam Barati.

Also makes sure that the order of arguments of PushWithScope, op_push_with_scope, JSWithScope::create()
and so on is consistent (always parentScope first, the new scopeObject second). We used to go from one
to the other at different step which was quite confusing. I picked this order for consistency with CreateActivation
that takes its parentScope argument first.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitPushWithScope):
* debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::evaluateWithScopeExtension):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compilePushWithScope):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compilePushWithScope):
* jit/JITOperations.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/Completion.cpp:
(JSC::evaluateWithScopeExtension):
* runtime/JSWithScope.cpp:
(JSC::JSWithScope::create):
* runtime/JSWithScope.h:

Source/WebCore:
Change the order of arguments of JSWithScope::create() for consistency
https://bugs.webkit.org/show_bug.cgi?id=175585

Patch by Robin Morisset <rmorisset@apple.com> on 2017-08-15
Reviewed by Saam Barati.

No change of behavior.

* bindings/js/JSHTMLElementCustom.cpp:
(WebCore::JSHTMLElement::pushEventHandlerScope const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220778 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
JSTests/ChangeLog
JSTests/stress/with.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/JITOperations.h
Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Source/JavaScriptCore/runtime/Completion.cpp
Source/JavaScriptCore/runtime/JSWithScope.cpp
Source/JavaScriptCore/runtime/JSWithScope.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSHTMLElementCustom.cpp

index 3cf3738..652834a 100644 (file)
@@ -1,3 +1,18 @@
+2017-08-15  Robin Morisset  <rmorisset@apple.com>
+
+        Support the 'with' keyword in FTL.
+        https://bugs.webkit.org/show_bug.cgi?id=175585
+
+        Reviewed by Saam Barati.
+
+        Also improve the JSTest/stress/with.js file to test
+        what happens when non-objects are passed to with.
+
+        * stress/with.js:
+        (foo):
+        (i.catch):
+        (i.with): Deleted.
+
 2017-08-14  Keith Miller  <keith_miller@apple.com>
 
         Add testing tool to lie to the DFG about profiles
index ad8c293..b9ad0c2 100644 (file)
@@ -1,36 +1,24 @@
-for (var i = 0; i < 10000; ++i) {
-    var x = 1;
-    var y = 2;
-
-    var z = {a: 42};
-    with (z) {
-        x = y;
-    }
-    if (x !== 2 || y !== 2 || z.a !== 42) {
-        throw "Error: bad result, first case, for i = " + i;
-    }
-
-    z = {y: 42}
-    with (z) {
-        x = y;
-    }
-    if (x !== 42 || y !== 2 || z.y !== 42) {
-        throw "Error: bad result, second case, for i = " + i;
-    }
-
-    z = {x: 13};
-    with (z) {
+function foo (x, y, z, newX, checkZ, errorMessage) {
+    with(z) {
         x = y;
     }
-    if (x !== 42 || y !== 2 || z.x !== 2) {
-        throw "Error: bad result, third case, for i = " + i;
+    if (x !== newX || !checkZ(z)) {
+        throw errorMessage;
     }
+}
 
-    z = {x:13, y:14};
-    with (z) {
-        x = y;
-    }
-    if (x !== 42 || y !== 2 || z.x !== 14 || z.y !== 14) {
-        throw "Error: bad result, fourth case, for i = " + i;
+for (var i = 0; i < 10000; ++i) {
+    foo(1, 2, {a:42}, 2, z => z.a === 42, "Error: bad result for non-overlapping case, i = " + i);
+    foo(1, 2, {x:42}, 1, z => z.x === 2, "Error: bad result for setter case, i = " + i);
+    foo(1, 2, {y:42}, 42, z => z.y === 42, "Error: bad result for getter case, i = " + i);
+    foo(1, 2, {x:42, y:13}, 1, z => z.x === 13 && z.y === 13, "Error: bad result for setter/getter case, i = " + i);
+    foo(1, 2, "toto", 2, z => z === "toto", "Error: bad result for string case, i = " + i);
+    try {
+        foo(1, 2, null, 2, z =>
+                {throw "Error: missing type error, i = " + i}, "Unreachable");
+    } catch (e) {
+        if (!(e instanceof TypeError)) {
+            throw e;
+        }
     }
 }
index 667d1c1..d12b1fe 100644 (file)
@@ -1,3 +1,39 @@
+2017-08-15  Robin Morisset  <rmorisset@apple.com>
+
+        Support the 'with' keyword in FTL
+        https://bugs.webkit.org/show_bug.cgi?id=175585
+
+        Reviewed by Saam Barati.
+
+        Also makes sure that the order of arguments of PushWithScope, op_push_with_scope, JSWithScope::create()
+        and so on is consistent (always parentScope first, the new scopeObject second). We used to go from one
+        to the other at different step which was quite confusing. I picked this order for consistency with CreateActivation
+        that takes its parentScope argument first.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitPushWithScope):
+        * debugger/DebuggerCallFrame.cpp:
+        (JSC::DebuggerCallFrame::evaluateWithScopeExtension):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compilePushWithScope):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compilePushWithScope):
+        * jit/JITOperations.cpp:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/Completion.cpp:
+        (JSC::evaluateWithScopeExtension):
+        * runtime/JSWithScope.cpp:
+        (JSC::JSWithScope::create):
+        * runtime/JSWithScope.h:
+
 2017-08-15  Saam Barati  <sbarati@apple.com>
 
         Make VM::scratchBufferForSize thread safe
index cc65dd5..e1cc3ee 100644 (file)
@@ -3766,8 +3766,8 @@ RegisterID* BytecodeGenerator::emitPushWithScope(RegisterID* objectScope)
 
     emitOpcode(op_push_with_scope);
     instructions().append(newScope->index());
-    instructions().append(objectScope->index());
     instructions().append(scopeRegister()->index());
+    instructions().append(objectScope->index());
 
     emitMove(scopeRegister(), newScope);
     m_lexicalScopeStack.append({ nullptr, newScope, true, 0 });
index a3ecc60..5653296 100644 (file)
@@ -255,7 +255,7 @@ JSValue DebuggerCallFrame::evaluateWithScopeExtension(const String& script, JSOb
     JSGlobalObject* globalObject = callFrame->vmEntryGlobalObject();
     if (scopeExtensionObject) {
         JSScope* ignoredPreviousScope = globalObject->globalScope();
-        globalObject->setGlobalScopeExtension(JSWithScope::create(vm, globalObject, scopeExtensionObject, ignoredPreviousScope));
+        globalObject->setGlobalScopeExtension(JSWithScope::create(vm, globalObject, ignoredPreviousScope, scopeExtensionObject));
     }
 
     JSValue thisValue = this->thisValue();
index fd0641a..5de7f3a 100644 (file)
@@ -5651,9 +5651,9 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         }
 
         case op_push_with_scope: {
-            Node* scopeObject = get(VirtualRegister(currentInstruction[2].u.operand));
-            Node* currentScope = get(VirtualRegister(currentInstruction[3].u.operand));
-            set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(PushWithScope, scopeObject, currentScope));
+            Node* currentScope = get(VirtualRegister(currentInstruction[2].u.operand));
+            Node* object = get(VirtualRegister(currentInstruction[3].u.operand));
+            set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(PushWithScope, currentScope, object));
             NEXT_OPCODE(op_push_with_scope);
         }
 
index ff89d0a..ae6e161 100644 (file)
@@ -1710,6 +1710,7 @@ private:
 
         case CreateScopedArguments:
         case CreateActivation:
+        case PushWithScope:
         case NewFunction:
         case NewGeneratorFunction:
         case NewAsyncFunction: {
@@ -1717,12 +1718,6 @@ private:
             break;
         }
 
-        case PushWithScope: {
-            // Child2 is always the current scope, which is guaranteed to be an object.
-            fixEdge<KnownCellUse>(node->child2());
-            break;
-        }
-
         case SetFunctionName: {
             // The first child is guaranteed to be a cell because op_set_function_name is only used
             // on a newly instantiated function object (the first child).
index 0548dd4..10a4153 100644 (file)
@@ -1126,16 +1126,17 @@ void SpeculativeJIT::compileDeleteByVal(Node* node)
 
 void SpeculativeJIT::compilePushWithScope(Node* node)
 {
-    JSValueOperand scopeObject(this, node->child1());
-    SpeculateCellOperand currentScope(this, node->child2());
-    JSValueRegs scopeObjectRegs = scopeObject.jsValueRegs();
+    SpeculateCellOperand currentScope(this, node->child1());
     GPRReg currentScopeGPR = currentScope.gpr();
 
+    JSValueOperand object(this, node->child2());
+    JSValueRegs objectRegs = object.jsValueRegs();
+
     GPRFlushedCallResult result(this);
     GPRReg resultGPR = result.gpr();
     
     flushRegisters();
-    callOperation(operationPushWithScope, resultGPR, currentScopeGPR, scopeObjectRegs);
+    callOperation(operationPushWithScope, resultGPR, currentScopeGPR, objectRegs);
     m_jit.exceptionCheck();
     
     cellResult(resultGPR, node);
index 0b45794..4066998 100644 (file)
@@ -115,6 +115,7 @@ inline CapabilityLevel canCompile(Node* node)
     case SkipScope:
     case GetGlobalObject:
     case CreateActivation:
+    case PushWithScope:
     case NewFunction:
     case NewGeneratorFunction:
     case NewAsyncFunction:
index 5d52118..966ed3a 100644 (file)
@@ -739,6 +739,9 @@ private:
         case CreateActivation:
             compileCreateActivation();
             break;
+        case PushWithScope:
+            compilePushWithScope();
+            break;
         case NewFunction:
         case NewGeneratorFunction:
         case NewAsyncFunction:
@@ -4262,7 +4265,17 @@ private:
             return;
         }
     }
-    
+
+    void compilePushWithScope()
+    {
+        LValue parentScope = lowCell(m_node->child1());
+        LValue object = lowJSValue(m_node->child2());
+
+        LValue result = vmCall(Int64, m_out.operation(operationPushWithScope), m_callFrame, parentScope, object);
+
+        setJSValue(result);
+    }
+
     void compileCreateActivation()
     {
         LValue scope = lowCell(m_node->child1());
index 6beed56..e864baf 100644 (file)
@@ -1996,18 +1996,18 @@ size_t JIT_OPERATION operationDeleteByVal(ExecState* exec, EncodedJSValue encode
     return couldDelete;
 }
 
-JSCell* JIT_OPERATION operationPushWithScope(ExecState* exec, JSCell* currentScopeCell, EncodedJSValue scopeObjectValue)
+JSCell* JIT_OPERATION operationPushWithScope(ExecState* exec, JSCell* currentScopeCell, EncodedJSValue objectValue)
 {
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* newScope = JSValue::decode(scopeObjectValue).toObject(exec);
+    JSObject* object = JSValue::decode(objectValue).toObject(exec);
     RETURN_IF_EXCEPTION(scope, nullptr);
 
     JSScope* currentScope = jsCast<JSScope*>(currentScopeCell);
 
-    return JSWithScope::create(vm, exec->lexicalGlobalObject(), newScope, currentScope);
+    return JSWithScope::create(vm, exec->lexicalGlobalObject(), currentScope, object);
 }
 
 EncodedJSValue JIT_OPERATION operationInstanceOf(ExecState* exec, EncodedJSValue encodedValue, EncodedJSValue encodedProto)
index 2bc4bfe..e28a895 100644 (file)
@@ -419,7 +419,7 @@ EncodedJSValue JIT_OPERATION operationDeleteByIdJSResult(ExecState*, EncodedJSVa
 size_t JIT_OPERATION operationDeleteById(ExecState*, EncodedJSValue base, UniquedStringImpl*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationDeleteByValJSResult(ExecState*, EncodedJSValue base, EncodedJSValue target) WTF_INTERNAL;
 size_t JIT_OPERATION operationDeleteByVal(ExecState*, EncodedJSValue base, EncodedJSValue target) WTF_INTERNAL;
-JSCell* JIT_OPERATION operationPushWithScope(ExecState*, JSCell* currentScopeCell, EncodedJSValue scopeObject) WTF_INTERNAL;
+JSCell* JIT_OPERATION operationPushWithScope(ExecState*, JSCell* currentScopeCell, EncodedJSValue object) WTF_INTERNAL;
 JSCell* JIT_OPERATION operationGetPNames(ExecState*, JSObject*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationInstanceOf(ExecState*, EncodedJSValue, EncodedJSValue proto) WTF_INTERNAL;
 int32_t JIT_OPERATION operationSizeFrameForForwardArguments(ExecState*, EncodedJSValue arguments, int32_t numUsedStackSlots, int32_t firstVarArgOffset) WTF_INTERNAL;
index 1761fc7..ffcae0c 100644 (file)
@@ -814,12 +814,12 @@ SLOW_PATH_DECL(slow_path_create_lexical_environment)
 SLOW_PATH_DECL(slow_path_push_with_scope)
 {
     BEGIN();
-    JSObject* newScope = OP_C(2).jsValue().toObject(exec);
+    JSObject* newScope = OP_C(3).jsValue().toObject(exec);
     CHECK_EXCEPTION();
 
-    int scopeReg = pc[3].u.operand;
+    int scopeReg = pc[2].u.operand;
     JSScope* currentScope = exec->uncheckedR(scopeReg).Register::scope();
-    RETURN(JSWithScope::create(vm, exec->lexicalGlobalObject(), newScope, currentScope));
+    RETURN(JSWithScope::create(vm, exec->lexicalGlobalObject(), currentScope, newScope));
 }
 
 SLOW_PATH_DECL(slow_path_resolve_scope_for_hoisting_func_decl_in_eval)
index 8823e6f..7dc8935 100644 (file)
@@ -123,7 +123,7 @@ JSValue evaluateWithScopeExtension(ExecState* exec, const SourceCode& source, JS
 
     if (scopeExtensionObject) {
         JSScope* ignoredPreviousScope = globalObject->globalScope();
-        globalObject->setGlobalScopeExtension(JSWithScope::create(exec->vm(), globalObject, scopeExtensionObject, ignoredPreviousScope));
+        globalObject->setGlobalScopeExtension(JSWithScope::create(exec->vm(), globalObject, ignoredPreviousScope, scopeExtensionObject));
     }
 
     JSValue returnValue = JSC::evaluate(globalObject->globalExec(), source, globalObject, returnedException);
index e83ed2a..9d4ad2c 100644 (file)
@@ -33,7 +33,7 @@ namespace JSC {
 const ClassInfo JSWithScope::s_info = { "WithScope", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSWithScope) };
 
 JSWithScope* JSWithScope::create(
-    VM& vm, JSGlobalObject* globalObject, JSObject* object, JSScope* next)
+    VM& vm, JSGlobalObject* globalObject, JSScope* next, JSObject* object)
 {
     Structure* structure = globalObject->withScopeStructure();
     JSWithScope* withScope = new (NotNull, allocateCell<JSWithScope>(vm.heap)) JSWithScope(vm, structure, object, next);
index 39e09e3..2847bd8 100644 (file)
@@ -33,7 +33,7 @@ class JSWithScope : public JSScope {
 public:
     typedef JSScope Base;
 
-    JS_EXPORT_PRIVATE static JSWithScope* create(VM&, JSGlobalObject*, JSObject*, JSScope* next);
+    JS_EXPORT_PRIVATE static JSWithScope* create(VM&, JSGlobalObject*, JSScope* next, JSObject*);
 
     JSObject* object() { return m_object.get(); }
 
index 9fae8c6..e5fec92 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-15  Robin Morisset  <rmorisset@apple.com>
+
+        Change the order of arguments of JSWithScope::create() for consistency
+        https://bugs.webkit.org/show_bug.cgi?id=175585
+
+        Reviewed by Saam Barati.
+
+        No change of behavior.
+
+        * bindings/js/JSHTMLElementCustom.cpp:
+        (WebCore::JSHTMLElement::pushEventHandlerScope const):
+
 2017-08-15  Youenn Fablet  <youenn@apple.com>
 
         [Cache API] Ensure ResourceResponse is not null when redirected/tainting/type fields are set
index 43792a9..0e495ed 100644 (file)
@@ -120,14 +120,14 @@ JSScope* JSHTMLElement::pushEventHandlerScope(ExecState* exec, JSScope* scope) c
     VM& vm = exec->vm();
     JSGlobalObject* lexicalGlobalObject = exec->lexicalGlobalObject();
     
-    scope = JSWithScope::create(vm, lexicalGlobalObject, asObject(toJS(exec, globalObject(), element.document())), scope);
+    scope = JSWithScope::create(vm, lexicalGlobalObject, scope, asObject(toJS(exec, globalObject(), element.document())));
 
     // The form is next, searched before the document, but after the element itself.
     if (HTMLFormElement* form = element.form())
-        scope = JSWithScope::create(vm, lexicalGlobalObject, asObject(toJS(exec, globalObject(), *form)), scope);
+        scope = JSWithScope::create(vm, lexicalGlobalObject, scope, asObject(toJS(exec, globalObject(), *form)));
 
     // The element is on top, searched first.
-    return JSWithScope::create(vm, lexicalGlobalObject, asObject(toJS(exec, globalObject(), element)), scope);
+    return JSWithScope::create(vm, lexicalGlobalObject, scope, asObject(toJS(exec, globalObject(), element)));
 }
 
 } // namespace WebCore