fix the order on setting write barrier bits

order:
1. update the verification bits for verification in background thread marking
2. update the target address
3. update card table

make sure the JIT code use the same order

lib/Backend/Lower.cpp

@@ -114,13 +114,15 @@ Lowerer::Lower()
                 IR::AddrOpnd* addrOpnd = instr->m_src1->AsAddrOpnd();
                 if (addrOpnd->GetAddrOpndKind() == IR::AddrOpndKindWriteBarrierCardTable)
                 {
-                    auto& leaInstr = instr->m_prev->m_prev;
+                    auto& leaInstr = instr->m_prev->m_prev->m_prev;
+                    auto& movInstr = instr->m_prev->m_prev;
                     auto& shrInstr = instr->m_prev;
                     Assert(leaInstr->m_opcode == Js::OpCode::LEA);
+                    Assert(movInstr->m_opcode == Js::OpCode::MOV);
                     Assert(shrInstr->m_opcode == Js::OpCode::SHR);
-                    m_lowererMD.LoadHelperArgument(shrInstr, leaInstr->m_dst);
+                    m_lowererMD.LoadHelperArgument(movInstr, leaInstr->m_dst);
                     IR::Instr* instrCall = IR::Instr::New(Js::OpCode::Call, m_func);
-                    shrInstr->InsertBefore(instrCall);
+                    movInstr->InsertBefore(instrCall);
                     m_lowererMD.ChangeToHelperCall(instrCall, IR::HelperWriteBarrierSetVerifyBit);
                 }
             }
@@ -10292,7 +10294,7 @@ Lowerer::LowerStSlot(IR::Instr *instr)
     }
     else
     {
-        m_lowererMD.ChangeToWriteBarrierAssign(instr, this->m_func);
+        instr = m_lowererMD.ChangeToWriteBarrierAssign(instr, this->m_func);
     }
 
     return instr;
