[JSC] Remove index-masking on ScopedArguments and put it in IsoSubspace
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 07:54:34 +0000 (07:54 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Nov 2019 07:54:34 +0000 (07:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204269

Reviewed by Saam Barati.

Source/JavaScriptCore:

We should remove index-masking for ScopedArguments. This patch reverts it.
We still use AuxiliaryBuffer, and avoid using variable sized cell. Then,
this patch also puts ScopedArguments in IsoSubspace.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::generateWithGuard):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnScopedArguments):
(JSC::DFG::SpeculativeJIT::compileGetArrayLength):
* ftl/FTLAbstractHeapRepository.h:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::LowerDFGToB3):
(JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
(JSC::FTL::DFG::LowerDFGToB3::preciseIndexMask64): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::preciseIndexMask32): Deleted.
* jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::emitPreparePreciseIndexMask32): Deleted.
* jit/AssemblyHelpers.h:
* jit/JIT.cpp:
(JSC::JIT::JIT):
* jit/JIT.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitScopedArgumentsGetByVal):
* runtime/ScopedArguments.cpp:
(JSC::ScopedArguments::ScopedArguments):
(JSC::ScopedArguments::createUninitialized):
(JSC::ScopedArguments::create):
(JSC::ScopedArguments::createByCopyingFrom):
(JSC::ScopedArguments::visitChildren):
(JSC::ScopedArguments::overrideThings):
(JSC::ScopedArguments::overrideThingsIfNecessary):
(JSC::ScopedArguments::unmapArgument):
* runtime/ScopedArguments.h:
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:

Source/WTF:

* wtf/MathExtras.h:
(WTF::computeIndexingMask): Deleted.
(WTF::preciseIndexMaskShiftForSize): Deleted.
(WTF::preciseIndexMaskShift): Deleted.
(WTF::opaque): Deleted.
(WTF::preciseIndexMaskPtr): Deleted.

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

16 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/AccessCase.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/AssemblyHelpers.cpp
Source/JavaScriptCore/jit/AssemblyHelpers.h
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JIT.h
Source/JavaScriptCore/jit/JITPropertyAccess.cpp
Source/JavaScriptCore/runtime/ScopedArguments.cpp
Source/JavaScriptCore/runtime/ScopedArguments.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/WTF/ChangeLog
Source/WTF/wtf/MathExtras.h

index 790eed6..7f80687 100644 (file)
@@ -1,3 +1,49 @@
+2019-11-15  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Remove index-masking on ScopedArguments and put it in IsoSubspace
+        https://bugs.webkit.org/show_bug.cgi?id=204269
+
+        Reviewed by Saam Barati.
+
+        We should remove index-masking for ScopedArguments. This patch reverts it.
+        We still use AuxiliaryBuffer, and avoid using variable sized cell. Then,
+        this patch also puts ScopedArguments in IsoSubspace.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateWithGuard):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnScopedArguments):
+        (JSC::DFG::SpeculativeJIT::compileGetArrayLength):
+        * ftl/FTLAbstractHeapRepository.h:
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::LowerDFGToB3):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::preciseIndexMask64): Deleted.
+        (JSC::FTL::DFG::LowerDFGToB3::preciseIndexMask32): Deleted.
+        * jit/AssemblyHelpers.cpp:
+        (JSC::AssemblyHelpers::emitPreparePreciseIndexMask32): Deleted.
+        * jit/AssemblyHelpers.h:
+        * jit/JIT.cpp:
+        (JSC::JIT::JIT):
+        * jit/JIT.h:
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitScopedArgumentsGetByVal):
+        * runtime/ScopedArguments.cpp:
+        (JSC::ScopedArguments::ScopedArguments):
+        (JSC::ScopedArguments::createUninitialized):
+        (JSC::ScopedArguments::create):
+        (JSC::ScopedArguments::createByCopyingFrom):
+        (JSC::ScopedArguments::visitChildren):
+        (JSC::ScopedArguments::overrideThings):
+        (JSC::ScopedArguments::overrideThingsIfNecessary):
+        (JSC::ScopedArguments::unmapArgument):
+        * runtime/ScopedArguments.h:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+
 2019-11-15  Eric Carlson  <eric.carlson@apple.com>
 
         Don't use AVCapture on watchOS and tvOS
