Skip to content

Commit 94c19ff

Browse files
agarwal-sandeepagarwal-sandeep
committed
[MERGE #1464 @agarwal-sandeep] Unify error handling for arguments, caller and callee accessor functions
Merge pull request #1464 from agarwal-sandeep:accessorfunction Accessing arguments, caller and callee accessor returns different functions differing in error message. Spec says they should be treated as same for == and ===, so we had special handling in interpreter. Handling the same case in JIT is overkill. So I unified all the three functions into one and made the error message same.
2 parents 07d12c6 + b04e071 commit 94c19ff

17 files changed

+61
-174
lines changed

‎lib/Parser/rterrors.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,8 @@ RT_ERROR_MSG(JSERR_CyclicProtoValue, 5040, "", "Cyclic __proto__ value", kjstErr
153153

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

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

214214
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)
215215
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)
216-
217-
RT_ERROR_MSG(JSERR_AccessArgumentsRestricted, 5095, "", "Accessing the 'arguments' property is restricted in this context", kjstTypeError, 0)
218-
216+
// 5095 - Removed
219217
RT_ERROR_MSG(JSERR_This_NeedInspectableObject, 5096, "%s: 'this' is not an Inspectable Object", "Inspectable Object expected", kjstTypeError, JSERR_NeedObject) // {Locked="\'this\'"}
220218

221219
RT_ERROR_MSG(JSERR_FunctionArgument_NeedWinRTChar, 5097, "%s: could not convert argument to type 'char'", "Could not convert argument to type 'char'", kjstTypeError, 0)

‎lib/Runtime/Debug/TTRuntmeInfoTracker.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -490,10 +490,7 @@ namespace TTD
490490
this->EnqueueRootPathObject(_u("_defaultAccessor"), ctx->GetLibrary()->GetDefaultAccessorFunction());
491491

492492
this->EnqueueRootPathObject(_u("_stackTraceAccessor"), ctx->GetLibrary()->GetStackTraceAccessorFunction());
493-
this->EnqueueRootPathObject(_u("_throwTypeErrorAccessor"), ctx->GetLibrary()->GetThrowTypeErrorAccessorFunction());
494-
this->EnqueueRootPathObject(_u("_throwTypeErrorCallerAccessor"), ctx->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction());
495-
this->EnqueueRootPathObject(_u("_throwTypeErrorCalleeAccessor"), ctx->GetLibrary()->GetThrowTypeErrorCalleeAccessorFunction());
496-
this->EnqueueRootPathObject(_u("_throwTypeErrorArgumentsAccessor"), ctx->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction());
493+
this->EnqueueRootPathObject(_u("_throwTypeErrorRestrictedPropertyAccessor"), ctx->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction());
497494

498495
//DEBUG
499496
uint32 counter = 0;

‎lib/Runtime/Language/JavascriptConversion.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -216,16 +216,7 @@ namespace Js
216216
}
217217
}
218218
return false;
219-
case TypeIds_Function:
220-
switch (rightType)
221-
{
222-
case TypeIds_Function:
223-
if (JavascriptFunction::FromVar(aLeft)->IsThrowTypeErrorFunction() &&
224-
JavascriptFunction::FromVar(aRight)->IsThrowTypeErrorFunction())
225-
{
226-
return true;
227-
}
228-
}
219+
default:
229220
break;
230221
}
231222
return aLeft == aRight;

‎lib/Runtime/Language/JavascriptExceptionOperators.cpp

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,27 +1032,10 @@ namespace Js
10321032
JavascriptError::ThrowReferenceError(scriptContext, MAKE_HR(messageId));
10331033
}
10341034

