[1.8>1.9] [MERGE #4589 @meg-gupta] Fix JsBuiltIn initialization code …

…to not read root property Merge pull request #4589 from meg-gupta:addCheckOnRootPropLd Also add assert so that in future we dont write js builtin code to access any root property Fixes OS#15467762
chakra-core · Jan 23, 2018 · b244840 · b244840
2 parents cfcbe3b + a5a6b65
commit b244840
Show file tree

Hide file tree

Showing 15 changed files with 896 additions and 838 deletions.
diff --git a/lib/Common/Exceptions/ReportError.h b/lib/Common/Exceptions/ReportError.h
@@ -29,6 +29,7 @@ enum ErrorReason
     Fatal_Failed_API_Result = 21,
     Fatal_OutOfMemory = 22,
     Fatal_RecyclerVisitedHost_LargeHeapBlock = 23,
+    Fatal_JsBuiltIn_Error = 24,
 };
 
 extern "C" void ReportFatalException(

diff --git a/lib/Common/Exceptions/Throw.cpp b/lib/Common/Exceptions/Throw.cpp
@@ -82,6 +82,14 @@ namespace Js {
         RaiseException((DWORD)DBG_TERMINATE_PROCESS, EXCEPTION_NONCONTINUABLE, 0, NULL);
     }
 
+#ifdef ENABLE_JS_BUILTINS
+    void Throw::FatalJsBuiltInError()
+    {
+        AssertMsg(false, "Could not initialize JsBuiltIns!");
+        ReportFatalException(NULL, E_UNEXPECTED, Fatal_JsBuiltIn_Error, 0);
+    }
+#endif
+
 #if ENABLE_JS_REENTRANCY_CHECK
     void Throw::FatalJsReentrancyError()
     {

diff --git a/lib/Common/Exceptions/Throw.h b/lib/Common/Exceptions/Throw.h
@@ -25,7 +25,9 @@ namespace Js {
 #if ENABLE_JS_REENTRANCY_CHECK
         static void __declspec(noreturn) FatalJsReentrancyError();
 #endif
-
+#ifdef ENABLE_JS_BUILTINS
+        static void __declspec(noreturn) FatalJsBuiltInError();
+#endif
         static void CheckAndThrowOutOfMemory(BOOLEAN status);
 
         static bool ReportAssert(__in LPCSTR fileName, uint lineNumber, __in LPCSTR error, __in LPCSTR message);

diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -76,8 +76,12 @@ namespace Js
         m_functionNumber(functionNumber),
         m_defaultEntryPointInfo(nullptr),
         m_displayNameIsRecyclerAllocated(false),
-        m_tag11(true)
+        m_tag11(true),
+        m_isJsBuiltInCode(false)
     {
+#if DBG
+        m_isJsBuiltInInitCode = false;
+#endif
         PERF_COUNTER_INC(Code, TotalFunction);
     }
 

diff --git a/lib/Runtime/Base/FunctionBody.h b/lib/Runtime/Base/FunctionBody.h
@@ -1307,6 +1307,11 @@ namespace Js
         void SetIsJsBuiltInCode() { m_isJsBuiltInCode = true; }
         bool IsJsBuiltInCode() const { return m_isJsBuiltInCode; }
 
+#if DBG
+        void SetIsJsBuiltInInitCode() { m_isJsBuiltInInitCode = true; }
+        bool IsJsBuiltInInitCode() { return m_isJsBuiltInInitCode; }
+#endif
+
 #if DBG
         bool HasValidEntryPoint() const;
 #if defined(ENABLE_SCRIPT_PROFILING) || defined(ENABLE_SCRIPT_DEBUGGING)
@@ -1367,6 +1372,9 @@ namespace Js
         FieldWithBarrier(bool) m_isTopLevel : 1; // Indicates that this function is top-level function, currently being used in script profiler and debugger
         FieldWithBarrier(bool) m_isPublicLibraryCode: 1; // Indicates this function is public boundary library code that should be visible in JS stack
         FieldWithBarrier(bool) m_isJsBuiltInCode: 1; // Indicates this function comes from the JS Built In implementation
+#if DBG
+        FieldWithBarrier(bool) m_isJsBuiltInInitCode: 1;
+#endif
         FieldWithBarrier(bool) m_canBeDeferred : 1;
         FieldWithBarrier(bool) m_displayNameIsRecyclerAllocated : 1;
 

diff --git a/lib/Runtime/Language/InterpreterStackFrame.cpp b/lib/Runtime/Language/InterpreterStackFrame.cpp
@@ -8372,6 +8372,7 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(uint loopId)
 
     Var InterpreterStackFrame::GetRootObject() const
     {
+        Assert(!this->GetFunctionBody()->IsJsBuiltInInitCode());
         Var rootObject = GetReg(Js::FunctionBody::RootObjectRegSlot);
         Assert(rootObject == this->GetFunctionBody()->LoadRootObject());
         return rootObject;

diff --git a/lib/Runtime/Library/EngineInterfaceObjectBuiltIns.h b/lib/Runtime/Library/EngineInterfaceObjectBuiltIns.h
@@ -10,6 +10,7 @@ GlobalBuiltInConstructor(String)
 GlobalBuiltInConstructor(Date)
 GlobalBuiltInConstructor(Error) /*This was added back in to allow assert errors*/
 GlobalBuiltInConstructor(Map)
+GlobalBuiltInConstructor(Symbol)
 
 GlobalBuiltIn(Math,Abs)
 GlobalBuiltIn(Math,Floor)

diff --git a/lib/Runtime/Library/JavascriptFunction.cpp b/lib/Runtime/Library/JavascriptFunction.cpp
@@ -33,12 +33,14 @@ namespace Js
         : DynamicObject(type), functionInfo(nullptr), constructorCache(&ConstructorCache::DefaultInstance)
     {
         Assert(this->constructorCache != nullptr);
+#if DBG
+        isJsBuiltInInitCode = false;
+#endif
     }
 
 
     JavascriptFunction::JavascriptFunction(DynamicType * type, FunctionInfo * functionInfo)
         : DynamicObject(type), functionInfo(functionInfo), constructorCache(&ConstructorCache::DefaultInstance)
-
     {
         Assert(this->constructorCache != nullptr);
         this->GetTypeHandler()->ClearHasOnlyWritableDataProperties(); // length is non-writable
@@ -50,11 +52,13 @@ namespace Js
             // GetScriptContext()->InvalidateStoreFieldCaches(PropertyIds::length);
             GetLibrary()->NoPrototypeChainsAreEnsuredToHaveOnlyWritableDataProperties();
         }
+#if DBG
+        isJsBuiltInInitCode = false;
+#endif
     }
 
     JavascriptFunction::JavascriptFunction(DynamicType * type, FunctionInfo * functionInfo, ConstructorCache* cache)
         : DynamicObject(type), functionInfo(functionInfo), constructorCache(cache)
-
     {
         Assert(this->constructorCache != nullptr);
         this->GetTypeHandler()->ClearHasOnlyWritableDataProperties(); // length is non-writable
@@ -66,6 +70,9 @@ namespace Js
             // GetScriptContext()->InvalidateStoreFieldCaches(PropertyIds::length);
             GetLibrary()->NoPrototypeChainsAreEnsuredToHaveOnlyWritableDataProperties();
         }
+#if DBG
+        isJsBuiltInInitCode = false;
+#endif
     }
 
     FunctionProxy *JavascriptFunction::GetFunctionProxy() const

diff --git a/lib/Runtime/Library/JavascriptFunction.h b/lib/Runtime/Library/JavascriptFunction.h
@@ -43,7 +43,9 @@ namespace Js
         Field(ConstructorCache*) constructorCache;
 
         Field(bool) isJsBuiltInCode;
-
+#if DBG
+        Field(bool) isJsBuiltInInitCode;
+#endif
     protected:
 
         Field(FunctionInfo *) functionInfo;  // Underlying function

diff --git a/lib/Runtime/Library/JsBuiltIn/JsBuiltIn.js b/lib/Runtime/Library/JsBuiltIn/JsBuiltIn.js
@@ -16,6 +16,7 @@
 
     var setPrototype = platform.builtInSetPrototype;
     var _objectDefineProperty = platform.builtInJavascriptObjectEntryDefineProperty;
+    var Symbol = platform.Symbol;
 
     // Object's getter and setter can get overriden on the prototype, in that case while setting the value attributes, we will end up with TypeError
     // So, we need to set the prototype of attributes to null