Bug 1393011 - Part 2: Fix PatchDataWithValueCheck flushing incorrect ICache locations. r=bbouvier
authorSean Stangl <sstangl@mozilla.com>
Wed, 13 Dec 2017 15:55:46 -0600
changeset 396318 5c155dd29e4c069cb8959021bdaf24c1d1c4805a
parent 396317 d467fdda8dae4be1842f57767c94cdeb28beae7a
child 396319 d05acb5f6a7b2b40d002c145ff8111d0011ed000
push id56975
push userdluca@mozilla.com
push dateThu, 14 Dec 2017 09:59:07 +0000
treeherderautoland@16bcfaad13e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier
bugs1393011
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1393011 - Part 2: Fix PatchDataWithValueCheck flushing incorrect ICache locations. r=bbouvier
js/src/jit/arm/Assembler-arm.cpp
js/src/jit/arm/MacroAssembler-arm.cpp
--- a/js/src/jit/arm/Assembler-arm.cpp
+++ b/js/src/jit/arm/Assembler-arm.cpp
@@ -3158,26 +3158,36 @@ Assembler::PatchWrite_NearCall(CodeLocat
 void
 Assembler::PatchDataWithValueCheck(CodeLocationLabel label, PatchedImmPtr newValue,
                                    PatchedImmPtr expectedValue)
 {
     Instruction* ptr = reinterpret_cast<Instruction*>(label.raw());
 
     Register dest;
     Assembler::RelocStyle rs;
-    DebugOnly<const uint32_t*> val = GetPtr32Target(InstructionIterator(ptr), &dest, &rs);
-    MOZ_ASSERT(uint32_t((const uint32_t*)val) == uint32_t(expectedValue.value));
-
-    MacroAssembler::ma_mov_patch(Imm32(int32_t(newValue.value)), dest, Always, rs,
-                                 InstructionIterator(ptr));
+
+#ifdef DEBUG
+    {
+        InstructionIterator iter(ptr);
+        const uint32_t* val = GetPtr32Target(iter, &dest, &rs);
+        MOZ_ASSERT(uint32_t((const uint32_t*)val) == uint32_t(expectedValue.value));
+    }
+#endif
+
+    // Patch over actual instructions.
+    {
+        InstructionIterator iter(ptr);
+        MacroAssembler::ma_mov_patch(Imm32(int32_t(newValue.value)), dest, Always, rs, iter);
+    }
 
     // L_LDR won't cause any instructions to be updated.
     if (rs != L_LDR) {
-        AutoFlushICache::flush(uintptr_t(ptr), 4);
-        AutoFlushICache::flush(uintptr_t(ptr->next()), 4);
+        InstructionIterator iter(ptr);
+        AutoFlushICache::flush(uintptr_t(iter.cur()), 4);
+        AutoFlushICache::flush(uintptr_t(iter.next()), 4);
     }
 }
 
 void
 Assembler::PatchDataWithValueCheck(CodeLocationLabel label, ImmPtr newValue, ImmPtr expectedValue)
 {
     PatchDataWithValueCheck(label,
                             PatchedImmPtr(newValue.value),
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -348,21 +348,20 @@ MacroAssemblerARM::ma_movPatchable(ImmPt
 }
 
 /* static */
 template<class Iter>
 void
 MacroAssemblerARM::ma_mov_patch(Imm32 imm32, Register dest, Assembler::Condition c,
                                 RelocStyle rs, Iter iter)
 {
+    // The current instruction must be an actual instruction,
+    // not automatically-inserted boilerplate.
     MOZ_ASSERT(iter.cur());
-
-    // Make sure the current instruction is not an artificial guard inserted
-    // by the assembler buffer.
-    iter.maybeSkipAutomaticInstructions();
+    MOZ_ASSERT(iter.cur() == iter.cur()->maybeSkipAutomaticInstructions());
 
     int32_t imm = imm32.value;
     switch(rs) {
       case L_MOVWT:
         Assembler::as_movw_patch(dest, Imm16(imm & 0xffff), c, iter.cur());
         Assembler::as_movt_patch(dest, Imm16(imm >> 16 & 0xffff), c, iter.next());
         break;
       case L_LDR: