Title: [228638] releases/WebKitGTK/webkit-2.20
Revision
228638
Author
carlo...@webkit.org
Date
2018-02-19 02:47:07 -0800 (Mon, 19 Feb 2018)

Log Message

Merge r228193 - put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
https://bugs.webkit.org/show_bug.cgi?id=182549
<rdar://problem/36189995>

Reviewed by Saam Barati.

JSTests:

* stress/var-injection-cache-invalidation.js: Added.
(allocateLotsOfThings):
(test):

Source/_javascript_Core:

Previously, the llint/baseline caching for put_to_scope and
get_from_scope would cache lexical environments when the
varInjectionWatchpoint had been fired for global properties. Code
in the DFG does not follow this same assumption so we could
potentially return the wrong result. Additionally, the baseline
would write barrier the global object rather than the lexical
enviroment object. This patch makes it so that we do not cache
anything other than the global object for when the resolve type is
GlobalPropertyWithVarInjectionChecks or GlobalProperty.

* assembler/MacroAssembler.cpp:
(JSC::MacroAssembler::jitAssert):
* assembler/MacroAssembler.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_from_scope):
(JSC::JIT::emit_op_put_to_scope):
* runtime/CommonSlowPaths.h:
(JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
(JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
* runtime/Options.h:

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog	2018-02-19 10:47:07 UTC (rev 228638)
@@ -1,3 +1,15 @@
+2018-02-06  Keith Miller  <keith_mil...@apple.com>
+
+        put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
+        https://bugs.webkit.org/show_bug.cgi?id=182549
+        <rdar://problem/36189995>
+
+        Reviewed by Saam Barati.
+
+        * stress/var-injection-cache-invalidation.js: Added.
+        (allocateLotsOfThings):
+        (test):
+
 2018-02-03  Yusuke Suzuki  <utatane....@gmail.com>
 
         Unreviewed, follow up for test262 update

Added: releases/WebKitGTK/webkit-2.20/JSTests/stress/var-injection-cache-invalidation.js (0 => 228638)


--- releases/WebKitGTK/webkit-2.20/JSTests/stress/var-injection-cache-invalidation.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.20/JSTests/stress/var-injection-cache-invalidation.js	2018-02-19 10:47:07 UTC (rev 228638)
@@ -0,0 +1,19 @@
+a = 0;
+
+function allocateLotsOfThings(array) {
+    for (let i = 0; i < 1e4; i++)
+        array[i] = { next: array[Math.floor(i / 2)] };
+}
+
+function test() {
+    a = 5;
+    for (var i = 0; i < 1e3; i++) {
+        allocateLotsOfThings([]);
+        edenGC();
+        eval("var a = new Int32Array(100);");
+    }
+}
+noInline(test);
+noDFG(test);
+
+test();

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-02-19 10:47:07 UTC (rev 228638)
@@ -1,3 +1,32 @@
+2018-02-06  Keith Miller  <keith_mil...@apple.com>
+
+        put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
+        https://bugs.webkit.org/show_bug.cgi?id=182549
+        <rdar://problem/36189995>
+
+        Reviewed by Saam Barati.
+
+        Previously, the llint/baseline caching for put_to_scope and
+        get_from_scope would cache lexical environments when the
+        varInjectionWatchpoint had been fired for global properties. Code
+        in the DFG does not follow this same assumption so we could
+        potentially return the wrong result. Additionally, the baseline
+        would write barrier the global object rather than the lexical
+        enviroment object. This patch makes it so that we do not cache
+        anything other than the global object for when the resolve type is
+        GlobalPropertyWithVarInjectionChecks or GlobalProperty.
+
+        * assembler/MacroAssembler.cpp:
+        (JSC::MacroAssembler::jitAssert):
+        * assembler/MacroAssembler.h:
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emit_op_get_from_scope):
+        (JSC::JIT::emit_op_put_to_scope):
+        * runtime/CommonSlowPaths.h:
+        (JSC::CommonSlowPaths::tryCachePutToScopeGlobal):
+        (JSC::CommonSlowPaths::tryCacheGetFromScopeGlobal):
+        * runtime/Options.h:
+
 2018-02-11  Guillaume Emont  <guijem...@igalia.com>
 
         [MIPS] JSC needs to be built with -latomic

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.cpp (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.cpp	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.cpp	2018-02-19 10:47:07 UTC (rev 228638)
@@ -28,13 +28,24 @@
 
 #if ENABLE(ASSEMBLER)
 
+#include "Options.h"
 #include "ProbeContext.h"
 #include <wtf/PrintStream.h>
+#include <wtf/ScopedLambda.h>
 
 namespace JSC {
 
 const double MacroAssembler::twoToThe32 = (double)0x100000000ull;
 
+void MacroAssembler::jitAssert(const ScopedLambda<Jump(void)>& functor)
+{
+    if (Options::enableJITDebugAssetions()) {
+        Jump passed = functor();
+        breakpoint();
+        passed.link(this);
+    }
+}
+
 #if ENABLE(MASM_PROBE)
 static void stdFunctionCallback(Probe::Context& context)
 {

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.h (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.h	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/assembler/MacroAssembler.h	2018-02-19 10:47:07 UTC (rev 228638)
@@ -61,6 +61,13 @@
 
 #include "MacroAssemblerHelpers.h"
 
+namespace WTF {
+
+template<typename FunctionType>
+class ScopedLambda;
+
+} // namespace WTF
+
 namespace JSC {
 
 #if ENABLE(MASM_PROBE)
@@ -1884,6 +1891,9 @@
         urshift32(src, trustedImm32ForShift(amount), dest);
     }
 
+    // If the result jump is taken that means the assert passed.
+    void jitAssert(const WTF::ScopedLambda<Jump(void)>&);
+
 #if ENABLE(MASM_PROBE)
     // This function emits code to preserve the CPUState (e.g. registers),
     // call a user supplied probe function, and restore the CPUState before

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jit/JITPropertyAccess.cpp (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2018-02-19 10:47:07 UTC (rev 228638)
@@ -43,6 +43,7 @@
 #include "ScopedArgumentsTable.h"
 #include "SlowPathCall.h"
 #include "StructureStubInfo.h"
+#include <wtf/ScopedLambda.h>
 #include <wtf/StringPrintStream.h>
 
 
@@ -857,12 +858,16 @@
         switch (resolveType) {
         case GlobalProperty:
         case GlobalPropertyWithVarInjectionChecks: {
-            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
+            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
             GPRReg base = regT0;
             GPRReg result = regT0;
             GPRReg offset = regT1;
             GPRReg scratch = regT2;
-            
+
+            jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
+                return branchPtr(Equal, base, TrustedImmPtr(m_codeBlock->globalObject()));
+            }));
+
             load32(operandSlot, offset);
             if (!ASSERT_DISABLED) {
                 Jump isOutOfLine = branch32(GreaterThanOrEqual, offset, TrustedImm32(firstOutOfLineOffset));
@@ -985,9 +990,13 @@
         switch (resolveType) {
         case GlobalProperty:
         case GlobalPropertyWithVarInjectionChecks: {
-            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection.
+            emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Additionally, resolve_scope handles checking for the var injection.
             emitGetVirtualRegister(value, regT2);
-            
+
+            jitAssert(scopedLambda<Jump(void)>([&] () -> Jump {
+                return branchPtr(Equal, regT0, TrustedImmPtr(m_codeBlock->globalObject()));
+            }));
+
             loadPtr(Address(regT0, JSObject::butterflyOffset()), regT0);
             loadPtr(operandSlot, regT1);
             negPtr(regT1);

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/CommonSlowPaths.h (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/CommonSlowPaths.h	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/CommonSlowPaths.h	2018-02-19 10:47:07 UTC (rev 228638)
@@ -138,8 +138,11 @@
     }
     
     if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
+        JSGlobalObject* globalObject = codeBlock->globalObject();
+        ASSERT(globalObject == scope || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
         if (!slot.isCacheablePut()
             || slot.base() != scope
+            || scope != globalObject
             || !scope->structure()->propertyAccessesAreCacheable())
             return;
         
@@ -183,9 +186,11 @@
     }
 
     // Covers implicit globals. Since they don't exist until they first execute, we didn't know how to cache them at compile time.
-    if (slot.isCacheableValue() && slot.slotBase() == scope && scope->structure()->propertyAccessesAreCacheable()) {
-        if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
-            CodeBlock* codeBlock = exec->codeBlock();
+    if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
+        CodeBlock* codeBlock = exec->codeBlock();
+        JSGlobalObject* globalObject = codeBlock->globalObject();
+        ASSERT(scope == globalObject || globalObject->varInjectionWatchpoint()->hasBeenInvalidated());
+        if (slot.isCacheableValue() && slot.slotBase() == scope && scope == globalObject && scope->structure()->propertyAccessesAreCacheable()) {
             Structure* structure = scope->structure(vm);
             {
                 ConcurrentJSLocker locker(codeBlock->m_lock);

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Options.h (228637 => 228638)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Options.h	2018-02-19 10:46:58 UTC (rev 228637)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Options.h	2018-02-19 10:47:07 UTC (rev 228638)
@@ -250,6 +250,7 @@
     v(bool, b3AlwaysFailsBeforeLink, false, Normal, nullptr) \
     v(bool, ftlCrashes, false, Normal, nullptr) /* fool-proof way of checking that you ended up in the FTL. ;-) */\
     v(bool, clobberAllRegsInFTLICSlowPath, !ASSERT_DISABLED, Normal, nullptr) \
+    v(bool, enableJITDebugAssetions, !ASSERT_DISABLED, Normal, nullptr) \
     v(bool, useAccessInlining, true, Normal, nullptr) \
     v(unsigned, maxAccessVariantListSize, 8, Normal, nullptr) \
     v(bool, usePolyvariantDevirtualization, true, Normal, nullptr) \
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to