Changeset 229518 in webkit

Timestamp:

Mar 11, 2018 2:09:20 PM (6 years ago)

Author:

fpizlo@apple.com

Message:

Split DirectArguments into JSValueOOB and JSValueStrict parts
​https://bugs.webkit.org/show_bug.cgi?id=183458

Reviewed by Yusuke Suzuki.
Source/JavaScriptCore:

Our Spectre plan for JSValue objects is to allow inline JSValue stores and loads guarded by
unmitigated structure checks. This works because objects reachable from JSValues (i.e. JSValue
objects, like String, Symbol, and any descendant of JSObject) will only contain fields that it's OK
to read and write within a Spectre mitigation window. Writes are important, because within the
window, a write could appear to be made speculatively and rolled out later. This means that:

• JSValue objects cannot have lengths, masks, or anything else inline.

• JSValue objects cannot have an inline type that is used as part of a Spectre mitigation for a type check, unless that type is in the form of a poison key.

This means that the dynamic poisoning that I previously landed for DirectArguments is wrong. It also
means that it's wrong for DirectArguments to have an inline length.

This changes DirectArguments to use poisoning according to the universal formula:

• The random accessed portions are out-of-line, pointed to by a poisoned pointer.

• No inline length.

Surprisingly, this is perf-neutral. It's probably perf-neutral because our compiler optimizations
amortize whatever cost there was.

• bytecode/AccessCase.cpp:

(JSC::AccessCase::generateWithGuard):

• dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:

(JSC::DFG::CallCreateDirectArgumentsSlowPathGenerator::CallCreateDirectArgumentsSlowPathGenerator):

• dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h: Added.

(JSC::DFG::CallCreateDirectArgumentsWithKnownLengthSlowPathGenerator::CallCreateDirectArgumentsWithKnownLengthSlowPathGenerator):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
(JSC::DFG::SpeculativeJIT::compileGetArrayLength):
(JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
(JSC::DFG::SpeculativeJIT::compileGetFromArguments):
(JSC::DFG::SpeculativeJIT::compilePutToArguments):

• ftl/FTLAbstractHeapRepository.h:
• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
(JSC::FTL::DFG::LowerDFGToB3::compileGetFromArguments):
(JSC::FTL::DFG::LowerDFGToB3::compilePutToArguments):
(JSC::FTL::DFG::LowerDFGToB3::compileCheckSubClass):
(JSC::FTL::DFG::LowerDFGToB3::allocateVariableSizedHeapCell):
(JSC::FTL::DFG::LowerDFGToB3::dynamicPoison): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::dynamicPoisonOnLoadedType): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::dynamicPoisonOnType): Deleted.

• heap/SecurityKind.h:
• jit/JITPropertyAccess.cpp:

(JSC::JIT::emit_op_get_from_arguments):
(JSC::JIT::emit_op_put_to_arguments):
(JSC::JIT::emitDirectArgumentsGetByVal):

• jit/JITPropertyAccess32_64.cpp:

(JSC::JIT::emit_op_get_from_arguments):
(JSC::JIT::emit_op_put_to_arguments):

• llint/LowLevelInterpreter.asm:
• llint/LowLevelInterpreter32_64.asm:
• llint/LowLevelInterpreter64.asm:
• runtime/DirectArguments.cpp:

(JSC::DirectArguments::DirectArguments):
(JSC::DirectArguments::createUninitialized):
(JSC::DirectArguments::create):
(JSC::DirectArguments::createByCopying):
(JSC::DirectArguments::estimatedSize):
(JSC::DirectArguments::visitChildren):
(JSC::DirectArguments::overrideThings):
(JSC::DirectArguments::copyToArguments):
(JSC::DirectArguments::mappedArgumentsSize):

• runtime/DirectArguments.h:
• runtime/JSCPoison.h:
• runtime/JSLexicalEnvironment.h:
• runtime/JSSymbolTableObject.h:
• runtime/VM.cpp:

(JSC::VM::VM):

• runtime/VM.h:

Source/WTF:

• wtf/MathExtras.h:

(WTF::dynamicPoison): Deleted.

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj (modified) (4 diffs)
• JavaScriptCore/bytecode/AccessCase.cpp (modified) (1 diff)
• JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h (modified) (4 diffs)
• JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h (added)
• JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (10 diffs)
• JavaScriptCore/ftl/FTLAbstractHeapRepository.h (modified) (2 diffs)
• JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (11 diffs)
• JavaScriptCore/heap/SecurityKind.h (modified) (2 diffs)
• JavaScriptCore/jit/JITPropertyAccess.cpp (modified) (3 diffs)
• JavaScriptCore/jit/JITPropertyAccess32_64.cpp (modified) (2 diffs)
• JavaScriptCore/llint/LowLevelInterpreter.asm (modified) (1 diff)
• JavaScriptCore/llint/LowLevelInterpreter32_64.asm (modified) (2 diffs)
• JavaScriptCore/llint/LowLevelInterpreter64.asm (modified) (2 diffs)
• JavaScriptCore/runtime/DirectArguments.cpp (modified) (10 diffs)
• JavaScriptCore/runtime/DirectArguments.h (modified) (9 diffs)
• JavaScriptCore/runtime/JSCPoison.h (modified) (1 diff)
• JavaScriptCore/runtime/JSLexicalEnvironment.h (modified) (1 diff)
• JavaScriptCore/runtime/JSSymbolTableObject.h (modified) (1 diff)
• JavaScriptCore/runtime/VM.cpp (modified) (1 diff)
• JavaScriptCore/runtime/VM.h (modified) (1 diff)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/MathExtras.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-03-09  Filip Pizlo  <fpizlo@apple.com>
+
+        Split DirectArguments into JSValueOOB and JSValueStrict parts
+        https://bugs.webkit.org/show_bug.cgi?id=183458
+
+        Reviewed by Yusuke Suzuki.
+        
+        Our Spectre plan for JSValue objects is to allow inline JSValue stores and loads guarded by
+        unmitigated structure checks. This works because objects reachable from JSValues (i.e. JSValue
+        objects, like String, Symbol, and any descendant of JSObject) will only contain fields that it's OK
+        to read and write within a Spectre mitigation window. Writes are important, because within the
+        window, a write could appear to be made speculatively and rolled out later. This means that:
+        
+        - JSValue objects cannot have lengths, masks, or anything else inline.
+        
+        - JSValue objects cannot have an inline type that is used as part of a Spectre mitigation for a type
+          check, unless that type is in the form of a poison key.
+        
+        This means that the dynamic poisoning that I previously landed for DirectArguments is wrong. It also
+        means that it's wrong for DirectArguments to have an inline length.
+        
+        This changes DirectArguments to use poisoning according to the universal formula:
+        
+        - The random accessed portions are out-of-line, pointed to by a poisoned pointer.
+        
+        - No inline length.
+        
+        Surprisingly, this is perf-neutral. It's probably perf-neutral because our compiler optimizations
+        amortize whatever cost there was.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateWithGuard):
+        * dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:
+        (JSC::DFG::CallCreateDirectArgumentsSlowPathGenerator::CallCreateDirectArgumentsSlowPathGenerator):
+        * dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h: Added.
+        (JSC::DFG::CallCreateDirectArgumentsWithKnownLengthSlowPathGenerator::CallCreateDirectArgumentsWithKnownLengthSlowPathGenerator):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
+        (JSC::DFG::SpeculativeJIT::compileGetArrayLength):
+        (JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
+        (JSC::DFG::SpeculativeJIT::compileGetFromArguments):
+        (JSC::DFG::SpeculativeJIT::compilePutToArguments):
+        * ftl/FTLAbstractHeapRepository.h:
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetFromArguments):
+        (JSC::FTL::DFG::LowerDFGToB3::compilePutToArguments):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCheckSubClass):
+        (JSC::FTL::DFG::LowerDFGToB3::allocateVariableSizedHeapCell):
+        (JSC::FTL::DFG::LowerDFGToB3::dynamicPoison): Deleted.
+        (JSC::FTL::DFG::LowerDFGToB3::dynamicPoisonOnLoadedType): Deleted.
+        (JSC::FTL::DFG::LowerDFGToB3::dynamicPoisonOnType): Deleted.
+        * heap/SecurityKind.h:
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emit_op_get_from_arguments):
+        (JSC::JIT::emit_op_put_to_arguments):
+        (JSC::JIT::emitDirectArgumentsGetByVal):
+        * jit/JITPropertyAccess32_64.cpp:
+        (JSC::JIT::emit_op_get_from_arguments):
+        (JSC::JIT::emit_op_put_to_arguments):
+        * llint/LowLevelInterpreter.asm:
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/DirectArguments.cpp:
+        (JSC::DirectArguments::DirectArguments):
+        (JSC::DirectArguments::createUninitialized):
+        (JSC::DirectArguments::create):
+        (JSC::DirectArguments::createByCopying):
+        (JSC::DirectArguments::estimatedSize):
+        (JSC::DirectArguments::visitChildren):
+        (JSC::DirectArguments::overrideThings):
+        (JSC::DirectArguments::copyToArguments):
+        (JSC::DirectArguments::mappedArgumentsSize):
+        * runtime/DirectArguments.h:
+        * runtime/JSCPoison.h:
+        * runtime/JSLexicalEnvironment.h:
+        * runtime/JSSymbolTableObject.h:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+
 2018-03-11  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -162 +162 @@
                 0F25F1B2181635F300522F39 /* FTLSlowPathCall.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F25F1AB181635F300522F39 /* FTLSlowPathCall.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 0F25F1B4181635F300522F39 /* FTLSlowPathCallKey.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F25F1AD181635F300522F39 /* FTLSlowPathCallKey.h */; settings = {ATTRIBUTES = (Private, ); }; };