index 8a5723a..857bc6e 100644 (file)
@@ -576,15 +576,12 @@ void AccessCase::generateWithGuard(
         fallThrough.append(
             jit.branchIfNotType(baseGPR, ScopedArgumentsType));
 
-        jit.loadPtr(
-            CCallHelpers::Address(baseGPR, ScopedArguments::offsetOfStorage()),
-            scratchGPR);
         fallThrough.append(
             jit.branchTest8(
                 CCallHelpers::NonZero,
-                CCallHelpers::Address(scratchGPR, ScopedArguments::offsetOfOverrodeThingsInStorage())));
+                CCallHelpers::Address(baseGPR, ScopedArguments::offsetOfOverrodeThings())));
         jit.load32(
-            CCallHelpers::Address(scratchGPR, ScopedArguments::offsetOfTotalLengthInStorage()),
+            CCallHelpers::Address(baseGPR, ScopedArguments::offsetOfTotalLength()),
             valueRegs.payloadGPR());
         jit.boxInt32(valueRegs.payloadGPR(), valueRegs);
         state.succeed();
index b849666..89634f3 100644 (file)
@@ -6967,14 +6967,12 @@ void SpeculativeJIT::compileGetByValOnScopedArguments(Node* node)
     JSValueRegsTemporary result(this);
     GPRTemporary scratch(this);
     GPRTemporary scratch2(this);
-    GPRTemporary indexMask(this);
     
     GPRReg baseReg = base.gpr();
     GPRReg propertyReg = property.gpr();
     JSValueRegs resultRegs = result.regs();
     GPRReg scratchReg = scratch.gpr();
     GPRReg scratch2Reg = scratch2.gpr();
-    GPRReg indexMaskReg = indexMask.gpr();
     
     if (!m_compileOkay)
         return;
@@ -6983,15 +6981,12 @@ void SpeculativeJIT::compileGetByValOnScopedArguments(Node* node)
     
     m_jit.loadPtr(
         MacroAssembler::Address(baseReg, ScopedArguments::offsetOfStorage()), resultRegs.payloadGPR());
-    m_jit.load32(
-        MacroAssembler::Address(resultRegs.payloadGPR(), ScopedArguments::offsetOfTotalLengthInStorage()),
-        scratchReg);
-    
+
     speculationCheck(
         ExoticObjectMode, JSValueSource(), nullptr,
-        m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, scratchReg));
-    
-    m_jit.emitPreparePreciseIndexMask32(propertyReg, scratchReg, indexMaskReg);
+        m_jit.branch32(
+            MacroAssembler::AboveOrEqual, propertyReg,
+            MacroAssembler::Address(baseReg, ScopedArguments::offsetOfTotalLength())));
     
     m_jit.loadPtr(MacroAssembler::Address(baseReg, ScopedArguments::offsetOfTable()), scratchReg);
     m_jit.load32(
@@ -7034,8 +7029,6 @@ void SpeculativeJIT::compileGetByValOnScopedArguments(Node* node)
     
     done.link(&m_jit);
     
-    m_jit.andPtr(indexMaskReg, resultRegs.payloadGPR());
-    
     jsValueResult(resultRegs, node);
 }
 
@@ -7166,7 +7159,7 @@ void SpeculativeJIT::compileGetArrayLength(Node* node)
     }
     case Array::ScopedArguments: {
         SpeculateCellOperand base(this, node->child1());
-        GPRTemporary result(this);
+        GPRTemporary result(this, Reuse, base);
         
         GPRReg baseReg = base.gpr();
         GPRReg resultReg = result.gpr();
@@ -7176,17 +7169,14 @@ void SpeculativeJIT::compileGetArrayLength(Node* node)
         
         ASSERT(ArrayMode(Array::ScopedArguments, Array::Read).alreadyChecked(m_jit.graph(), node, m_state.forNode(node->child1())));
         
-        m_jit.loadPtr(
-            MacroAssembler::Address(baseReg, ScopedArguments::offsetOfStorage()), resultReg);
-
         speculationCheck(
             ExoticObjectMode, JSValueSource(), 0,
             m_jit.branchTest8(
                 MacroAssembler::NonZero,
-                MacroAssembler::Address(resultReg, ScopedArguments::offsetOfOverrodeThingsInStorage())));
+                MacroAssembler::Address(baseReg, ScopedArguments::offsetOfOverrodeThings())));
         
         m_jit.load32(
-            MacroAssembler::Address(resultReg, ScopedArguments::offsetOfTotalLengthInStorage()), resultReg);
+            MacroAssembler::Address(baseReg, ScopedArguments::offsetOfTotalLength()), resultReg);
         
         int32Result(resultReg, node);
         break;
