[MERGE #4957 @sigatrev] OS#16050168: fix jit crash for splitscope using 'with' statements

Merge pull request #4957 from sigatrev:withBodyScope

When 'with' statements are used in inside param scopes when param and body scopes are split, there will not always be a local closure generated. See attached test for an example case.

The IRBuilder would attempt to build a RegOpnd for the closure, and would assert on a nullptr sym. The backward pass would later AV on that nullptr sym.

This commit instead assigns nullptr to the param closure when there is no local closure, which is the behavior of InterpreterStackFrame::OP_BeginBodyScope()

Author: sigatrev

lib/Backend/IRBuilder.cpp

@@ -6937,23 +6937,35 @@ IRBuilder::BuildEmpty(Js::OpCode newOpcode, uint32 offset)
         break;
 
     case Js::OpCode::BeginBodyScope:
+    {
         // This marks the end of a param socpe which is not merged with body scope.
         // So we have to first cache the closure so that we can use it to copy the initial values for
         // body syms from corresponding param syms (LdParamSlot). Body should get its own scope slot.
         Assert(!this->IsParamScopeDone());
         this->SetParamScopeDone();
 
+        IR::Opnd * localClosureOpnd;
+        if (this->m_func->GetLocalClosureSym() != nullptr)
+        {
+            localClosureOpnd = IR::RegOpnd::New(this->m_func->GetLocalClosureSym(), TyVar, this->m_func);
+        }
+        else
+        {
+            AssertOrFailFast(this->m_func->GetJITFunctionBody()->GetScopeSlotArraySize() == 0 && !this->m_func->GetJITFunctionBody()->HasScopeObject());
+            localClosureOpnd = IR::IntConstOpnd::New(0, TyVar, this->m_func);
+        }
+
         this->AddInstr(
             IR::Instr::New(
                 Js::OpCode::Ld_A,
                 this->BuildDstOpnd(this->m_func->GetJITFunctionBody()->GetParamClosureReg()),
-                IR::RegOpnd::New(this->m_func->GetLocalClosureSym(), TyVar, this->m_func),
+                localClosureOpnd,
                 this->m_func),
             offset);
 
         // Create a new local closure for the body when either body scope has scope slots allocated or
         // eval is present which can leak declarations.
-        if (this->m_func->GetJITFunctionBody()->GetScopeSlotArraySize()  > 0 || this->m_func->GetJITFunctionBody()->HasScopeObject())
+        if (this->m_func->GetJITFunctionBody()->GetScopeSlotArraySize() > 0 || this->m_func->GetJITFunctionBody()->HasScopeObject())
         {
             if (this->m_func->GetJITFunctionBody()->HasScopeObject())
             {
@@ -6994,6 +7006,7 @@ IRBuilder::BuildEmpty(Js::OpCode newOpcode, uint32 offset)
             lfd->isNonFastPathFrameDisplay = true;
         }
         break;
+    }
 
     default:
         this->AddInstr(instr, offset);

test/Bugs/rlexe.xml

@@ -496,4 +496,9 @@
       <compile-flags>-force:deferparse -parserstatecache -useparserstatecache</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>withSplitScope.js</files>
+    </default>
+  </test>
 </regress-exe>

test/Bugs/withSplitScope.js

@@ -0,0 +1,23 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function foo()
+{
+    (function bar(a = 
+        (function() 
+        {
+            with (1)
+            {
+                bar;
+            }
+        })()
+    ){})();
+}
+
+foo();
+foo();
+foo();
+
+WScript.Echo("Pass");