+                0F26A7A32053895C0090A141 /* DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F26A7A2205389580090A141 /* DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h */; };
                 0F2AC5671E8A0B790001EE3F /* AirFixSpillsAfterTerminals.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F2AC5651E8A0B760001EE3F /* AirFixSpillsAfterTerminals.h */; };
                 0F2AC56B1E8A0BD50001EE3F /* AirAllocateRegistersAndStackByLinearScan.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F2AC5691E8A0BD10001EE3F /* AirAllocateRegistersAndStackByLinearScan.h */; };
@@ -2060 +2061 @@
                 0F25F1AC181635F300522F39 /* FTLSlowPathCallKey.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = FTLSlowPathCallKey.cpp; path = ftl/FTLSlowPathCallKey.cpp; sourceTree = "<group>"; };
                 0F25F1AD181635F300522F39 /* FTLSlowPathCallKey.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = FTLSlowPathCallKey.h; path = ftl/FTLSlowPathCallKey.h; sourceTree = "<group>"; };
+                0F26A7A2205389580090A141 /* DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h; path = dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h; sourceTree = "<group>"; };
                 0F275F2C1ECE079600620D47 /* Intrinsic.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Intrinsic.cpp; sourceTree = "<group>"; };
                 0F2AC5641E8A0B760001EE3F /* AirFixSpillsAfterTerminals.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = AirFixSpillsAfterTerminals.cpp; path = b3/air/AirFixSpillsAfterTerminals.cpp; sourceTree = "<group>"; };
@@ -7134 +7136 @@
                                 0F256C341627B0AA007F2783 /* DFGCallArrayAllocatorSlowPathGenerator.h */,
                                 0FBDB9AC1AB0FBC6000B57E5 /* DFGCallCreateDirectArgumentsSlowPathGenerator.h */,
+                                0F26A7A2205389580090A141 /* DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h */,
                                 0FD82E1E14172C2F00179C94 /* DFGCapabilities.cpp */,
                                 0FD82E1F14172C2F00179C94 /* DFGCapabilities.h */,
@@ -8747 +8750 @@
                                 52B310FB1974AE610080857C /* FunctionHasExecutedCache.h in Headers */,
                                 FE4BFF2C1AD476E700088F87 /* FunctionOverrides.h in Headers */,
+                                0F26A7A32053895C0090A141 /* DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h in Headers */,
                                 BC18C4050E16F5CD00B34460 /* FunctionPrototype.h in Headers */,
                                 62D2D3901ADF103F000206C1 /* FunctionRareData.h in Headers */,

trunk/Source/JavaScriptCore/bytecode/AccessCase.cpp

@@ -384 +384 @@
                 CCallHelpers::NonZero,
                 CCallHelpers::Address(baseGPR, DirectArguments::offsetOfMappedArguments())));
+        jit.loadPtr(
+            CCallHelpers::Address(baseGPR, DirectArguments::offsetOfStorage()),
+            valueRegs.payloadGPR());
+        jit.xorPtr(CCallHelpers::TrustedImmPtr(DirectArgumentsPoison::key()), valueRegs.payloadGPR());
         jit.load32(
-            CCallHelpers::Address(baseGPR, DirectArguments::offsetOfLength()),
+            CCallHelpers::Address(valueRegs.payloadGPR(), DirectArguments::offsetOfLengthInStorage()),
             valueRegs.payloadGPR());
         jit.boxInt32(valueRegs.payloadGPR(), valueRegs);

trunk/Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -35 +35 @@
 namespace JSC { namespace DFG {
 
-// This calls operationCreateDirectArguments but then restores the value of lengthGPR.
+// This calls operationCreateDirectArguments but then restores the value of lengthGPR and storageGPR.
 class CallCreateDirectArgumentsSlowPathGenerator : public JumpingSlowPathGenerator<MacroAssembler::JumpList> {
 public:
     CallCreateDirectArgumentsSlowPathGenerator(
         MacroAssembler::JumpList from, SpeculativeJIT* jit, GPRReg resultGPR, RegisteredStructure structure,
-        GPRReg lengthGPR, unsigned minCapacity)
+        GPRReg lengthGPR, GPRReg storageGPR, unsigned minCapacity)
         : JumpingSlowPathGenerator<MacroAssembler::JumpList>(from, jit)
         , m_resultGPR(resultGPR)
         , m_structure(structure)
         , m_lengthGPR(lengthGPR)
+        , m_storageGPR(storageGPR)
         , m_minCapacity(minCapacity)
     {
@@ -62 +63 @@
         jit->m_jit.exceptionCheck();
         jit->m_jit.loadPtr(
-            MacroAssembler::Address(m_resultGPR, DirectArguments::offsetOfLength()), m_lengthGPR);
+            MacroAssembler::Address(m_resultGPR, DirectArguments::offsetOfStorage()), m_storageGPR);
+        jit->m_jit.xorPtr(
+            MacroAssembler::TrustedImmPtr(DirectArgumentsPoison::key()), m_storageGPR);
+        jit->m_jit.load32(
+            MacroAssembler::Address(m_storageGPR, DirectArguments::offsetOfLengthInStorage()), m_lengthGPR);
         jumpTo(jit);
     }
@@ -70 +75 @@
     RegisteredStructure m_structure;
     GPRReg m_lengthGPR;
+    GPRReg m_storageGPR;
     unsigned m_minCapacity;
     Vector<SilentRegisterSavePlan, 2> m_plans;

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -34 +34 @@
 #include "DFGCallArrayAllocatorSlowPathGenerator.h"
 #include "DFGCallCreateDirectArgumentsSlowPathGenerator.h"
+#include "DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h"
 #include "DFGCapabilities.h"
 #include "DFGMayExit.h"
@@ -6426 +6427 @@
 #endif
     GPRTemporary scratch(this);
+    GPRTemporary storage(this);
     
     GPRReg baseReg = base.gpr();
@@ -6437 +6439 @@
 #endif
     GPRReg scratchReg = scratch.gpr();
+    GPRReg storageReg = storage.gpr();
     
     if (!m_compileOkay)
@@ -6448 +6451 @@
             MacroAssembler::NonZero,
             MacroAssembler::Address(baseReg, DirectArguments::offsetOfMappedArguments())));
-
-    m_jit.load32(CCallHelpers::Address(baseReg, DirectArguments::offsetOfLength()), scratchReg);
+    
+    m_jit.loadPtr(CCallHelpers::Address(baseReg, DirectArguments::offsetOfStorage()), storageReg);
+    m_jit.xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), storageReg);
+    
+    m_jit.load32(CCallHelpers::Address(storageReg, DirectArguments::offsetOfLengthInStorage()), scratchReg);
     auto isOutOfBounds = m_jit.branch32(CCallHelpers::AboveOrEqual, propertyReg, scratchReg);
     if (node->arrayMode().isInBounds())
         speculationCheck(OutOfBounds, JSValueSource(), 0, isOutOfBounds);
     
