Skip to content

Commit

Permalink
[MERGE #2303 @kunalspathak] TypeSharing Bug fix
Browse files Browse the repository at this point in the history
Merge pull request #2303 from kunalspathak:typesharingfix

See: https://microsoft.visualstudio.com/DefaultCollection/OS/_workItems?id=10058868&triage=true&_a=edit

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.
  • Loading branch information
kunalspathak committed Jan 4, 2017
2 parents c65d768 + 8edb0c8 commit 1c38588
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
10 changes: 10 additions & 0 deletions lib/Runtime/Types/PathTypeHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/Prototypes/ChangePrototype.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -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__.
24 changes: 23 additions & 1 deletion test/Prototypes/ChangePrototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
test2();
test3();

0 comments on commit 1c38588

Please sign in to comment.