Skip to content

Commit

Permalink
[MERGE #5405 @rhuanjl] Fix configurability of function.length and bou…
Browse files Browse the repository at this point in the history 
…ndFunction.length

Merge pull request #5405 from rhuanjl:boundFunction

Fix: #4122

This PR addresses several issues with the length property of JavascriptFunctions and BoundFunctions.

Length should be configurable (previously worked for JavascriptFunctions but not BoundFunctions)
Deleting length should succeed (previously failed for both)
Deleting length should not throw in strict mode (previously failed for both)
value after deletion = 0 (previously failed for both)
using defineProperty to redefine length should work (previously failed silently for both)
  • Loading branch information
@jackhorton
jackhorton committed Aug 20, 2018
2 parents 1ef7ea8 + cbada71 commit 7699222
Show file tree
Hide file tree
Showing 11 changed files with 255 additions and 288 deletions.
8 changes: 1 addition & 7 deletions lib/Runtime/Debug/DiagObjectModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2547,7 +2547,6 @@ namespace Js
// We need to special-case RegExp constructor here because it has some special properties (above) and some
// special enumerable properties which should all show up in the debugger.
JavascriptRegExpConstructor* regExp = scriptContext->GetLibrary()->GetRegExpConstructor();
Js::JavascriptFunction* jsFunction = Js::JavascriptFunction::FromVar(object);

if (regExp == object)
{
Expand All @@ -2563,11 +2562,6 @@ namespace Js
InsertItem(originalObject, object, propertyId, isConst, isUnscoped, &pMethodsGroupWalker);
}
}
else if ((jsFunction->IsScriptFunction() && !jsFunction->GetFunctionProxy()->IsJsBuiltInCode()) || jsFunction->IsBoundFunction())
{
// Adding special property length for the ScriptFunction, like it is done in JavascriptFunction::GetSpecialNonEnumerablePropertyName
InsertItem(originalObject, object, PropertyIds::length, true/*not editable*/, false /*isUnscoped*/, &pMethodsGroupWalker);
}
}
}

Expand Down Expand Up @@ -4216,4 +4210,4 @@ namespace Js
}
#endif
}
#endif
#endif
191 changes: 13 additions & 178 deletions lib/Runtime/Library/BoundFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ namespace Js
count(0),
boundArgs(nullptr)
{

DebugOnly(VerifyEntryPoint());
AssertMsg(args.Info.Count > 0, "wrong number of args in BoundFunction");

Expand All @@ -44,21 +43,19 @@ namespace Js
}
type->SetPrototype(proto);
}

int len = 0;
// If targetFunction is proxy, need to make sure that traps are called in right order as per 19.2.3.2 in RC#4 dated April 3rd 2015.
// Here although we won't use value of length, this is just to make sure that we call traps involved with HasOwnProperty(Target, "length") and Get(Target, "length")
if (JavascriptProxy::Is(targetFunction))
// additionally need to get the correct length value for the boundFunctions' length property
if (JavascriptOperators::HasOwnProperty(targetFunction, PropertyIds::length, scriptContext, nullptr) == TRUE)
{
if (JavascriptOperators::HasOwnProperty(targetFunction, PropertyIds::length, scriptContext, nullptr) == TRUE)
Var varLength;
if (targetFunction->GetProperty(targetFunction, PropertyIds::length, &varLength, nullptr, scriptContext))
{
int len = 0;
Var varLength;
if (targetFunction->GetProperty(targetFunction, PropertyIds::length, &varLength, nullptr, scriptContext))
{
len = JavascriptConversion::ToInt32(varLength, scriptContext);
}
len = JavascriptConversion::ToInt32(varLength, scriptContext);
}
GetTypeHandler()->EnsureObjectReady(this);
}
GetTypeHandler()->EnsureObjectReady(this);

if (args.Info.Count > 1)
{
Expand All @@ -84,27 +81,12 @@ namespace Js
// If no "this" is passed, "undefined" is used
boundThis = scriptContext->GetLibrary()->GetUndefined();
}
}