-    m_jit.emitDynamicPoisonOnType(baseReg, resultReg, DirectArgumentsType);
     m_jit.emitPreparePreciseIndexMask32(propertyReg, scratchReg, scratchReg);
 
     m_jit.loadValue(
         MacroAssembler::BaseIndex(
-            baseReg, propertyReg, MacroAssembler::TimesEight, DirectArguments::storageOffset()),
+            storageReg, propertyReg, MacroAssembler::TimesEight),
         resultRegs);
     
@@ -6644 +6649 @@
                 MacroAssembler::Address(baseReg, DirectArguments::offsetOfMappedArguments())));
         
+        m_jit.loadPtr(
+            MacroAssembler::Address(baseReg, DirectArguments::offsetOfStorage()), resultReg);
+        m_jit.xorPtr(
+            TrustedImmPtr(DirectArgumentsPoison::key()), resultReg);
         m_jit.load32(
-            MacroAssembler::Address(baseReg, DirectArguments::offsetOfLength()), resultReg);
+            MacroAssembler::Address(resultReg, DirectArguments::offsetOfLengthInStorage()), resultReg);
         
         int32Result(resultReg, node);
@@ -6968 +6977 @@
 void SpeculativeJIT::compileCreateDirectArguments(Node* node)
 {
-    // FIXME: A more effective way of dealing with the argument count and callee is to have
-    // them be explicit arguments to this node.
-    // https://bugs.webkit.org/show_bug.cgi?id=142207
-    
-    GPRTemporary result(this);
-    GPRTemporary scratch1(this);
-    GPRTemporary scratch2(this);
-    GPRTemporary length;
-    GPRReg resultGPR = result.gpr();
-    GPRReg scratch1GPR = scratch1.gpr();
-    GPRReg scratch2GPR = scratch2.gpr();
-    GPRReg lengthGPR = InvalidGPRReg;
-    JSValueRegs valueRegs = JSValueRegs::withTwoAvailableRegs(scratch1GPR, scratch2GPR);
-        
+    VM& vm = *m_jit.vm();
+    
+    bool lengthIsKnown = node->origin.semantic.inlineCallFrame
+        && !node->origin.semantic.inlineCallFrame->isVarargs();
+    unsigned knownLength = UINT_MAX;
+    
+    RegisteredStructure structure =
+        m_jit.graph().registerStructure(m_jit.graph().globalObjectFor(node->origin.semantic)->directArgumentsStructure());
     unsigned minCapacity = m_jit.graph().baselineCodeBlockFor(node->origin.semantic)->numParameters() - 1;
-        
-    unsigned knownLength;
-    bool lengthIsKnown; // if false, lengthGPR will have the length.
-    if (node->origin.semantic.inlineCallFrame
-        && !node->origin.semantic.inlineCallFrame->isVarargs()) {
-        knownLength = node->origin.semantic.inlineCallFrame->argumentCountIncludingThis - 1;
-        lengthIsKnown = true;
-    } else {
-        knownLength = UINT_MAX;
-        lengthIsKnown = false;
-            
-        GPRTemporary realLength(this);
-        length.adopt(realLength);
-        lengthGPR = length.gpr();
-
+    
+    GPRReg resultGPR;
+    GPRReg lengthGPR;
+    GPRReg scratch1GPR;
+    GPRReg scratch2GPR;
+    GPRReg storageGPR;
+    JSValueRegs valueRegs;
+    
+    auto loadLength = [&] () {
         VirtualRegister argumentCountRegister = m_jit.argumentCount(node->origin.semantic);
         m_jit.load32(JITCompiler::payloadFor(argumentCountRegister), lengthGPR);
         m_jit.sub32(TrustedImm32(1), lengthGPR);
-    }
-        
-    RegisteredStructure structure =
-        m_jit.graph().registerStructure(m_jit.graph().globalObjectFor(node->origin.semantic)->directArgumentsStructure());
-        
-    // Use a different strategy for allocating the object depending on whether we know its
-    // size statically.
-    JITCompiler::JumpList slowPath;
-    if (lengthIsKnown) {
+    };
+        
+    GPRTemporary result;
+    GPRTemporary scratch1;
+    GPRTemporary scratch2;
+    GPRTemporary storage;
+    GPRTemporary length;
+
+    if (isX86() && is32Bit() && !lengthIsKnown) {
+        GPRFlushedCallResult result(this);
+        resultGPR = result.gpr();
+        RELEASE_ASSERT(resultGPR == GPRInfo::regT0);
+        flushRegisters();
+        lengthGPR = GPRInfo::regT1; // Can be anything we like because registers are flushed.
+        scratch1GPR = GPRInfo::regT2;
+        scratch2GPR = GPRInfo::regT3;
+        storageGPR = GPRInfo::regT4;
+        valueRegs = JSValueRegs::withTwoAvailableRegs(scratch1GPR, scratch2GPR);
+        loadLength();
+        callOperation(operationCreateDirectArguments, resultGPR, structure, lengthGPR, minCapacity);
+        m_jit.exceptionCheck();
+        m_jit.loadPtr(MacroAssembler::Address(resultGPR, DirectArguments::offsetOfStorage()), storageGPR);
+        m_jit.xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), storageGPR);
+        m_jit.load32(MacroAssembler::Address(storageGPR, DirectArguments::offsetOfLengthInStorage()), lengthGPR);
+    } else {
+        // FIXME: A more effective way of dealing with the argument count and callee is to have
+        // them be explicit arguments to this node.
+        // https://bugs.webkit.org/show_bug.cgi?id=142207
+    
+        GPRTemporary realResult(this);
+        result.adopt(realResult);
+        GPRTemporary realScratch1(this);
+        scratch1.adopt(realScratch1);
+        GPRTemporary realScratch2(this);
+        scratch2.adopt(realScratch2);
+        GPRTemporary realStorage(this);
+        storage.adopt(realStorage);
+        resultGPR = result.gpr();
+        scratch1GPR = scratch1.gpr();
+        scratch2GPR = scratch2.gpr();
+        lengthGPR = InvalidGPRReg;
+        storageGPR = storage.gpr();
+        valueRegs = JSValueRegs::withTwoAvailableRegs(scratch1GPR, scratch2GPR);
+        
+        if (lengthIsKnown)
+            knownLength = node->origin.semantic.inlineCallFrame->argumentCountIncludingThis - 1;
+        else {
+            GPRTemporary realLength(this);
+            length.adopt(realLength);
+            lengthGPR = length.gpr();
+            
+            loadLength();
+        }
+        
+        // Use a different strategy for allocating the object depending on whether we know its
+        // size statically.
+        JITCompiler::JumpList slowPath;
+        if (lengthIsKnown) {
+            JITAllocator storageAllocator = JITAllocator::constant(
+                vm.jsValueGigacageAuxiliarySpace.allocatorForNonVirtual(
+                    DirectArguments::storageSize(std::max(knownLength, minCapacity)),
+                    AllocatorForMode::AllocatorIfExists));
+        
+            m_jit.emitAllocate(storageGPR, storageAllocator, scratch1GPR, scratch2GPR, slowPath);
+            m_jit.addPtr(TrustedImmPtr(DirectArguments::storageHeaderSize()), storageGPR);
+            m_jit.store32(
+                TrustedImm32(knownLength),
+                JITCompiler::Address(storageGPR, DirectArguments::offsetOfLengthInStorage()));
+        } else {
+            JITCompiler::Jump tooFewArguments;
+            if (minCapacity) {
+                tooFewArguments =
+                    m_jit.branch32(JITCompiler::Below, lengthGPR, TrustedImm32(minCapacity));
+            }
+            m_jit.lshift32(lengthGPR, TrustedImm32(3), scratch1GPR);
+            m_jit.add32(TrustedImm32(DirectArguments::storageHeaderSize()), scratch1GPR);
+            if (minCapacity) {
+                JITCompiler::Jump done = m_jit.jump();
+                tooFewArguments.link(&m_jit);
+                m_jit.move(TrustedImm32(DirectArguments::storageSize(minCapacity)), scratch1GPR);
+                done.link(&m_jit);
+            }
+        
+            m_jit.emitAllocateVariableSized(
+                storageGPR, vm.jsValueGigacageAuxiliarySpace, scratch1GPR, scratch1GPR, scratch2GPR, slowPath);
+            m_jit.addPtr(TrustedImmPtr(DirectArguments::storageHeaderSize()), storageGPR);
+            m_jit.store32(
+                lengthGPR,
+                JITCompiler::Address(storageGPR, DirectArguments::offsetOfLengthInStorage()));
+        }
+    
+        m_jit.store32(
+            TrustedImm32(minCapacity),
+            JITCompiler::Address(storageGPR, DirectArguments::offsetOfMinCapacityInStorage()));
+    
         auto butterfly = TrustedImmPtr(nullptr);
         auto mask = TrustedImm32(0);
-        emitAllocateJSObjectWithKnownSize<DirectArguments>(
-            resultGPR, TrustedImmPtr(structure), butterfly, mask, scratch1GPR, scratch2GPR,
-            slowPath, DirectArguments::allocationSize(std::max(knownLength, minCapacity)));
-            
-        m_jit.store32(
-            TrustedImm32(knownLength),
-            JITCompiler::Address(resultGPR, DirectArguments::offsetOfLength()));
-    } else {
-        JITCompiler::Jump tooFewArguments;
-        if (minCapacity) {
-            tooFewArguments =
-                m_jit.branch32(JITCompiler::Below, lengthGPR, TrustedImm32(minCapacity));
-        }
-        m_jit.lshift32(lengthGPR, TrustedImm32(3), scratch1GPR);
-        m_jit.add32(TrustedImm32(DirectArguments::storageOffset()), scratch1GPR);
-        if (minCapacity) {
-            JITCompiler::Jump done = m_jit.jump();
-            tooFewArguments.link(&m_jit);
-            m_jit.move(TrustedImm32(DirectArguments::allocationSize(minCapacity)), scratch1GPR);
-            done.link(&m_jit);
-        }
-        
-        emitAllocateVariableSizedJSObject<DirectArguments>(
-            resultGPR, TrustedImmPtr(structure), scratch1GPR, scratch1GPR, scratch2GPR,
-            slowPath);
-            
-        m_jit.store32(
-            lengthGPR, JITCompiler::Address(resultGPR, DirectArguments::offsetOfLength()));
-    }
-        
-    m_jit.store32(
-        TrustedImm32(minCapacity),
-        JITCompiler::Address(resultGPR, DirectArguments::offsetOfMinCapacity()));
-        
-    m_jit.storePtr(
-        TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, DirectArguments::offsetOfMappedArguments()));
-
-    m_jit.storePtr(
-        TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, DirectArguments::offsetOfModifiedArgumentsDescriptor()));
-    
-    if (lengthIsKnown) {
-        addSlowPathGenerator(
-            slowPathCall(
-                slowPath, this, operationCreateDirectArguments, resultGPR, structure,
-                knownLength, minCapacity));
-    } else {
-        auto generator = std::make_unique<CallCreateDirectArgumentsSlowPathGenerator>(
-            slowPath, this, resultGPR, structure, lengthGPR, minCapacity);
+        emitAllocateJSObject<DirectArguments>(resultGPR, TrustedImmPtr(structure), butterfly, mask, scratch1GPR, scratch2GPR, slowPath);
+        m_jit.move(storageGPR, scratch1GPR);
+        m_jit.xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), scratch1GPR);
+        m_jit.storePtr(scratch1GPR, JITCompiler::Address(resultGPR, DirectArguments::offsetOfStorage()));
+        
+        m_jit.storePtr(
+            TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, DirectArguments::offsetOfMappedArguments()));
+
+        m_jit.storePtr(
+            TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, DirectArguments::offsetOfModifiedArgumentsDescriptor()));
+    
+        std::unique_ptr<SlowPathGenerator> generator;
+        if (lengthIsKnown) {
+            generator = std::make_unique<CallCreateDirectArgumentsWithKnownLengthSlowPathGenerator>(
+                slowPath, this, resultGPR, structure, knownLength, storageGPR, minCapacity);
+        } else {
+            generator = std::make_unique<CallCreateDirectArgumentsSlowPathGenerator>(
+                slowPath, this, resultGPR, structure, lengthGPR, storageGPR, minCapacity);
+        }
         addSlowPathGenerator(WTFMove(generator));
     }
