[MERGE #2293 @leirocks] some write barrier fixes

Merge pull request #2293 from leirocks:wb3

- false poisitive on EntryPointInfo->jsMethod after native address reuse in recycler
- false poisitive in arena allocator cacheBlockEnd reused in leaf block in recycler (sharing same page allocator)
- overload ChangeToAssign in JIT code, check all posible assign for write barrier
- missing barriers in various location

Author: leirocks

CMakeLists.txt

@@ -292,9 +292,13 @@ endif(CLR_CMAKE_PLATFORM_XPLAT)
 if(ENABLE_FULL_LTO_SH)
     unset(DENABLE_FULL_LTO_SH CACHE)
     add_compile_options(-flto)
+    set (CMAKE_SHARED_LINKER_FLAGS "-flto")
+    set (CMAKE_EXE_LINKER_FLAGS "-flto")
 elseif(ENABLE_THIN_LTO_SH)
     unset(ENABLE_THIN_LTO_SH CACHE)
     add_compile_options(-flto=thin)
+    set (CMAKE_SHARED_LINKER_FLAGS "-flto=thin")
+    set (CMAKE_EXE_LINKER_FLAGS "-flto=thin")
 endif()
 
 if(CMAKE_BUILD_TYPE STREQUAL Debug)

bin/ch/ch.cpp

@@ -615,6 +615,7 @@ HRESULT ExecuteTest(const char* fileName)
 HRESULT ExecuteTestWithMemoryCheck(char* fileName)
 {
     HRESULT hr = E_FAIL;
+#ifdef _WIN32 // looks on linux it always leak ThreadContextTLSEntry since there's no DllMain
 #ifdef CHECK_MEMORY_LEAK
     // Always check memory leak, unless user specified the flag already
     if (!ChakraRTInterface::IsEnabledCheckMemoryFlag())
@@ -626,6 +627,7 @@ HRESULT ExecuteTestWithMemoryCheck(char* fileName)
     // We will re-enable it if there is no unhandled exceptions
     ChakraRTInterface::SetEnableCheckMemoryLeakOutput(false);
 #endif
+#endif
 
 #ifdef _WIN32
     __try

lib/Backend/BailOut.cpp

@@ -1494,7 +1494,7 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
 
             newInstance->m_reader.Create(executeFunction);
 
-            generator->SetFrame(newInstance);
+            generator->SetFrame(newInstance, varSizeInBytes);
         }
     }
     else

lib/Backend/Func.cpp

@@ -143,7 +143,9 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
     , m_forInEnumeratorArrayOffset(-1)
     , argInsCount(0)
     , m_globalObjTypeSpecFldInfoArray(nullptr)
+#ifdef RECYCLER_WRITE_BARRIER_JIT
     , m_lowerer(nullptr)
+#endif
 {
 
     Assert(this->IsInlined() == !!runtimeInfo);

lib/Backend/JITTimeConstructorCache.cpp

@@ -51,7 +51,7 @@ JITTimeConstructorCache::Clone(JitArenaAllocator* allocator) const
 BVSparse<JitArenaAllocator>*
 JITTimeConstructorCache::GetGuardedPropOps() const
 {
-    return (BVSparse<JitArenaAllocator>*)m_data.guardedPropOps;
+    return (BVSparse<JitArenaAllocator>*)(m_data.guardedPropOps & ~(intptr_t)1);
 }
 
 void
@@ -60,6 +60,7 @@ JITTimeConstructorCache::EnsureGuardedPropOps(JitArenaAllocator* allocator)
     if (GetGuardedPropOps() == nullptr)
     {
         m_data.guardedPropOps = (intptr_t)Anew(allocator, BVSparse<JitArenaAllocator>, allocator);
+        m_data.guardedPropOps |= 1; // tag it to prevent false positive after the arena address reuse in recycler
     }
 }
 

lib/Backend/Lower.cpp

@@ -101,6 +101,34 @@ Lowerer::Lower()
 
     this->LowerRange(m_func->m_headInstr, m_func->m_tailInstr, defaultDoFastPath, loopFastPath);
 
