Attachment 343704 Details for Bug 187091 – proposed patch.

[patch] proposed patch.

bug-187091.patch (text/plain), 5.24 KB, created by Mark Lam on 2018-06-27 02:31:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-06-27 02:31:05 PDT

Size: 5.24 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 233250)
>+++ JSTests/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        * stress/regress-187091.js: Added.
>+
> 2018-06-26  Mark Lam  <mark.lam@apple.com>
> 
>         eval() is wrong about the LiteralParser never throwing any exceptions.
>Index: JSTests/stress/regress-187091.js
>===================================================================
>--- JSTests/stress/regress-187091.js	(nonexistent)
>+++ JSTests/stress/regress-187091.js	(working copy)
>@@ -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: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 233250)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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-26  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [JSC] Pass VM& to functions more
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 233250)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -8936,13 +8936,13 @@ void SpeculativeJIT::compileAllocateProp
>     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::compileReallocatePr
>     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::compileReallocatePr
>     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);
> }
>

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 187091: 343704