Make exception handling in typeof spec-complient. Move re-throw out of catch to avoid OS stack overflow.

lib/Runtime/Language/JavascriptOperators.h

@@ -22,36 +22,47 @@ namespace Js
     CONTEXT                 ep##c;                  \
     EXCEPTION_POINTERS      ep = {&ep##er, &ep##c};
 
+    // Typeof operator should return 'undefined' for undeclared or hoisted vars
+    // and propagate all other exceptions.
+    //
+    // NB: Re-throw from catch unwinds the active frame but doesn't clear the stack
+    // (catch clauses keep accumulating at the top of the stack until a catch 
+    // that doesn't re-throw). This is problematic if we've detected potential
+    // stack overflow and report it via exceptions: the handling of throw
+    // might actually overflow the stack and cause system SO exception.
+    // Thus, use catch to cache the exception, and re-throw it outside of the catch.
 #define TYPEOF_ERROR_HANDLER_CATCH(scriptContext, var) \
     } \
     catch (const JavascriptException& err) \
     { \
-        JavascriptExceptionObject* exceptionObject = err.GetAndClear(); \
+        exceptionObject = err.GetAndClear(); \
+        var = scriptContext->GetLibrary()->GetUndefined(); \
+
+#define TYPEOF_ERROR_HANDLER_THROW(scriptContext, var) \
+    } \
+    if (exceptionObject != nullptr) \
+    { \
         Js::Var errorObject = exceptionObject->GetThrownObject(nullptr); \
-        if (errorObject != nullptr && Js::JavascriptError::Is(errorObject)) \
+        HRESULT hr = (errorObject != nullptr && Js::JavascriptError::Is(errorObject)) \
+                     ? Js::JavascriptError::GetRuntimeError(Js::RecyclableObject::FromVar(errorObject), nullptr) \
+                     : S_OK; \
+        if (JavascriptError::GetErrorNumberFromResourceID(JSERR_UndefVariable) != (int32)hr) \
         { \
-            HRESULT hr = Js::JavascriptError::GetRuntimeError(Js::RecyclableObject::FromVar(errorObject), nullptr); \
-            if (JavascriptError::GetErrorNumberFromResourceID(JSERR_Property_CannotGet_NullOrUndefined) == (int32)hr \
-                || JavascriptError::GetErrorNumberFromResourceID(JSERR_UseBeforeDeclaration) == (int32)hr) \
+            if (scriptContext->IsScriptContextInDebugMode()) \
             { \
-                if (scriptContext->IsScriptContextInDebugMode()) \
-                { \
-                    JavascriptExceptionOperators::ThrowExceptionObject(exceptionObject, scriptContext, true); \
-                } \
-                else \
-                { \
-                    JavascriptExceptionOperators::DoThrow(exceptionObject, scriptContext); \
-                } \
+                JavascriptExceptionOperators::ThrowExceptionObject(exceptionObject, scriptContext, true); \
+            } \
+            else \
+            { \
+                JavascriptExceptionOperators::DoThrowCheckClone(exceptionObject, scriptContext); \
             } \
         } \
-        var = scriptContext->GetLibrary()->GetUndefined();
-
-#define TYPEOF_ERROR_HANDLER_THROW(scriptContext, var) \
     } \
     if (scriptContext->IsUndeclBlockVar(var)) \
     { \
         JavascriptError::ThrowReferenceError(scriptContext, JSERR_UseBeforeDeclaration); \
-    }
+    } \
+}
 
 #ifdef ENABLE_SCRIPT_DEBUGGING
 #define BEGIN_TYPEOF_ERROR_HANDLER_DEBUGGER_THROW_IS_INTERNAL \
@@ -80,6 +91,8 @@ namespace Js
 #endif
 
 #define BEGIN_TYPEOF_ERROR_HANDLER(scriptContext)  \