index 5df43f7..8740572 100644 (file)
@@ -133,11 +133,11 @@ namespace JSC { namespace FTL {
     macro(ShadowChicken_Packet_scope, OBJECT_OFFSETOF(ShadowChicken::Packet, scope)) \
     macro(ShadowChicken_Packet_codeBlock, OBJECT_OFFSETOF(ShadowChicken::Packet, codeBlock)) \
     macro(ShadowChicken_Packet_callSiteIndex, OBJECT_OFFSETOF(ShadowChicken::Packet, callSiteIndex)) \
-    macro(ScopedArguments_Storage_overrodeThings, ScopedArguments::offsetOfOverrodeThingsInStorage()) \
-    macro(ScopedArguments_Storage_totalLength, ScopedArguments::offsetOfTotalLengthInStorage()) \
-    macro(ScopedArguments_storage, ScopedArguments::offsetOfStorage()) \
+    macro(ScopedArguments_overrodeThings, ScopedArguments::offsetOfOverrodeThings()) \
     macro(ScopedArguments_scope, ScopedArguments::offsetOfScope()) \
+    macro(ScopedArguments_storage, ScopedArguments::offsetOfStorage()) \
     macro(ScopedArguments_table, ScopedArguments::offsetOfTable()) \
+    macro(ScopedArguments_totalLength, ScopedArguments::offsetOfTotalLength()) \
     macro(ScopedArgumentsTable_arguments, ScopedArgumentsTable::offsetOfArguments()) \
     macro(ScopedArgumentsTable_length, ScopedArgumentsTable::offsetOfLength()) \
     macro(StringImpl_data, StringImpl::dataOffset()) \
index 37f0a96..e5e3753 100644 (file)
@@ -158,7 +158,6 @@ public:
         , m_availabilityCalculator(m_graph)
         , m_state(state.graph)
         , m_interpreter(state.graph, m_state)
-        , m_indexMaskingMode(Options::enableSpectreMitigations() ?  IndexMaskingEnabled : IndexMaskingDisabled)
     {
         if (Options::validateAbstractInterpreterState()) {
             performLivenessAnalysis(m_graph);
@@ -4183,11 +4182,10 @@ private:
             
         case Array::ScopedArguments: {
             LValue arguments = lowCell(m_node->child1());
-            LValue storage = m_out.loadPtr(arguments, m_heaps.ScopedArguments_storage);
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
-                m_out.notZero32(m_out.load8ZeroExt32(storage, m_heaps.ScopedArguments_Storage_overrodeThings)));
-            setInt32(m_out.load32NonNegative(storage, m_heaps.ScopedArguments_Storage_totalLength));
+                m_out.notZero32(m_out.load8ZeroExt32(arguments, m_heaps.ScopedArguments_overrodeThings)));
+            setInt32(m_out.load32NonNegative(arguments, m_heaps.ScopedArguments_totalLength));
             return;
         }
             
@@ -4382,12 +4380,11 @@ private:
             LValue base = lowCell(m_graph.varArgChild(m_node, 0));
             LValue index = lowInt32(m_graph.varArgChild(m_node, 1));
             
-            LValue storage = m_out.loadPtr(base, m_heaps.ScopedArguments_storage);
-            LValue totalLength = m_out.load32NonNegative(
-                storage, m_heaps.ScopedArguments_Storage_totalLength);
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
-                m_out.aboveOrEqual(index, totalLength));
+                m_out.aboveOrEqual(
+                    index,
+                    m_out.load32NonNegative(base, m_heaps.ScopedArguments_totalLength)));
             
             LValue table = m_out.loadPtr(base, m_heaps.ScopedArguments_table);
             LValue namedLength = m_out.load32(table, m_heaps.ScopedArgumentsTable_length);
@@ -4419,6 +4416,7 @@ private:
             
             m_out.appendTo(overflowCase, continuation);
             
+            LValue storage = m_out.loadPtr(base, m_heaps.ScopedArguments_storage);
             address = m_out.baseIndex(
                 m_heaps.ScopedArguments_Storage_storage, storage,
                 m_out.zeroExtPtr(m_out.sub(index, namedLength)));
@@ -4428,11 +4426,7 @@ private:
             m_out.jump(continuation);
             
             m_out.appendTo(continuation, lastNext);
-            
-            LValue result = m_out.phi(Int64, namedResult, overflowResult);
-            result = preciseIndexMask32(result, index, totalLength);
-            
-            setJSValue(result);
+            setJSValue(m_out.phi(Int64, namedResult, overflowResult));
             return;
         }
             
@@ -4609,7 +4603,6 @@ private:
             LValue pointer = m_out.baseIndex(
                 base.value(), m_out.zeroExt(index, pointerType()), ScaleEight);
             result = m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer));
-            result = preciseIndexMask32(result, indexToCheck, numberOfArgs);
         } else
             result = m_out.constInt64(JSValue::encode(jsUndefined()));
         