@@ -14533,12 +14535,12 @@ IR::Instr *Lowerer::InsertMove(IR::Opnd *dst, IR::Opnd *src, IR::Instr *const in
     {
         src = src->UseWithNewType(dst->GetType(), func);
     }
-    IR::Instr *const instr = IR::Instr::New(Js::OpCode::Ld_A, dst, src, func);
+    IR::Instr * instr = IR::Instr::New(Js::OpCode::Ld_A, dst, src, func);
 
     insertBeforeInstr->InsertBefore(instr);
     if (generateWriteBarrier)
     {
-        LowererMD::ChangeToWriteBarrierAssign(instr, func);
+        instr = LowererMD::ChangeToWriteBarrierAssign(instr, func);
     }
     else
     {

lib/Backend/LowerMDShared.cpp

@@ -4762,7 +4762,7 @@ LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
         && assignInstr->m_opcode == Js::OpCode::MOV // ignore SSE instructions like MOVSD
         && assignInstr->GetSrc1()->IsWriteBarrierTriggerableValue())
     {
-        LowererMD::GenerateWriteBarrier(assignInstr);
+        instr = LowererMD::GenerateWriteBarrier(assignInstr);
     }
 #endif
 
@@ -4837,7 +4837,7 @@ LowererMD::GenerateWriteBarrier(IR::Instr * assignInstr)
     assignInstr->InsertBefore(loadIndexInstr);
     IR::Instr * shiftBitInstr = IR::Instr::New(Js::OpCode::SHR, indexOpnd, indexOpnd,
         IR::IntConstOpnd::New(12 /* 1 << 12 = 4096 */, TyInt8, assignInstr->m_func), assignInstr->m_func);
-    assignInstr->InsertBefore(shiftBitInstr);
+    assignInstr->InsertAfter(shiftBitInstr);
 
     // The cardtable address is likely 64 bits already so we have to load it to a register
     // That is, we have to do the following:
@@ -4857,13 +4857,13 @@ LowererMD::GenerateWriteBarrier(IR::Instr * assignInstr)
     IR::Instr * cardTableAddrInstr = IR::Instr::New(Js::OpCode::MOV, cardTableRegOpnd,
         IR::AddrOpnd::New(RecyclerWriteBarrierManager::GetAddressOfCardTable(), IR::AddrOpndKindWriteBarrierCardTable, assignInstr->m_func),
         assignInstr->m_func);
-    assignInstr->InsertBefore(cardTableAddrInstr);
+    shiftBitInstr->InsertAfter(cardTableAddrInstr);
 
     IR::IndirOpnd * cardTableEntryOpnd = IR::IndirOpnd::New(cardTableRegOpnd, indexOpnd,
         TyInt8, assignInstr->m_func);
     IR::Instr * movInstr = IR::Instr::New(Js::OpCode::MOV, cardTableEntryOpnd, IR::IntConstOpnd::New(1, TyInt8, assignInstr->m_func), assignInstr->m_func);
-    assignInstr->InsertBefore(movInstr);
-    return movInstr;
+    cardTableAddrInstr->InsertAfter(movInstr);
+    return loadIndexInstr;
 #else
     Assert(writeBarrierAddrRegOpnd->IsRegOpnd());
     IR::RegOpnd * shiftBitOpnd = IR::RegOpnd::New(TyInt32, assignInstr->m_func);

lib/Common/Memory/Recycler.cpp

@@ -8538,8 +8538,11 @@ Recycler::RegisterPendingWriteBarrierBlock(void* address, size_t bytes)
 {
     if (CONFIG_FLAG(ForceSoftwareWriteBarrier))
     {
-        RecyclerWriteBarrierManager::WriteBarrier(address, bytes);
+#if DBG
+        WBSetBitRange((char*)address, (uint)bytes/sizeof(void*));
+#endif
         pendingWriteBarrierBlockMap.Item(address, bytes);
+        RecyclerWriteBarrierManager::WriteBarrier(address, bytes);
     }
 }
 void
@@ -8571,31 +8574,37 @@ Recycler::WBVerifyBitIsSet(char* addr, char* target)
 void
 Recycler::WBSetBit(char* addr)
 {
-    Recycler* recycler = Recycler::recyclerList;
-    while (recycler)
+    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(VerifyBarrierBit))
     {
-        auto heapBlock = recycler->FindHeapBlock((void*)((UINT_PTR)addr&~HeapInfo::ObjectAlignmentMask));
-        if (heapBlock)
+        Recycler* recycler = Recycler::recyclerList;
+        while (recycler)
         {
-            heapBlock->WBSetBit(addr);
-            break;
+            auto heapBlock = recycler->FindHeapBlock((void*)((UINT_PTR)addr&~HeapInfo::ObjectAlignmentMask));
+            if (heapBlock)
+            {
+                heapBlock->WBSetBit(addr);
+                break;
+            }
+            recycler = recycler->next;
         }
-        recycler = recycler->next;
     }
 }
 void
 Recycler::WBSetBitRange(char* addr, uint count)
 {
-    Recycler* recycler = Recycler::recyclerList;
-    while (recycler)
+    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(VerifyBarrierBit))
     {
-        auto heapBlock = recycler->FindHeapBlock((void*)((UINT_PTR)addr&~HeapInfo::ObjectAlignmentMask));
-        if (heapBlock)
+        Recycler* recycler = Recycler::recyclerList;
+        while (recycler)
         {
-            heapBlock->WBSetBitRange(addr, count);
-            break;
+            auto heapBlock = recycler->FindHeapBlock((void*)((UINT_PTR)addr&~HeapInfo::ObjectAlignmentMask));
+            if (heapBlock)
+            {
+                heapBlock->WBSetBitRange(addr, count);
+                break;
+            }
+            recycler = recycler->next;
         }
-        recycler = recycler->next;
     }
 }
 bool

lib/Common/Memory/RecyclerPointers.h

@@ -117,6 +117,9 @@ struct _ArrayWriteBarrier
 #endif
 #endif
     }
