Bug 1445235 part 3 - Use Spectre-safe bounds check for LStoreTypedArrayElementHole. r=nbp, a=RyanVM
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 28 Mar 2018 16:07:10 +0200
changeset 463129 93bd90643c5b5bb0e90eee0dd3f71645c638d656
parent 463128 1f055e9f1c51662c21c961480dec854d2c1c6bc9
child 463130 707fd3a2a6886566d9f7a56017c4983c869b5f2c
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, RyanVM
bugs1445235
milestone60.0
Bug 1445235 part 3 - Use Spectre-safe bounds check for LStoreTypedArrayElementHole. r=nbp, a=RyanVM
js/src/jit/CodeGenerator.cpp
js/src/jit/Lowering.cpp
js/src/jit/shared/LIR-shared.h
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -11604,52 +11604,30 @@ void
 CodeGenerator::visitStoreTypedArrayElementHole(LStoreTypedArrayElementHole* lir)
 {
     Register elements = ToRegister(lir->elements());
     const LAllocation* value = lir->value();
 
     Scalar::Type arrayType = lir->mir()->arrayType();
     int width = Scalar::byteSize(arrayType);
 
-    const LAllocation* index = lir->index();
+    Register index = ToRegister(lir->index());
     const LAllocation* length = lir->length();
-
-    bool guardLength = true;
-    if (index->isConstant() && length->isConstant()) {
-        uint32_t idx = ToInt32(index);
-        uint32_t len = ToInt32(length);
-        if (idx >= len)
-            return;
-        guardLength = false;
-    }
+    Register spectreTemp = ToTempRegisterOrInvalid(lir->spectreTemp());
+
     Label skip;
-    if (index->isConstant()) {
-        uint32_t idx = ToInt32(index);
-        if (guardLength) {
-            if (length->isRegister())
-                masm.branch32(Assembler::BelowOrEqual, ToRegister(length), Imm32(idx), &skip);
-            else
-                masm.branch32(Assembler::BelowOrEqual, ToAddress(length), Imm32(idx), &skip);
-        }
-        Address dest(elements, idx * width);
-        StoreToTypedArray(masm, arrayType, value, dest);
-    } else {
-        Register idxReg = ToRegister(index);
-        MOZ_ASSERT(guardLength);
-        if (length->isConstant())
-            masm.branch32(Assembler::AboveOrEqual, idxReg, Imm32(ToInt32(length)), &skip);
-        else if (length->isRegister())
-            masm.branch32(Assembler::BelowOrEqual, ToRegister(length), idxReg, &skip);
-        else
-            masm.branch32(Assembler::BelowOrEqual, ToAddress(length), idxReg, &skip);
-        BaseIndex dest(elements, ToRegister(index), ScaleFromElemWidth(width));
-        StoreToTypedArray(masm, arrayType, value, dest);
-    }
-    if (guardLength)
-        masm.bind(&skip);
+    if (length->isRegister())
+        masm.spectreBoundsCheck32(index, ToRegister(length), spectreTemp, &skip);
+    else
+        masm.spectreBoundsCheck32(index, ToAddress(length), spectreTemp, &skip);
+
+    BaseIndex dest(elements, index, ScaleFromElemWidth(width));
+    StoreToTypedArray(masm, arrayType, value, dest);
+
+    masm.bind(&skip);
 }
 
 void
 CodeGenerator::visitAtomicIsLockFree(LAtomicIsLockFree* lir)
 {
     Register value = ToRegister(lir->value());
     Register output = ToRegister(lir->output());
 
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -3776,26 +3776,29 @@ LIRGenerator::visitStoreTypedArrayElemen
     if (ins->isFloatWrite()) {
         MOZ_ASSERT_IF(ins->arrayType() == Scalar::Float32, ins->value()->type() == MIRType::Float32);
         MOZ_ASSERT_IF(ins->arrayType() == Scalar::Float64, ins->value()->type() == MIRType::Double);
     } else {
         MOZ_ASSERT(ins->value()->type() == MIRType::Int32);
     }
 
     LUse elements = useRegister(ins->elements());
-    LAllocation length = useAnyOrConstant(ins->length());
-    LAllocation index = useRegisterOrConstant(ins->index());
+    LAllocation length = useAny(ins->length());
+    LAllocation index = useRegister(ins->index());
+    // For byte arrays, the value has to be in a byte register on x86.
     LAllocation value;
-
-    // For byte arrays, the value has to be in a byte register on x86.
     if (ins->isByteWrite())
         value = useByteOpRegisterOrNonDoubleConstant(ins->value());
     else
         value = useRegisterOrNonDoubleConstant(ins->value());
-    add(new(alloc()) LStoreTypedArrayElementHole(elements, length, index, value), ins);
+
+    LDefinition spectreTemp = JitOptions.spectreIndexMasking ? temp() : LDefinition::BogusTemp();
+    auto* lir =
+        new(alloc()) LStoreTypedArrayElementHole(elements, length, index, value, spectreTemp);
+    add(lir, ins);
 }
 
 void
 LIRGenerator::visitLoadFixedSlot(MLoadFixedSlot* ins)
 {
     MDefinition* obj = ins->object();
     MOZ_ASSERT(obj->type() == MIRType::Object);
 
--- a/js/src/jit/shared/LIR-shared.h
+++ b/js/src/jit/shared/LIR-shared.h
@@ -6868,29 +6868,31 @@ class LStoreUnboxedScalar : public LInst
     const LAllocation* index() {
         return getOperand(1);
     }
     const LAllocation* value() {
         return getOperand(2);
     }
 };
 
-class LStoreTypedArrayElementHole : public LInstructionHelper<0, 4, 0>
+class LStoreTypedArrayElementHole : public LInstructionHelper<0, 4, 1>
 {
   public:
     LIR_HEADER(StoreTypedArrayElementHole)
 
     LStoreTypedArrayElementHole(const LAllocation& elements, const LAllocation& length,
-                                const LAllocation& index, const LAllocation& value)
+                                const LAllocation& index, const LAllocation& value,
+                                const LDefinition& spectreTemp)
       : LInstructionHelper(classOpcode)
     {
         setOperand(0, elements);
         setOperand(1, length);
         setOperand(2, index);
         setOperand(3, value);
+        setTemp(0, spectreTemp);
     }
 
     const MStoreTypedArrayElementHole* mir() const {
         return mir_->toStoreTypedArrayElementHole();
     }
     const LAllocation* elements() {
         return getOperand(0);
     }
@@ -6898,16 +6900,19 @@ class LStoreTypedArrayElementHole : publ
         return getOperand(1);
     }
     const LAllocation* index() {
         return getOperand(2);
     }
     const LAllocation* value() {
         return getOperand(3);
     }
+    const LDefinition* spectreTemp() {
+        return getTemp(0);
+    }
 };
 
 class LAtomicIsLockFree : public LInstructionHelper<1, 1, 0>
 {
   public:
     LIR_HEADER(AtomicIsLockFree)
 
     explicit LAtomicIsLockFree(const LAllocation& value)