@@ -17349,22 +17342,6 @@ private:
         m_out.appendTo(continuation, lastNext);
     }
     
-    LValue preciseIndexMask64(LValue value, LValue index, LValue limit)
-    {
-        return m_out.bitAnd(
-            value,
-            m_out.aShr(
-                m_out.sub(
-                    index,
-                    m_out.opaque(limit)),
-                m_out.constInt32(63)));
-    }
-    
-    LValue preciseIndexMask32(LValue value, LValue index, LValue limit)
-    {
-        return preciseIndexMask64(value, m_out.zeroExt(index, Int64), m_out.zeroExt(limit, Int64));
-    }
-    
     template<typename OperationType, typename... Args>
     LValue vmCall(LType type, OperationType function, Args&&... args)
     {
@@ -18060,10 +18037,6 @@ private:
     DFG::BasicBlock* m_nextHighBlock;
     LBasicBlock m_nextLowBlock;
 
-    enum IndexMaskingMode { IndexMaskingDisabled, IndexMaskingEnabled };
-
-    IndexMaskingMode m_indexMaskingMode;
-
     NodeOrigin m_origin;
     unsigned m_nodeIndex;
     Node* m_node;
index 1cac1c4..35c595e 100644 (file)
@@ -988,18 +988,6 @@ void AssemblyHelpers::sanitizeStackInline(VM& vm, GPRReg scratch)
     storePtr(scratch, vm.addressOfLastStackTop());
 }
 
-void AssemblyHelpers::emitPreparePreciseIndexMask32(GPRReg index, GPRReg length, GPRReg result)
-{
-    if (length == result) {
-        negPtr(length);
-        addPtr(index, length);
-    } else {
-        move(index, result);
-        subPtr(length, result);
-    }
-    rshiftPtr(TrustedImm32(preciseIndexMaskShift<void*>()), result);
-}
-
 } // namespace JSC
 
 #endif // ENABLE(JIT)
index ca21195..22009c5 100644 (file)
@@ -1907,11 +1907,6 @@ public:
 #if USE(JSVALUE64)
     void wangsInt64Hash(GPRReg inputAndResult, GPRReg scratch);
 #endif
-    
-    // This assumes that index and length are 32-bit. This also assumes that they are already
-    // zero-extended. Also this does not clobber index, which is useful in the baseline JIT. This
-    // permits length and result to be in the same register.
-    void emitPreparePreciseIndexMask32(GPRReg index, GPRReg length, GPRReg result);
 
 #if ENABLE(WEBASSEMBLY)
     void loadWasmContextInstance(GPRReg dst);
index 1ff8b47..28a7317 100644 (file)
@@ -81,7 +81,6 @@ JIT::JIT(VM& vm, CodeBlock* codeBlock, BytecodeIndex loopOSREntryBytecodeIndex)
     , m_pcToCodeOriginMapBuilder(vm)
     , m_canBeOptimized(false)
     , m_shouldEmitProfiling(false)
-    , m_shouldUseIndexMasking(Options::enableSpectreMitigations())
     , m_loopOSREntryBytecodeIndex(loopOSREntryBytecodeIndex)
 {
 }
index 2b4a5d6..d7fe4ae 100644 (file)
@@ -967,7 +967,6 @@ namespace JSC {
         bool m_canBeOptimized;
         bool m_canBeOptimizedOrInlined;
         bool m_shouldEmitProfiling;
-        bool m_shouldUseIndexMasking;
         BytecodeIndex m_loopOSREntryBytecodeIndex;
     };
 
