DFG's compileReallocatePropertyStorage() and compileAllocatePropertyStorage() slow...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jun 2018 11:19:46 +0000 (11:19 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jun 2018 11:19:46 +0000 (11:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187091
<rdar://problem/41395624>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/regress-187091.js: Added.

Source/JavaScriptCore:

Previously, when compileReallocatePropertyStorage() and compileAllocatePropertyStorage()
take their slow paths, the slow path would jump back to the fast path right after
the emitted code which clears the unused property values.  As a result, the
unused properties are not initialized.  We've fixed this by adding the slow path
generators before we emit the code to clear the unused properties.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileAllocatePropertyStorage):
(JSC::DFG::SpeculativeJIT::compileReallocatePropertyStorage):

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

JSTests/ChangeLog
JSTests/stress/regress-187091.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

index 7336e5c..f213286 100644 (file)
@@ -1,3 +1,13 @@
+2018-06-27  Mark Lam  <mark.lam@apple.com>
+
+        DFG's compileReallocatePropertyStorage() and compileAllocatePropertyStorage() slow paths should also clear unused properties.
+        https://bugs.webkit.org/show_bug.cgi?id=187091
+        <rdar://problem/41395624>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/regress-187091.js: Added.
+
 2018-06-27  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] ArrayPatternNode::emitDirectBinding does not return assignment target value if dst is nullptr
diff --git a/JSTests/stress/regress-187091.js b/JSTests/stress/regress-187091.js
new file mode 100644 (file)
index 0000000..ac819f7
--- /dev/null
@@ -0,0 +1,20 @@
+// This test should not crash.
+
+function foo(x) {
+    x.a0 = 0;
+    Object.defineProperty(x, "a0", { value: 42 });
+    x.a6 = 6;
+}
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+    var x = { }
+    x.a1 = 1;
+    x.a2 = 2;
+    x.a3 = 3;
+    x.a4 = 4;
+    x.a7 = 7;
+    x.a5 = 5;
+
+    foo(x);
+}
index d4b4c7e..9483c96 100644 (file)
@@ -1,3 +1,21 @@
+2018-06-27  Mark Lam  <mark.lam@apple.com>
+
+        DFG's compileReallocatePropertyStorage() and compileAllocatePropertyStorage() slow paths should also clear unused properties.
+        https://bugs.webkit.org/show_bug.cgi?id=187091
+        <rdar://problem/41395624>
+
+        Reviewed by Yusuke Suzuki.
+
+        Previously, when compileReallocatePropertyStorage() and compileAllocatePropertyStorage()
+        take their slow paths, the slow path would jump back to the fast path right after
+        the emitted code which clears the unused property values.  As a result, the
+        unused properties are not initialized.  We've fixed this by adding the slow path
+        generators before we emit the code to clear the unused properties.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileAllocatePropertyStorage):
+        (JSC::DFG::SpeculativeJIT::compileReallocatePropertyStorage):
+
 2018-06-27  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] ArrayPatternNode::emitDirectBinding does not return assignment target value if dst is nullptr
index b04bde2..7deb898 100644 (file)
@@ -8936,13 +8936,13 @@ void SpeculativeJIT::compileAllocatePropertyStorage(Node* node)
     JITCompiler::JumpList slowPath;
     m_jit.emitAllocate(scratchGPR1, JITAllocator::constant(allocator), scratchGPR2, scratchGPR3, slowPath);
     m_jit.addPtr(JITCompiler::TrustedImm32(size + sizeof(IndexingHeader)), scratchGPR1);
-    
-    for (ptrdiff_t offset = 0; offset < static_cast<ptrdiff_t>(size); offset += sizeof(void*))
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratchGPR1, -(offset + sizeof(JSValue) + sizeof(void*))));
-        
+
     addSlowPathGenerator(
         slowPathCall(slowPath, this, operationAllocateSimplePropertyStorageWithInitialCapacity, scratchGPR1));
 
+    for (ptrdiff_t offset = 0; offset < static_cast<ptrdiff_t>(size); offset += sizeof(void*))
+        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratchGPR1, -(offset + sizeof(JSValue) + sizeof(void*))));
+
     storageResult(scratchGPR1, node);
 }
 
@@ -8973,7 +8973,7 @@ void SpeculativeJIT::compileReallocatePropertyStorage(Node* node)
     GPRTemporary scratch1(this);
     GPRTemporary scratch2(this);
     GPRTemporary scratch3(this);
-        
+
     GPRReg oldStorageGPR = oldStorage.gpr();
     GPRReg scratchGPR1 = scratch1.gpr();
     GPRReg scratchGPR2 = scratch2.gpr();
@@ -8983,19 +8983,19 @@ void SpeculativeJIT::compileReallocatePropertyStorage(Node* node)
     m_jit.emitAllocate(scratchGPR1, JITAllocator::constant(allocator), scratchGPR2, scratchGPR3, slowPath);
     
     m_jit.addPtr(JITCompiler::TrustedImm32(newSize + sizeof(IndexingHeader)), scratchGPR1);
-        
-    for (ptrdiff_t offset = oldSize; offset < static_cast<ptrdiff_t>(newSize); offset += sizeof(void*))
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratchGPR1, -(offset + sizeof(JSValue) + sizeof(void*))));
 
     addSlowPathGenerator(
         slowPathCall(slowPath, this, operationAllocateSimplePropertyStorage, scratchGPR1, newSize / sizeof(JSValue)));
 
+    for (ptrdiff_t offset = oldSize; offset < static_cast<ptrdiff_t>(newSize); offset += sizeof(void*))
+        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratchGPR1, -(offset + sizeof(JSValue) + sizeof(void*))));
+
     // We have scratchGPR1 = new storage, scratchGPR2 = scratch
     for (ptrdiff_t offset = 0; offset < static_cast<ptrdiff_t>(oldSize); offset += sizeof(void*)) {
         m_jit.loadPtr(JITCompiler::Address(oldStorageGPR, -(offset + sizeof(JSValue) + sizeof(void*))), scratchGPR2);
         m_jit.storePtr(scratchGPR2, JITCompiler::Address(scratchGPR1, -(offset + sizeof(JSValue) + sizeof(void*))));
     }
-        
+
     storageResult(scratchGPR1, node);
 }