+
+    template <class T>
+    static void WriteBarrierSetVerifyBits(T * address, size_t count) {   }
 };
 
 #ifdef RECYCLER_WRITE_BARRIER
@@ -128,6 +131,14 @@ struct _ArrayWriteBarrier<_write_barrier_policy>
     {
         RecyclerWriteBarrierManager::WriteBarrier(address, sizeof(T) * count);
     }
+
+    template <class T>
+    static void WriteBarrierSetVerifyBits(T * address, size_t count)
+    {
+#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
+        Recycler::WBSetBitRange((char*)address, (uint)(sizeof(T) * count / sizeof(void*)));
+#endif
+    }
 };
 #endif
 
@@ -163,16 +174,26 @@ void ArrayWriteBarrier(T * address, size_t count)
     return _ArrayWriteBarrier<Policy>::WriteBarrier(address, count);
 }
 
+template <class T, class PolicyType = T, class Allocator = Recycler>
+void ArrayWriteBarrierVerifyBits(T * address, size_t count)
+{
+    typedef typename _ArrayItemWriteBarrierPolicy<PolicyType>::Policy ItemPolicy;
+    typedef typename AllocatorWriteBarrierPolicy<Allocator, ItemPolicy>::Policy Policy;
+    return _ArrayWriteBarrier<Policy>::WriteBarrierSetVerifyBits(address, count);
+}
+
 // Copy array content. Triggers write barrier on the dst array content if if
 // Allocator and element type determines write barrier is needed.
 //
 template <class T, class PolicyType = T, class Allocator = Recycler>
 void CopyArray(T* dst, size_t dstCount, const T* src, size_t srcCount)
 {
-    ArrayWriteBarrier<T, PolicyType, Allocator>(dst, dstCount);
+    ArrayWriteBarrierVerifyBits<T, PolicyType, Allocator>(dst, dstCount);
 
     js_memcpy_s(reinterpret_cast<void*>(dst), sizeof(T) * dstCount,
                 reinterpret_cast<const void*>(src), sizeof(T) * srcCount);
+
+    ArrayWriteBarrier<T, PolicyType, Allocator>(dst, dstCount);
 }
 template <class T, class PolicyType = T, class Allocator = Recycler>
 void CopyArray(WriteBarrierPtr<T>& dst, size_t dstCount,
@@ -212,9 +233,12 @@ void CopyArray(WriteBarrierPtr<T>* dst, size_t dstCount,
 template <class T, class PolicyType = T, class Allocator = Recycler>
 void MoveArray(T* dst, const T* src, size_t count)
 {
+    ArrayWriteBarrierVerifyBits<T, PolicyType, Allocator>(dst, count);
+
     memmove(reinterpret_cast<void*>(dst),
             reinterpret_cast<const void*>(src),
             sizeof(T) * count);
+
     ArrayWriteBarrier<T, PolicyType, Allocator>(dst, count);
 }
 
@@ -369,10 +393,17 @@ public:
     }
     void WriteBarrierSet(T * ptr)
     {
+#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
+        // set the verification bits before updating the reference so it's ready to verify while background marking hit the reference
+        Recycler::WBSetBit((char*)this);
+#endif
+
+        NoWriteBarrierSet(ptr);
+
 #ifdef RECYCLER_WRITE_BARRIER
+        // set the barrier bit after updating the reference to prevent a race issue that background thread is resetting all the dirty pages
         RecyclerWriteBarrierManager::WriteBarrier(this);
 #endif
-        NoWriteBarrierSet(ptr);
     }
 
     WriteBarrierPtr& operator=(WriteBarrierPtr const& other)
@@ -383,10 +414,15 @@ public:
 
     WriteBarrierPtr& operator++()  // prefix ++
     {
+#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
+        Recycler::WBSetBit((char*)this);
+#endif
+
+        ++ptr;
+
 #ifdef RECYCLER_WRITE_BARRIER
         RecyclerWriteBarrierManager::WriteBarrier(this);
 #endif
-        ++ptr;
         return *this;
     }
 
@@ -399,10 +435,15 @@ public:
 
     WriteBarrierPtr& operator--()  // prefix --
     {
+#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
+        Recycler::WBSetBit((char*)this);
+#endif
+
+        --ptr;
+
 #ifdef RECYCLER_WRITE_BARRIER
         RecyclerWriteBarrierManager::WriteBarrier(this);
 #endif
-        --ptr;
         return *this;
     }
 

lib/Common/Memory/RecyclerWriteBarrierManager.cpp

@@ -330,12 +330,6 @@ RecyclerWriteBarrierManager::WriteBarrier(void * address)
         Output::Print(_u("Writing to 0x%p (CIndex: %u)\n"), address, index);
     }
 #endif