1035-
Var JavascriptExceptionOperators::ThrowTypeErrorAccessor(RecyclableObject* function, CallInfo callInfo, ...)
1035+
// Throw type error on access 'arguments', 'callee' or 'caller' when in a restricted context
1036+
Var JavascriptExceptionOperators::ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...)
10361037
{
1037-
JavascriptError::ThrowTypeError(function->GetScriptContext(), VBSERR_ActionNotSupported);
1038-
}
1039-
1040-
// Throw type error on access caller when in a restricted context
1041-
Var JavascriptExceptionOperators::ThrowTypeErrorCallerAccessor(RecyclableObject* function, CallInfo callInfo, ...)
1042-
{
1043-
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessCallerRestricted);
1044-
}
1045-
1046-
// Throw type error on access on callee when strict mode
1047-
Var JavascriptExceptionOperators::ThrowTypeErrorCalleeAccessor(RecyclableObject* function, CallInfo callInfo, ...)
1048-
{
1049-
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessCallee);
1050-
}
1051-
1052-
// Throw type error on access arguments when in a restricted context
1053-
Var JavascriptExceptionOperators::ThrowTypeErrorArgumentsAccessor(RecyclableObject* function, CallInfo callInfo, ...)
1054-
{
1055-
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessArgumentsRestricted);
1038+
JavascriptError::ThrowTypeError(function->GetScriptContext(), JSERR_AccessRestrictedProperty);
10561039
}
10571040

10581041
Var JavascriptExceptionOperators::StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...)

‎lib/Runtime/Language/JavascriptExceptionOperators.h

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,7 @@ namespace Js
7070
static void __declspec(noreturn) ThrowStackOverflow(ScriptContext* scriptContext, PVOID returnAddress);
7171

7272
static uint64 GetStackTraceLimit(Var thrownObject, ScriptContext* scriptContext);
73-
static Var ThrowTypeErrorAccessor(RecyclableObject* function, CallInfo callInfo, ...);
74-
static Var ThrowTypeErrorCallerAccessor(RecyclableObject* function, CallInfo callInfo, ...);
75-
static Var ThrowTypeErrorCalleeAccessor(RecyclableObject* function, CallInfo callInfo, ...);
76-
static Var ThrowTypeErrorArgumentsAccessor(RecyclableObject* function, CallInfo callInfo, ...);
73+
static Var ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...);
7774
static Var StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...);
7875
static void WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException = true, bool resetSatck = false);
7976
static void AddStackTraceToObject(Var obj, JavascriptExceptionContext::StackTrace* stackTrace, ScriptContext& scriptContext, bool isThrownException = true, bool resetSatck = false);
@@ -82,13 +79,10 @@ namespace Js
8279
class EntryInfo
8380
{
8481
public:
85-
static FunctionInfo ThrowTypeErrorAccessor;
8682
static FunctionInfo StackTraceAccessor;
8783

8884
// For strict mode
89-
static FunctionInfo ThrowTypeErrorCallerAccessor;
90-
static FunctionInfo ThrowTypeErrorCalleeAccessor;
91-
static FunctionInfo ThrowTypeErrorArgumentsAccessor;
85+
static FunctionInfo ThrowTypeErrorRestrictedPropertyAccessor;
9286
};
9387

9488
private:

‎lib/Runtime/Language/JavascriptOperators.cpp

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,19 +1016,6 @@ namespace Js
10161016
}
10171017
break;
10181018