+#if DBG
+    // TODO: (leish)(swb) implement for arm
+#if defined(_M_IX86) || defined(_M_AMD64)
+    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(RecyclerVerifyMark))
+    {
+        // find out all write barrier setting instr, call Recycler::WBSetBit for verification purpose
+        // should do this in LowererMD::GenerateWriteBarrier, however, can't insert call instruction there
+        FOREACH_INSTR_EDITING(instr, instrNext, m_func->m_headInstr)
+            if (instr->m_src1 && instr->m_src1->IsAddrOpnd())
+            {
+                IR::AddrOpnd* addrOpnd = instr->m_src1->AsAddrOpnd();
+                if (addrOpnd->GetAddrOpndKind() == IR::AddrOpndKindWriteBarrierCardTable)
+                {
+                    auto& leaInstr = instr->m_prev->m_prev;
+                    auto& shrInstr = instr->m_prev;
+                    Assert(leaInstr->m_opcode == Js::OpCode::LEA);
+                    Assert(shrInstr->m_opcode == Js::OpCode::SHR);
+                    m_lowererMD.LoadHelperArgument(shrInstr, leaInstr->m_dst);
+                    IR::Instr* instrCall = IR::Instr::New(Js::OpCode::Call, m_func);
+                    shrInstr->InsertBefore(instrCall);
+                    m_lowererMD.ChangeToHelperCall(instrCall, IR::HelperWriteBarrierSetVerifyBit);
+                }
+            }
+        NEXT_INSTR_EDITING
+    }
+#endif
+#endif
+
     this->m_func->ClearCloneMap();
 
     if (m_func->HasAnyStackNestedFunc())
@@ -14530,7 +14558,7 @@ IR::Instr *Lowerer::InsertMove(IR::Opnd *dst, IR::Opnd *src, IR::Instr *const in
     }
     else
     {
-        LowererMD::ChangeToAssign(instr);
+        LowererMD::ChangeToAssignNoBarrierCheck(instr);
     }
 
     return instr;

lib/Backend/Lower.h

@@ -57,11 +57,15 @@ class Lowerer
         , m_int64RegPairMap(nullptr)
 #endif
     {
+#ifdef RECYCLER_WRITE_BARRIER_JIT
         m_func->m_lowerer = this;
+#endif
     }
     ~Lowerer()
     {
+#ifdef RECYCLER_WRITE_BARRIER_JIT
         m_func->m_lowerer = nullptr;
+#endif
     }
 
     void Lower();

lib/Backend/LowerMDShared.cpp

@@ -756,11 +756,17 @@ IR::Instr* LowererMD::ChangeToHelperCallMem(IR::Instr * instr,  IR::JnHelperMeth
 ///----------------------------------------------------------------------------
 
 IR::Instr *
-LowererMD::ChangeToAssign(IR::Instr * instr)
+LowererMD::ChangeToAssignNoBarrierCheck(IR::Instr * instr)
 {
     return ChangeToAssign(instr, instr->GetDst()->GetType());
 }
 
+IR::Instr *
+LowererMD::ChangeToAssign(IR::Instr * instr)
+{
+    return ChangeToWriteBarrierAssign(instr, instr->m_func);
+}
+
 IR::Instr *
 LowererMD::ChangeToAssign(IR::Instr * instr, IRType type)
 {
@@ -4712,7 +4718,7 @@ LowererMD::GenerateLoadPolymorphicInlineCacheSlot(IR::Instr * instrLdSt, IR::Reg
     instrLdSt->InsertBefore(instr);
 }
 
-void
+IR::Instr *
 LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
 {
 #ifdef RECYCLER_WRITE_BARRIER_JIT
@@ -4747,17 +4753,19 @@ LowererMD::ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func)
     }
 #endif
 
-    ChangeToAssign(assignInstr);
+    IR::Instr * instr = ChangeToAssignNoBarrierCheck(assignInstr);
 
     // Now insert write barrier if necessary
 #ifdef RECYCLER_WRITE_BARRIER_JIT
     if (isPossibleBarrieredDest 
         && assignInstr->m_opcode == Js::OpCode::MOV // ignore SSE instructions like MOVSD
         && assignInstr->GetSrc1()->IsWriteBarrierTriggerableValue())
     {
-        func->GetTopFunc()->m_lowerer->GetLowererMD()->GenerateWriteBarrier(assignInstr);
+        LowererMD::GenerateWriteBarrier(assignInstr);
     }
 #endif
+
+    return instr;
 }
 
 void