-        
+    
     if (node->origin.semantic.inlineCallFrame) {
         if (node->origin.semantic.inlineCallFrame->isClosureCall) {
@@ -7088 +7142 @@
             m_jit.loadValue(JITCompiler::addressFor(start + i), valueRegs);
             m_jit.storeValue(
-                valueRegs, JITCompiler::Address(resultGPR, DirectArguments::offsetOfSlot(i)));
+                valueRegs, JITCompiler::Address(storageGPR, i * sizeof(WriteBarrier<Unknown>)));
         }
     } else {
@@ -7109 +7163 @@
             valueRegs,
             JITCompiler::BaseIndex(
-                resultGPR, lengthGPR, JITCompiler::TimesEight,
-                DirectArguments::storageOffset()));
+                storageGPR, lengthGPR, JITCompiler::TimesEight));
         m_jit.branchTest32(MacroAssembler::NonZero, lengthGPR).linkTo(loop, &m_jit);
         if (done.isSet())
             done.link(&m_jit);
     }
-        
-    m_jit.mutatorFence(*m_jit.vm());
+    
+    m_jit.mutatorFence(vm);
         
     cellResult(resultGPR, node);
@@ -7129 +7182 @@
     JSValueRegs resultRegs = result.regs();
     
-    m_jit.loadValue(JITCompiler::Address(argumentsGPR, DirectArguments::offsetOfSlot(node->capturedArgumentsOffset().offset())), resultRegs);
+    m_jit.loadPtr(JITCompiler::Address(argumentsGPR, DirectArguments::offsetOfStorage()), resultRegs.payloadGPR());
+    m_jit.xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), resultRegs.payloadGPR());
+    m_jit.loadValue(JITCompiler::Address(resultRegs.payloadGPR(), node->capturedArgumentsOffset().offset() * sizeof(WriteBarrier<Unknown>)), resultRegs);
     jsValueResult(resultRegs, node);
 }
@@ -7137 +7192 @@
     SpeculateCellOperand arguments(this, node->child1());
     JSValueOperand value(this, node->child2());
+    GPRTemporary storage(this);
     
     GPRReg argumentsGPR = arguments.gpr();
+    GPRReg storageGPR = storage.gpr();
     JSValueRegs valueRegs = value.jsValueRegs();
     
-    m_jit.storeValue(valueRegs, JITCompiler::Address(argumentsGPR, DirectArguments::offsetOfSlot(node->capturedArgumentsOffset().offset())));
+    m_jit.loadPtr(JITCompiler::Address(argumentsGPR, DirectArguments::offsetOfStorage()), storageGPR);
+    m_jit.xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), storageGPR);
+    m_jit.storeValue(valueRegs, JITCompiler::Address(storageGPR, node->capturedArgumentsOffset().offset() * sizeof(WriteBarrier<Unknown>)));
     noResult(node);
 }

trunk/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h

@@ -52 +52 @@
     macro(CallFrame_callerFrame, CallFrame::callerFrameOffset()) \
     macro(ClassInfo_parentClass, ClassInfo::offsetOfParentClass()) \
+    macro(DirectArguments_Storage_length, DirectArguments::offsetOfLengthInStorage()) \
+    macro(DirectArguments_Storage_minCapacity, DirectArguments::offsetOfMinCapacityInStorage()) \
     macro(DirectArguments_callee, DirectArguments::offsetOfCallee()) \
-    macro(DirectArguments_length, DirectArguments::offsetOfLength()) \
-    macro(DirectArguments_minCapacity, DirectArguments::offsetOfMinCapacity()) \
     macro(DirectArguments_mappedArguments, DirectArguments::offsetOfMappedArguments()) \
     macro(DirectArguments_modifiedArgumentsDescriptor, DirectArguments::offsetOfModifiedArgumentsDescriptor()) \
+    macro(DirectArguments_storage, DirectArguments::offsetOfStorage()) \
     macro(GetterSetter_getter, GetterSetter::offsetOfGetter()) \
     macro(GetterSetter_setter, GetterSetter::offsetOfSetter()) \
@@ -134 +135 @@
     macro(ArrayStorage_vector, ArrayStorage::vectorOffset(), sizeof(WriteBarrier<Unknown>)) \
     macro(CompleteSubspace_allocatorForSizeStep, CompleteSubspace::offsetOfAllocatorForSizeStep(), sizeof(Allocator)) \