BoundFunction::BoundFunction(RecyclableObject* targetFunction, Var boundThis, Var* args, uint argsCount, DynamicType * type)
: JavascriptFunction(type, &functionInfo),
count(argsCount),
boundArgs(nullptr)
{
DebugOnly(VerifyEntryPoint());

this->targetFunction = targetFunction;
this->boundThis = boundThis;

if (argsCount != 0)
{
this->boundArgs = RecyclerNewArray(this->GetScriptContext()->GetRecycler(), Field(Var), argsCount);

for (uint i = 0; i < argsCount; i++)
{
this->boundArgs[i] = args[i];
}
}
// Reduce length number of bound args
len = len - this->count;
len = max(len, 0);

SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(len), PropertyConfigurable, nullptr, PropertyOperation_None, SideEffects_None);
}

BoundFunction* BoundFunction::New(ScriptContext* scriptContext, ArgumentReader args)
Expand Down Expand Up @@ -307,108 +289,11 @@ namespace Js
return false;
}

PropertyQueryFlags BoundFunction::HasPropertyQuery(PropertyId propertyId, _Inout_opt_ PropertyValueInfo* info)
{
if (propertyId == PropertyIds::length)
{
return PropertyQueryFlags::Property_Found;
}

return JavascriptFunction::HasPropertyQuery(propertyId, info);
}

PropertyQueryFlags BoundFunction::GetPropertyQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
{
BOOL result;
if (GetPropertyBuiltIns(originalInstance, propertyId, value, info, requestContext, &result))
{
return JavascriptConversion::BooleanToPropertyQueryFlags(result);
}

return JavascriptFunction::GetPropertyQuery(originalInstance, propertyId, value, info, requestContext);
}

PropertyQueryFlags BoundFunction::GetPropertyQuery(Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
{
BOOL result;
PropertyRecord const* propertyRecord;
this->GetScriptContext()->FindPropertyRecord(propertyNameString, &propertyRecord);

if (propertyRecord != nullptr && GetPropertyBuiltIns(originalInstance, propertyRecord->GetPropertyId(), value, info, requestContext, &result))
{
return JavascriptConversion::BooleanToPropertyQueryFlags(result);
}

return JavascriptFunction::GetPropertyQuery(originalInstance, propertyNameString, value, info, requestContext);
}

bool BoundFunction::GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result)
{
if (propertyId == PropertyIds::length)
{
// Get the "length" property of the underlying target function
int len = 0;
Var varLength;
if (targetFunction->GetProperty(targetFunction, PropertyIds::length, &varLength, nullptr, requestContext))
{
len = JavascriptConversion::ToInt32(varLength, requestContext);
}

// Reduce by number of bound args
len = len - this->count;
len = max(len, 0);

*value = JavascriptNumber::ToVar(len, requestContext);
*result = true;
return true;
}

return false;
}

PropertyQueryFlags BoundFunction::GetPropertyReferenceQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
{
return BoundFunction::GetPropertyQuery(originalInstance, propertyId, value, info, requestContext);
}

BOOL BoundFunction::SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info)
{
BOOL result;
if (SetPropertyBuiltIns(propertyId, value, flags, info, &result))
{
return result;
}

return JavascriptFunction::SetProperty(propertyId, value, flags, info);
}

BOOL BoundFunction::SetProperty(JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info)
{
BOOL result;
PropertyRecord const* propertyRecord;
this->GetScriptContext()->FindPropertyRecord(propertyNameString, &propertyRecord);

if (propertyRecord != nullptr && SetPropertyBuiltIns(propertyRecord->GetPropertyId(), value, flags, info, &result))
{
return result;
}

return JavascriptFunction::SetProperty(propertyNameString, value, flags, info);
}

bool BoundFunction::SetPropertyBuiltIns(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info, BOOL* result)
{
if (propertyId == PropertyIds::length)
{
JavascriptError::ThrowCantAssignIfStrictMode(flags, this->GetScriptContext());

*result = false;
return true;
}

return false;
}