index ff39b22..4f867da 100644 (file)
@@ -1634,20 +1634,17 @@ JIT::JumpList JIT::emitScopedArgumentsGetByVal(const Instruction*, PatchableJump
     JSValueRegs result = JSValueRegs(regT0);
     RegisterID scratch = regT3;
     RegisterID scratch2 = regT4;
-    RegisterID scratch3 = regT5;
 #else
     RegisterID base = regT0;
     RegisterID property = regT2;
     JSValueRegs result = JSValueRegs(regT1, regT0);
     RegisterID scratch = regT3;
     RegisterID scratch2 = regT4;
-    RegisterID scratch3 = regT5;
 #endif
 
     load8(Address(base, JSCell::typeInfoTypeOffset()), scratch);
     badType = patchableBranch32(NotEqual, scratch, TrustedImm32(ScopedArgumentsType));
-    loadPtr(Address(base, ScopedArguments::offsetOfStorage()), scratch3);
-    slowCases.append(branch32(AboveOrEqual, property, Address(scratch3, ScopedArguments::offsetOfTotalLengthInStorage())));
+    slowCases.append(branch32(AboveOrEqual, property, Address(base, ScopedArguments::offsetOfTotalLength())));
     
     loadPtr(Address(base, ScopedArguments::offsetOfTable()), scratch);
     load32(Address(scratch, ScopedArgumentsTable::offsetOfLength()), scratch2);
@@ -1661,14 +1658,11 @@ JIT::JumpList JIT::emitScopedArgumentsGetByVal(const Instruction*, PatchableJump
     overflowCase.link(this);
     sub32(property, scratch2);
     neg32(scratch2);
-    loadValue(BaseIndex(scratch3, scratch2, TimesEight), result);
+    loadPtr(Address(base, ScopedArguments::offsetOfStorage()), scratch);
+    loadValue(BaseIndex(scratch, scratch2, TimesEight), result);
     slowCases.append(branchIfEmpty(result));
     done.link(this);
     
-    load32(Address(scratch3, ScopedArguments::offsetOfTotalLengthInStorage()), scratch);
-    emitPreparePreciseIndexMask32(property, scratch, scratch2);
-    andPtr(scratch2, result.payloadGPR());
-    
     return slowCases;
 }
 
index f480b17..081d978 100644 (file)
@@ -35,11 +35,12 @@ STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(ScopedArguments);
 
 const ClassInfo ScopedArguments::s_info = { "Arguments", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(ScopedArguments) };
 
-ScopedArguments::ScopedArguments(VM& vm, Structure* structure, WriteBarrier<Unknown>* storage)
+ScopedArguments::ScopedArguments(VM& vm, Structure* structure, WriteBarrier<Unknown>* storage, unsigned totalLength)
     : GenericArguments(vm, structure)
-    , m_storage(vm, this, storage)
+    , m_totalLength(totalLength)
 {
-    ASSERT(!storageHeader(storage).overrodeThings);
+    if (storage)
+        m_storage.set(vm, this, storage);
 }
 
 void ScopedArguments::finishCreation(VM& vm, JSFunction* callee, ScopedArgumentsTable* table, JSLexicalEnvironment* scope)
@@ -52,22 +53,16 @@ void ScopedArguments::finishCreation(VM& vm, JSFunction* callee, ScopedArguments
 
 ScopedArguments* ScopedArguments::createUninitialized(VM& vm, Structure* structure, JSFunction* callee, ScopedArgumentsTable* table, JSLexicalEnvironment* scope, unsigned totalLength)
 {
-    unsigned overflowLength;
-    if (totalLength > table->length())
-        overflowLength = totalLength - table->length();
-    else
-        overflowLength = 0;
-    
-    void* rawStoragePtr = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(
-        vm, storageSize(overflowLength), nullptr, AllocationFailureMode::Assert);
-    WriteBarrier<Unknown>* storage = static_cast<WriteBarrier<Unknown>*>(rawStoragePtr) + 1;
-    storageHeader(storage).overrodeThings = false;
-    storageHeader(storage).totalLength = totalLength;
-    
+    WriteBarrier<Unknown>* storage = nullptr;
+    if (totalLength > table->length()) {
+        Checked<unsigned> overflowLength = totalLength - table->length();
+        storage = static_cast<WriteBarrier<Unknown>*>(vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, (overflowLength * sizeof(WriteBarrier<Unknown>)).unsafeGet(), nullptr, AllocationFailureMode::Assert));
+    }
+
     ScopedArguments* result = new (
         NotNull,
         allocateCell<ScopedArguments>(vm.heap))
-        ScopedArguments(vm, structure, storage);
+        ScopedArguments(vm, structure, storage, totalLength);
     result->finishCreation(vm, callee, table, scope);
     return result;
 }
@@ -79,7 +74,7 @@ ScopedArguments* ScopedArguments::create(VM& vm, Structure* structure, JSFunctio
 
     unsigned namedLength = table->length();
     for (unsigned i = namedLength; i < totalLength; ++i)
-        result->overflowStorage()[i - namedLength].clear();
+        result->storage()[i - namedLength].clear();
     
     return result;
 }
@@ -99,7 +94,7 @@ ScopedArguments* ScopedArguments::createByCopyingFrom(VM& vm, Structure* structu
     
     unsigned namedLength = table->length();
     for (unsigned i = namedLength; i < totalLength; ++i)
-        result->overflowStorage()[i - namedLength].set(vm, result, argumentsStart[i].jsValue());
+        result->storage()[i - namedLength].set(vm, result, argumentsStart[i].jsValue());
     
     return result;
 }
@@ -114,11 +109,10 @@ void ScopedArguments::visitChildren(JSCell* cell, SlotVisitor& visitor)
     visitor.append(thisObject->m_table);
     visitor.append(thisObject->m_scope);
     
-    visitor.markAuxiliary(&thisObject->storageHeader());
-    
-    if (thisObject->storageHeader().totalLength > thisObject->m_table->length()) {
-        visitor.appendValues(
-            thisObject->overflowStorage(), thisObject->storageHeader().totalLength - thisObject->m_table->length());
+    if (WriteBarrier<Unknown>* storage = thisObject->m_storage.get()) {
+        visitor.markAuxiliary(storage);
+        if (thisObject->m_totalLength > thisObject->m_table->length())
+            visitor.appendValues(storage, thisObject->m_totalLength - thisObject->m_table->length());
     }
 }
 
@@ -129,29 +123,29 @@ Structure* ScopedArguments::createStructure(VM& vm, JSGlobalObject* globalObject
 
 void ScopedArguments::overrideThings(VM& vm)
 {
-    RELEASE_ASSERT(!storageHeader().overrodeThings);
+    RELEASE_ASSERT(!m_overrodeThings);
     
     putDirect(vm, vm.propertyNames->length, jsNumber(m_table->length()), static_cast<unsigned>(PropertyAttribute::DontEnum));
     putDirect(vm, vm.propertyNames->callee, m_callee.get(), static_cast<unsigned>(PropertyAttribute::DontEnum));
     putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject(vm)->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
     
-    storageHeader().overrodeThings = true;
+    m_overrodeThings = true;
 }
 
 void ScopedArguments::overrideThingsIfNecessary(VM& vm)
 {
-    if (!storageHeader().overrodeThings)
+    if (!m_overrodeThings)
         overrideThings(vm);
 }
 
 void ScopedArguments::unmapArgument(VM& vm, uint32_t i)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(i < storageHeader().totalLength);
+    ASSERT_WITH_SECURITY_IMPLICATION(i < m_totalLength);
     unsigned namedLength = m_table->length();
     if (i < namedLength)
         m_table.set(vm, this, m_table->set(vm, i, ScopeOffset()));
     else
-        overflowStorage()[i - namedLength].clear();
+        storage()[i - namedLength].clear();
 }
 
 void ScopedArguments::copyToArguments(JSGlobalObject* globalObject, CallFrame* callFrame, VirtualRegister firstElementDest, unsigned offset, unsigned length)
index fd1844d..f85e47d 100644 (file)
@@ -38,16 +38,16 @@ namespace JSC {
 // lookups.
 class ScopedArguments final : public GenericArguments<ScopedArguments> {
 private:
-    ScopedArguments(VM&, Structure*, WriteBarrier<Unknown>* storage);
+    ScopedArguments(VM&, Structure*, WriteBarrier<Unknown>* storage, unsigned totalLength);
     void finishCreation(VM&, JSFunction* callee, ScopedArgumentsTable*, JSLexicalEnvironment*);
     using Base = GenericArguments<ScopedArguments>;
 
 public:
     template<typename CellType, SubspaceAccess>
-    static CompleteSubspace* subspaceFor(VM& vm)
+    static IsoSubspace* subspaceFor(VM& vm)
     {
         static_assert(!CellType::needsDestruction, "");
-        return &vm.variableSizedCellSpace;
+        return &vm.scopedArgumentsSpace;
     }
 
     // Creates an arguments object but leaves it uninitialized. This is dangerous if we GC right
@@ -68,14 +68,14 @@ public:
     
     uint32_t internalLength() const
     {
-        return storageHeader().totalLength;
+        return m_totalLength;
     }
     
     uint32_t length(JSGlobalObject* globalObject) const
     {
         VM& vm = getVM(globalObject);
         auto scope = DECLARE_THROW_SCOPE(vm);
-        if (UNLIKELY(storageHeader().overrodeThings)) {
+        if (UNLIKELY(m_overrodeThings)) {
             auto value = get(globalObject, vm.propertyNames->length);
             RETURN_IF_EXCEPTION(scope, 0);
             RELEASE_AND_RETURN(scope, value.toUInt32(globalObject));
@@ -85,13 +85,12 @@ public:
     
     bool isMappedArgument(uint32_t i) const
     {
-        WriteBarrier<Unknown>* storage = overflowStorage();
-        if (i >= storageHeader(storage).totalLength)
+        if (i >= m_totalLength)
             return false;
         unsigned namedLength = m_table->length();
         if (i < namedLength)
             return !!m_table->get(i);
-        return !!storage[i - namedLength].get();
+        return !!storage()[i - namedLength].get();
     }
 
     bool isMappedArgumentInDFG(uint32_t i) const
@@ -102,24 +101,20 @@ public:
     JSValue getIndexQuickly(uint32_t i) const
     {
         ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
-        WriteBarrier<Unknown>* storage = overflowStorage();
-        unsigned totalLength = storageHeader(storage).totalLength;
         unsigned namedLength = m_table->length();
         if (i < namedLength)
-            return preciseIndexMaskPtr(i, totalLength, &m_scope->variableAt(m_table->get(i)))->get();
-        return preciseIndexMaskPtr(i, totalLength, storage + (i - namedLength))->get();
+            return m_scope->variableAt(m_table->get(i)).get();
+        return storage()[i - namedLength].get();
     }
 
     void setIndexQuickly(VM& vm, uint32_t i, JSValue value)
     {
         ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
-        WriteBarrier<Unknown>* storage = overflowStorage();
-        unsigned totalLength = storageHeader(storage).totalLength;
         unsigned namedLength = m_table->length();
         if (i < namedLength)
-            preciseIndexMaskPtr(i, totalLength, &m_scope->variableAt(m_table->get(i)))->set(vm, m_scope.get(), value);
+            m_scope->variableAt(m_table->get(i)).set(vm, m_scope.get(), value);
         else
-            preciseIndexMaskPtr(i, totalLength, storage + (i - namedLength))->set(vm, this, value);
+            storage()[i - namedLength].set(vm, this, value);
     }
 
     JSFunction* callee()
@@ -127,7 +122,7 @@ public:
         return m_callee.get();
     }
 
-    bool overrodeThings() const { return storageHeader().overrodeThings; }
+    bool overrodeThings() const { return m_overrodeThings; }
     void overrideThings(VM&);
     void overrideThingsIfNecessary(VM&);
     void unmapArgument(VM&, uint32_t index);
@@ -153,47 +148,20 @@ public:
     
     static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
     
-    static ptrdiff_t offsetOfStorage() { return OBJECT_OFFSETOF(ScopedArguments, m_storage); }
-    static ptrdiff_t offsetOfOverrodeThingsInStorage() { return OBJECT_OFFSETOF(StorageHeader, overrodeThings) - sizeof(WriteBarrier<Unknown>); }
-    static ptrdiff_t offsetOfTotalLengthInStorage() { return OBJECT_OFFSETOF(StorageHeader, totalLength) - sizeof(WriteBarrier<Unknown>); }
+    static ptrdiff_t offsetOfOverrodeThings() { return OBJECT_OFFSETOF(ScopedArguments, m_overrodeThings); }
+    static ptrdiff_t offsetOfTotalLength() { return OBJECT_OFFSETOF(ScopedArguments, m_totalLength); }
     static ptrdiff_t offsetOfTable() { return OBJECT_OFFSETOF(ScopedArguments, m_table); }
     static ptrdiff_t offsetOfScope() { return OBJECT_OFFSETOF(ScopedArguments, m_scope); }
-    
-    static size_t allocationSize(size_t inlineSize)
-    {
-        RELEASE_ASSERT(!inlineSize);
-        return sizeof(ScopedArguments);
-    }
-    
-    static size_t storageSize(Checked<size_t> capacity)
-    {
-        return (sizeof(WriteBarrier<Unknown>) * (capacity + static_cast<size_t>(1))).unsafeGet();
-    }
-    
-    static size_t storageHeaderSize() { return sizeof(WriteBarrier<Unknown>); }
+    static ptrdiff_t offsetOfStorage() { return OBJECT_OFFSETOF(ScopedArguments, m_storage); }
     
 private:
-    struct StorageHeader {
-        unsigned totalLength;
-        bool overrodeThings; // True if length, callee, and caller are fully materialized in the object.
-    };
-    
-    WriteBarrier<Unknown>* overflowStorage() const
+    WriteBarrier<Unknown>* storage() const
     {
         return m_storage.get();
     }
     
-    static StorageHeader& storageHeader(WriteBarrier<Unknown>* storage)
-    {
-        static_assert(sizeof(StorageHeader) <= sizeof(WriteBarrier<Unknown>), "StorageHeader needs to be no bigger than a JSValue");
-        return *bitwise_cast<StorageHeader*>(storage - 1);
-    }
-    
-    StorageHeader& storageHeader() const
-    {
-        return storageHeader(overflowStorage());
-    }
-    
+    bool m_overrodeThings { false }; // True if length, callee, and caller are fully materialized in the object.
+    unsigned m_totalLength; // The length of declared plus overflow arguments.
     WriteBarrier<JSFunction> m_callee;
     WriteBarrier<ScopedArgumentsTable> m_table;
     WriteBarrier<JSLexicalEnvironment> m_scope;
index 075c9f8..74d40f4 100644 (file)
 #include "RegisterAtOffsetList.h"
 #include "RuntimeType.h"
 #include "SamplingProfiler.h"
+#include "ScopedArguments.h"
 #include "ShadowChicken.h"
 #include "SimpleTypedArrayController.h"
 #include "SourceProviderCache.h"
@@ -282,6 +283,7 @@ VM::VM(VMType vmType, HeapType heapType)
     , nativeExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), NativeExecutable) // Hash:0x67567f95
     , propertyTableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), PropertyTable) // Hash:0xc6bc9f12
     , ropeStringSpace ISO_SUBSPACE_INIT(heap, stringHeapCellType.get(), JSRopeString)
+    , scopedArgumentsSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), ScopedArguments)
     , sparseArrayValueMapSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), SparseArrayValueMap)
     , stringSpace ISO_SUBSPACE_INIT(heap, stringHeapCellType.get(), JSString) // Hash:0x90cf758f
     , structureRareDataSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), StructureRareData) // Hash:0xaca4e62d
