From 8edb0c8939ac9e391618710cd85b41a672f23acc Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 3 Jan 2017 10:46:54 -0800 Subject: [PATCH] TypeSharing Bug fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we share type for ChangePrototype scenario, there is an instance where we could assign type with slotCapacity different than the layout of underline object. For example, initially we cache the map of oldType to newCachedType (both didn’t have properties) in prototype object `P`. Later, newCachedType evolves when properties were added on it. Also, oldType’s slotCapacity got shrunk when we update the ctor cache of an object. In future, when we try to set same object `P` as prototype to another object, we notice that currentType aka oldType’s slotcapacity was shrunk. So we try to shrink the newCachedType’s slotCapacity as well, but it doesn’t shrink to the extent oldType was shrunk. The reason being because newCachedType has successors added that were not present on oldType. Thus we end up assigning a type to an object with different slotCapacity than object’s layout. Any further operations on this object can give undefined behavior. Fix: We try to shrink slotCapacity of newCachedType and after shrinking if it still doesn’t match oldType’s slotCapacity, we create newType and set it on object. We also update the cache with this newType. --- lib/Runtime/Types/PathTypeHandler.cpp | 10 ++++++++++ test/Prototypes/ChangePrototype.baseline | 3 +++ test/Prototypes/ChangePrototype.js | 24 +++++++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/Runtime/Types/PathTypeHandler.cpp b/lib/Runtime/Types/PathTypeHandler.cpp index c704a9e8e59..63620e7eefc 100644 --- a/lib/Runtime/Types/PathTypeHandler.cpp +++ b/lib/Runtime/Types/PathTypeHandler.cpp @@ -1552,6 +1552,16 @@ namespace Js Assert(cachedDynamicTypeHandler->GetInlineSlotCapacity() >= roundedInlineSlotCapacity); Assert(cachedDynamicTypeHandler->GetInlineSlotCapacity() >= GetPropertyCount()); cachedDynamicTypeHandler->ShrinkSlotAndInlineSlotCapacity(); + + // If slot capacity doesn't match after shrinking, it could be because oldType was shrunk and + // newType evolved. In that case, we should update the cache with new type + if (cachedDynamicTypeHandler->GetInlineSlotCapacity() != roundedInlineSlotCapacity) + { + cachedDynamicType = nullptr; +#if DBG + swprintf_s(reason, 1024, _u("InlineSlotCapacity mismatch after shrinking. Required = %d, Cached = %d"), roundedInlineSlotCapacity, cachedDynamicTypeHandler->GetInlineSlotCapacity()); +#endif + } } } } diff --git a/test/Prototypes/ChangePrototype.baseline b/test/Prototypes/ChangePrototype.baseline index 3d992a12c0e..41b7a933802 100644 --- a/test/Prototypes/ChangePrototype.baseline +++ b/test/Prototypes/ChangePrototype.baseline @@ -26,3 +26,6 @@ Changing __proto__ TypeSharing: Updating prototype object's DictionarySlot cache in __proto__. Changing __proto__ TypeSharing: Reusing prototype object's DictionarySlot cache in __proto__. +TypeSharing: Updating prototype object's InlineSlot cache in CreateObject. +TypeSharing: Updating prototype object's DictionarySlot cache in __proto__. +TypeSharing: Updating prototype object's DictionarySlot cache in __proto__. diff --git a/test/Prototypes/ChangePrototype.js b/test/Prototypes/ChangePrototype.js index 3e47c8bd907..677b4d77927 100644 --- a/test/Prototypes/ChangePrototype.js +++ b/test/Prototypes/ChangePrototype.js @@ -80,5 +80,27 @@ function test2() { y.__proto__ = obj; // cached T2's inlineSlotCapacity doesn't match y's T1 } +function test3() { + // no switches needed + var proto = {}; + + function foo() { + } + + var x = new foo(); + var y = new foo(); + y.__proto__ = proto; // empty type cached in map of proto object + y._a = 1; // evolve cached type created above + y._b = 1; + y._c = 1; + y._d = 1; + var z = new foo(); // this shrunk oldType's slotCapacity from 8 to 2. + + // retrived the cached type which was evolved. + // Realized that oldType's slotCapacity has shrunk, we shrink slot capacity of cachedType but it doesn't match because cachedType has evolved + z.__proto__ = proto; +} + test1(); -test2(); \ No newline at end of file +test2(); +test3(); \ No newline at end of file