Skip to content

Unify error handling for arguments, caller and callee accessor functions #1464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chakrabot merged 2 commits into from
Aug 19, 2016
Merged

Unify error handling for arguments, caller and callee accessor functions #1464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eeb057e
Unify error handling for arguments, caller and callee accessor functions
agarwal-sandeep Aug 18, 2016
b04e071
Incorporate review comment
agarwal-sandeep Aug 19, 2016
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
8 changes: 3 additions & 5 deletions lib/Parser/rterrors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ RT_ERROR_MSG(JSERR_CyclicProtoValue, 5040, "", "Cyclic __proto__ value", kjstErr

RT_ERROR_MSG(JSERR_CantDeleteExpr, 5041, "Calling delete on '%s' is not allowed in strict mode", "Object member not configurable", kjstTypeError, 0) // string 4
RT_ERROR_MSG(JSERR_RefErrorUndefVariable, 5042, "", "Variable undefined in strict mode", kjstReferenceError, 0) // string 10
RT_ERROR_MSG(JSERR_AccessCallerRestricted, 5043, "", "Accessing the 'caller' property is restricted in this context", kjstTypeError, 0)
RT_ERROR_MSG(JSERR_AccessCallee, 5044, "", "Accessing the 'callee' property of an arguments object is not allowed in strict mode", kjstTypeError, 0) // string 2
RT_ERROR_MSG(JSERR_AccessRestrictedProperty, 5043, "", "'arguments', 'callee' and 'caller' are restricted function properties and cannot be accessed in this context", kjstTypeError, 0)
// 5044 - Removed
RT_ERROR_MSG(JSERR_CantAssignToReadOnly, 5045, "", "Assignment to read-only properties is not allowed in strict mode", kjstTypeError, 0) // string 5
RT_ERROR_MSG(JSERR_NonExtensibleObject, 5046, "", "Cannot create property for a non-extensible object", kjstTypeError, 0) // string 6

Expand Down Expand Up @@ -213,9 +213,7 @@ RT_ERROR_MSG(JSERR_InvalidPropertySignature, 5092, "The property '%s' has an inv

RT_ERROR_MSG(JSERR_InvalidRTCPropertyValueIn, 5093, "The runtimeclass %s that has Windows.Foundation.IPropertyValue as default interface is not supported as an input parameter type", "invalid input parameter type", kjstTypeError, 0)
RT_ERROR_MSG(JSERR_RTCInvalidRTCPropertyValueOut, 5094, "The object with interface Windows.Foundation.IPropertyValue that has runtimeclass name %s is not supported as an output parameter", "invalid output parameter", kjstTypeError, 0)

RT_ERROR_MSG(JSERR_AccessArgumentsRestricted, 5095, "", "Accessing the 'arguments' property is restricted in this context", kjstTypeError, 0)

// 5095 - Removed
RT_ERROR_MSG(JSERR_This_NeedInspectableObject, 5096, "%s: 'this' is not an Inspectable Object", "Inspectable Object expected", kjstTypeError, JSERR_NeedObject) // {Locked="\'this\'"}

RT_ERROR_MSG(JSERR_FunctionArgument_NeedWinRTChar, 5097, "%s: could not convert argument to type 'char'", "Could not convert argument to type 'char'", kjstTypeError, 0)
Expand Down
5 changes: 1 addition & 4 deletions lib/Runtime/Debug/TTRuntmeInfoTracker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,10 +490,7 @@ namespace TTD
this->EnqueueRootPathObject(_u("_defaultAccessor"), ctx->GetLibrary()->GetDefaultAccessorFunction());

this->EnqueueRootPathObject(_u("_stackTraceAccessor"), ctx->GetLibrary()->GetStackTraceAccessorFunction());
this->EnqueueRootPathObject(_u("_throwTypeErrorAccessor"), ctx->GetLibrary()->GetThrowTypeErrorAccessorFunction());
this->EnqueueRootPathObject(_u("_throwTypeErrorCallerAccessor"), ctx->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction());
this->EnqueueRootPathObject(_u("_throwTypeErrorCalleeAccessor"), ctx->GetLibrary()->GetThrowTypeErrorCalleeAccessorFunction());
this->EnqueueRootPathObject(_u("_throwTypeErrorArgumentsAccessor"), ctx->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction());
this->EnqueueRootPathObject(_u("_throwTypeErrorRestrictedPropertyAccessor"), ctx->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction());

//DEBUG
uint32 counter = 0;
Expand Down
11 changes: 1 addition & 10 deletions lib/Runtime/Language/JavascriptConversion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,7 @@ namespace Js
}
}
return false;
case TypeIds_Function:
switch (rightType)
{
case TypeIds_Function:
if (JavascriptFunction::FromVar(aLeft)->IsThrowTypeErrorFunction() &&
JavascriptFunction::FromVar(aRight)->IsThrowTypeErrorFunction())
{
return true;
}
}
default:
break;
}
return aLeft == aRight;
Expand Down
23 changes: 3 additions & 20 deletions lib/Runtime/Language/JavascriptExceptionOperators.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,27 +1032,10 @@ namespace Js
JavascriptError::ThrowReferenceError(scriptContext, MAKE_HR(messageId));
}