-    macro(DirectArguments_storage, DirectArguments::storageOffset(), sizeof(EncodedJSValue)) \
+    macro(DirectArguments_Storage_storage, 0, sizeof(EncodedJSValue)) \
     macro(JSLexicalEnvironment_variables, JSLexicalEnvironment::offsetOfVariables(), sizeof(EncodedJSValue)) \
     macro(JSPropertyNameEnumerator_cachedPropertyNamesVectorContents, 0, sizeof(WriteBarrier<JSString>)) \

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -3674 +3674 @@
                 ExoticObjectMode, noValue(), nullptr,
                 m_out.notNull(m_out.loadPtr(arguments, m_heaps.DirectArguments_mappedArguments)));
-            setInt32(m_out.load32NonNegative(arguments, m_heaps.DirectArguments_length));
+            LValue storage = m_out.bitXor(
+                m_out.loadPtr(lowCell(m_node->child1()), m_heaps.DirectArguments_storage),
+                m_out.constIntPtr(DirectArgumentsPoison::key()));
+            setInt32(m_out.load32NonNegative(storage, m_heaps.DirectArguments_Storage_length));
             return;
         }
@@ -3840 +3843 @@
                 m_out.notNull(m_out.loadPtr(base, m_heaps.DirectArguments_mappedArguments)));
 
-            LValue length = m_out.load32NonNegative(base, m_heaps.DirectArguments_length);
+            LValue storage = m_out.loadPtr(base, m_heaps.DirectArguments_storage);
+            storage = m_out.bitXor(storage, m_out.constIntPtr(DirectArgumentsPoison::key()));
+            
+            LValue length = m_out.load32NonNegative(storage, m_heaps.DirectArguments_Storage_length);
             auto isOutOfBounds = m_out.aboveOrEqual(index, length);
             if (m_node->arrayMode().isInBounds()) {
                 speculate(OutOfBounds, noValue(), nullptr, isOutOfBounds);
                 TypedPointer address = m_out.baseIndex(
-                    m_heaps.DirectArguments_storage, base, m_out.zeroExtPtr(index));
-                setJSValue(m_out.load64(address));
+                    m_heaps.DirectArguments_Storage_storage, storage, m_out.zeroExtPtr(index));
+                setJSValue(preciseIndexMask32(m_out.load64(address), index, length));
                 return;
             }
@@ -3858 +3864 @@
             LBasicBlock lastNext = m_out.appendTo(inBounds, slowCase);
             TypedPointer address = m_out.baseIndex(
-                m_heaps.DirectArguments_storage,
-                dynamicPoisonOnType(base, DirectArgumentsType),
+                m_heaps.DirectArguments_Storage_storage, storage,
                 m_out.zeroExt(index, pointerType()));
             ValueFromBlock fastResult = m_out.anchor(
@@ -5158 +5163 @@
         ArgumentsLength length = getArgumentsLength();
         
+        LValue fastStorage;
         LValue fastObject;
         if (length.isKnown) {
-            fastObject = allocateObject<DirectArguments>(
-                DirectArguments::allocationSize(std::max(length.known, minCapacity)), structure,
-                m_out.intPtrZero, m_out.int32Zero, slowPath);
+            LValue allocator = m_out.constInt32(
+                vm().jsValueGigacageAuxiliarySpace.allocatorForNonVirtual(
+                    DirectArguments::storageSize(std::max(length.known, minCapacity)),
+                    AllocatorForMode::AllocatorIfExists).offset());
+
+            fastStorage = allocateHeapCell(allocator, slowPath);
         } else {
             LValue size = m_out.add(
                 m_out.shl(length.value, m_out.constInt32(3)),
-                m_out.constInt32(DirectArguments::storageOffset()));
+                m_out.constInt32(DirectArguments::storageHeaderSize()));
             
             size = m_out.select(
                 m_out.aboveOrEqual(length.value, m_out.constInt32(minCapacity)),
-                size, m_out.constInt32(DirectArguments::allocationSize(minCapacity)));
-            
-            fastObject = allocateVariableSizedObject<DirectArguments>(
-                m_out.zeroExtPtr(size), structure, m_out.intPtrZero, m_out.int32Zero, slowPath);
-        }
-        
-        m_out.store32(length.value, fastObject, m_heaps.DirectArguments_length);
-        m_out.store32(m_out.constInt32(minCapacity), fastObject, m_heaps.DirectArguments_minCapacity);
+                size, m_out.constInt32(DirectArguments::storageSize(minCapacity)));
+        
+            fastStorage = allocateVariableSizedHeapCell(
+                vm().jsValueGigacageAuxiliarySpace, m_out.zeroExtPtr(size), slowPath);
+        }
+        
+        fastStorage = m_out.add(fastStorage, m_out.constIntPtr(DirectArguments::storageHeaderSize()));
+        m_out.store32(length.value, fastStorage, m_heaps.DirectArguments_Storage_length);
+        m_out.store32(m_out.constInt32(minCapacity), fastStorage, m_heaps.DirectArguments_Storage_minCapacity);
+        
+        fastObject = allocateObject<DirectArguments>(structure, m_out.intPtrZero, m_out.intPtrZero, slowPath);
+ 
+        m_out.storePtr(m_out.bitXor(fastStorage, m_out.constIntPtr(DirectArgumentsPoison::key())), fastObject, m_heaps.DirectArguments_storage);
         m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.DirectArguments_mappedArguments);
         m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.DirectArguments_modifiedArgumentsDescriptor);
         
+        ValueFromBlock fastStorageAnchor = m_out.anchor(fastStorage);
         ValueFromBlock fastResult = m_out.anchor(fastObject);
         m_out.jump(continuation);
@@ -5194 +5209 @@
             }, length.value);
         ValueFromBlock slowResult = m_out.anchor(callResult);
+        ValueFromBlock slowStorage = m_out.anchor(
+            m_out.bitXor(
+                m_out.loadPtr(callResult, m_heaps.DirectArguments_storage),
+                m_out.constIntPtr(DirectArgumentsPoison::key())));
         m_out.jump(continuation);
         
         m_out.appendTo(continuation, lastNext);
+        LValue storage = m_out.phi(pointerType(), fastStorageAnchor, slowStorage);
         LValue result = m_out.phi(pointerType(), fastResult, slowResult);
 
@@ -5206 +5226 @@
                 m_out.store64(
                     m_out.load64(addressFor(start + i)),
-                    result, m_heaps.DirectArguments_storage[i]);
+                    storage, m_heaps.DirectArguments_Storage_storage[i]);
             }
         } else {
@@ -5234 +5254 @@
             m_out.store64(
                 m_out.load64(m_out.baseIndex(m_heaps.variables, stackBase, index)),
-                m_out.baseIndex(m_heaps.DirectArguments_storage, result, index));
+                m_out.baseIndex(m_heaps.DirectArguments_Storage_storage, storage, index));
             ValueFromBlock nextIndex = m_out.anchor(index);
             m_out.addIncomingToPhi(previousIndex, nextIndex);
@@ -6691 +6711 @@
     void compileGetFromArguments()
     {
+        LValue storage = m_out.bitXor(
+            m_out.loadPtr(lowCell(m_node->child1()), m_heaps.DirectArguments_storage),
+            m_out.constIntPtr(DirectArgumentsPoison::key()));
         setJSValue(
             m_out.load64(
-                lowCell(m_node->child1()),
-                m_heaps.DirectArguments_storage[m_node->capturedArgumentsOffset().offset()]));
+                storage,
+                m_heaps.DirectArguments_Storage_storage[m_node->capturedArgumentsOffset().offset()]));
     }
     
     void compilePutToArguments()
     {
+        LValue storage = m_out.bitXor(
+            m_out.loadPtr(lowCell(m_node->child1()), m_heaps.DirectArguments_storage),
+            m_out.constIntPtr(DirectArgumentsPoison::key()));
         m_out.store64(
-            lowJSValue(m_node->child2()),
-            lowCell(m_node->child1()),
-            m_heaps.DirectArguments_storage[m_node->capturedArgumentsOffset().offset()]);
+            lowJSValue(m_node->child2()), storage,
+            m_heaps.DirectArguments_Storage_storage[m_node->capturedArgumentsOffset().offset()]);
     }
 
@@ -11842 +11867 @@
             LValue structure = loadStructure(cell);
             LValue poisonedClassInfo = m_out.loadPtr(structure, m_heaps.Structure_classInfo);