_Check_return_ _Success_(return) BOOL BoundFunction::GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext)
{
return DynamicObject::GetAccessors(propertyId, getter, setter, requestContext);
Expand All @@ -429,56 +314,6 @@ namespace Js
return SetProperty(propertyId, value, PropertyOperation_None, info);
}

BOOL BoundFunction::DeleteProperty(PropertyId propertyId, PropertyOperationFlags flags)
{
if (propertyId == PropertyIds::length)
{
return false;
}

return JavascriptFunction::DeleteProperty(propertyId, flags);
}

BOOL BoundFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
{
if (BuiltInPropertyRecords::length.Equals(propertyNameString))
{
return false;
}

return JavascriptFunction::DeleteProperty(propertyNameString, flags);
}

BOOL BoundFunction::IsWritable(PropertyId propertyId)
{
if (propertyId == PropertyIds::length)
{
return false;
}

return JavascriptFunction::IsWritable(propertyId);
}

BOOL BoundFunction::IsConfigurable(PropertyId propertyId)
{
if (propertyId == PropertyIds::length)
{
return false;
}

return JavascriptFunction::IsConfigurable(propertyId);
}

BOOL BoundFunction::IsEnumerable(PropertyId propertyId)
{
if (propertyId == PropertyIds::length)
{
return false;
}

return JavascriptFunction::IsEnumerable(propertyId);
}

BOOL BoundFunction::HasInstance(Var instance, ScriptContext* scriptContext, IsInstInlineCache* inlineCache)
{
return this->targetFunction->HasInstance(instance, scriptContext, inlineCache);
Expand Down
13 changes: 1 addition & 12 deletions lib/Runtime/Library/BoundFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,20 @@ namespace Js
protected:
BoundFunction(DynamicType * type);
BoundFunction(Arguments args, DynamicType * type);
BoundFunction(RecyclableObject* targetFunction, Var boundThis, Var* args, uint argsCount, DynamicType *type);

public:
static BoundFunction* New(ScriptContext* scriptContext, ArgumentReader args);

static bool Is(Var func){ return JavascriptFunction::Is(func) && JavascriptFunction::UnsafeFromVar(func)->IsBoundFunction(); }
static Var NewInstance(RecyclableObject* function, CallInfo callInfo, ...);
virtual JavascriptString* GetDisplayNameImpl() const override;
virtual PropertyQueryFlags HasPropertyQuery(PropertyId propertyId, _Inout_opt_ PropertyValueInfo* info) override;
virtual PropertyQueryFlags GetPropertyQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
virtual PropertyQueryFlags GetPropertyQuery(Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
virtual PropertyQueryFlags GetPropertyReferenceQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) override;
virtual BOOL SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
virtual BOOL SetProperty(JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;

_Check_return_ _Success_(return) virtual BOOL GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext) override;
virtual DescriptorFlags GetSetter(PropertyId propertyId, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
virtual DescriptorFlags GetSetter(JavascriptString* propertyNameString, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;

virtual BOOL InitProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags = PropertyOperation_None, PropertyValueInfo* info = NULL) override;
virtual BOOL DeleteProperty(PropertyId propertyId, PropertyOperationFlags flags) override;
virtual BOOL DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags) override;

virtual BOOL IsWritable(PropertyId propertyId) override;
virtual BOOL IsConfigurable(PropertyId propertyId) override;
virtual BOOL IsEnumerable(PropertyId propertyId) override;
virtual BOOL HasInstance(Var instance, ScriptContext* scriptContext, IsInstInlineCache* inlineCache = NULL) override;
virtual inline BOOL IsConstructor() const override;

Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Library/JavascriptExternalFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace Js

bool __cdecl JavascriptExternalFunction::DeferredLengthInitializer(DynamicObject * instance, DeferredTypeHandlerBase * typeHandler, DeferredInitializeMode mode)
{
Js::JavascriptLibrary::InitializeFunction<true>(instance, typeHandler, mode);
Js::JavascriptLibrary::InitializeFunction<true, true, true>(instance, typeHandler, mode);

JavascriptExternalFunction* object = static_cast<JavascriptExternalFunction*>(instance);

Expand Down
Loading

0 comments on commit 7699222

Please sign in to comment.