[MERGE #5734 @wyrichte] Fixes #5576 - Chakra should not redeclare "undefined" keyword

Merge pull request #5734 from wyrichte:build/wyrichte/5576_1

Fixed by adding opcode OP_EnsureNoRootRedeclFunc that enforces the section of #sec-globaldeclarationinstantiation where #CanDeclareGlobalFunction is called. This enforces that when a global function that is not configurable, not writable, and not enumerable is overwritten a TypeError is thrown.

Author: wyrichte

lib/Backend/JnHelperMethodList.h

@@ -66,6 +66,7 @@ HELPERCALLCHK(Op_LdElemUndefDynamic, Js::JavascriptOperators::OP_LoadUndefinedTo
 HELPERCALLCHK(Op_LdElemUndefScoped, Js::JavascriptOperators::OP_LoadUndefinedToElementScoped, AttrCanNotBeReentrant)
 HELPERCALLCHK(Op_EnsureNoRootProperty, Js::JavascriptOperators::OP_EnsureNoRootProperty, AttrCanThrow | AttrCanNotBeReentrant)
 HELPERCALLCHK(Op_EnsureNoRootRedeclProperty, Js::JavascriptOperators::OP_EnsureNoRootRedeclProperty, AttrCanThrow | AttrCanNotBeReentrant)
+HELPERCALLCHK(Op_EnsureCanDeclGloFunc, Js::JavascriptOperators::OP_EnsureCanDeclGloFunc, AttrCanThrow | AttrCanNotBeReentrant)
 HELPERCALLCHK(Op_EnsureNoRedeclPropertyScoped, Js::JavascriptOperators::OP_ScopedEnsureNoRedeclProperty, AttrCanThrow | AttrCanNotBeReentrant)
 
 HELPERCALLCHK(Op_ToSpreadedFunctionArgument, Js::JavascriptOperators::OP_LdCustomSpreadIteratorList, AttrCanThrow)

lib/Backend/Lower.cpp

@@ -2491,6 +2491,10 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
             this->LowerElementUndefined(instr, IR::HelperOp_EnsureNoRootRedeclProperty);
             break;
 
+        case Js::OpCode::EnsureCanDeclGloFunc:
+            this->LowerElementUndefined(instr, IR::HelperOp_EnsureCanDeclGloFunc);
+            break;
+
         case Js::OpCode::ScopedEnsureNoRedeclFld:
             this->LowerElementUndefinedScoped(instr, IR::HelperOp_EnsureNoRedeclPropertyScoped);
             break;

lib/Parser/rterrors.h

@@ -370,6 +370,7 @@ RT_ERROR_MSG(JSERR_RegExpTooManyCapturingGroups, 5675, "", "Regular expression c
 RT_ERROR_MSG(JSERR_ProxyHandlerReturnedFalse, 5676, "Proxy %s handler returned false", "Proxy handler returned false", kjstTypeError, 0)
 RT_ERROR_MSG(JSERR_UnicodeRegExpRangeContainsCharClass, 5677, "%s", "Character classes not allowed in a RegExp class range.", kjstSyntaxError, 0)
 RT_ERROR_MSG(JSERR_DuplicateKeysFromOwnPropertyKeys, 5678, "%s", "Proxy's ownKeys trap returned duplicate keys", kjstTypeError, 0)
+RT_ERROR_MSG(JSERR_InvalidGloFuncDecl, 5679, "The global property %s is not configurable, writable, nor enumerable, therefore cannot be declared as a function", "", kjstTypeError, 0)
 
 //Host errors
 RT_ERROR_MSG(JSERR_HostMaybeMissingPromiseContinuationCallback, 5700, "", "Host may not have set any promise continuation callback. Promises may not be executed.", kjstTypeError, 0)

lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h

@@ -4,6 +4,6 @@
 //-------------------------------------------------------------------------------------------------------
 // NOTE: If there is a merge conflict the correct fix is to make a new GUID.
 
-// {BFA0BED0-7B1A-4018-83DA-B1D989D38BFD}
+// {9FAAF688-ACBA-4092-BA5B-77D97D3CD53A}
 const GUID byteCodeCacheReleaseFileVersion =
-{ 0xBFA0BED0, 0x7B1A, 0x4018, { 0x83, 0xDA, 0xB1, 0xD9, 0x89, 0xD3, 0x8B, 0xFD } };
+{ 0x9FAAF688, 0xACBA, 0x4092, { 0xBA, 0x5B, 0x77, 0xD9, 0x7D, 0x3C, 0xD5, 0x3A } };

lib/Runtime/ByteCode/ByteCodeDumper.cpp

@@ -630,6 +630,11 @@ namespace Js
                 Output::Print(_u(" root.%s"), pPropertyName->GetBuffer());
                 break;
             }
+            case OpCode::EnsureCanDeclGloFunc:
+            {
+                Output::Print(_u(" root.%s"), pPropertyName->GetBuffer());
+                break;
+            }
             case OpCode::LdLocalElemUndef:
             {
                 Output::Print(_u(" %s = undefined"), pPropertyName->GetBuffer());

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -5588,10 +5588,17 @@ void ByteCodeGenerator::EnsureNoRedeclarations(ParseNodeBlock *pnodeBlock, FuncI
         });
     }
 
-    auto emitRedeclCheck = [this](Symbol * sym, FuncInfo * funcInfo)
+    auto emitRedeclCheck = [this](Symbol * sym, FuncInfo * funcInfo, bool isFncDecl = false)
     {
         Js::PropertyId propertyId = sym->EnsurePosition(this);
 
+        // Global function declarations must pass #sec-candeclareglobalfunction
+        // which is enforced by EnsureCanDeclGloFunc
+        if (isFncDecl)
+        {
+            this->m_writer.ElementRootU(Js::OpCode::EnsureCanDeclGloFunc, funcInfo->FindOrAddReferencedPropertyId(propertyId));
+        }
+
         if (this->flags & fscrEval)
         {
             if (!funcInfo->byteCodeFunction->GetIsStrictMode())
@@ -5615,7 +5622,7 @@ void ByteCodeGenerator::EnsureNoRedeclarations(ParseNodeBlock *pnodeBlock, FuncI
         case knopFncDecl:
             if (pnode->AsParseNodeFnc()->IsDeclaration())
             {
-                emitRedeclCheck(pnode->AsParseNodeFnc()->pnodeName->sym, funcInfo);
+                emitRedeclCheck(pnode->AsParseNodeFnc()->pnodeName->sym, funcInfo, true);
             }
 
             pnode = pnode->AsParseNodeFnc()->pnodeNext;

lib/Runtime/ByteCode/OpCodes.h

@@ -71,10 +71,10 @@
     MACRO_WMS_WITH_DBG_ATTR(opcode, layout, attr, OpDbgAttr_LoadRoot)
 
 #define MACRO_EXTENDED_ROOT(opcode, layout, attr) \
-    MACRO_EXTENDED_WITH_DBG_ATTR(opcode, layout, attr, OpDbgAttr_LoadRoot)
+    MACRO_EXTEND_WITH_DBG_ATTR(opcode, layout, attr, OpDbgAttr_LoadRoot)
 
 #define MACRO_EXTEND_WMS_ROOT(opcode, layout, attr) \
-    MACRO_EXTENDED_WMS_WITH_DBG_ATTR(opcode, layout, attr, OpDbgAttr_LoadRoot)
+    MACRO_EXTEND_WMS_WITH_DBG_ATTR(opcode, layout, attr, OpDbgAttr_LoadRoot)
 
 #define MACRO_WMS_PROFILED( opcode, layout, attr) \
     MACRO_WMS(opcode, layout, OpHasProfiled|attr) \
@@ -354,6 +354,7 @@ MACRO_WMS(              ChkUndecl,                  Reg1,           OpSideEffect
 
 MACRO_WMS_ROOT(         EnsureNoRootFld,            ElementRootU,   OpSideEffect)
 MACRO_WMS_ROOT(         EnsureNoRootRedeclFld,      ElementRootU,   OpSideEffect)
+MACRO_EXTEND_WMS_ROOT(  EnsureCanDeclGloFunc,       ElementRootU,   OpSideEffect)
 MACRO_WMS(              ScopedEnsureNoRedeclFld,    ElementScopedC, OpSideEffect)
 
 MACRO_WMS(              InitUndecl,                 Reg1,           OpCanCSE)

lib/Runtime/Language/InterpreterHandler.inl

@@ -142,6 +142,7 @@ EXDEF2_WMS(A2toXX,                  SetComputedNameVar,         JavascriptOperat
   DEF2_WMS(XXtoA1,                  InitUndecl,                 OP_InitUndecl)
   DEF2_WMS(ELEM_RtU_to_XX,          EnsureNoRootFld,            OP_EnsureNoRootProperty)
   DEF2_WMS(ELEM_RtU_to_XX,          EnsureNoRootRedeclFld,      OP_EnsureNoRootRedeclProperty)
+EXDEF2_WMS(ELEM_RtU_to_XX,          EnsureCanDeclGloFunc,       OP_EnsureCanDeclGloFunc)
   DEF2_WMS(ELEM_C2_to_XX,           ScopedEnsureNoRedeclFld,    OP_ScopedEnsureNoRedeclProperty)
   DEF2_WMS(A1toA1Profiled,          ProfiledBeginSwitch,        PROFILEDOP(ProfiledSwitch<true>, ProfiledSwitch<false>))
   DEF2_WMS(XXtoA1Mem,               LdC_A_Null,                 JavascriptOperators::OP_LdNull)

lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -6526,6 +6526,12 @@ namespace Js
         JavascriptOperators::OP_EnsureNoRootRedeclProperty(instance, this->m_functionBody->GetReferencedPropertyId(propertyIdIndex));
     }
 
+    void InterpreterStackFrame::OP_EnsureCanDeclGloFunc(uint propertyIdIndex)
+    {
+        Var instance = this->GetRootObject();
+        JavascriptOperators::OP_EnsureCanDeclGloFunc(instance, this->m_functionBody->GetReferencedPropertyId(propertyIdIndex));
+    }
+
     void InterpreterStackFrame::OP_ScopedEnsureNoRedeclProperty(Var aValue, uint propertyIdIndex, Var aValue2)
     {
         Js::PropertyId propertyId = this->m_functionBody->GetReferencedPropertyId(propertyIdIndex);

lib/Runtime/Language/InterpreterStackFrame.h

@@ -656,6 +656,7 @@ namespace Js
 
         void OP_EnsureNoRootProperty(uint propertyIdIndex);
         void OP_EnsureNoRootRedeclProperty(uint propertyIdIndex);
+        void OP_EnsureCanDeclGloFunc(uint propertyIdIndex);
         void OP_ScopedEnsureNoRedeclProperty(Var aValue, uint propertyIdIndex, Var aValue2);
         Var OP_InitUndecl();
         void OP_InitUndeclSlot(Var aValue, int32 slot);

lib/Runtime/Language/JavascriptOperators.cpp

@@ -7572,6 +7572,15 @@ using namespace Js;
         JIT_HELPER_END(Op_EnsureNoRootRedeclProperty);
     }
 
+    void JavascriptOperators::OP_EnsureCanDeclGloFunc(Var instance, PropertyId propertyId)
+    {
+        JIT_HELPER_NOT_REENTRANT_NOLOCK_HEADER(Op_EnsureCanDeclGloFunc);
+        Assert(VarIs<RootObjectBase>(instance));
+        RootObjectBase *obj = VarTo<RootObjectBase>(instance);
+        obj->EnsureCanDeclGloFunc(propertyId);
+        JIT_HELPER_END(Op_EnsureCanDeclGloFunc);
+    }
+
     void JavascriptOperators::OP_ScopedEnsureNoRedeclProperty(FrameDisplay *pDisplay, PropertyId propertyId, Var defaultInstance)
     {
         JIT_HELPER_NOT_REENTRANT_NOLOCK_HEADER(Op_EnsureNoRedeclPropertyScoped);

lib/Runtime/Language/JavascriptOperators.h

@@ -173,6 +173,7 @@ namespace Js
         static BOOL EnsureProperty(Var instance, PropertyId propertyId);
         static void OP_EnsureNoRootProperty(Var instance, PropertyId propertyId);
         static void OP_EnsureNoRootRedeclProperty(Var instance, PropertyId propertyId);
+        static void OP_EnsureCanDeclGloFunc(Var instance, PropertyId propertyId);
         static void OP_ScopedEnsureNoRedeclProperty(FrameDisplay *pDisplay, PropertyId propertyId, Var instanceDefault);
         static JavascriptArray*  GetOwnPropertyNames(Var instance, ScriptContext *scriptContext);
         static JavascriptArray*  GetOwnPropertySymbols(Var instance, ScriptContext *scriptContext);