-            LValue classInfo = m_out.bitXor(poisonedClassInfo, m_out.constInt64(GlobalDataPoison::key()));
+            LValue classInfo = m_out.bitXor(poisonedClassInfo, m_out.constIntPtr(GlobalDataPoison::key()));
             ValueFromBlock otherAtStart = m_out.anchor(classInfo);
             m_out.jump(loop);
@@ -12615 +12640 @@
         LValue allocator = allocatorForSize(*subspaceFor<ClassType>(vm()), size, slowPath);
         return allocateCell(allocator, structure, slowPath);
+    }
+    
+    LValue allocateVariableSizedHeapCell(CompleteSubspace& subspace, LValue size, LBasicBlock slowPath)
+    {
+        LValue allocator = allocatorForSize(subspace, size, slowPath);
+        return allocateHeapCell(allocator, slowPath);
     }
     
@@ -15573 +15604 @@
     }
     
-    LValue dynamicPoison(LValue value, LValue poison)
-    {
-        return m_out.add(
-            value,
-            m_out.shl(
-                m_out.zeroExt(poison, pointerType()),
-                m_out.constInt32(40)));
-    }
-    
-    LValue dynamicPoisonOnLoadedType(LValue value, LValue actualType, JSType expectedType)
-    {
-        return dynamicPoison(
-            value,
-            m_out.bitXor(
-                m_out.opaque(actualType),
-                m_out.constInt32(expectedType)));
-    }
-    
-    LValue dynamicPoisonOnType(LValue value, JSType expectedType)
-    {
-        return dynamicPoisonOnLoadedType(
-            value,
-            m_out.load8ZeroExt32(value, m_heaps.JSCell_typeInfoType),
-            expectedType);
-    }
-
     template<typename... Args>
     LValue vmCall(LType type, LValue function, Args&&... args)

trunk/Source/JavaScriptCore/heap/SecurityKind.h

@@ -28 +28 @@
 namespace JSC {
 
+// NOTE: SecurityKind is for distancing. But caging implies distancing. So, things that have their own
+// cages (like typed arrays) don't need to worry about the security kind.
 enum class SecurityKind : uint8_t {
     // The JSValueOOB security kind is for cells that contain JValues and can be accessed out-of-bounds
@@ -42 +44 @@
     // out-of-bounds. Currently, it's not essential to keep this separate from DangerousBits. We're
     // using this to get some wiggle room for how we handle array elements. For example, we might want
-    // to allow OOB reads but not OOB writes.
+    // to allow OOB reads but not OOB writes, since JSValueStrict contains only JSValues and length fields.
+    // Using Spectre to read the length fields is not useful for attackers since they can read them anyway.
+    // So, they will only want to write to length fields, in order to confuse a subsequent bounds check.
+    // They can do that within a speculation window. However, we currently use precise index masking for
+    // this.
     //
     // It's illegal to use this for any subclass of JSObject, JSString, or Symbol, or any other cell

trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

@@ -1101 +1101 @@
     
     emitGetVirtualRegister(arguments, regT0);
-    load64(Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>)), regT0);
+    loadPtr(Address(regT0, DirectArguments::offsetOfStorage()), regT0);
+    xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), regT0);
+    load64(Address(regT0, index * sizeof(WriteBarrier<Unknown>)), regT0);
     emitValueProfilingSite();
     emitPutVirtualRegister(dst);
@@ -1114 +1116 @@
     emitGetVirtualRegister(arguments, regT0);
     emitGetVirtualRegister(value, regT1);
-    store64(regT1, Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>)));
+    loadPtr(Address(regT0, DirectArguments::offsetOfStorage()), regT0);
+    xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), regT0);
+    store64(regT1, Address(regT0, index * sizeof(WriteBarrier<Unknown>)));
 
     emitWriteBarrier(arguments, value, ShouldFilterValue);
@@ -1413 +1417 @@
     load8(Address(base, JSCell::typeInfoTypeOffset()), scratch);
     badType = patchableBranch32(NotEqual, scratch, TrustedImm32(DirectArgumentsType));
-    emitDynamicPoisonOnLoadedType(base, scratch, DirectArgumentsType);
-    
-    load32(Address(base, DirectArguments::offsetOfLength()), scratch2);
+    
+    loadPtr(Address(base, DirectArguments::offsetOfStorage()), scratch);
+    xorPtr(TrustedImmPtr(DirectArgumentsPoison::key()), scratch);
+    
+    load32(Address(scratch, DirectArguments::offsetOfLengthInStorage()), scratch2);
     slowCases.append(branch32(AboveOrEqual, property, scratch2));
     slowCases.append(branchTestPtr(NonZero, Address(base, DirectArguments::offsetOfMappedArguments())));
     
     emitPreparePreciseIndexMask32(property, scratch2, scratch2);
-    loadValue(BaseIndex(base, property, TimesEight, DirectArguments::storageOffset()), result);
+    loadValue(BaseIndex(scratch, property, TimesEight), result);
     andPtr(scratch2, result.payloadGPR());
     

trunk/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp

@@ -1119 +1119 @@
     
     emitLoadPayload(arguments, regT0);
-    load32(Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>) + TagOffset), regT1);
-    load32(Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>) + PayloadOffset), regT0);
+    loadPtr(Address(regT0, DirectArguments::offsetOfStorage()), regT0);
+    load32(Address(regT0, index * sizeof(WriteBarrier<Unknown>) + TagOffset), regT1);
+    load32(Address(regT0, index * sizeof(WriteBarrier<Unknown>) + PayloadOffset), regT0);
     emitValueProfilingSite();
     emitStore(dst, regT1, regT0);
@@ -1134 +1135 @@
     
     emitLoadPayload(arguments, regT0);
+    loadPtr(Address(regT0, DirectArguments::offsetOfStorage()), regT0);
     emitLoad(value, regT1, regT2);
-    store32(regT1, Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>) + TagOffset));
-    store32(regT2, Address(regT0, DirectArguments::storageOffset() + index * sizeof(WriteBarrier<Unknown>) + PayloadOffset));
+    store32(regT1, Address(regT0, index * sizeof(WriteBarrier<Unknown>) + TagOffset));
+    store32(regT2, Address(regT0, index * sizeof(WriteBarrier<Unknown>) + PayloadOffset));
 }
 

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

@@ -165 +165 @@
 
 const JSLexicalEnvironment_variables = (sizeof JSLexicalEnvironment + SlotSize - 1) & ~(SlotSize - 1)
-const DirectArguments_storage = (sizeof DirectArguments + SlotSize - 1) & ~(SlotSize - 1)
 
 const StackAlignment = 16

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

@@ -2497 +2497 @@
     loadi PayloadOffset[cfr, t0, 8], t0
     loadi 12[PC], t1
-    loadi DirectArguments_storage + TagOffset[t0, t1, 8], t2
-    loadi DirectArguments_storage + PayloadOffset[t0, t1, 8], t3
+    loadp DirectArguments::m_storage[t0], t0
+    loadi TagOffset[t0, t1, 8], t2
+    loadi PayloadOffset[t0, t1, 8], t3
     loadisFromInstruction(1, t1)
     valueProfile(t2, t3, 16, t0)
@@ -2513 +2514 @@
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2, t3)
+    loadp DirectArguments::m_storage[t0], t0
     loadi 8[PC], t1
-    storei t2, DirectArguments_storage + TagOffset[t0, t1, 8]
-    storei t3, DirectArguments_storage + PayloadOffset[t0, t1, 8]
+    storei t2, TagOffset[t0, t1, 8]
+    storei t3, PayloadOffset[t0, t1, 8]
     dispatch(4)
 

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -2530 +2530 @@
     loadVariable(2, t0)
     loadi 24[PB, PC, 8], t1
-    loadq DirectArguments_storage[t0, t1, 8], t0
+    loadp DirectArguments::m_storage[t0], t0
+    unpoison(_g_DirectArgumentsPoison, t0, t3)
+    loadq [t0, t1, 8], t0
     valueProfile(t0, 4, t1)
     loadisFromInstruction(1, t1)
@@ -2543 +2545 @@
     loadisFromInstruction(3, t3)
     loadConstantOrVariable(t3, t2)
-    storeq t2, DirectArguments_storage[t0, t1, 8]
+    loadp DirectArguments::m_storage[t0], t0
+    unpoison(_g_DirectArgumentsPoison, t0, t3)
+    storeq t2, [t0, t1, 8]
     writeBarrierOnOperands(1, 3)
     dispatch(constexpr op_put_to_arguments_length)

