Attachment 348953 Details for Bug 188917 – Patch for landing

[patch] Patch for landing

bug-188917-20180905122103.patch (text/plain), 8.35 KB, created by Keith Miller on 2018-09-05 12:21:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-09-05 12:21:06 PDT

Size: 8.35 KB

patch

obsolete

>Subversion Revision: 235439
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 1993b800ea71d6485b3d509ebb2b74267716d5c2..5effcd6fe7beeb137678120f5b33c3030bb62dfd 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-09-04  Keith Miller  <keith_miller@apple.com>
>+
>+        RELEASE_ASSERT at ../../Source/JavaScriptCore/heap/MarkedSpace.h:83
>+        https://bugs.webkit.org/show_bug.cgi?id=188917
>+
>+        Reviewed by Mark Lam.
>+
>+        Our allocators should be able to handle allocating a zero-sized object.
>+        Zero-sized objects will be allocated into the smallest size class.
>+
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileNewTypedArrayWithSize):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileNewTypedArray):
>+        (JSC::FTL::DFG::LowerDFGToB3::allocatorForSize):
>+        * heap/MarkedSpace.h:
>+        (JSC::MarkedSpace::sizeClassToIndex):
>+        (JSC::MarkedSpace::indexToSizeClass):
>+        * jit/AssemblyHelpers.cpp:
>+        (JSC::AssemblyHelpers::emitAllocateVariableSized):
>+        * runtime/JSArrayBufferView.cpp:
>+        (JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
>+
> 2018-08-28  Keith Miller  <keith_miller@apple.com>
> 
>         Add nullablity attributes to JSValue
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index bd18f9df9b7e8530923edf6e0b4108f6d297a067..3c104b292a1534ffc48c64b4435034b3781bacf5 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -9532,7 +9532,6 @@ void SpeculativeJIT::compileNewTypedArrayWithSize(Node* node)
> 
>     slowCases.append(m_jit.branch32(
>         MacroAssembler::Above, sizeGPR, TrustedImm32(JSArrayBufferView::fastSizeLimit)));
>-    slowCases.append(m_jit.branchTest32(MacroAssembler::Zero, sizeGPR));
>     
>     m_jit.move(sizeGPR, scratchGPR);
>     m_jit.lshift32(TrustedImm32(logElementSize(typedArrayType)), scratchGPR);
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index e0298d321d607cfb5a8d6619e83cf775d52e424e..640382fa699f5f238131588ad8acf6310e264071 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -5992,7 +5992,6 @@ private:
>             LValue size = lowInt32(m_node->child1());
> 
>             LBasicBlock smallEnoughCase = m_out.newBlock();
>-            LBasicBlock nonZeroCase = m_out.newBlock();
>             LBasicBlock slowCase = m_out.newBlock();
>             LBasicBlock continuation = m_out.newBlock();
>             
>@@ -6002,11 +6001,7 @@ private:
>                 m_out.above(size, m_out.constInt32(JSArrayBufferView::fastSizeLimit)),
>                 rarely(slowCase), usually(smallEnoughCase));
> 
>-            LBasicBlock lastNext = m_out.appendTo(smallEnoughCase, nonZeroCase);
>-
>-            m_out.branch(m_out.notZero32(size), usually(nonZeroCase), rarely(slowCase));
>-
>-            m_out.appendTo(nonZeroCase, slowCase);
>+            LBasicBlock lastNext = m_out.appendTo(smallEnoughCase, slowCase);
> 
>             LValue byteSize =
>                 m_out.shl(m_out.zeroExtPtr(size), m_out.constInt32(logElementSize(typedArrayType)));
>@@ -13151,7 +13146,7 @@ private:
>         if (subspace->hasIntPtr() && size->hasIntPtr()) {
>             CompleteSubspace* actualSubspace = bitwise_cast<CompleteSubspace*>(subspace->asIntPtr());
>             size_t actualSize = size->asIntPtr();
>-            
>+
>             Allocator actualAllocator = actualSubspace->allocatorForNonVirtual(actualSize, AllocatorForMode::AllocatorIfExists);
>             if (!actualAllocator) {
>                 LBasicBlock continuation = m_out.newBlock();
>@@ -13160,7 +13155,7 @@ private:
>                 m_out.appendTo(continuation, lastNext);
>                 return m_out.intPtrZero;
>             }
>-            
>+
>             return m_out.constIntPtr(actualAllocator.localAllocator());
>         }
>         
>@@ -13183,7 +13178,7 @@ private:
>         return m_out.loadPtr(
>             m_out.baseIndex(
>                 m_heaps.CompleteSubspace_allocatorForSizeStep,
>-                subspace, m_out.sub(sizeClassIndex, m_out.intPtrOne)));
>+                subspace, sizeClassIndex));
>     }
>     
>     LValue allocatorForSize(CompleteSubspace& subspace, LValue size, LBasicBlock slowPath)
>diff --git a/Source/JavaScriptCore/heap/MarkedSpace.cpp b/Source/JavaScriptCore/heap/MarkedSpace.cpp
>index 97216644f5669576c03faf534a103670db34191d..e3872a73cd78d246cfd4958a07d0c43a8412a7ca 100644
>--- a/Source/JavaScriptCore/heap/MarkedSpace.cpp
>+++ b/Source/JavaScriptCore/heap/MarkedSpace.cpp
>@@ -171,6 +171,7 @@ void buildSizeClassTable(TableType& table, const SizeClassCons& cons, const Defa
>             table[i] = entry;
>         nextIndex = index + 1;
>     }
>+    ASSERT(MarkedSpace::sizeClassToIndex(MarkedSpace::largeCutoff - 1) < MarkedSpace::numSizeClasses);
>     for (size_t i = nextIndex; i < MarkedSpace::numSizeClasses; ++i)
>         table[i] = defaultCons(MarkedSpace::indexToSizeClass(i));
> }
>diff --git a/Source/JavaScriptCore/heap/MarkedSpace.h b/Source/JavaScriptCore/heap/MarkedSpace.h
>index a6c089ea1cbfdd2eaa0c4a726b5d9d2c4000ec10..0d769817aee578814b3fc9eea7d38714be386bac 100644
>--- a/Source/JavaScriptCore/heap/MarkedSpace.h
>+++ b/Source/JavaScriptCore/heap/MarkedSpace.h
>@@ -64,7 +64,8 @@ public:
>     // into one block.
>     static constexpr size_t largeCutoff = (blockPayload / 2) & ~(sizeStep - 1);
> 
>-    static constexpr size_t numSizeClasses = largeCutoff / sizeStep;
>+    // We have an extra size class for size zero.
>+    static constexpr size_t numSizeClasses = largeCutoff / sizeStep + 1;
>     
>     static constexpr HeapVersion nullVersion = 0; // The version of freshly allocated blocks.
>     static constexpr HeapVersion initialVersion = 2; // The version that the heap starts out with. Set to make sure that nextVersion(nullVersion) != initialVersion.
>@@ -79,13 +80,14 @@ public:
>     
>     static size_t sizeClassToIndex(size_t size)
>     {
>-        RELEASE_ASSERT(size);
>-        return (size + sizeStep - 1) / sizeStep - 1;
>+        return (size + sizeStep - 1) / sizeStep;
>     }
>     
>     static size_t indexToSizeClass(size_t index)
>     {
>-        return (index + 1) * sizeStep;
>+        size_t result = index * sizeStep;
>+        ASSERT(sizeClassToIndex(result) == index);
>+        return result;
>     }
>     
>     MarkedSpace(Heap*);
>diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>index 8cc878f55c68c32669decb3d0c8901d93d91ba94..7bb9e3eca49aa203ed073a1209b4e777715181bc 100644
>--- a/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.cpp
>@@ -641,7 +641,7 @@ void AssemblyHelpers::emitAllocateVariableSized(GPRReg resultGPR, CompleteSubspa
>     add32(TrustedImm32(MarkedSpace::sizeStep - 1), allocationSize, scratchGPR1);
>     urshift32(TrustedImm32(stepShift), scratchGPR1);
>     slowPath.append(branch32(Above, scratchGPR1, TrustedImm32(MarkedSpace::largeCutoff >> stepShift)));
>-    move(TrustedImmPtr(subspace.allocatorForSizeStep() - 1), scratchGPR2);
>+    move(TrustedImmPtr(subspace.allocatorForSizeStep()), scratchGPR2);
>     loadPtr(BaseIndex(scratchGPR2, scratchGPR1, timesPtr()), scratchGPR1);
>     
>     emitAllocate(resultGPR, JITAllocator::variable(), scratchGPR1, scratchGPR2, slowPath);
>diff --git a/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp b/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
>index a900daef319f2e70c8315b06fb20759ee91ca581..b3039a236bc7d9262b3ee0bc9f4faeef63b7357c 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
>+++ b/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
>@@ -69,12 +69,9 @@ JSArrayBufferView::ConstructionContext::ConstructionContext(
>         // Attempt GC allocation.
>         void* temp;
>         size_t size = sizeOf(length, elementSize);
>-        if (size) {
>-            temp = vm.primitiveGigacageAuxiliarySpace.allocateNonVirtual(vm, size, nullptr, AllocationFailureMode::ReturnNull);
>-            if (!temp)
>-                return;
>-        } else
>-            temp = nullptr;
>+        temp = vm.primitiveGigacageAuxiliarySpace.allocateNonVirtual(vm, size, nullptr, AllocationFailureMode::ReturnNull);
>+        if (!temp)
>+            return;
> 
>         m_structure = structure;
>         m_vector = temp;

Actions: View | Formatted Diff | Diff

Attachments on bug 188917: 348132 | 348133 | 348134 | 348137 | 348935 | 348952 | 348953