[MERGE #6135 @boingoing] SetProperty doesn't handle the case where the receiver object has a non-writable property

Merge pull request #6135 from boingoing:nonwritablepropertyreceiver

SetProperty doesn't handle the case where the receiver object has a non-writable property

When checking for non-writable properties while trying to set a property, we only check the object. If the receiver has a non-writable property we should throw in strict mode.

Also adds StSuperFldStrict and a profiled version because strictness is not preserved in some backend calls to StSuperFld.

Fixes: #5948

Author: boingoing

lib/Backend/BackwardPass.cpp

@@ -3442,6 +3442,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
                 case Js::OpCode::StSlotBoxTemp:
                 case Js::OpCode::StSlotChkUndecl:
                 case Js::OpCode::StSuperFld:
+                case Js::OpCode::StSuperFldStrict:
                 case Js::OpCode::ProfiledStElemI_A:
                 case Js::OpCode::ProfiledStElemI_A_Strict:
                 case Js::OpCode::ProfiledStFld:
@@ -3450,6 +3451,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
                 case Js::OpCode::ProfiledStRootFld:
                 case Js::OpCode::ProfiledStRootFldStrict:
                 case Js::OpCode::ProfiledStSuperFld:
+                case Js::OpCode::ProfiledStSuperFldStrict:
                     // Unfortunately, being fed into a store means that we could have aliasing, and the
                     // consequence is that it may be re-read and then dereferenced. Note that we can do
                     // this case if we poison any array symbol that we store to on the way out, but the

lib/Backend/GlobOpt.cpp

@@ -13636,6 +13636,7 @@ GlobOpt::CheckJsArrayKills(IR::Instr *const instr)
         case Js::OpCode::StFld:
         case Js::OpCode::StFldStrict:
         case Js::OpCode::StSuperFld:
+        case Js::OpCode::StSuperFldStrict:
         {
             Assert(instr->GetDst());
 

lib/Backend/GlobOptExpr.cpp

@@ -828,6 +828,7 @@ GlobOpt::ProcessArrayValueKills(IR::Instr *instr)
     case Js::OpCode::StFldStrict:
     case Js::OpCode::StRootFldStrict:
     case Js::OpCode::StSuperFld:
+    case Js::OpCode::StSuperFldStrict:
     case Js::OpCode::StSlot:
     case Js::OpCode::StSlotChkUndecl:
     case Js::OpCode::DeleteFld:

lib/Backend/GlobOptFields.cpp

@@ -429,6 +429,7 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse<JitArenaAllocator> *bv, bo
     case Js::OpCode::StSlot:
     case Js::OpCode::StSlotChkUndecl:
     case Js::OpCode::StSuperFld:
+    case Js::OpCode::StSuperFldStrict:
         Assert(dstOpnd != nullptr);
         sym = dstOpnd->AsSymOpnd()->m_sym;
         if (inGlobOpt)

lib/Backend/IR.cpp

@@ -1036,7 +1036,8 @@ bool IR::Instr::IsStFldVariant() const
         this->m_opcode == Js::OpCode::StLocalFld ||
         this->m_opcode == Js::OpCode::StRootFld ||
         this->m_opcode == Js::OpCode::StRootFldStrict ||
-        this->m_opcode == Js::OpCode::StSuperFld;
+        this->m_opcode == Js::OpCode::StSuperFld ||
+        this->m_opcode == Js::OpCode::StSuperFldStrict;
 }
 
 bool IR::Instr::IsStElemVariant() const

lib/Backend/IRBuilder.cpp

@@ -4763,10 +4763,12 @@ IRBuilder::BuildElementC2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot insta
         break;
 
     case Js::OpCode::ProfiledStSuperFld:
+    case Js::OpCode::ProfiledStSuperFldStrict:
         Js::OpCodeUtil::ConvertNonCallOpToNonProfiled(newOpcode);
         // fall-through
 
     case Js::OpCode::StSuperFld:
+    case Js::OpCode::StSuperFldStrict:
     {
         propertyId = m_func->GetJITFunctionBody()->GetPropertyIdFromCacheId(propertyIdIndex);
         fieldSymOpnd = this->BuildFieldOpnd(newOpcode, instanceSlot, propertyId, (Js::PropertyIdIndexType) - 1, PropertyKindData, propertyIdIndex);

lib/Backend/JnHelperMethodList.h

@@ -413,6 +413,7 @@ HELPERCALLCHK(ProfiledLdRootFld, Js::ProfilingHelpers::ProfiledLdRootFld_Jit, 0)
 HELPERCALLCHK(ProfiledLdRootMethodFld, Js::ProfilingHelpers::ProfiledLdRootMethodFld_Jit, 0)
 HELPERCALLCHK(ProfiledStFld, Js::ProfilingHelpers::ProfiledStFld_Jit, 0)
 HELPERCALLCHK(ProfiledStSuperFld, Js::ProfilingHelpers::ProfiledStSuperFld_Jit, 0)
+HELPERCALLCHK(ProfiledStSuperFld_Strict, Js::ProfilingHelpers::ProfiledStSuperFld_Strict_Jit, 0)
 HELPERCALLCHK(ProfiledStFld_Strict, Js::ProfilingHelpers::ProfiledStFld_Strict_Jit, 0)
 HELPERCALLCHK(ProfiledStRootFld, Js::ProfilingHelpers::ProfiledStRootFld_Jit, 0)
 HELPERCALLCHK(ProfiledStRootFld_Strict, Js::ProfilingHelpers::ProfiledStRootFld_Strict_Jit, 0)

lib/Backend/Lower.cpp

@@ -159,7 +159,6 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
 {
     bool noMathFastPath;
     bool noFieldFastPath;
-    bool isStrictMode = this->m_func->GetJITFunctionBody()->IsStrictMode();
     noFieldFastPath = !defaultDoFastPath;
     noMathFastPath = !defaultDoFastPath;
 
@@ -538,7 +537,12 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
 
         case Js::OpCode::StSuperFld:
             instrPrev = GenerateCompleteStFld(instr, !noFieldFastPath, IR::HelperOp_PatchPutValueWithThisPtrNoLocalFastPath, IR::HelperOp_PatchPutValueWithThisPtrNoLocalFastPathPolymorphic,
-                IR::HelperOp_PatchPutValueWithThisPtr, IR::HelperOp_PatchPutValueWithThisPtrPolymorphic, true, isStrictMode ? Js::PropertyOperation_StrictMode : Js::PropertyOperation_None);
+                IR::HelperOp_PatchPutValueWithThisPtr, IR::HelperOp_PatchPutValueWithThisPtrPolymorphic, true, Js::PropertyOperation_None);
+            break;
+
+        case Js::OpCode::StSuperFldStrict:
+            instrPrev = GenerateCompleteStFld(instr, !noFieldFastPath, IR::HelperOp_PatchPutValueWithThisPtrNoLocalFastPath, IR::HelperOp_PatchPutValueWithThisPtrNoLocalFastPathPolymorphic,
+                IR::HelperOp_PatchPutValueWithThisPtr, IR::HelperOp_PatchPutValueWithThisPtrPolymorphic, true, Js::PropertyOperation_StrictMode);
             break;
 
         case Js::OpCode::StRootFld:
@@ -7132,7 +7136,7 @@ Lowerer::LowerProfiledStFld(IR::JitProfilingInstr *stFldInstr, Js::PropertyOpera
 
     m_lowererMD.LoadHelperArgument(stFldInstr, IR::Opnd::CreateFramePointerOpnd(m_func));
 
-    if (stFldInstr->m_opcode == Js::OpCode::StSuperFld)
+    if (stFldInstr->m_opcode == Js::OpCode::StSuperFld || stFldInstr->m_opcode == Js::OpCode::StSuperFldStrict)
     {
         m_lowererMD.LoadHelperArgument(stFldInstr, stFldInstr->UnlinkSrc2());
     }
@@ -7159,6 +7163,10 @@ Lowerer::LowerProfiledStFld(IR::JitProfilingInstr *stFldInstr, Js::PropertyOpera
         helper = IR::HelperProfiledStSuperFld;
         break;
 
+    case Js::OpCode::StSuperFldStrict:
+        helper = IR::HelperProfiledStSuperFld_Strict;
+        break;
+
     default:
         helper =
             flags & Js::PropertyOperation_Root
@@ -7224,7 +7232,7 @@ Lowerer::LowerStFld(
     }
 
     IR::Opnd *src = stFldInstr->UnlinkSrc1();
-    if (stFldInstr->m_opcode == Js::OpCode::StSuperFld)
+    if (stFldInstr->m_opcode == Js::OpCode::StSuperFld || stFldInstr->m_opcode == Js::OpCode::StSuperFldStrict)
     {
         m_lowererMD.LoadHelperArgument(stFldInstr, stFldInstr->UnlinkSrc2());
     }

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.
 
-// {2734B2C7-9500-45AF-AAA6-FABB7B9C3F81}
+// {5EEF3B18-B808-42EF-BA86-A68AA3DBEEBA}
 const GUID byteCodeCacheReleaseFileVersion =
-{ 0x2734b2c7, 0x9500, 0x45af, { 0xaa, 0xa6, 0xfa, 0xbb, 0x7b, 0x9c, 0x3f, 0x81 } };
+{ 0x5eef3b18, 0xb808, 0x42ef, { 0xba, 0x86, 0xa6, 0x8a, 0xa3, 0xdb, 0xee, 0xba } };

lib/Runtime/ByteCode/ByteCodeDumper.cpp

@@ -767,12 +767,14 @@ namespace Js
                 break;
             }
             case OpCode::StSuperFld:
+            case OpCode::StSuperFldStrict:
             {
                 Output::Print(_u(" R%d.%s(this=R%d) = R%d #%d"), data->Instance, pPropertyName->GetBuffer(),
                     data->Value2, data->Value, data->PropertyIdIndex);
                 break;
             }
             case OpCode::ProfiledStSuperFld:
+            case OpCode::ProfiledStSuperFldStrict:
             {
                 Output::Print(_u(" R%d.%s(this=R%d) = R%d #%d"), data->Instance, pPropertyName->GetBuffer(),
                     data->Value2, data->Value, data->PropertyIdIndex);