Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions lib/Runtime/Language/JavascriptOperators.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(); \
Copy link
Collaborator

@LouisLaf LouisLaf Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still seem wrong though to not propagate the stack overflow in this case. The rest of the change looks fine. #Closed

Copy link
Contributor

@rajatd rajatd Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should propagate instead of swallow, exceptions other than the script exceptions we explicitly swallow. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propagating it now and added a test for it.


In reply to: 204181201 [](ancestors = 204181201)

Copy link
Contributor Author

@IrinaYatsenko IrinaYatsenko Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per http://es-discourse.com/t/why-typeof-is-no-longer-safe/15 it seems that the general expectation for "typeof" to not throw so I'm going to keep it as is (I couldn't find it in the spec though)


In reply to: 204187371 [](ancestors = 204187371)

Copy link
Contributor

@MSLaguana MSLaguana Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my reading of the spec, I think that we should throw. The spec says that first you evaluate the expression, then you set val to ? GetValue(val). GetValue checks if val is an "abrupt completion", which includes a throw, and returns it if so, and the ? expression syntax means that you should do the same; if the expression returns an abrupt completion, then return that abrupt completion.

If I've parsed that out right, then typeof(expressionThatThrows) should have the exception propagate.

@zenparsing does that sound correct? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If typeof operator isn't supposed to be safe, why do we have BEGIN_TYPEOF_ERROR_HANDLER and END_TYPEOF_ERROR_HANDLER macros at all? Are there any exceptions that must be swallowed?


In reply to: 204924632 [](ancestors = 204924632)

Copy link
Contributor

@MSLaguana MSLaguana Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me like some reference errors (or more specifically, some circumstances which would result in a reference error) should be swallowed. There is some subtlety, since if x has not been declared or assigned to, then typeof(x) should return undefined, but typeof((() => x)()) should throw ReferenceError: x is not defined #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jimmy, could you please take a look at the tests I've added for typeof? Anything missing? Currently only the test for throwing getter is failing.


In reply to: 205165507 [](ancestors = 205165507)

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 \
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion test/Basics/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@
<test>
<default>
<files>typeof.js</files>
<baseline>typeof.baseline</baseline>
<compile-flags>-Intl-</compile-flags>
</default>
</test>
Expand Down
21 changes: 0 additions & 21 deletions test/Basics/typeof.baseline

This file was deleted.

137 changes: 85 additions & 52 deletions test/Basics/typeof.js
Original file line number Diff line number Diff line change
Expand Up @@ -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*/ });
63 changes: 54 additions & 9 deletions test/EH/StackOverflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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*/ });