trunk/Source/JavaScriptCore/runtime/DirectArguments.cpp

@@ -37 +37 @@
 const ClassInfo DirectArguments::s_info = { "Arguments", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(DirectArguments) };
 
-DirectArguments::DirectArguments(VM& vm, Structure* structure, unsigned length, unsigned capacity)
+DirectArguments::DirectArguments(VM& vm, Structure* structure, WriteBarrier<Unknown>* storage)
     : GenericArguments(vm, structure)
-    , m_length(length)
-    , m_minCapacity(capacity)
+    , m_storage(vm, this, storage)
 {
     // When we construct the object from C++ code, we expect the capacity to be at least as large as
     // length. JIT-allocated DirectArguments objects play evil tricks, though.
-    ASSERT(capacity >= length);
+    ASSERT(storageHeader(storage).minCapacity >= storageHeader(storage).length);
 }
 
@@ -50 +49 @@
     VM& vm, Structure* structure, unsigned length, unsigned capacity)
 {
+    void* rawStoragePtr = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(
+        vm, storageSize(capacity), nullptr, AllocationFailureMode::Assert);
+    WriteBarrier<Unknown>* storage = static_cast<WriteBarrier<Unknown>*>(rawStoragePtr) + 1;
+    storageHeader(storage).length = length;
+    storageHeader(storage).minCapacity = capacity;
+    
     DirectArguments* result =
-        new (NotNull, allocateCell<DirectArguments>(vm.heap, allocationSize(capacity)))
-        DirectArguments(vm, structure, length, capacity);
+        new (NotNull, allocateCell<DirectArguments>(vm.heap))
+        DirectArguments(vm, structure, storage);
     result->finishCreation(vm);
     return result;
@@ -60 +65 @@
 {
     DirectArguments* result = createUninitialized(vm, structure, length, capacity);
-    
+
+    WriteBarrier<Unknown>* storage = result->storage();
     for (unsigned i = capacity; i--;)
-        result->storage()[i].clear();
+        storage[i].clear();
     
     return result;
@@ -76 +82 @@
         vm, exec->lexicalGlobalObject()->directArgumentsStructure(), length, capacity);
     
+    WriteBarrier<Unknown>* storage = result->storage();
     for (unsigned i = capacity; i--;)
-        result->storage()[i].set(vm, result, exec->getArgumentUnsafe(i));
+        storage[i].set(vm, result, exec->getArgumentUnsafe(i));
     
     result->callee().set(vm, result, jsCast<JSFunction*>(exec->jsCallee()));
@@ -88 +95 @@
     DirectArguments* thisObject = jsCast<DirectArguments*>(cell);
     size_t mappedArgumentsSize = thisObject->m_mappedArguments ? thisObject->mappedArgumentsSize() * sizeof(bool) : 0;
-    size_t modifiedArgumentsSize = thisObject->m_modifiedArgumentsDescriptor ? thisObject->m_length * sizeof(bool) : 0;
+    size_t modifiedArgumentsSize = thisObject->m_modifiedArgumentsDescriptor ? thisObject->storageHeader().length * sizeof(bool) : 0;
     return Base::estimatedSize(cell) + mappedArgumentsSize + modifiedArgumentsSize;
 }
@@ -98 +105 @@
     Base::visitChildren(thisObject, visitor);
 
-    visitor.appendValues(thisObject->storage(), std::max(thisObject->m_length, thisObject->m_minCapacity));
+    visitor.markAuxiliary(&thisObject->storageHeader());
+    visitor.appendValues(thisObject->storage(), std::max(thisObject->storageHeader().length, thisObject->storageHeader().minCapacity));
+    
     visitor.append(thisObject->m_callee);
 
     if (thisObject->m_mappedArguments)
         visitor.markAuxiliary(thisObject->m_mappedArguments.get());
+    
     GenericArguments<DirectArguments>::visitChildren(thisCell, visitor);
 }
@@ -115 +125 @@
     RELEASE_ASSERT(!m_mappedArguments);
     
-    putDirect(vm, vm.propertyNames->length, jsNumber(m_length), static_cast<unsigned>(PropertyAttribute::DontEnum));
+    putDirect(vm, vm.propertyNames->length, jsNumber(storageHeader().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()->arrayProtoValuesFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
@@ -122 +132 @@
     bool* overrides = static_cast<bool*>(backingStore);
     m_mappedArguments.set(vm, this, overrides);
-    for (unsigned i = m_length; i--;)
+    for (unsigned i = storageHeader().length; i--;)
         overrides[i] = false;
 }
@@ -141 +151 @@
 {
     if (!m_mappedArguments) {
-        unsigned limit = std::min(length + offset, m_length);
+        unsigned limit = std::min(length + offset, storageHeader().length);
         unsigned i;
         VirtualRegister start = firstElementDest - offset;
+        WriteBarrier<Unknown>* storage = this->storage();
         for (i = offset; i < limit; ++i)
-            exec->r(start + i) = storage()[i].get();
+            exec->r(start + i) = storage[i].get();
         for (; i < length; ++i)
             exec->r(start + i) = get(exec, i);
@@ -159 +170 @@
     // still allocate so that m_mappedArguments is non-null. We use that to indicate that the other properties
     // (length, etc) are overridden.
-    return WTF::roundUpToMultipleOf<8>(m_length ? m_length : 1);
+    return WTF::roundUpToMultipleOf<8>(storageHeader().length ? storageHeader().length : 1);
 }
 

trunk/Source/JavaScriptCore/runtime/DirectArguments.h

@@ -33 +33 @@
 namespace JSC {
 
+class LLIntOffsetsExtractor;
+    
 // This is an Arguments-class object that we create when you say "arguments" inside a function,
 // and none of the arguments are captured in the function's activation. The function will copy all
@@ -39 +41 @@
 // being deleted (something like "delete arguments[0]") or reconfigured (broadly, we say deletions
 // and reconfigurations mean that the respective argument was "overridden").
-//
-// To speed allocation, this object will hold all of the arguments in-place. The arguments as well
-// as a table of flags saying which arguments were overridden.
 class DirectArguments final : public GenericArguments<DirectArguments> {
 private:
-    DirectArguments(VM&, Structure*, unsigned length, unsigned capacity);
+    DirectArguments(VM&, Structure*, WriteBarrier<Unknown>* storage);
     
 public:
     template<typename CellType>
-    static CompleteSubspace* subspaceFor(VM& vm)
+    static IsoSubspace* subspaceFor(VM& vm)
     {
         RELEASE_ASSERT(!CellType::needsDestruction);
-        return &vm.jsValueGigacageCellSpace;
+        return &vm.directArgumentsSpace;
     }
 
@@ -70 +69 @@
     uint32_t internalLength() const
     {
-        return m_length;
+        return storageHeader().length;
     }
     
@@ -83 +82 @@
             return value.toUInt32(exec);
         }
-        return m_length;
+        return storageHeader().length;
     }
     
     bool isMappedArgument(uint32_t i) const
     {
-        return i < m_length && (!m_mappedArguments || !m_mappedArguments[i]);
+        return i < storageHeader().length && (!m_mappedArguments || !m_mappedArguments[i]);
     }
 
     bool isMappedArgumentInDFG(uint32_t i) const
     {
-        return i < m_length && !overrodeThings();
-    }
-
+        return i < storageHeader().length && !overrodeThings();
+    }
+    
     JSValue getIndexQuickly(uint32_t i) const
     {
         ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
-        auto* ptr = &const_cast<DirectArguments*>(this)->storage()[i];
-        return preciseIndexMaskPtr(i, m_length, dynamicPoison(type(), DirectArgumentsType, ptr))->get();
+        WriteBarrier<Unknown>* storage = this->storage();
+        auto* ptr = &storage[i];
+        return preciseIndexMaskPtr(i, storageHeader(storage).length, ptr)->get();
     }
     
@@ -106 +106 @@
     {
         ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
-        auto* ptr = &storage()[i];
-        preciseIndexMaskPtr(i, m_length, dynamicPoison(type(), DirectArgumentsType, ptr))->set(vm, this, value);
+        WriteBarrier<Unknown>* storage = this->storage();
+        auto* ptr = &storage[i];
+        preciseIndexMaskPtr(i, storageHeader(storage).length, ptr)->set(vm, this, value);
     }
     
@@ -118 +119 @@
     {
         ASSERT(offset);
-        ASSERT_WITH_SECURITY_IMPLICATION(offset.offset() < std::max(m_length, m_minCapacity));
-        auto* ptr = &storage()[offset.offset()];
-        return *preciseIndexMaskPtr(offset.offset(), std::max(m_length, m_minCapacity), dynamicPoison(type(), DirectArgumentsType, ptr));
+        ASSERT_WITH_SECURITY_IMPLICATION(offset.offset() < std::max(storageHeader().length, storageHeader().minCapacity));
+        WriteBarrier<Unknown>* storage = this->storage();
+        auto* ptr = &storage[offset.offset()];
+        return *preciseIndexMaskPtr(
+            offset.offset(),
+            std::max(storageHeader(storage).length, storageHeader(storage).minCapacity),
+            ptr);
     }
     
@@ -131 +136 @@
     void initModifiedArgumentsDescriptorIfNecessary(VM& vm)
     {
-        GenericArguments<DirectArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, m_length);
+        GenericArguments<DirectArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, storageHeader().length);
     }
 
     void setModifiedArgumentDescriptor(VM& vm, unsigned index)
     {
-        GenericArguments<DirectArguments>::setModifiedArgumentDescriptor(vm, index, m_length);
+        GenericArguments<DirectArguments>::setModifiedArgumentDescriptor(vm, index, storageHeader().length);
     }
 
     bool isModifiedArgumentDescriptor(unsigned index)
     {
-        return GenericArguments<DirectArguments>::isModifiedArgumentDescriptor(index, m_length);
+        return GenericArguments<DirectArguments>::isModifiedArgumentDescriptor(index, storageHeader().length);
     }
 
