From 6e5d041b838592fafea9497e37a1ec07d22cb97f Mon Sep 17 00:00:00 2001 From: Akrosh Gandhi Date: Fri, 4 Oct 2019 16:05:08 -0700 Subject: [PATCH] Issue in setting accessor in PathTypeHandlerWithAttr 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. --- lib/Runtime/Types/PathTypeHandler.cpp | 2 ++ lib/Runtime/Types/SimpleTypeHandler.cpp | 20 ++++++++++++++++++ lib/Runtime/Types/SimpleTypeHandler.h | 2 ++ test/Bugs/bug_6271.js | 27 +++++++++++++++++++++++++ test/Bugs/rlexe.xml | 5 +++++ 5 files changed, 56 insertions(+) create mode 100644 test/Bugs/bug_6271.js diff --git a/lib/Runtime/Types/PathTypeHandler.cpp b/lib/Runtime/Types/PathTypeHandler.cpp index e41c031a562..856e6c2fa53 100644 --- a/lib/Runtime/Types/PathTypeHandler.cpp +++ b/lib/Runtime/Types/PathTypeHandler.cpp @@ -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()) diff --git a/lib/Runtime/Types/SimpleTypeHandler.cpp b/lib/Runtime/Types/SimpleTypeHandler.cpp index 8de4ea88105..ccb43254a9b 100644 --- a/lib/Runtime/Types/SimpleTypeHandler.cpp +++ b/lib/Runtime/Types/SimpleTypeHandler.cpp @@ -95,6 +95,11 @@ namespace Js } } + if (HasDeletedProperties()) + { + return false; + } + return true; } @@ -172,6 +177,21 @@ namespace Js return newTypeHandler; } + template + bool SimpleTypeHandler::HasDeletedProperties() + { + for (PropertyIndex i = 0; i < propertyCount; i++) + { + if (descriptors[i].Attributes & PropertyDeleted) + { + return true; + } + } + + return false; + } + + template PathTypeHandlerBase* SimpleTypeHandler::ConvertToPathType(DynamicObject* instance) { diff --git a/lib/Runtime/Types/SimpleTypeHandler.h b/lib/Runtime/Types/SimpleTypeHandler.h index b658f14f0df..fb214d30abb 100644 --- a/lib/Runtime/Types/SimpleTypeHandler.h +++ b/lib/Runtime/Types/SimpleTypeHandler.h @@ -92,6 +92,8 @@ namespace Js ES5ArrayTypeHandler* ConvertToES5ArrayType(DynamicObject* instance); SimpleTypeHandler* ConvertToNonSharedSimpleType(DynamicObject * instance); + bool HasDeletedProperties(); + BOOL GetDescriptor(PropertyId propertyId, PropertyIndex * index); BOOL SetAttribute(DynamicObject* instance, PropertyIndex index, PropertyAttributes attribute); BOOL ClearAttribute(DynamicObject* instance, PropertyIndex index, PropertyAttributes attribute); diff --git a/test/Bugs/bug_6271.js b/test/Bugs/bug_6271.js new file mode 100644 index 00000000000..df47aae1c52 --- /dev/null +++ b/test/Bugs/bug_6271.js @@ -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'); + })(); + \ No newline at end of file diff --git a/test/Bugs/rlexe.xml b/test/Bugs/rlexe.xml index 4d1afffbc85..0c40cf01dec 100644 --- a/test/Bugs/rlexe.xml +++ b/test/Bugs/rlexe.xml @@ -623,6 +623,11 @@ bug_6277.js + + + bug_6271.js + + bug_OS23102586.js