-#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
-    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(VerifyBarrierBit))
-    {
-        Recycler::WBSetBit((char*)address);
-    }
-#endif
 }
 
 void
@@ -352,13 +346,6 @@ RecyclerWriteBarrierManager::WriteBarrier(void * address, size_t bytes)
     memset(cardTable + startIndex, WRITE_BARRIER_PAGE_BIT | DIRTYBIT, endIndex - startIndex);
     GlobalSwbVerboseTrace(_u("Writing to 0x%p (CIndex: %u-%u)\n"), address, startIndex, endIndex);
 
-#if DBG && GLOBAL_ENABLE_WRITE_BARRIER
-    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(VerifyBarrierBit))
-    {
-        Recycler::WBSetBitRange((char*)address, (uint)bytes / sizeof(void*));
-    }
-#endif
-
 #else
     uint bitShift = (((uint)address) >> s_BitArrayCardTableShift);
     uint bitMask = 0xFFFFFFFF << bitShift;

lib/Runtime/Base/AuxPtrs.h

@@ -128,6 +128,7 @@ namespace Js
     template<class T, typename FieldsEnum>
     AuxPtrs<T, FieldsEnum>::AuxPtrs(uint8 capacity, AuxPtrs* ptr)
     {
+        ArrayWriteBarrierVerifyBits(&this->ptrs, ptr->count);
         memcpy(this, ptr, offsetof(AuxPtrs, ptrs) + ptr->count * sizeof(void*));
         ArrayWriteBarrier(&this->ptrs, ptr->count);
         this->capacity = capacity;

lib/Runtime/Library/CompoundString.cpp

@@ -37,6 +37,7 @@ namespace Js
         Assert(buffer);
         Assert(charLength <= charCapacity);
 
+        ArrayWriteBarrierVerifyBits(Block::Pointers(Chars()), Block::PointerLengthFromCharLength(charLength));
         js_wmemcpy_s(Chars(), charLength, Chars(buffer), charLength);
         // SWB: buffer may contain chars or pointers. Trigger write barrier for the whole buffer.
         ArrayWriteBarrier(Pointers(), PointerLengthFromCharLength(charLength));
@@ -382,6 +383,8 @@ namespace Js
         {
             AllocateBuffer(charCapacity, recycler);
             charLength = usedCharLength;
+            
+            ArrayWriteBarrierVerifyBits(Block::Pointers(Chars()), Block::PointerLengthFromCharLength(charCapacity));
             js_wmemcpy_s(Chars(), charCapacity, (const char16*)(buffer), usedCharLength);
             // SWB: buffer may contain chars or pointers. Trigger write barrier for the whole buffer.
             ArrayWriteBarrier(Pointers(), PointerLength());
@@ -403,6 +406,8 @@ namespace Js
             void *const newBuffer = RecyclerNewArray(recycler, char16, newCharCapacity);
             charCapacity = newCharCapacity;
             const CharCount charLength = CharLength();
+
+            ArrayWriteBarrierVerifyBits(Block::Pointers(newBuffer), Block::PointerLengthFromCharLength(charCapacity));
             js_wmemcpy_s((char16*)newBuffer, charCapacity, (char16*)PointerValue(buffer), charLength);
             buffer = newBuffer;
             // SWB: buffer may contain chars or pointers. Trigger write barrier for the whole buffer.