Skip to content

Commit

Permalink
Issue in setting accessor in PathTypeHandlerWithAttr
Browse files Browse the repository at this point in the history
When the property is deleted (say when we are in SimpleTypeHandler) and then later
if we create an accessor on the same property we have converted the type handler to PathTypeHandlerWithAttr.
Since a property got deleted and PathTypeHandler should not handle that deleted property we need to convert this to
Dictionary type.
In order to fix that When we add the accessor on any property we need to check if any property deleted upon that we need
to convert to Dictionary type instead of type.
  • Loading branch information
akroshg committed Oct 8, 2019
1 parent 6b98ef8 commit 3e080f7
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 1 deletion.
2 changes: 2 additions & 0 deletions lib/Runtime/Types/PathTypeHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,8 @@ namespace Js
}
}

Assert((attr & ObjectSlotAttr_Deleted) != ObjectSlotAttr_Deleted);

SetAttributesHelper(instance, propertyId, propertyIndex, attributes, (ObjectSlotAttributes)(attr | ObjectSlotAttr_Accessor));
// SetAttributesHelper can convert to dictionary in corner cases, e.g., if we haven't got a full path from the root. Remove this check when that's fixed.
if (!instance->GetTypeHandler()->IsPathTypeHandler())
Expand Down
19 changes: 18 additions & 1 deletion lib/Runtime/Types/SimpleTypeHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,26 @@ namespace Js
return newTypeHandler;
}

template<size_t size>
bool SimpleTypeHandler<size>::HasAnyPropertyDeleted()
{
for (PropertyIndex i = 0; i < propertyCount; i++)
{
if (descriptors[i].Attributes & PropertyDeleted)
{
return true;
}
}

return false;
}


template<size_t size>
PathTypeHandlerBase* SimpleTypeHandler<size>::ConvertToPathType(DynamicObject* instance)
{
Assert(!HasAnyPropertyDeleted());

ScriptContext *scriptContext = instance->GetScriptContext();
PathTypeHandlerBase* newTypeHandler =
PathTypeHandlerNoAttr::New(
Expand Down Expand Up @@ -956,7 +973,7 @@ namespace Js
template<size_t size>
BOOL SimpleTypeHandler<size>::SetAccessors(DynamicObject* instance, PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags)
{
if (DoConvertToPathType(instance->GetDynamicType()))
if (DoConvertToPathType(instance->GetDynamicType()) && !HasAnyPropertyDeleted())
{
return ConvertToPathType(instance)->SetAccessors(instance, propertyId, getter, setter, flags);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/Runtime/Types/SimpleTypeHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ namespace Js
ES5ArrayTypeHandler* ConvertToES5ArrayType(DynamicObject* instance);
SimpleTypeHandler<size>* ConvertToNonSharedSimpleType(DynamicObject * instance);

bool HasAnyPropertyDeleted();

BOOL GetDescriptor(PropertyId propertyId, PropertyIndex * index);
BOOL SetAttribute(DynamicObject* instance, PropertyIndex index, PropertyAttributes attribute);
BOOL ClearAttribute(DynamicObject* instance, PropertyIndex index, PropertyAttributes attribute);
Expand Down
27 changes: 27 additions & 0 deletions test/Bugs/bug_6271.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

function testReconfigureAsAccessorProperty(f) {
var length = 2;
Object.defineProperty(f, 'length', {
get: function () {
return length;
},
set: function (v) {
length = v;
}
});
}
(function testSetOnInstance() {
function f() {}
delete f.length;
testReconfigureAsAccessorProperty(f);
Object.defineProperty(Function.prototype, 'length', {
writable: true
});
f.length = 123;
f.length == 123 ? print("Pass") : print('fail');
})();

5 changes: 5 additions & 0 deletions test/Bugs/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,11 @@
<files>bug_6277.js</files>
</default>
</test>
<test>
<default>
<files>bug_6271.js</files>
</default>
</test>
<test>
<default>
<files>bug_OS23102586.js</files>
Expand Down

0 comments on commit 3e080f7

Please sign in to comment.