Var JavascriptExceptionOperators::ThrowTypeErrorAccessor(RecyclableObject* function, CallInfo callInfo, ...)
// Throw type error on access 'arguments', 'callee' or 'caller' when in a restricted context
Var JavascriptExceptionOperators::ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...)
{
JavascriptError::ThrowTypeError(function->GetScriptContext(), VBSERR_ActionNotSupported);
}

// Throw type error on access caller when in a restricted context
Var JavascriptExceptionOperators::ThrowTypeErrorCallerAccessor(RecyclableObject* function, CallInfo callInfo, ...)
{
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessCallerRestricted);
}

// Throw type error on access on callee when strict mode
Var JavascriptExceptionOperators::ThrowTypeErrorCalleeAccessor(RecyclableObject* function, CallInfo callInfo, ...)
{
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessCallee);
}

// Throw type error on access arguments when in a restricted context
Var JavascriptExceptionOperators::ThrowTypeErrorArgumentsAccessor(RecyclableObject* function, CallInfo callInfo, ...)
{
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessArgumentsRestricted);
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessRestrictedProperty);
}

Var JavascriptExceptionOperators::StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...)
Expand Down
10 changes: 2 additions & 8 deletions lib/Runtime/Language/JavascriptExceptionOperators.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,7 @@ namespace Js
static void __declspec(noreturn) ThrowStackOverflow(ScriptContext* scriptContext, PVOID returnAddress);

static uint64 GetStackTraceLimit(Var thrownObject, ScriptContext* scriptContext);
static Var ThrowTypeErrorAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var ThrowTypeErrorCallerAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var ThrowTypeErrorCalleeAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var ThrowTypeErrorArgumentsAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static void WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException = true, bool resetSatck = false);
static void AddStackTraceToObject(Var obj, JavascriptExceptionContext::StackTrace* stackTrace, ScriptContext& scriptContext, bool isThrownException = true, bool resetSatck = false);
Expand All @@ -82,13 +79,10 @@ namespace Js
class EntryInfo
{
public:
static FunctionInfo ThrowTypeErrorAccessor;
static FunctionInfo StackTraceAccessor;

// For strict mode
static FunctionInfo ThrowTypeErrorCallerAccessor;
static FunctionInfo ThrowTypeErrorCalleeAccessor;
static FunctionInfo ThrowTypeErrorArgumentsAccessor;
static FunctionInfo ThrowTypeErrorRestrictedPropertyAccessor;
};

private:
Expand Down
20 changes: 3 additions & 17 deletions lib/Runtime/Language/JavascriptOperators.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,19 +1016,6 @@ namespace Js
}
break;

case TypeIds_Function:
if (rightType == TypeIds_Function)
{
// In ES5 in certain cases (ES5 10.6.14(strict), 13.2.19(strict), 15.3.4.5.20-21) we return a function that throws type error.
// For different scenarios we return different instances of the function, which differ by exception/error message.
// According to ES5, this is the same [[ThrowTypeError]] (thrower) internal function, thus they should be equal.
if (JavascriptFunction::FromVar(aLeft)->IsThrowTypeErrorFunction() &&
JavascriptFunction::FromVar(aRight)->IsThrowTypeErrorFunction())
{
return true;
}
}
break;
#ifdef ENABLE_SIMDJS
case TypeIds_SIMDBool8x16:
case TypeIds_SIMDInt8x16:
Expand Down Expand Up @@ -6904,11 +6891,10 @@ namespace Js
JavascriptOperators::SetProperty(argsObj, argsObj, PropertyIds::_symbolIterator, library->GetArrayPrototypeValuesFunction(), scriptContext);
if (funcCallee->IsStrictMode())
{
JavascriptFunction* callerAccessor = library->GetThrowTypeErrorCallerAccessorFunction();
argsObj->SetAccessors(PropertyIds::caller, callerAccessor, callerAccessor, PropertyOperation_NonFixedValue);
JavascriptFunction* restrictedPropertyAccessor = library->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
argsObj->SetAccessors(PropertyIds::caller, restrictedPropertyAccessor, restrictedPropertyAccessor, PropertyOperation_NonFixedValue);

JavascriptFunction* calleeAccessor = library->GetThrowTypeErrorCalleeAccessorFunction();
argsObj->SetAccessors(PropertyIds::callee, calleeAccessor, calleeAccessor, PropertyOperation_NonFixedValue);
argsObj->SetAccessors(PropertyIds::callee, restrictedPropertyAccessor, restrictedPropertyAccessor, PropertyOperation_NonFixedValue);

}
else
Expand Down
5 changes: 1 addition & 4 deletions lib/Runtime/Library/JavascriptBuiltInFunctionList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
#endif