@@ -151 +156 @@
     
     static ptrdiff_t offsetOfCallee() { return OBJECT_OFFSETOF(DirectArguments, m_callee); }
-    static ptrdiff_t offsetOfLength() { return OBJECT_OFFSETOF(DirectArguments, m_length); }
-    static ptrdiff_t offsetOfMinCapacity() { return OBJECT_OFFSETOF(DirectArguments, m_minCapacity); }
     static ptrdiff_t offsetOfMappedArguments() { return OBJECT_OFFSETOF(DirectArguments, m_mappedArguments); }
     static ptrdiff_t offsetOfModifiedArgumentsDescriptor() { return OBJECT_OFFSETOF(DirectArguments, m_modifiedArgumentsDescriptor); }
-    
-    static size_t storageOffset()
-    {
-        return WTF::roundUpToMultipleOf<sizeof(WriteBarrier<Unknown>)>(sizeof(DirectArguments));
-    }
-    
-    static size_t offsetOfSlot(Checked<size_t> index)
-    {
-        return (storageOffset() + sizeof(WriteBarrier<Unknown>) * index).unsafeGet();
-    }
-    
-    static size_t allocationSize(Checked<size_t> capacity)
-    {
-        return offsetOfSlot(capacity);
+    static ptrdiff_t offsetOfStorage() { return OBJECT_OFFSETOF(DirectArguments, m_storage); }
+
+    static ptrdiff_t offsetOfLengthInStorage() { return OBJECT_OFFSETOF(StorageHeader, length) - sizeof(WriteBarrier<Unknown>); }
+    static ptrdiff_t offsetOfMinCapacityInStorage() { return OBJECT_OFFSETOF(StorageHeader, minCapacity) - sizeof(WriteBarrier<Unknown>); }
+    
+    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 size_t allocationSize(size_t inlineSize)
+    {
+        RELEASE_ASSERT(!inlineSize);
+        return sizeof(DirectArguments);
     }
     
 private:
-    WriteBarrier<Unknown>* storage()
-    {
-        return bitwise_cast<WriteBarrier<Unknown>*>(bitwise_cast<char*>(this) + storageOffset());
+    friend class LLIntOffsetsExtractor;
+    
+    struct StorageHeader {
+        uint32_t length; // Always the actual length of captured arguments and never what was stored into the length property.
+        uint32_t minCapacity; // The max of this and length determines the capacity of this object. It may be the actual capacity, or maybe something smaller. We arrange it this way to be kind to the JITs.
+    };
+    
+    WriteBarrier<Unknown>* storage() const
+    {
+        return m_storage.get().unpoisoned();
+    }
+    
+    static StorageHeader& storageHeader(WriteBarrier<Unknown>* storage)
+    {
+        static_assert(sizeof(StorageHeader) == sizeof(WriteBarrier<Unknown>), "StorageHeader needs to be the same size as a JSValue");
+        return *bitwise_cast<StorageHeader*>(storage - 1);
+    }
+
+    StorageHeader& storageHeader() const
+    {
+        return storageHeader(storage());
     }
     
@@ -180 +203 @@
     
     WriteBarrier<JSFunction> m_callee;
-    uint32_t m_length; // Always the actual length of captured arguments and never what was stored into the length property.
-    uint32_t m_minCapacity; // The max of this and length determines the capacity of this object. It may be the actual capacity, or maybe something smaller. We arrange it this way to be kind to the JITs.
     CagedBarrierPtr<Gigacage::Primitive, bool> m_mappedArguments; // If non-null, it means that length, callee, and caller are fully materialized properties.
+    AuxiliaryBarrier<Poisoned<DirectArgumentsPoison, WriteBarrier<Unknown>*>> m_storage;
 };
 

trunk/Source/JavaScriptCore/runtime/JSCPoison.h

@@ -35 +35 @@
     v(CodeBlock) \
     v(DateInstance) \
+    v(DirectArguments) \
     v(GlobalData) \
     v(JITCode) \

trunk/Source/JavaScriptCore/runtime/JSLexicalEnvironment.h

@@ -37 +37 @@
 class LLIntOffsetsExtractor;
 
+// This is Spectre-safe because it doesn't have its length inline.
 class JSLexicalEnvironment : public JSSymbolTableObject {
     friend class JIT;

trunk/Source/JavaScriptCore/runtime/JSSymbolTableObject.h

@@ -74 +74 @@
     
 private:
+    // FIXME: This needs to be poisoned.
     WriteBarrier<SymbolTable> m_symbolTable;
 };

trunk/Source/JavaScriptCore/runtime/VM.cpp

@@ -262 +262 @@
     , boundFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSBoundFunction)
     , customGetterSetterFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSCustomGetterSetterFunction)
+    , directArgumentsSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), DirectArguments)
     , directEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), DirectEvalExecutable)
     , executableToCodeBlockEdgeSpace ISO_SUBSPACE_INIT(heap, cellDangerousBitsHeapCellType.get(), ExecutableToCodeBlockEdge)

trunk/Source/JavaScriptCore/runtime/VM.h

@@ -346 +346 @@
     IsoSubspace boundFunctionSpace;
     IsoSubspace customGetterSetterFunctionSpace;
+    IsoSubspace directArgumentsSpace;
     IsoSubspace directEvalExecutableSpace;
     IsoSubspace executableToCodeBlockEdgeSpace;

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2018-03-11  Filip Pizlo  <fpizlo@apple.com>
+
+        Split DirectArguments into JSValueOOB and JSValueStrict parts
+        https://bugs.webkit.org/show_bug.cgi?id=183458
+
+        Reviewed by Yusuke Suzuki.
+
+        * wtf/MathExtras.h:
+        (WTF::dynamicPoison): Deleted.
+
 2018-03-11  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/WTF/wtf/MathExtras.h

@@ -501 +501 @@
 }
 
+// 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)
@@ -510 +512 @@
 }
 
-constexpr unsigned bytePoisonShift = 40;
-
-template<typename T, typename U>
-inline T* dynamicPoison(U actual, U expected, T* pointer)
-{
-    static_assert(sizeof(U) == 1, "Poisoning only works for bytes at the moment");
-#if CPU(X86_64) || (CPU(ARM64) && !defined(__ILP32__))
-    return bitwise_cast<T*>(
-        bitwise_cast<char*>(pointer) +
-        (static_cast<uintptr_t>(opaque(actual) ^ expected) << bytePoisonShift));
-#else
-    UNUSED_PARAM(actual);
-    UNUSED_PARAM(expected);
-    return pointer;
-#endif
-}
-
 template<typename VectorType, typename RandomFunc>
 void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc)
@@ -542 +527 @@
 } // namespace WTF
 
-using WTF::dynamicPoison;
 using WTF::opaque;
 using WTF::preciseIndexMaskPtr;