OS:12625415:Don't add let attribute for built-in arguments symbol when function has non-simple parameters

In cached scope when we create the scope object we pass let attribute for all formals. This was marking the built-in arguments symbol also with let. This change removes it.

lib/Runtime/Language/JavascriptOperators.cpp

@@ -5706,11 +5706,18 @@ SetElementIHelper_INDEX_TYPE_IS_NUMBER:
                 uint formalsSlotLimit = (firstFuncSlot != Constants::NoProperty) ? (uint)firstFuncSlot :
                                         (firstVarSlot != Constants::NoProperty) ? (uint)firstVarSlot :
                                         propIds->count;
-                type = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
+                if (func->GetFunctionBody()->HasReferenceableBuiltInArguments())
+                {
+                    type = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
+                }
+                else
+                {
+                    type = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
+                }
             }
             else
             {
-                type = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, type, propIds);
+                type = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, type, propIds);
             }
             *literalType = type;
         }
@@ -7167,7 +7174,26 @@ SetElementIHelper_INDEX_TYPE_IS_NUMBER:
                 // CONSIDER : When we delay type sharing until the second instance is created, pass an argument indicating we want the types
                 // and handlers created here to be marked as shared up-front. This is to ensure we don't get any fixed fields and that the handler
                 // is ready for storing values directly to slots.
-                DynamicType* newType = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, frameObject->GetDynamicType(), propIds, nonSimpleParamList ? PropertyLetDefaults : PropertyNone);
+                DynamicType* newType = nullptr;
+                if (nonSimpleParamList)
+                {
+                    bool skipLetAttrForArguments = ((JavascriptGeneratorFunction::Is(funcCallee) || JavascriptAsyncFunction::Is(funcCallee)) ?
+                        JavascriptGeneratorFunction::FromVar(funcCallee)->GetGeneratorVirtualScriptFunction()->GetFunctionBody()->HasReferenceableBuiltInArguments()
+                        : funcCallee->GetFunctionBody()->HasReferenceableBuiltInArguments());
+
+                    if (skipLetAttrForArguments)
+                    {
+                        newType = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, frameObject->GetDynamicType(), propIds, PropertyLetDefaults);
+                    }
+                    else
+                    {
+                        newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), propIds, PropertyLetDefaults);
+                    }
+                }
+                else
+                {
+                    newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), propIds);
+                }
                 int oldSlotCapacity = frameObject->GetDynamicType()->GetTypeHandler()->GetSlotCapacity();
                 int newSlotCapacity = newType->GetTypeHandler()->GetSlotCapacity();
                 __analysis_assume((uint32)newSlotCapacity >= formalsCount);
@@ -7267,7 +7293,22 @@ SetElementIHelper_INDEX_TYPE_IS_NUMBER:
         // CONSIDER : When we delay type sharing until the second instance is created, pass an argument indicating we want the types
         // and handlers created here to be marked as shared up-front. This is to ensure we don't get any fixed fields and that the handler
         // is ready for storing values directly to slots.
-        DynamicType* newType = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), nonSimpleParamList ? PropertyLetDefaults : PropertyNone);
+        DynamicType* newType = nullptr;
+        if (nonSimpleParamList)
+        {
+            if (calleeBody->HasReferenceableBuiltInArguments())
+            {
+                newType = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), PropertyLetDefaults);
+            }
+            else
+            {
+                newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), PropertyLetDefaults);
+            }
+        }
+        else
+        {
+            newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray());
+        }
 
         int oldSlotCapacity = frameObject->GetDynamicType()->GetTypeHandler()->GetSlotCapacity();
         int newSlotCapacity = newType->GetTypeHandler()->GetSlotCapacity();

lib/Runtime/Types/PathTypeHandler.cpp

@@ -1880,8 +1880,8 @@ namespace Js
         return type;
     }
 
-    DynamicType *
-    PathTypeHandlerBase::CreateNewScopeObject(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount)
+    template <bool skipLetAttrForArguments>
+    DynamicType * PathTypeHandlerBase::CreateNewScopeObject(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount)
     {
         uint count = propIds->count;
 
@@ -1899,6 +1899,11 @@ namespace Js
             if (i < extraAttributesSlotCount)
             {
                 attributes |= extraAttributes;
+                if (skipLetAttrForArguments && propertyId == PropertyIds::arguments)
+                {
+                    // Do not add let attribute for built-in arguments symbol
+                    attributes &= ~PropertyLet;
+                }
             }
             typeHandler->Add(propertyRecord, attributes, scriptContext);
         }
@@ -1912,6 +1917,8 @@ namespace Js
 
         return type;
     }
+    template DynamicType * PathTypeHandlerBase::CreateNewScopeObject<true>(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount);
+    template DynamicType * PathTypeHandlerBase::CreateNewScopeObject<false>(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount);
 
     template <bool isObjectLiteral>
     DynamicType* PathTypeHandlerBase::PromoteType(DynamicType* predecessorType, const PathTypeSuccessorKey key, bool shareType, ScriptContext* scriptContext, DynamicObject* instance, PropertyIndex* propertyIndex)

lib/Runtime/Types/PathTypeHandler.h

@@ -127,7 +127,7 @@ namespace Js
         static bool ObjectSlotAttributesContains(const PropertyAttributes attr) { return attr == (attr & ObjectSlotAttr_PropertyAttributesMask); }
         static bool UsePathTypeHandlerForObjectLiteral(const PropertyIdArray *const propIds, bool *const check__proto__Ref = nullptr);
         static DynamicType* CreateTypeForNewScObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, bool shareType);
-        static DynamicType* CreateNewScopeObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, PropertyAttributes extraAttributes = PropertyNone, uint extraAttributesSlotCount = UINT_MAX);
+        template <bool skipLetAttrForArguments> static DynamicType* CreateNewScopeObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, PropertyAttributes extraAttributes = PropertyNone, uint extraAttributesSlotCount = UINT_MAX);
 
         static PathTypeHandlerBase * FromTypeHandler(DynamicTypeHandler * const typeHandler) { Assert(typeHandler->IsPathTypeHandler()); return static_cast<PathTypeHandlerBase*>(typeHandler); }
 

test/Bugs/ArgumentsAttrIssue.js

@@ -0,0 +1,12 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function f1(...args) {
+    eval("var arguments = 1;");
+}
+f1();
+f1();
+f1(); // Shouldn't throw.
+print("PASSED");

test/Bugs/rlexe.xml

@@ -517,4 +517,10 @@
       <compile-flags>-forceundodefer</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>ArgumentsAttrIssue.js</files>
+      <compile-flags>-force:cachedScope</compile-flags>
+    </default>
+  </test>
 </regress-exe>