Fixes bug where ImplicitCallFlags and DisableImplicitFlags were not restored to previous values held.

lib/Runtime/Base/ThreadContext.h

@@ -347,6 +347,25 @@ public:
         WorkerThread(HANDLE handle = nullptr) :threadHandle(handle){};
     };
 
+    struct AutoRestoreImplicitFlags
+    {
+        ThreadContext * threadContext;
+        Js::ImplicitCallFlags savedImplicitCallFlags;
+        DisableImplicitFlags savedDisableImplicitFlags;
+        AutoRestoreImplicitFlags(ThreadContext *threadContext, Js::ImplicitCallFlags implicitCallFlags, DisableImplicitFlags disableImplicitFlags) :
+            threadContext(threadContext),
+            savedImplicitCallFlags(implicitCallFlags),
+            savedDisableImplicitFlags(disableImplicitFlags)
+        {
+        }
+
+        ~AutoRestoreImplicitFlags()
+        {
+            threadContext->SetImplicitCallFlags((Js::ImplicitCallFlags)(savedImplicitCallFlags));
+            threadContext->SetDisableImplicitFlags((DisableImplicitFlags)savedDisableImplicitFlags);
+        }
+    };
+
     void SetCurrentThreadId(DWORD threadId) { this->currentThreadId = threadId; }
     DWORD GetCurrentThreadId() const { return this->currentThreadId; }
     void SetIsThreadBound()

lib/Runtime/Language/InlineCache.inl

@@ -184,13 +184,12 @@ namespace Js
         bool canSetField; // To verify if we can set a field on the object
         Var setterValue = nullptr;
         {
-            // We need to disable implicit call to ensure the check doesn't cause unwanted side effects in debug code
-            // Save old disableImplicitFlags and implicitCallFlags and disable implicit call and exception
+            // We need to disable implicit call to ensure the check doesn't cause unwanted side effects in debug
+            // code Save old disableImplicitFlags and implicitCallFlags and disable implicit call and exception.
             ThreadContext * threadContext = requestContext->GetThreadContext();
-            DisableImplicitFlags disableImplicitFlags = *threadContext->GetAddressOfDisableImplicitFlags();
-            Js::ImplicitCallFlags implicitCallFlags = threadContext->GetImplicitCallFlags();
+            ThreadContext::AutoRestoreImplicitFlags autoRestoreImplicitFlags(threadContext, threadContext->GetImplicitCallFlags(), threadContext->GetDisableImplicitFlags());
             threadContext->ClearImplicitCallFlags();
-            *threadContext->GetAddressOfDisableImplicitFlags() = DisableImplicitCallAndExceptionFlag;
+            threadContext->SetDisableImplicitFlags(DisableImplicitCallAndExceptionFlag);
 
             DescriptorFlags flags = DescriptorFlags::None;
             canSetField = !JavascriptOperators::CheckPrototypesForAccessorOrNonWritablePropertySlow(object, propertyId, &setterValue, &flags, isRoot, requestContext);
@@ -199,15 +198,12 @@ namespace Js
                 canSetField = true; // If there was an implicit call, inconclusive. Disable debug check.
                 setterValue = nullptr;
             }
-            else
-                if ((flags & Accessor) == Accessor)
+            else if ((flags & Accessor) == Accessor)
             {
                 Assert(setterValue != nullptr);
             }
 
-            // Restore old disableImplicitFlags and implicitCallFlags
-            *threadContext->GetAddressOfDisableImplicitFlags() = disableImplicitFlags;
-            threadContext->SetImplicitCallFlags(implicitCallFlags);
+            // ImplicitCallFlags and DisableImplicitFlags restored by AutoRestoreImplicitFlags' destructor.
         }
 #endif
 

lib/Runtime/Library/JsBuiltInEngineInterfaceExtensionObject.cpp

@@ -131,26 +131,6 @@ namespace Js
             return;
         }
 