+{ \
+    JavascriptExceptionObject* exceptionObject = nullptr; \
     try { \
         Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext); \
         BEGIN_TYPEOF_ERROR_HANDLER_DEBUGGER_THROW_IS_INTERNAL

test/Basics/rlexe.xml

@@ -148,7 +148,6 @@
   <test>
     <default>
       <files>typeof.js</files>
-      <baseline>typeof.baseline</baseline>
       <compile-flags>-Intl-</compile-flags>
     </default>
   </test>

test/Basics/typeof.baseline

@@ -1,21 +0,0 @@
-typeof (new String()) : object
-typeof () : string
-typeof (new Boolean(false)) : object
-typeof (false) : boolean
-typeof (new Number(0)) : object
-typeof (0) : number
-typeof (new Number(12345.678)) : object
-typeof (12345.678) : number
-typeof function : function
-typeof function returning function : function
-Caught JS error on undefined var
-undefined
-typeof empty obj : object
-typeof obj : object
-typeof array element with number : number
-typeof undef element array : undefined
-typeof array : object
-typeof (err) : object
-typeof ( new Date) : object
-typeof (new Array()) : object
-typeof(regex) : object

test/Basics/typeof.js

@@ -3,63 +3,96 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
-WScript.Echo("typeof (new String()) : " + typeof (new String("")));
-WScript.Echo("typeof () : " + typeof (""));
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
 
-WScript.Echo("typeof (new Boolean(false)) : " + typeof (new Boolean(false)));
-WScript.Echo("typeof (false) : " + typeof (false));
+var tests = [
+    {
+        name: "typeof of literals, built-in types and object wrappers",
+        body: function () {
+            var arr = [42];
 
-WScript.Echo("typeof (new Number(0)) : " + typeof (new Number(0)));
-WScript.Echo("typeof (0) : " + typeof (0));
+            assert.areEqual("object", typeof null, "typeof null");
+            assert.areEqual("undefined", typeof undefined, "typeof undefined");
+            assert.areEqual("string", typeof "", "typeof empty string");
+            assert.areEqual("boolean", typeof false, "typeof false");
+            assert.areEqual("number", typeof 0, "typeof 0");
+            assert.areEqual("number", typeof 12345.678, "typeof 12345.678");
+            assert.areEqual("object", typeof {}, "typeof {}");
+            assert.areEqual("object", typeof arr, "typeof array");
+            assert.areEqual("number", typeof arr[0], "typeof arr[0]");
+            assert.areEqual("undefined", typeof arr[1], "typeof arr[1]");
+            assert.areEqual("object", typeof /abc/, "typeof /abc/");
+            assert.areEqual("function", typeof (function (){}), "typeof (function (){})");
+            assert.areEqual("function", typeof (() => 42), "typeof (() => 42)");
+            assert.areEqual("symbol", typeof Symbol(), "typeof Symbol()");
 
+            // built-in object and object wrappers
+            assert.areEqual("object", typeof (new String("")), "typeof empty string object wrapper");
+            assert.areEqual("object", typeof (new Boolean(false)), "typeof (new Boolean(false))");
+            assert.areEqual("object", typeof (new Number(0)), "typeof (new Number(0))");
+            assert.areEqual("object", typeof (new Number(12345.678)), "typeof (new Number(12345.678))");
+            assert.areEqual("object", typeof (new Object()), "typeof (new Object())");
+            assert.areEqual("object", typeof (new Array()), "typeof (new Array())");
+            assert.areEqual("object", typeof (new Date(123)), "typeof (new Date(123))");
+        }
+    },
+    {
+        name: "typeof of expressions",
+        body: function () {
+            function f() {
+                function g() { }
+                return g;
+            }
+            assert.areEqual("function", typeof f(), "typeof of function call");
+            assert.areEqual("number", typeof eval(7*6), "typeof of direct eval");
+        }
+    },
+    {
+        name: "typeof handling of undefined variables",
+        body: function () {
+            assert.areEqual("undefined", typeof x, "typeof of undeclaired var");
+            assert.areEqual("undefined", typeof {}.x, "typeof {}.x");
 
-WScript.Echo("typeof (new Number(12345.678)) : " + typeof (new Number(12345.678)));
-WScript.Echo("typeof (12345.678) : " + typeof (12345.678));
+            assert.areEqual("undefined", typeof hoisted, "typeof of hoisted var");
+            var hoisted = 42;
 
-function f() {
-    function g() { }
-    return g;
-}
+            function inner() { var scoped = 42; }
+            assert.areEqual("undefined", typeof scoped, "typeof of var when it's out of scope");
 
-WScript.Echo("typeof function : " + typeof (f));
-WScript.Echo("typeof function returning function : " + typeof (f()));
+            assert.throws(()=>{ typeof x.y; }, ReferenceError, "typeof of property on undefined obj", "'x' is not defined");
+            assert.throws(()=>{ typeof x[0]; }, ReferenceError, "typeof of property on undefined obj", "'x' is not defined");
+            assert.throws(()=>{ typeof (()=>x)(); }, ReferenceError, "reference error in the function invoked by typeof", "'x' is not defined");
 
-function exc() {
-    try {
-        WScript.Echo(q);
-    }
-    catch (e) {
-        WScript.Echo("Caught JS error on undefined var");
-        WScript.Echo(typeof (q));
-    }
-}
-exc();
+            assert.throws(()=>{ typeof x_let; }, ReferenceError, "typeof of 'let' variable in its dead zone", "Use before declaration");
+            let x_let = 42;
 
-var x = {};
-WScript.Echo("typeof empty obj : " + typeof (x));
-WScript.Echo("typeof obj : " + typeof (new Object()));
-
-var y = [];
-y[0] = 0;
-WScript.Echo("typeof array element with number : " + typeof (y[0]));
-WScript.Echo("typeof undef element array : " + typeof (y[1]));
-WScript.Echo("typeof array : " + typeof (y));
-
-var verr = new Error("aaa");
-WScript.Echo("typeof (err) : " + typeof (verr));
-
-var vDate = new Date(123);
-WScript.Echo("typeof ( new Date) : " + typeof (vDate));
-
-WScript.Echo("typeof (new Array()) : " + typeof (new Array()));
-
-var regx = /abc/;
-WScript.Echo("typeof(regex) : " + typeof (regx));
-
-var s;
-var map = {};
-function CEvent() {
-    do { 
-    } while(typeof (s = map.x) != "undefined");
-}
-CEvent();
+            assert.throws(()=>{ typeof x_const; }, ReferenceError, "typeof of 'const' variable in its dead zone", "Use before declaration");
+            const x_const = 42;
+        }
+    },
+    {
+        name: "typeof should propagate user exceptions",
+        body: function () {
+            function foo() { throw new Error("abc"); }
+            assert.throws(()=>{typeof foo()}, Error, "exception raised from the invoked function", "abc");
+            
+            var obj = { get x() { throw new Error("xyz")}};
+            assert.throws(()=>{typeof obj.x}, Error, "exception raised from the getter", "xyz");
+        }
+    },
+    {
+        name: "typeof should propagate stack overflow",
+        body: function () {
+            var obj = {};
+            var handler = {
+                get: function () {
+                    return obj.x;
+                }
+            };
+            obj = new Proxy(obj, handler);
+            assert.throws(()=>{typeof obj.x}, Error, "recursive proxy should hit SO", "Out of stack space");
+        }
+    },
+];
+ 
+testRunner.runTests(tests, { verbose: false /*so no need to provide baseline*/ });

test/EH/StackOverflow.js

@@ -2,14 +2,59 @@
 // // Copyright (C) Microsoft. All rights reserved.
 // // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 // //-------------------------------------------------------------------------------------------------------
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
 
-function foo(a)
-{ 
-    try { a[0] } finally {}
-    try { foo(0) } catch(e) {}
-    try { foo() } catch(e) {}
-}
+var tests = [
+    {
+        name: "plain recursive call with modified arguments",
+        body: function () {
+            function recursive(a) {
+                recursive(a + 1);
+            }
+            try {
+                recursive(42);
+                assert(false); // should never reach this line
+            }
+            catch (e) {
+                assert.areNotEqual(-1, e.message.indexOf("Out of stack space"), "Should be SO exception");
+            }
+        }
+    },
+    {
+        name: "plain recursive call with no arguments",
+        body: function () {
+            function recursive() {
+                recursive();
+            }
+            try {
+                recursive();
+                assert(false); // should never reach this line
+            }
+            catch (e) {
+                assert.areNotEqual(-1, e.message.indexOf("Out of stack space"), "Should be SO exception");
+            }
+        }
+    },
+    {
+        name: "recursive call to getter via proxy",
+        body: function () {
+            var obj = {};
+            var handler = {
+                get: function () {
+                    return obj.x;
+                }
+            };
+            obj = new Proxy(obj, handler);
 
-foo(0)
-
-WScript.Echo("PASS");
+            try {
+                var y = obj.x;
+                assert(false); // should never reach this line
+            }
+            catch (e) {
+                assert.areNotEqual(-1, e.message.indexOf("Out of stack space"), "Should be SO exception");
+            }
+        }
+    },
+];
+ 
+testRunner.runTests(tests, { verbose: false /*so no need to provide baseline*/ });