index e6d6cbe..f660654 100644 (file)
@@ -383,6 +383,7 @@ public:
     IsoSubspace nativeExecutableSpace;
     IsoSubspace propertyTableSpace;
     IsoSubspace ropeStringSpace;
+    IsoSubspace scopedArgumentsSpace;
     IsoSubspace sparseArrayValueMapSpace;
     IsoSubspace stringSpace;
     IsoSubspace structureRareDataSpace;
index e98d7d9..6aaa439 100644 (file)
@@ -1,3 +1,17 @@
+2019-11-15  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Remove index-masking on ScopedArguments and put it in IsoSubspace
+        https://bugs.webkit.org/show_bug.cgi?id=204269
+
+        Reviewed by Saam Barati.
+
+        * wtf/MathExtras.h:
+        (WTF::computeIndexingMask): Deleted.
+        (WTF::preciseIndexMaskShiftForSize): Deleted.
+        (WTF::preciseIndexMaskShift): Deleted.
+        (WTF::opaque): Deleted.
+        (WTF::preciseIndexMaskPtr): Deleted.
+
 2019-11-14  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] BlockDirectory's bits should be compact
index fddcbb6..8ec8987 100644 (file)
@@ -564,44 +564,6 @@ inline bool rangesOverlap(T leftMin, T leftMax, T rightMin, T rightMax)
     return nonEmptyRangesOverlap(leftMin, leftMax, rightMin, rightMax);
 }
 