@@ -4826,18 +4834,6 @@ LowererMD::GenerateWriteBarrier(IR::Instr * assignInstr)
     IR::RegOpnd * indexOpnd = IR::RegOpnd::New(TyMachPtr, assignInstr->m_func);
     IR::Instr * loadIndexInstr = IR::Instr::New(Js::OpCode::LEA, indexOpnd, assignInstr->GetDst(), assignInstr->m_func);
     assignInstr->InsertBefore(loadIndexInstr);
-
-#if DBG
-    // CALL Recycler::WBSetBitJIT
-    if (CONFIG_FLAG(ForceSoftwareWriteBarrier) && CONFIG_FLAG(RecyclerVerifyMark))
-    {
-        this->LoadHelperArgument(assignInstr, indexOpnd);
-        IR::Instr* instrCall = IR::Instr::New(Js::OpCode::Call, m_func);
-        assignInstr->InsertBefore(instrCall);
-        this->ChangeToHelperCall(instrCall, IR::HelperWriteBarrierSetVerifyBit);
-    }
-#endif
-
     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);

lib/Backend/LowerMDShared.h

@@ -85,6 +85,7 @@ class LowererMD
     static  IR::Instr *     CreateAssign(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instrInsertPt, bool generateWriteBarrier = true);
 
     static  IR::Instr *     ChangeToAssign(IR::Instr * instr);
+    static  IR::Instr *     ChangeToAssignNoBarrierCheck(IR::Instr * instr);
     static  IR::Instr *     ChangeToAssign(IR::Instr * instr, IRType type);
     static  IR::Instr *     ChangeToLea(IR::Instr *const instr, bool postRegAlloc = false);
     static  void            ImmedSrcToReg(IR::Instr * instr, IR::Opnd * newOpnd, int srcNum);
@@ -305,7 +306,7 @@ class LowererMD
 
             void GenerateWriteBarrierAssign(IR::IndirOpnd * opndDst, IR::Opnd * opndSrc, IR::Instr * insertBeforeInstr);
             void GenerateWriteBarrierAssign(IR::MemRefOpnd * opndDst, IR::Opnd * opndSrc, IR::Instr * insertBeforeInstr);
-            static void ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
+            static IR::Instr * ChangeToWriteBarrierAssign(IR::Instr * assignInstr, const Func* func);
 
             static IR::BranchInstr * GenerateLocalInlineCacheCheck(IR::Instr * instrLdSt, IR::RegOpnd * opndType, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelNext, bool checkTypeWithoutProperty = false);
             static IR::BranchInstr * GenerateProtoInlineCacheCheck(IR::Instr * instrLdSt, IR::RegOpnd * opndType, IR::RegOpnd * opndInlineCache, IR::LabelInstr * labelNext);
@@ -398,7 +399,7 @@ class LowererMD
 
     IR::LabelInstr*   EmitLoadFloatCommon(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool needLabelHelper);
 #ifdef RECYCLER_WRITE_BARRIER
-    IR::Instr* GenerateWriteBarrier(IR::Instr * assignInstr);
+    static IR::Instr* GenerateWriteBarrier(IR::Instr * assignInstr);
 #endif
 
     // Data

lib/Backend/Sym.inl

@@ -86,7 +86,7 @@ StackSym::IsArgSlotSym() const
 inline bool
 StackSym::IsParamSlotSym() const
 {
-	return m_isParamSym;
+    return m_isParamSym;
 }
 
 ///----------------------------------------------------------------------------