[MERGE #1735 @jianchun] swb: run write-barrier check/analyze from build.sh

Merge pull request #1735 from jianchun:swbscript

Enable build.sh to run write-barrier check/analyze:

```
build.sh -n -d --wb-check lib/Runtime/Base/FunctionBody.cpp
```

Also fix some build breaks and failures/assertions, fix copyright headers and tabs.

Author: Jianchun Xu

CMakeLists.txt

@@ -310,6 +310,28 @@ endif(WITHOUT_FEATURES_SH)
 
 enable_language(ASM)
 
+################# Write-barrier check/analyze ##################
+if (WB_CHECK_SH OR WB_ANALYZE_SH)
+    add_definitions(
+        -Xclang -load
+        -Xclang ${CMAKE_CURRENT_SOURCE_DIR}/tools/RecyclerChecker/Build/libclangRecyclerChecker.so
+    )
+endif()
+if (WB_CHECK_SH)
+    unset(WB_CHECK_SH CACHE)    # don't cache
+    add_definitions(
+        -Xclang -plugin
+        -Xclang check-recycler
+    )
+endif()
+if (WB_ANALYZE_SH)
+    unset(WB_ANALYZE_SH CACHE)  # don't cache
+    add_definitions(
+        -Xclang -analyze
+        -Xclang -analyzer-checker=chakra.RecyclerChecker
+    )
+endif()
+
 include_directories(
     .
     lib/Common

build.sh

@@ -49,6 +49,10 @@ PRINT_USAGE() {
     echo "      --without=FEATURE,FEATURE,..."
     echo "                       Disable FEATUREs from JSRT experimental"
     echo "                       features."
+    echo "      --wb-check CPPFILE"
+    echo "                       Write-barrier check given CPPFILE (git path)"
+    echo "      --wb-analyze CPPFILE"
+    echo "                       Write-barrier analyze given CPPFILE (git path)"
     echo ""
     echo "example:"
     echo "  ./build.sh --cxx=/path/to/clang++ --cc=/path/to/clang -j"
@@ -75,6 +79,8 @@ ARCH="-DCC_TARGETS_AMD64_SH=1"
 OS_LINUX=0
 OS_APT_GET=0
 OS_UNIX=0
+WB_CHECK=
+WB_ANALYZE=
 
 if [ -f "/proc/version" ]; then
     OS_LINUX=1
@@ -234,6 +240,24 @@ while [[ $# -gt 0 ]]; do
         done
         ;;
 
+    --wb-check)
+        if [[ "$2" =~ ^[^-] ]]; then
+            WB_CHECK="$2"
+            shift
+        else
+            PRINT_USAGE && exit 1
+        fi
+        ;;
+
+    --wb-analyze)
+        if [[ "$2" =~ ^[^-] ]]; then
+            WB_ANALYZE="$2"
+            shift
+        else
+            PRINT_USAGE && exit 1
+        fi
+        ;;
+
     *)
         echo "Unknown option $1"
         PRINT_USAGE
@@ -306,6 +330,43 @@ if [[ ${#_CXX} > 0 ]]; then
     CC_PREFIX="-DCMAKE_CXX_COMPILER=$_CXX -DCMAKE_C_COMPILER=$_CC"
 fi
 
+################# Write-barrier check/analyze run #################
+WB_FLAG=
+WB_TARGET=
+if [[ $WB_CHECK || $WB_ANALYZE ]]; then
+    if [[ $MAKE != 'ninja' ]]; then
+        echo "--wb-check/wb-analyze only works with --ninja" && exit 1
+    fi
+    if [[ $WB_CHECK && $WB_ANALYZE ]]; then
+        echo "Please run only one of --wb-check or --wb-analyze" && exit 1
+    fi
+    if [[ $WB_CHECK ]]; then
+        WB_FLAG="-DWB_CHECK_SH=1"
+        WB_FILE=$WB_CHECK
+    fi
+    if [[ $WB_ANALYZE ]]; then
+        WB_FLAG="-DWB_ANALYZE_SH=1"
+        WB_FILE=$WB_ANALYZE
+    fi
+
+    if [[ -f $CHAKRACORE_DIR/$WB_FILE ]]; then
+        touch $CHAKRACORE_DIR/$WB_FILE
+    else
+        echo "$CHAKRACORE_DIR/$WB_FILE not found. Please use full git path for $WB_FILE." && exit 1
+    fi
+
+    WB_FILE_DIR=`dirname $WB_FILE`
+    WB_FILE_BASE=`basename $WB_FILE`
+
+    WB_FILE_CMAKELISTS="$CHAKRACORE_DIR/$WB_FILE_DIR/CMakeLists.txt"
+    if [[ -f $WB_FILE_CMAKELISTS ]]; then
+        SUBDIR=$(grep -i add_library $WB_FILE_CMAKELISTS | sed -r "s/.*\((\S+) .*/\1/")
+    else
+        echo "$WB_FILE_CMAKELISTS not found." && exit 1
+    fi
+    WB_TARGET="$WB_FILE_DIR/CMakeFiles/$SUBDIR.dir/$WB_FILE_BASE.o"
+fi
+
 build_directory="$CHAKRACORE_DIR/BuildLinux/${BUILD_TYPE:0}"
 if [ ! -d "$build_directory" ]; then
     SAFE_RUN `mkdir -p $build_directory`
@@ -322,12 +383,14 @@ fi
 
 echo Generating $BUILD_TYPE makefiles
 cmake $CMAKE_GEN $CC_PREFIX $ICU_PATH $STATIC_LIBRARY $ARCH \
-    -DCMAKE_BUILD_TYPE=$BUILD_TYPE $SANITIZE $NO_JIT $WITHOUT_FEATURES ../..
+    -DCMAKE_BUILD_TYPE=$BUILD_TYPE $SANITIZE $NO_JIT $WITHOUT_FEATURES \
+    $WB_FLAG \
+    ../..
 
 _RET=$?
 if [[ $? == 0 ]]; then
     if [[ $MAKE != 0 ]]; then
-        $MAKE $MULTICORE_BUILD 2>&1 | tee build.log
+        $MAKE $MULTICORE_BUILD $WB_TARGET 2>&1 | tee build.log
         _RET=${PIPESTATUS[0]}
     else
         echo "Visit given folder above for xcode project file ----^"

jenkins/check_copyright.sh

@@ -32,13 +32,16 @@ git diff --name-only `git merge-base origin/master HEAD` HEAD |
     grep -v -E '\.nuspec$' |
     grep -v -E '\.def$' |
     grep -v -E '\.inc$' |
+    grep -v -E '\.cmake$' |
     grep -v -E 'test/benchmarks/.*\.js$' |
     grep -v -E 'bin/External/.*$' |
     grep -v -E 'bin/NativeTests/Scripts/splay.js$' |
     grep -v -E 'pal/.*' |
     grep -v -E 'libChakraCoreLib.version|ch.version' |
     xargs -I % sh -c "echo 'Check Copyright > Checking %'; python jenkins/check_copyright.py % > $ERRFILETEMP || cat $ERRFILETEMP >> $ERRFILE"
 
+rm -f $ERRFILETEMP
+
 if [ -e $ERRFILE ]; then # if error file exists then there were errors
     >&2 echo "--------------" # leading >&2 means echo to stderr
     >&2 echo "--- ERRORS ---"

lib/Backend/LowerMDShared.cpp

@@ -1675,7 +1675,8 @@ LowererMD::Legalize(IR::Instr *const instr, bool fPostRegAlloc)
 
         case Js::OpCode::LEA:
             Assert(instr->GetDst()->IsRegOpnd());
-            Assert(instr->GetSrc1()->IsIndirOpnd() || instr->GetSrc1()->IsSymOpnd());
+            Assert(instr->GetSrc1()->IsIndirOpnd() || instr->GetSrc1()->IsSymOpnd()
+                   || instr->GetSrc1()->IsMemRefOpnd());  // We may convert IndirOpnd to MemRefOpnd
             Assert(!instr->GetSrc2());
             break;
         case Js::OpCode::PSRLDQ:
@@ -5031,7 +5032,7 @@ LowererMD::GenerateUntagVar(IR::RegOpnd * src, IR::LabelInstr * labelFail, IR::I
     if (generateTagCheck)
     {
         Assert(!opnd->IsTaggedInt());
-        this->GenerateSmIntTest(opnd, insertBeforeInstr, labelFail);
+        this->GenerateSmIntTest(opnd, assignInstr, labelFail);
     }
 
     // Moving into r2 clears the tag bits on AMD64.

lib/Common/CommonDefines.h

@@ -154,8 +154,6 @@
 #ifdef RECYCLER_WRITE_BARRIER
 #if defined(__clang__) 
 #define GLOBAL_FORCE_USE_WRITE_BARRIER 1
-#else
-#define GLOBAL_FORCE_USE_WRITE_BARRIER 1            // force turn on write barrier in windows
 #endif
 #endif
 

lib/Common/Memory/Recycler.h

@@ -129,7 +129,7 @@ struct InfoBitsWrapper{};
 #define RecyclerNewFinalized(recycler,T,...) static_cast<T *>(static_cast<FinalizableObject *>(AllocatorNewBase(Recycler, recycler, AllocFinalizedInlined, T, __VA_ARGS__)))
 #define RecyclerNewFinalizedPlus(recycler, size, T,...) static_cast<T *>(static_cast<FinalizableObject *>(AllocatorNewPlusBase(Recycler, recycler, AllocFinalized, size, T, __VA_ARGS__)))
 #define RecyclerNewTracked(recycler,T,...) static_cast<T *>(static_cast<FinalizableObject *>(AllocatorNewBase(Recycler, recycler, AllocTrackedInlined, T, __VA_ARGS__)))
-#define RecyclerNewEnumClass(recycler, enumClass, T, ...) new (TRACK_ALLOC_INFO(static_cast<Recycler *>(recycler), T, Recycler, 0, (size_t)-1), enumClass) T(__VA_ARGS__)
+#define RecyclerNewEnumClass(recycler, enumClass, T, ...) new (TRACK_ALLOC_INFO(static_cast<Recycler *>(recycler), T, Recycler, 0, (size_t)-1), InfoBitsWrapper<enumClass>()) T(__VA_ARGS__)
 #define RecyclerNewWithInfoBits(recycler, infoBits, T, ...) new (TRACK_ALLOC_INFO(static_cast<Recycler *>(recycler), T, Recycler, 0, (size_t)-1), InfoBitsWrapper<infoBits>()) T(__VA_ARGS__)
 #define RecyclerNewFinalizedClientTracked(recycler,T,...) static_cast<T *>(static_cast<FinalizableObject *>(AllocatorNewBase(Recycler, recycler, AllocFinalizedClientTrackedInlined, T, __VA_ARGS__)))
 #endif
@@ -1351,7 +1351,7 @@ class Recycler
     char * AllocEnumClass(DECLSPEC_GUARD_OVERFLOW size_t size)
     {
         Assert((enumClass & EnumClassMask) != 0);
-        Assert((enumClass & ~EnumClassMask & ~WithBarrierBit) == 0);
+        //Assert((enumClass & ~EnumClassMask & ~WithBarrierBit) == 0);
         return AllocWithAttributes<(ObjectInfoBits)(enumClass), /* nothrow = */ false>(size);
     }
 

lib/Runtime/Base/AuxPtrs.h

@@ -25,16 +25,19 @@ namespace Js
     template<typename FieldsEnum, uint8 size, uint8 _MaxCount = (size - 1) / (1 + sizeof(void*))>
     struct AuxPtrsFix
     {
-        static const uint8 MaxCount = _MaxCount;
-        Field(uint8) count;                // always saving maxCount
-        Field(FieldsEnum) type[MaxCount];  // save instantiated pointer enum
-        Pointer(void) ptr[MaxCount];       // save instantiated pointer address
+        static const uint8 MaxCount;
+        Field(uint8) count;                 // always saving maxCount
+        Field(FieldsEnum) type[_MaxCount];  // save instantiated pointer enum
+        Pointer(void) ptr[_MaxCount];       // save instantiated pointer address
         AuxPtrsFix();
         AuxPtrsFix(AuxPtrsFix<FieldsEnum, 16>* ptr16); // called when promoting from AuxPtrs16 to AuxPtrs32
         void* Get(FieldsEnum e);
         bool Set(FieldsEnum e, void* p);
     };
 
+    template<typename FieldsEnum, uint8 size, uint8 _MaxCount>
+    const uint8 AuxPtrsFix<FieldsEnum, size, _MaxCount>::MaxCount = _MaxCount;
+
     // Use flexible size structure to save pointers. when pointer count exceeds AuxPtrsFix<FieldsEnum, 32>::MaxCount,
     // it will promote to this structure to save the pointers
     // Layout:

lib/Runtime/Language/ModuleNamespace.cpp

@@ -16,7 +16,7 @@ namespace Js
     Js::FunctionInfo ModuleNamespace::EntryInfo::SymbolIterator(ModuleNamespace::EntrySymbolIterator);
 
     ModuleNamespace::ModuleNamespace(ModuleRecordBase* moduleRecord, DynamicType* type) :
-        moduleRecord(moduleRecord), DynamicObject(type), unambiguousNonLocalExports(nullptr), 
+        moduleRecord(moduleRecord), DynamicObject(type), unambiguousNonLocalExports(nullptr),
         sortedExportedNames(nullptr), nsSlots(nullptr)
     {
 
@@ -56,7 +56,7 @@ namespace Js
 
         DynamicType* type = library->CreateFunctionWithLengthType(&EntryInfo::SymbolIterator);
         RuntimeFunction* iteratorFunction = RecyclerNewEnumClass(scriptContext->GetRecycler(),
-            library->EnumFunctionClass, RuntimeFunction,
+            JavascriptLibrary::EnumFunctionClass, RuntimeFunction,
             type, &EntryInfo::SymbolIterator);
         DynamicObject::SetPropertyWithAttributes(PropertyIds::_symbolIterator, iteratorFunction, PropertyBuiltInMethodDefaults, nullptr);
 
@@ -89,7 +89,7 @@ namespace Js
         // update the local slot to use the storage for local exports.
         SetNSSlotsForModuleNS(sourceTextModuleRecord->GetLocalExportSlots());
 
-        // For items that are not in the local export list, we need to resolve them to get it 
+        // For items that are not in the local export list, we need to resolve them to get it
         ExportedNames* exportedNames = sourceTextModuleRecord->GetExportedNames(nullptr);
         ModuleNameRecord* moduleNameRecord = nullptr;
 #if DBG

lib/Runtime/Library/JavascriptLibrary.cpp

@@ -5024,7 +5024,7 @@ namespace Js
 
     Js::RecyclableObject* JavascriptLibrary::CreateRevokeFunction_TTD(RecyclableObject* proxy)
     {
-        RuntimeFunction* revoker = RecyclerNewEnumClass(this->scriptContext->GetRecycler(), this->EnumFunctionClass, RuntimeFunction, this->CreateFunctionWithLengthType(&JavascriptProxy::EntryInfo::Revoke), &JavascriptProxy::EntryInfo::Revoke);
+        RuntimeFunction* revoker = RecyclerNewEnumClass(this->scriptContext->GetRecycler(), JavascriptLibrary::EnumFunctionClass, RuntimeFunction, this->CreateFunctionWithLengthType(&JavascriptProxy::EntryInfo::Revoke), &JavascriptProxy::EntryInfo::Revoke);
 
         revoker->SetPropertyWithAttributes(Js::PropertyIds::length, Js::TaggedInt::ToVarUnchecked(0), PropertyNone, NULL);
         revoker->SetInternalProperty(Js::InternalPropertyIds::RevocableProxy, proxy, PropertyOperationFlags::PropertyOperation_Force, nullptr);

lib/Runtime/Library/JavascriptProxy.cpp

@@ -126,7 +126,7 @@ namespace Js
         JavascriptLibrary* library = scriptContext->GetLibrary();
         DynamicType* type = library->CreateFunctionWithLengthType(&EntryInfo::Revoke);
         RuntimeFunction* revoker = RecyclerNewEnumClass(scriptContext->GetRecycler(),
-            library->EnumFunctionClass, RuntimeFunction,
+            JavascriptLibrary::EnumFunctionClass, RuntimeFunction,
             type, &EntryInfo::Revoke);
 
         revoker->SetPropertyWithAttributes(Js::PropertyIds::length, Js::TaggedInt::ToVarUnchecked(0), PropertyNone, NULL);
@@ -906,16 +906,16 @@ namespace Js
             if (!threadContext->RecordImplicitException())
                 return FALSE;
             JavascriptError::ThrowTypeError(GetScriptContext(), JSERR_ErrorOnRevokedProxy, _u("ownKeys"));
-        }    
-        
+        }
+
         Var propertyName = nullptr;
         PropertyId propertyId;
         int index = 0;
         JsUtil::BaseDictionary<const char16*, Var, Recycler> dict(requestContext->GetRecycler());
         JavascriptArray* arrResult = requestContext->GetLibrary()->CreateArray();
 
         // 13.7.5.15 EnumerateObjectProperties(O) (https://tc39.github.io/ecma262/#sec-enumerate-object-properties)
-        // for (let key of Reflect.ownKeys(obj)) {    
+        // for (let key of Reflect.ownKeys(obj)) {
         Var trapResult = JavascriptOperators::GetOwnPropertyNames(this, requestContext);
         if (JavascriptArray::Is(trapResult))
         {
@@ -941,7 +941,7 @@ namespace Js
                         dict.Add(str->GetSz(), prop);
                         // if (desc.enumerable) yield key;
                         if (desc.IsEnumerable())
-                        {                            
+                        {
                             ret = arrResult->SetItem(index++, CrossSite::MarshalVar(requestContext, prop), PropertyOperation_None);
                             Assert(ret);
                         }
@@ -1230,7 +1230,7 @@ namespace Js
         //7. ReturnIfAbrupt(keys).
         JavascriptArray* resultArray = JavascriptOperators::GetOwnPropertyKeys(obj, scriptContext);
 
-        const PropertyRecord* propertyRecord;        
+        const PropertyRecord* propertyRecord;
         if (integrityLevel == IntegrityLevel::IntegrityLevel_sealed)
         {
             //8. If level is "sealed", then