-// This mask is not necessarily the minimal mask, specifically if size is
-// a power of 2. It has the advantage that it's fast to compute, however.
-inline uint32_t computeIndexingMask(uint32_t size)
-{
-    return static_cast<uint64_t>(static_cast<uint32_t>(-1)) >> std::clz(size);
-}
-
-constexpr unsigned preciseIndexMaskShiftForSize(unsigned size)
-{
-    return size * 8 - 1;
-}
-
-template<typename T>
-constexpr unsigned preciseIndexMaskShift()
-{
-    return preciseIndexMaskShiftForSize(sizeof(T));
-}
-
-template<typename T>
-T opaque(T pointer)
-{
-#if !OS(WINDOWS)
-    asm("" : "+r"(pointer));
-#endif
-    return pointer;
-}
-
-// This masks the given pointer with 0xffffffffffffffff (ptrwidth) if `index <
-// length`. Otherwise, it masks the pointer with 0. Similar to Linux kernel's array_ptr.
-template<typename T>
-inline T* preciseIndexMaskPtr(uintptr_t index, uintptr_t length, T* value)
-{
-    uintptr_t result = bitwise_cast<uintptr_t>(value) & static_cast<uintptr_t>(
-        static_cast<intptr_t>(index - opaque(length)) >>
-        static_cast<intptr_t>(preciseIndexMaskShift<T*>()));
-    return bitwise_cast<T*>(result);
-}
-
 template<typename VectorType, typename RandomFunc>
 void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc)
 {
@@ -735,10 +697,6 @@ constexpr unsigned getMSBSetConstexpr(T t)
 
 } // namespace WTF
 
-using WTF::opaque;
-using WTF::preciseIndexMaskPtr;
-using WTF::preciseIndexMaskShift;
-using WTF::preciseIndexMaskShiftForSize;
 using WTF::shuffleVector;
 using WTF::clz;
 using WTF::ctz;