1019-
case TypeIds_Function:
1020-
if (rightType == TypeIds_Function)
1021-
{
1022-
// 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.
1023-
// For different scenarios we return different instances of the function, which differ by exception/error message.
1024-
// According to ES5, this is the same [[ThrowTypeError]] (thrower) internal function, thus they should be equal.
1025-
if (JavascriptFunction::FromVar(aLeft)->IsThrowTypeErrorFunction() &&
1026-
JavascriptFunction::FromVar(aRight)->IsThrowTypeErrorFunction())
1027-
{
1028-
return true;
1029-
}
1030-
}
1031-
break;
10321019
#ifdef ENABLE_SIMDJS
10331020
case TypeIds_SIMDBool8x16:
10341021
case TypeIds_SIMDInt8x16:
@@ -6904,11 +6891,10 @@ namespace Js
69046891
JavascriptOperators::SetProperty(argsObj, argsObj, PropertyIds::_symbolIterator, library->GetArrayPrototypeValuesFunction(), scriptContext);
69056892
if (funcCallee->IsStrictMode())
69066893
{
6907-
JavascriptFunction* callerAccessor = library->GetThrowTypeErrorCallerAccessorFunction();
6908-
argsObj->SetAccessors(PropertyIds::caller, callerAccessor, callerAccessor, PropertyOperation_NonFixedValue);
6894+
JavascriptFunction* restrictedPropertyAccessor = library->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
6895+
argsObj->SetAccessors(PropertyIds::caller, restrictedPropertyAccessor, restrictedPropertyAccessor, PropertyOperation_NonFixedValue);
69096896

6910-
JavascriptFunction* calleeAccessor = library->GetThrowTypeErrorCalleeAccessorFunction();
6911-
argsObj->SetAccessors(PropertyIds::callee, calleeAccessor, calleeAccessor, PropertyOperation_NonFixedValue);
6897+
argsObj->SetAccessors(PropertyIds::callee, restrictedPropertyAccessor, restrictedPropertyAccessor, PropertyOperation_NonFixedValue);
69126898

69136899
}
69146900
else

‎lib/Runtime/Library/JavascriptBuiltInFunctionList.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@
88
#endif
99

1010
BUILTIN(JavascriptExceptionOperators, StackTraceAccessor, StackTraceAccessor, FunctionInfo::DoNotProfile)
11-
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorAccessor, ThrowTypeErrorAccessor, FunctionInfo::DoNotProfile)
12-
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorCallerAccessor, ThrowTypeErrorCallerAccessor, FunctionInfo::DoNotProfile)
13-
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorCalleeAccessor, ThrowTypeErrorCalleeAccessor, FunctionInfo::DoNotProfile)
14-
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorArgumentsAccessor, ThrowTypeErrorArgumentsAccessor, FunctionInfo::DoNotProfile)
11+
BUILTIN(JavascriptExceptionOperators, ThrowTypeErrorRestrictedPropertyAccessor, ThrowTypeErrorRestrictedPropertyAccessor, FunctionInfo::DoNotProfile)
1512
BUILTIN(JavascriptOperators, DefaultAccessor, DefaultAccessor, FunctionInfo::DoNotProfile)
1613
BUILTIN(GlobalObject, Eval, EntryEval, FunctionInfo::ErrorOnNew)
1714
BUILTIN(GlobalObject, ParseInt, EntryParseInt, FunctionInfo::ErrorOnNew)

‎lib/Runtime/Library/JavascriptFunction.cpp

Lines changed: 5 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -371,18 +371,6 @@ namespace Js
371371
return library->GetUndefined();
372372
}
373373

374-
BOOL JavascriptFunction::IsThrowTypeErrorFunction()
375-
{
376-
ScriptContext* scriptContext = this->GetScriptContext();
377-
Assert(scriptContext);
378-
379-
return
380-
this == scriptContext->GetLibrary()->GetThrowTypeErrorAccessorFunction() ||
381-
this == scriptContext->GetLibrary()->GetThrowTypeErrorCalleeAccessorFunction() ||
382-
this == scriptContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction() ||
383-
this == scriptContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
384-
}
385-
386374
enum : unsigned { STACK_ARGS_ALLOCA_THRESHOLD = 8 }; // Number of stack args we allow before using _alloca
387375

