move recycler verify mark false reference checks to runtime

The checks were in Common.Memory and had no access to runtime headers. We had
to hard code runtime data field offsets. This was inconvenient and unstable.

This change moves the checks to runtime so that we can use exact field offsets
with "offsetof".

Added one more GC false reference filter (native array elements).

Added one unused field initialization to avoid false reference.

bin/GCStress/StubExternalApi.cpp

@@ -110,4 +110,12 @@ HRESULT MemProtectHeapSynchronizeWithCollector(void * heapHandle) { return E_NOT
 
 #if DBG && defined(INTERNAL_MEM_PROTECT_HEAP_ALLOC)
 void MemProtectHeapSetDisableConcurrentThreadExitedCheck(void * heapHandle) {};
-#endif
+#endif
+
+#if DBG && defined(RECYCLER_VERIFY_MARK)
+bool IsLikelyRuntimeFalseReference(char* objectStartAddress, size_t offset,
+    const char* typeName)
+{
+    return false;
+}
+#endif

lib/Common/Memory/HeapBlock.cpp

@@ -885,54 +885,7 @@ void HeapBlock::PrintVerifyMarkFailure(Recycler* recycler, char* objectAddress,
                     }
                 };
 
-                if (strstr(typeName, "Js::DynamicProfileInfo") != nullptr)
-                {
-                    // Js::DynamicProfileInfo allocate with non-Leaf in test/chk build
-                    // TODO: (leish)(swb) find a way to set barrier for the Js::DynamicProfileInfo plus allocation
-                    dumpFalsePositive();
-                    return;
-                }
-
-                if (offset <= Math::Align((3 * sizeof(uint)), sizeof(void*)) // left, length, size
-                    && strstr(typeName, "Js::SparseArraySegment") != nullptr)
-                {
-                    // Js::SparseArraySegmentBase left, length and size can easily form a false positive
-                    // TODO: (leish)(swb) find a way to tag these fields
-                    dumpFalsePositive();
-                    return;
-                }
-
-                if (
-                    offset >=// m_data offset on JavascriptDate
-#ifdef _M_X64_OR_ARM64
-                    0x20
-#else
-                    0x10
-#endif
-                    && strstr(typeName, "Js::JavascriptDate") != nullptr)
-                {
-                    // the fields on Js::DateImplementation can easily form a false positive
-                    // TODO: (leish)(swb) find a way to tag these
-                    dumpFalsePositive();
-                    return;
-                }
-
-                if (offset >= 0x30 && (offset & 0xf) == 0 // symbol array at the end of scopeInfo, can point to arena allocated propertyRecord
-                    && strstr(typeName, "Js::ScopeInfo") != nullptr)
-                {
-                    dumpFalsePositive();
-                    return;
-                }
-
-                // Js::Type::entryPoint may contain outdated data uncleared, and reused by recycler
-                // Most often occurs with script function Type
-                if (offset ==
-#if TARGET_64
-                    0x18
-#else
-                    0x10
-#endif
-                    && strstr(typeName, "Js::ScriptFunctionType"))
+                if (IsLikelyRuntimeFalseReference(objectStartAddress, offset, typeName))
                 {
                     dumpFalsePositive();
                     return;
@@ -941,7 +894,7 @@ void HeapBlock::PrintVerifyMarkFailure(Recycler* recycler, char* objectAddress,
                 //TODO: (leish)(swb) analyze pdb to check if the field is a pointer field or not
                 Output::Print(_u("Missing Barrier\nOn type %S+0x%x\n"), typeName, offset);
             }
-        }        
+        }
 
 
         targetStartAddress = target - targetOffset;
@@ -979,7 +932,7 @@ void HeapBlock::PrintVerifyMarkFailure(Recycler* recycler, char* objectAddress,
     Output::Print(_u("Missing barrier on 0x%p, target is 0x%p\n"), objectAddress, target);
     AssertMsg(false, "Missing barrier.");
 }
-#endif
+#endif  // DBG
 
 template <class TBlockAttributes>
 void
@@ -1185,7 +1138,7 @@ SmallHeapBlockT<TBlockAttributes>::AdjustPartialUncollectedAllocBytes(RecyclerSw
 
     recyclerSweep.SubtractSweepNewObjectAllocBytes(newObjectExpectSweepCount * this->objectSize);
 }
-#endif
+#endif  // RECYCLER_VERIFY_MARK
 
 template <class TBlockAttributes>
 uint

lib/Common/Memory/Recycler.h

@@ -2514,3 +2514,14 @@ operator new(DECLSPEC_GUARD_OVERFLOW size_t byteSize, Recycler * recycler, const
     Assume(buffer != nullptr);
     return buffer;
 }
+
+#if DBG && defined(RECYCLER_VERIFY_MARK)
+extern bool IsLikelyRuntimeFalseReference(
+    char* objectStartAddress, size_t offset, const char* typeName);
+#define DECLARE_RECYCLER_VERIFY_MARK_FRIEND() \
+    private: \
+        friend bool ::IsLikelyRuntimeFalseReference( \
+            char* objectStartAddress, size_t offset, const char* typeName);
+#else
+#define DECLARE_RECYCLER_VERIFY_MARK_FRIEND()
+#endif

lib/Runtime/ByteCode/ScopeInfo.h