-        struct AutoRestoreFlags
-        {
-            ThreadContext * ctx;
-            ImplicitCallFlags savedImplicitCallFlags;
-            DisableImplicitFlags savedDisableImplicitFlags;
-            AutoRestoreFlags(ThreadContext *ctx, Js::ImplicitCallFlags implFlags, DisableImplicitFlags disableImplFlags) :
-                ctx(ctx),
-                savedImplicitCallFlags(implFlags),
-                savedDisableImplicitFlags(disableImplFlags)
-            {
-                ctx->ClearDisableImplicitFlags();
-            }
-
-            ~AutoRestoreFlags()
-            {
-                ctx->SetImplicitCallFlags((Js::ImplicitCallFlags)(savedImplicitCallFlags));
-                ctx->SetDisableImplicitFlags((DisableImplicitFlags)savedDisableImplicitFlags);
-            }
-        };
-
         try {
             EnsureJsBuiltInByteCode(scriptContext);
             Assert(jsBuiltInByteCode != nullptr);
@@ -189,7 +169,8 @@ namespace Js
 #endif
             // Clear disable implicit call bit as initialization code doesn't have any side effect
             {
-                AutoRestoreFlags autoRestoreFlags(scriptContext->GetThreadContext(), scriptContext->GetThreadContext()->GetImplicitCallFlags(), scriptContext->GetThreadContext()->GetDisableImplicitFlags());
+                ThreadContext::AutoRestoreImplicitFlags autoRestoreImplicitFlags(scriptContext->GetThreadContext(), scriptContext->GetThreadContext()->GetImplicitCallFlags(), scriptContext->GetThreadContext()->GetDisableImplicitFlags());
+                scriptContext->GetThreadContext()->ClearDisableImplicitFlags();
                 JavascriptFunction::CallRootFunctionInScript(functionGlobal, Js::Arguments(callInfo, args));
             }
 
@@ -198,7 +179,8 @@ namespace Js
 
             // Clear disable implicit call bit as initialization code doesn't have any side effect
             {
-                AutoRestoreFlags autoRestoreFlags(scriptContext->GetThreadContext(), scriptContext->GetThreadContext()->GetImplicitCallFlags(), scriptContext->GetThreadContext()->GetDisableImplicitFlags());
+                ThreadContext::AutoRestoreImplicitFlags autoRestoreImplicitFlags(scriptContext->GetThreadContext(), scriptContext->GetThreadContext()->GetImplicitCallFlags(), scriptContext->GetThreadContext()->GetDisableImplicitFlags());
+                scriptContext->GetThreadContext()->ClearDisableImplicitFlags();
                 JavascriptFunction::CallRootFunctionInScript(functionBuiltins, Js::Arguments(callInfo, args));
             }
 

test/TryCatch/TryCatchStackOverflow.js

@@ -0,0 +1,16 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var a = [];
+function f() {
+    try {
+        f();
+    } catch (e) {
+        a.foo = 100;
+    }
+}
+f();
+
+print("pass");

test/TryCatch/rlexe.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<regress-exe>
+  <test>
+    <default>
+      <files>TryCatchStackOverflow.js</files>
+    </default>
+  </test>
+</regress-exe>

test/rlexedirs.xml

@@ -1,370 +1,375 @@
-<?xml version="1.0" encoding="utf-8"?>
-<regress-exe>
-<dir>
-  <default>
-    <files>262</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>UnitTestFramework</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Array</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Basics</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>ConfigParsing</files>
-    <tags>exclude_jshost,exclude_xplat</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>es5</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>EH</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>InlineCaches</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>LetConst</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Basics</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Generated</files>
-    <tags>Slow</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Closures</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Strings</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Date</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Intl</files>
-    <tags>exclude_noicu</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Error</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Boolean</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Number</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>BigInt</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>ControlFlow</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Math</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Array</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>TaggedIntegers</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>TaggedFloats</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Optimizer</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Function</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Object</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Regex</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Prototypes</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>GlobalFunctions</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Operators</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Conversions</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>RWC</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>VT_DATE</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Lib</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>JSON</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Bugs</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>strict</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Utf8</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>UnifiedRegex</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>typedarray</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>bailout</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Loop</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>StackTrace</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Miscellaneous</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>fieldopts</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>FixedFields</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>inlining</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>DynamicCode</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>es6</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>es6module</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>es7</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>switchStatement</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>stackfunc</files>
-    <tags>exclude_serialized,require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>PerfHint</files>
-    <tags>exclude_serialized,exclude_snap,require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>AsmJs</files>
-    <tags>exclude_serialized,require_backend,require_asmjs</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>AsmJsFloat</files>
-    <tags>exclude_serialized,require_backend,require_asmjs</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>AsmJsParser</files>
-    <tags>exclude_serialized,require_backend,require_asmjs</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>WasmSpec</files>
-    <tags>exclude_serialized,require_backend,exclude_jshost,exclude_win7,require_wasm</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>WasmSpec.MultiValue</files>
-    <tags>exclude_serialized,require_backend,require_wasm</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>wasm</files>
-    <tags>exclude_serialized,require_backend,require_wasm</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Debugger</files>
-    <tags>exclude_serialized,exclude_jshost,exclude_snap,require_debugger,exclude_sanitize_address</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>DebuggerCommon</files>
-    <tags>exclude_serialized,exclude_jshost,exclude_snap,require_debugger,exclude_sanitize_address</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>TTExecuteBasic</files>
-    <tags>sequential,exclude_dynapogo,exclude_jshost,exclude_snap,exclude_serialized,require_debugger,exclude_sanitize_address</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>TTBasic</files>
-    <tags>sequential,exclude_dynapogo,exclude_jshost,exclude_snap,exclude_serialized,require_debugger,exclude_sanitize_address</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>Scanner</files>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>wasm.simd</files>
-    <tags>require_wasm,exclude_serialized,require_backend,exclude_xplat</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>PRE</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-<dir>
-  <default>
-    <files>FlowGraph</files>
-    <tags>require_backend</tags>
-  </default>
-</dir>
-</regress-exe>
+<?xml version="1.0" encoding="utf-8"?>
+<regress-exe>
+<dir>
+  <default>
+    <files>262</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>UnitTestFramework</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Array</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Basics</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>ConfigParsing</files>
+    <tags>exclude_jshost,exclude_xplat</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>es5</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>EH</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>InlineCaches</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>LetConst</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Basics</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Generated</files>
+    <tags>Slow</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Closures</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Strings</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Date</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Intl</files>
+    <tags>exclude_noicu</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Error</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Boolean</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Number</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>BigInt</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>ControlFlow</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Math</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Array</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>TaggedIntegers</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>TaggedFloats</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>TryCatch</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Optimizer</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Function</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Object</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Regex</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Prototypes</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>GlobalFunctions</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Operators</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Conversions</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>RWC</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>VT_DATE</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Lib</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>JSON</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Bugs</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>strict</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Utf8</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>UnifiedRegex</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>typedarray</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>bailout</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Loop</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>StackTrace</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Miscellaneous</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>fieldopts</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>FixedFields</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>inlining</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>DynamicCode</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>es6</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>es6module</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>es7</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>switchStatement</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>stackfunc</files>
+    <tags>exclude_serialized,require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>PerfHint</files>
+    <tags>exclude_serialized,exclude_snap,require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>AsmJs</files>
+    <tags>exclude_serialized,require_backend,require_asmjs</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>AsmJsFloat</files>
+    <tags>exclude_serialized,require_backend,require_asmjs</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>AsmJsParser</files>
+    <tags>exclude_serialized,require_backend,require_asmjs</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>WasmSpec</files>
+    <tags>exclude_serialized,require_backend,exclude_jshost,exclude_win7,require_wasm</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>WasmSpec.MultiValue</files>
+    <tags>exclude_serialized,require_backend,require_wasm</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>wasm</files>
+    <tags>exclude_serialized,require_backend,require_wasm</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Debugger</files>
+    <tags>exclude_serialized,exclude_jshost,exclude_snap,require_debugger,exclude_sanitize_address</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>DebuggerCommon</files>
+    <tags>exclude_serialized,exclude_jshost,exclude_snap,require_debugger,exclude_sanitize_address</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>TTExecuteBasic</files>
+    <tags>sequential,exclude_dynapogo,exclude_jshost,exclude_snap,exclude_serialized,require_debugger,exclude_sanitize_address</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>TTBasic</files>
+    <tags>sequential,exclude_dynapogo,exclude_jshost,exclude_snap,exclude_serialized,require_debugger,exclude_sanitize_address</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>Scanner</files>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>wasm.simd</files>
+    <tags>require_wasm,exclude_serialized,require_backend,exclude_xplat</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>PRE</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+<dir>
+  <default>
+    <files>FlowGraph</files>
+    <tags>require_backend</tags>
+  </default>
+</dir>
+</regress-exe>