BUILTIN(JavascriptExceptionOperators, StackTraceAccessor, StackTraceAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorAccessor, ThrowTypeErrorAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorCallerAccessor, ThrowTypeErrorCallerAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorCalleeAccessor, ThrowTypeErrorCalleeAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorArgumentsAccessor, ThrowTypeErrorArgumentsAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorRestrictedPropertyAccessor, ThrowTypeErrorRestrictedPropertyAccessor, FunctionInfo::DoNotProfile)
BUILTIN(JavascriptOperators, DefaultAccessor, DefaultAccessor, FunctionInfo::DoNotProfile)
BUILTIN(GlobalObject, Eval, EntryEval, FunctionInfo::ErrorOnNew)
BUILTIN(GlobalObject, ParseInt, EntryParseInt, FunctionInfo::ErrorOnNew)
Expand Down
44 changes: 5 additions & 39 deletions lib/Runtime/Library/JavascriptFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,18 +371,6 @@ namespace Js
return library->GetUndefined();
}

BOOL JavascriptFunction::IsThrowTypeErrorFunction()
{
ScriptContext* scriptContext = this->GetScriptContext();
Assert(scriptContext);

return
this == scriptContext->GetLibrary()->GetThrowTypeErrorAccessorFunction() ||
this == scriptContext->GetLibrary()->GetThrowTypeErrorCalleeAccessorFunction() ||
this == scriptContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction() ||
this == scriptContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
}

enum : unsigned { STACK_ARGS_ALLOCA_THRESHOLD = 8 }; // Number of stack args we allow before using _alloca

// ES5 15.3.4.3
Expand Down Expand Up @@ -2255,17 +2243,10 @@ namespace Js
switch (propertyId)
{
case PropertyIds::caller:
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
{
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
return true;
}
break;

case PropertyIds::arguments:
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
{
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
return true;
}
break;
Expand Down Expand Up @@ -2309,27 +2290,12 @@ namespace Js
switch (propertyId)
{
case PropertyIds::caller:
if (this->HasRestrictedProperties()) {
PropertyValueInfo::SetNoCache(info, this);
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
{
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
*descriptorFlags = Accessor;
}
else
{
*descriptorFlags = Data;
}
return true;
}
break;

case PropertyIds::arguments:
if (this->HasRestrictedProperties()) {
PropertyValueInfo::SetNoCache(info, this);
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
{
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
*descriptorFlags = Accessor;
}
else
Expand Down Expand Up @@ -2507,7 +2473,7 @@ namespace Js
{
if (scriptContext->GetThreadContext()->RecordImplicitException())
{
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
*value = CALL_FUNCTION(accessor, CallInfo(1), originalInstance);
}
return true;
Expand Down Expand Up @@ -2562,7 +2528,7 @@ namespace Js
// Note that for caller coming from remote context (see the check right above) we can't call IsStrictMode()
// unless CheckCrossDomainScriptContext succeeds. If it fails we don't know whether caller is strict mode
// function or not and throw if it's not, so just return Null.
JavascriptError::ThrowTypeError(scriptContext, JSERR_AccessCallerRestricted);
JavascriptError::ThrowTypeError(scriptContext, JSERR_AccessRestrictedProperty);
}
}

Expand All @@ -2582,7 +2548,7 @@ namespace Js
{
if (scriptContext->GetThreadContext()->RecordImplicitException())
{
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
*value = CALL_FUNCTION(accessor, CallInfo(1), originalInstance);
}
return true;
Expand Down
2 changes: 0 additions & 2 deletions lib/Runtime/Library/JavascriptFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ namespace Js
virtual bool IsBoundFunction() const { return false; }
virtual bool IsGeneratorFunction() const { return false; }

BOOL IsThrowTypeErrorFunction();

void SetEntryPoint(JavascriptMethod method);
#if DBG
void VerifyEntryPoint();
Expand Down
13 changes: 2 additions & 11 deletions lib/Runtime/Library/JavascriptLibrary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,17 +1107,8 @@ namespace Js
stackTraceAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
}

throwTypeErrorAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorAccessor);
throwTypeErrorAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);

throwTypeErrorCallerAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorCallerAccessor);
throwTypeErrorCallerAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);

throwTypeErrorCalleeAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorCalleeAccessor);
throwTypeErrorCalleeAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);

throwTypeErrorArgumentsAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorArgumentsAccessor);
throwTypeErrorArgumentsAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
throwTypeErrorRestrictedPropertyAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorRestrictedPropertyAccessor);
throwTypeErrorRestrictedPropertyAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);

__proto__getterFunction = CreateNonProfiledFunction(&ObjectPrototypeObject::EntryInfo::__proto__getter);
__proto__getterFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
Expand Down
5 changes: 1 addition & 4 deletions lib/Runtime/Library/JavascriptLibrary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,7 @@ namespace Js
DynamicType * GetListIteratorType() const { return listIteratorType; }
JavascriptFunction* GetDefaultAccessorFunction() const { return defaultAccessorFunction; }
JavascriptFunction* GetStackTraceAccessorFunction() const { return stackTraceAccessorFunction; }
JavascriptFunction* GetThrowTypeErrorAccessorFunction() const { return throwTypeErrorAccessorFunction; }
JavascriptFunction* GetThrowTypeErrorCallerAccessorFunction() const { return throwTypeErrorCallerAccessorFunction; }
JavascriptFunction* GetThrowTypeErrorCalleeAccessorFunction() const { return throwTypeErrorCalleeAccessorFunction; }
JavascriptFunction* GetThrowTypeErrorArgumentsAccessorFunction() const { return throwTypeErrorArgumentsAccessorFunction; }
JavascriptFunction* GetThrowTypeErrorRestrictedPropertyAccessorFunction() const { return throwTypeErrorRestrictedPropertyAccessorFunction; }
JavascriptFunction* Get__proto__getterFunction() const { return __proto__getterFunction; }
JavascriptFunction* Get__proto__setterFunction() const { return __proto__setterFunction; }

Expand Down
5 changes: 1 addition & 4 deletions lib/Runtime/Library/JavascriptLibraryBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,7 @@ namespace Js

JavascriptFunction* defaultAccessorFunction;
JavascriptFunction* stackTraceAccessorFunction;
JavascriptFunction* throwTypeErrorAccessorFunction;
JavascriptFunction* throwTypeErrorCallerAccessorFunction;
JavascriptFunction* throwTypeErrorCalleeAccessorFunction;
JavascriptFunction* throwTypeErrorArgumentsAccessorFunction;
JavascriptFunction* throwTypeErrorRestrictedPropertyAccessorFunction;
Copy link
Collaborator

@ianwjhalliday ianwjhalliday Aug 19, 2016

Choose a reason for hiding this comment

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

heads up, this will break the Edge binary consistency check until we get a new Edge build that includes this change

JavascriptFunction* debugObjectNonUserGetterFunction;
JavascriptFunction* debugObjectNonUserSetterFunction;
JavascriptFunction* debugObjectDebugModeGetterFunction;
Expand Down
8 changes: 0 additions & 8 deletions lib/Runtime/Types/RecyclableObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,14 +680,6 @@ namespace Js
case TypeIds_Function:
if (rightType == TypeIds_Function)
{
// In ES5 in certain cases (ES5 10.6.14(strict), 13.2.19(strict), 15.3.4.5.20-21) we return a function that throws type error.
// For different scenarios we return different instances of the function, which differ by exception/error message.
// According to ES5, this is the same [[ThrowTypeError]] (thrower) internal function, thus they should be equal.
if (JavascriptFunction::FromVar(aLeft)->IsThrowTypeErrorFunction() &&
JavascriptFunction::FromVar(aRight)->IsThrowTypeErrorFunction())
{
goto ReturnTrue;
}
goto ReturnFalse;
}
// Fall through to do normal object comparison on function object.
Expand Down
Loading