@@ -11,6 +11,8 @@ namespace Js {
     //
     class ScopeInfo
     {
+        DECLARE_RECYCLER_VERIFY_MARK_FRIEND()
+
         struct MapSymbolData
         {
             FuncInfo* func;

lib/Runtime/Language/AsmJsModule.cpp

@@ -2287,7 +2287,10 @@ namespace Js
             {
                 mModuleMemory.mMemorySize = (int)((mModuleMemory.mSimdOffset + mSimdVarSpace.GetTotalVarCount()) * WAsmJs::SIMD_SLOTS_SPACE);
             }
-
+        }
+        else
+        {
+            mModuleMemory.mSimdOffset = 0;  // initialize to avoid GC false reference
         }
     }
 

lib/Runtime/Library/CMakeLists.txt

@@ -133,6 +133,7 @@ set(CRLIB_SOURCE_CODES
     TypedArray.cpp
     TypedArrayIndexEnumerator.cpp
     UriHelper.cpp
+    VerifyMarkFalseReference.cpp
     ${Wasm_dep}
     )
 

lib/Runtime/Library/Chakra.Runtime.Library.vcxproj

@@ -165,8 +165,9 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)WebAssemblyTable.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)WebAssemblyEnvironment.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)WabtInterface.cpp" />
-    <ClCompile Include="CustomExternalIterator.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)CustomExternalIterator.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)JavascriptExceptionMetadata.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)VerifyMarkFalseReference.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\InternalPropertyList.h" />
@@ -354,4 +355,4 @@
     <Import Project="$(VCTargetsPath)\BuildCustomizations\masm.targets" />
     <Import Project="$(BuildConfig_ARMASM_Path)armasm.targets" />
   </ImportGroup>
-</Project>
+</Project>

lib/Runtime/Library/Chakra.Runtime.Library.vcxproj.filters

@@ -119,7 +119,8 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)WebAssemblyTable.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)WebAssemblyEnvironment.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)WabtInterface.cpp" />
-    <ClCompile Include="CustomExternalIterator.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)CustomExternalIterator.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)VerifyMarkFalseReference.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\InternalPropertyList.h" />
@@ -315,4 +316,4 @@
     <ARMASM Include="$(MSBuildThisFileDirectory)arm64\arm64_CallFunction.asm" />
     <ARMASM Include="$(MSBuildThisFileDirectory)arm64\arm64_DeferredParsingThunk.asm" />
   </ItemGroup>
-</Project>
+</Project>

lib/Runtime/Library/CustomExternalIterator.cpp

@@ -53,7 +53,7 @@ namespace Js
         ScriptContext *scriptContext = function->GetScriptContext();
 
         AssertOrFailFast(RecyclableObject::Is(function->m_prototypeForIterator));
-        DynamicObject *prototype = static_cast<DynamicObject*>(function->m_prototypeForIterator);
+        DynamicObject *prototype = static_cast<DynamicObject*>(PointerValue(function->m_prototypeForIterator));
         Js::DynamicType *type = scriptContext->GetLibrary()->CreateObjectTypeNoCache(prototype, TypeIds_ExternalIterator);
 
         AssertOrFailFast(function->m_extraByteCount >= sizeof(void*));

lib/Runtime/Library/JavascriptDate.h

@@ -8,6 +8,8 @@ namespace Js
 {
     class JavascriptDate : public DynamicObject
     {
+        DECLARE_RECYCLER_VERIFY_MARK_FRIEND()
+
     protected:
         Field(DateImplementation) m_date;
 

lib/Runtime/Library/VerifyMarkFalseReference.cpp

@@ -0,0 +1,62 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+#include "RuntimeLibraryPch.h"
+
+#if DBG && defined(RECYCLER_VERIFY_MARK)
+
+#include "ByteCode/ScopeInfo.h"
+
+bool IsLikelyRuntimeFalseReference(char* objectStartAddress, size_t offset,
+    const char* typeName)
+{
+    // Js::DynamicProfileInfo allocate with non-Leaf in test/chk build
+    // TODO: (leish)(swb) find a way to set barrier for the Js::DynamicProfileInfo plus allocation
+    if (strstr(typeName, "Js::DynamicProfileInfo"))
+    {
+        return true;
+    }
+
+    // the fields on Js::DateImplementation can easily form a false positive
+    // TODO: (leish)(swb) find a way to tag these
+    if (VirtualTableInfo<Js::JavascriptDate>::HasVirtualTable(objectStartAddress) ||
+        VirtualTableInfo<Js::CrossSiteObject<Js::JavascriptDate>>::HasVirtualTable(objectStartAddress))
+    {
+        return offset >= offsetof(Js::JavascriptDate, m_date);
+    }
+
+    // symbol array at the end of scopeInfo, can point to arena allocated propertyRecord
+    if (offset >= offsetof(Js::ScopeInfo, symbols) && strstr(typeName, "Js::ScopeInfo"))
+    {
+        return true;
+    }
+
+    // Js::Type::entryPoint may contain outdated data uncleared, and reused by recycler
+    // Most often occurs with script function Type
+    if (offset == Js::Type::GetOffsetOfEntryPoint() && strstr(typeName, "Js::ScriptFunctionType"))
+    {
+        return true;
+    }
+
+    if (strstr(typeName, "Js::SparseArraySegment"))
+    {
+        // Js::SparseArraySegmentBase left, length and size can easily form a false positive
+        // TODO: (leish)(swb) find a way to tag these fields
+        if (offset < Js::SparseArraySegmentBase::GetOffsetOfNext())  // left, length, size
+        {
+            return true;
+        }
+
+        // Native array elements may form false positives
+        if (offset > Js::SparseArraySegmentBase::GetOffsetOfNext() &&  // elements
+            (strstr(typeName, "<double>") || strstr(typeName, "<int>")))
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+#endif  // DBG && defined(RECYCLER_VERIFY_MARK)