388376
// ES5 15.3.4.3
@@ -2255,17 +2243,10 @@ namespace Js
22552243
switch (propertyId)
22562244
{
22572245
case PropertyIds::caller:
2258-
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
2259-
{
2260-
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
2261-
return true;
2262-
}
2263-
break;
2264-
22652246
case PropertyIds::arguments:
22662247
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
22672248
{
2268-
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
2249+
*setter = *getter = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
22692250
return true;
22702251
}
22712252
break;
@@ -2309,27 +2290,12 @@ namespace Js
23092290
switch (propertyId)
23102291
{
23112292
case PropertyIds::caller:
2312-
if (this->HasRestrictedProperties()) {
2313-
PropertyValueInfo::SetNoCache(info, this);
2314-
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
2315-
{
2316-
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
2317-
*descriptorFlags = Accessor;
2318-
}
2319-
else
2320-
{
2321-
*descriptorFlags = Data;
2322-
}
2323-
return true;
2324-
}
2325-
break;
2326-
23272293
case PropertyIds::arguments:
23282294
if (this->HasRestrictedProperties()) {
23292295
PropertyValueInfo::SetNoCache(info, this);
23302296
if (this->GetEntryPoint() == JavascriptFunction::PrototypeEntryPoint)
23312297
{
2332-
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
2298+
*setterValue = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
23332299
*descriptorFlags = Accessor;
23342300
}
23352301
else
@@ -2507,7 +2473,7 @@ namespace Js
25072473
{
25082474
if (scriptContext->GetThreadContext()->RecordImplicitException())
25092475
{
2510-
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorCallerAccessorFunction();
2476+
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
25112477
*value = CALL_FUNCTION(accessor, CallInfo(1), originalInstance);
25122478
}
25132479
return true;
@@ -2562,7 +2528,7 @@ namespace Js
25622528
// Note that for caller coming from remote context (see the check right above) we can't call IsStrictMode()
25632529
// unless CheckCrossDomainScriptContext succeeds. If it fails we don't know whether caller is strict mode
25642530
// function or not and throw if it's not, so just return Null.
2565-
JavascriptError::ThrowTypeError(scriptContext, JSERR_AccessCallerRestricted);
2531+
JavascriptError::ThrowTypeError(scriptContext, JSERR_AccessRestrictedProperty);
25662532
}
25672533
}
25682534

@@ -2582,7 +2548,7 @@ namespace Js
25822548
{
25832549
if (scriptContext->GetThreadContext()->RecordImplicitException())
25842550
{
2585-
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorArgumentsAccessorFunction();
2551+
JavascriptFunction* accessor = requestContext->GetLibrary()->GetThrowTypeErrorRestrictedPropertyAccessorFunction();
25862552
*value = CALL_FUNCTION(accessor, CallInfo(1), originalInstance);
25872553
}
25882554
return true;

‎lib/Runtime/Library/JavascriptFunction.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,6 @@ namespace Js
191191
virtual bool IsBoundFunction() const { return false; }
192192
virtual bool IsGeneratorFunction() const { return false; }
193193

194-
BOOL IsThrowTypeErrorFunction();
195-
196194
void SetEntryPoint(JavascriptMethod method);
197195
#if DBG
198196
void VerifyEntryPoint();

‎lib/Runtime/Library/JavascriptLibrary.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,17 +1107,8 @@ namespace Js
11071107
stackTraceAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
11081108
}
11091109

1110-
throwTypeErrorAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorAccessor);
1111-
throwTypeErrorAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
1112-
1113-
throwTypeErrorCallerAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorCallerAccessor);
1114-
throwTypeErrorCallerAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
1115-
1116-
throwTypeErrorCalleeAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorCalleeAccessor);
1117-
throwTypeErrorCalleeAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
1118-
1119-
throwTypeErrorArgumentsAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorArgumentsAccessor);
1120-
throwTypeErrorArgumentsAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
1110+
throwTypeErrorRestrictedPropertyAccessorFunction = CreateNonProfiledFunction(&JavascriptExceptionOperators::EntryInfo::ThrowTypeErrorRestrictedPropertyAccessor);
1111+
throwTypeErrorRestrictedPropertyAccessorFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);
11211112

11221113
__proto__getterFunction = CreateNonProfiledFunction(&ObjectPrototypeObject::EntryInfo::__proto__getter);
11231114
__proto__getterFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyNone, nullptr);

0 commit comments

Comments
 (0)