Skip to content

Commit 86333c2

Browse files
author
Kevin Smith
committed
Don't copy object directly if internal properties are set
1 parent 7a4a283 commit 86333c2

File tree

8 files changed

+73
-42
lines changed

8 files changed

+73
-42
lines changed

‎lib/Runtime/Library/JavascriptArray.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12279,15 +12279,23 @@ using namespace Js;
1227912279
JS_REENTRANCY_LOCK(jsReentLock, scriptContext->GetThreadContext());
1228012280
SETOBJECT_FOR_MUTATION(jsReentLock, originalArray);
1228112281

12282-
if (JavascriptArray::IsNonES5Array(originalArray)
12283-
&& !UnsafeVarTo<DynamicObject>(originalArray)->GetDynamicType()->GetTypeHandler()->GetIsNotPathTypeHandlerOrHasUserDefinedCtor()
12284-
&& UnsafeVarTo<DynamicObject>(originalArray)->GetPrototype() == scriptContext->GetLibrary()->GetArrayPrototype()
12285-
&& !scriptContext->GetLibrary()->GetArrayObjectHasUserDefinedSpecies())
12282+
auto* library = scriptContext->GetLibrary();
12283+
12284+
if (JavascriptArray::IsNonES5Array(originalArray))
1228612285
{
12287-
return nullptr;
12286+
auto* dynamicObject = UnsafeVarTo<DynamicObject>(originalArray);
12287+
auto* typeHandler = dynamicObject->GetDynamicType()->GetTypeHandler();
12288+
12289+
if (typeHandler->IsPathTypeHandler()
12290+
&& !PathTypeHandlerBase::FromTypeHandler(typeHandler)->HasUserDefinedCtor()
12291+
&& dynamicObject->GetPrototype() == library->GetArrayPrototype()
12292+
&& !library->GetArrayObjectHasUserDefinedSpecies())
12293+
{
12294+
return nullptr;
12295+
}
1228812296
}
1228912297

12290-
Var constructor = scriptContext->GetLibrary()->GetUndefined();
12298+
Var constructor = library->GetUndefined();
1229112299

1229212300
JS_REENTRANT(jsReentLock, BOOL isArray = JavascriptOperators::IsArray(originalArray));
1229312301
if (isArray)
@@ -12305,7 +12313,7 @@ using namespace Js;
1230512313
{
1230612314
if (constructorScriptContext->GetLibrary()->GetArrayConstructor() == constructor)
1230712315
{
12308-
constructor = scriptContext->GetLibrary()->GetUndefined();
12316+
constructor = library->GetUndefined();
1230912317
}
1231012318
}
1231112319
}
@@ -12321,14 +12329,14 @@ using namespace Js;
1232112329
}
1232212330
return nullptr;
1232312331
}
12324-
if (constructor == scriptContext->GetLibrary()->GetNull())
12332+
if (constructor == library->GetNull())
1232512333
{
12326-
constructor = scriptContext->GetLibrary()->GetUndefined();
12334+
constructor = library->GetUndefined();
1232712335
}
1232812336
}
1232912337
}
1233012338

12331-
if (constructor == scriptContext->GetLibrary()->GetUndefined() || constructor == scriptContext->GetLibrary()->GetArrayConstructor())
12339+
if (constructor == library->GetUndefined() || constructor == library->GetArrayConstructor())
1233212340
{
1233312341
if (length > UINT_MAX)
1233412342
{
@@ -12337,7 +12345,7 @@ using namespace Js;
1233712345

1233812346
if (nullptr == pIsIntArray)
1233912347
{
12340-
return scriptContext->GetLibrary()->CreateArray(static_cast<uint32>(length));
12348+
return library->CreateArray(static_cast<uint32>(length));
1234112349
}
1234212350
else
1234312351
{

‎lib/Runtime/Types/DynamicObject.cpp

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -866,11 +866,11 @@ namespace Js
866866
}
867867
return false;
868868
}
869-
if (!from->GetTypeHandler()->IsPathTypeHandler())
869+
if (!from->GetTypeHandler()->IsObjectCopyable())
870870
{
871871
if (PHASE_TRACE1(ObjectCopyPhase))
872872
{
873-
Output::Print(_u("ObjectCopy: Can't copy: Don't have PathTypeHandler\n"));
873+
Output::Print(_u("ObjectCopy: Can't copy: from obj does not have copyable type handler\n"));
874874
}
875875
return false;
876876
}
@@ -890,14 +890,6 @@ namespace Js
890890
}
891891
return false;
892892
}
893-
if (PathTypeHandlerBase::FromTypeHandler(from->GetTypeHandler())->HasAccessors())
894-
{
895-
if (PHASE_TRACE1(ObjectCopyPhase))
896-
{
897-
Output::Print(_u("ObjectCopy: Can't copy: type handler has accessors\n"));
898-
}
899-
return false;
900-
}
901893
if (this->GetPrototype() != from->GetPrototype())
902894
{
903895
if (PHASE_TRACE1(ObjectCopyPhase))
@@ -906,14 +898,6 @@ namespace Js
906898
}
907899
return false;
908900
}
909-
if (!from->GetTypeHandler()->AllPropertiesAreEnumerable())
910-
{
911-
if (PHASE_TRACE1(ObjectCopyPhase))
912-
{
913-
Output::Print(_u("ObjectCopy: Can't copy: from obj has non-enumerable properties\n"));
914-
}
915-
return false;
916-
}
917901
if (from->IsExternal())
918902
{
919903
if (PHASE_TRACE1(ObjectCopyPhase))

‎lib/Runtime/Types/PathTypeHandler.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,20 @@ namespace Js
235235
DynamicTypeHandler(slotCapacity, inlineSlotCapacity, offsetOfInlineSlots, DefaultFlags | (isLocked ? IsLockedFlag : 0) | (isShared ? (MayBecomeSharedFlag | IsSharedFlag) : 0)),
236236
typePath(typePath),
237237
predecessorType(predecessorType),
238-
successorInfo(nullptr)
238+
successorInfo(nullptr),
239+
hasUserDefinedCtor(false),
240+
hasInternalProperty(false)
239241
{
240242
Assert(pathLength <= slotCapacity);
241243
Assert(inlineSlotCapacity <= slotCapacity);
242244
SetUnusedBytesValue(pathLength);
243-
isNotPathTypeHandlerOrHasUserDefinedCtor = predecessorType == nullptr ? false : predecessorType->GetTypeHandler()->GetIsNotPathTypeHandlerOrHasUserDefinedCtor();
245+
246+
if (predecessorType != nullptr && predecessorType->GetTypeHandler()->IsPathTypeHandler())
247+
{
248+
auto* handler = PathTypeHandlerBase::FromTypeHandler(predecessorType->GetTypeHandler());
249+
hasUserDefinedCtor = handler->hasUserDefinedCtor;
250+
hasInternalProperty = handler->hasInternalProperty;
251+
}
244252
}
245253

246254
int PathTypeHandlerBase::GetPropertyCount()
@@ -2166,7 +2174,12 @@ namespace Js
21662174

21672175
if (key.GetPropertyId() == PropertyIds::constructor)
21682176
{
2169-
nextPath->isNotPathTypeHandlerOrHasUserDefinedCtor = true;
2177+
nextPath->hasUserDefinedCtor = true;
2178+
}
2179+
2180+
if (IsInternalPropertyId(key.GetPropertyId()))
2181+
{
2182+
nextPath->hasInternalProperty = true;
21702183
}
21712184

21722185
#ifdef PROFILE_TYPES

‎lib/Runtime/Types/PathTypeHandler.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ namespace Js
102102
Field(DynamicType*) predecessorType; // Strong reference to predecessor type so that predecessor types remain in the cache even though they might not be used
103103
Field(TypePath*) typePath;
104104
Field(PathTypeSuccessorInfo*) successorInfo;
105+
Field(bool) hasUserDefinedCtor;
106+
Field(bool) hasInternalProperty;
105107

106108
public:
107109
DEFINE_GETCPPNAME();
@@ -119,6 +121,8 @@ namespace Js
119121
return nullptr;
120122
}
121123

124+
bool HasUserDefinedCtor() { return this->hasUserDefinedCtor; }
125+
122126
virtual BOOL IsLockable() const override { return true; }
123127
virtual BOOL IsSharable() const override { return true; }
124128

@@ -459,6 +463,8 @@ namespace Js
459463
DEFINE_VTABLE_CTOR_NO_REGISTER(PathTypeHandlerNoAttr, PathTypeHandlerBase);
460464

461465
public:
466+
virtual bool IsObjectCopyable() const override { return !this->hasInternalProperty; }
467+
462468
static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, TypePath* typePath, uint16 pathLength, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots, bool isLocked = false, bool isShared = false, DynamicType* predecessorType = nullptr);
463469
static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, TypePath* typePath, uint16 pathLength, const PropertyIndex slotCapacity, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots, bool isLocked = false, bool isShared = false, DynamicType* predecessorType = nullptr);
464470
static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, PathTypeHandlerNoAttr * typeHandler, bool isLocked, bool isShared);
@@ -534,6 +540,7 @@ namespace Js
534540
return FindNextPropertyHelper(scriptContext, this->attributes, index, propertyString, propertyId, attributes, type, typeToEnumerate, flags, instance, info);
535541
}
536542
virtual BOOL AllPropertiesAreEnumerable() sealed override { return false; }
543+
virtual bool IsObjectCopyable() const override { return false; }
537544
#if ENABLE_NATIVE_CODEGEN
538545
virtual bool IsObjTypeSpecEquivalent(const Type* type, const TypeEquivalenceRecord& record, uint& failedPropertyIndex) override;
539546
virtual bool IsObjTypeSpecEquivalent(const Type* type, const EquivalentPropertyEntry* entry) override;

‎lib/Runtime/Types/TypeHandler.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ using namespace Js;
7777
? RoundUpObjectHeaderInlinedInlineSlotCapacity(inlineSlotCapacity)
7878
: RoundUpInlineSlotCapacity(inlineSlotCapacity);
7979
this->slotCapacity = RoundUpSlotCapacity(slotCapacity, inlineSlotCapacity);
80-
this->isNotPathTypeHandlerOrHasUserDefinedCtor = true;
8180

8281
Assert(IsObjectHeaderInlinedTypeHandler() == IsObjectHeaderInlined(offsetOfInlineSlots));
8382
}
@@ -884,6 +883,5 @@ using namespace Js;
884883
Output::Print(_u("%*sslotCapacity: %d\n"), fieldIndent, padding, this->slotCapacity);
885884
Output::Print(_u("%*sunusedBytes: %u\n"), fieldIndent, padding, this->unusedBytes);
886885
Output::Print(_u("%*sinlineSlotCapacty: %u\n"), fieldIndent, padding, this->inlineSlotCapacity);
887-
Output::Print(_u("%*sisNotPathTypeHandlerOrHasUserDefinedCtor: %d\n"), fieldIndent, padding, static_cast<int>(this->isNotPathTypeHandlerOrHasUserDefinedCtor));
888886
}
889887
#endif

‎lib/Runtime/Types/TypeHandler.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ namespace Js
4848
Field(int) slotCapacity;
4949
Field(uint16) unusedBytes; // This always has it's lowest bit set to avoid false references
5050
Field(uint16) inlineSlotCapacity;
51-
Field(bool) isNotPathTypeHandlerOrHasUserDefinedCtor;
5251
Field(bool) protoCachesWereInvalidated;
5352

5453
public:
@@ -58,7 +57,6 @@ namespace Js
5857
propertyTypes(typeHandler->propertyTypes),
5958
slotCapacity(typeHandler->slotCapacity),
6059
offsetOfInlineSlots(typeHandler->offsetOfInlineSlots),
61-
isNotPathTypeHandlerOrHasUserDefinedCtor(typeHandler->isNotPathTypeHandlerOrHasUserDefinedCtor),
6260
unusedBytes(typeHandler->unusedBytes),
6361
protoCachesWereInvalidated(false),
6462
inlineSlotCapacity(typeHandler->inlineSlotCapacity)
@@ -427,7 +425,6 @@ namespace Js
427425
}
428426

429427
BOOL Freeze(DynamicObject *instance, bool isConvertedType = false) { return FreezeImpl(instance, isConvertedType); }
430-
bool GetIsNotPathTypeHandlerOrHasUserDefinedCtor() const { return this->isNotPathTypeHandlerOrHasUserDefinedCtor; }
431428

432429
virtual BOOL IsStringTypeHandler() const { return false; }
433430

@@ -560,6 +557,8 @@ namespace Js
560557
virtual BOOL IsSimpleDictionaryTypeHandler() const {return FALSE; }
561558
virtual BOOL IsDictionaryTypeHandler() const {return FALSE;}
562559

560+
virtual bool IsObjectCopyable() const { return false; }
561+
563562
static bool IsolatePrototypes() { return CONFIG_FLAG(IsolatePrototypes); }
564563
static bool ChangeTypeOnProto() { return CONFIG_FLAG(ChangeTypeOnProto); }
565564
static bool ShouldFixMethodProperties() { return !PHASE_OFF1(FixMethodPropsPhase); }

‎test/Object/assign.baseline

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
ObjectCopy succeeded
2-
ObjectCopy: Can't copy: Don't have PathTypeHandler
3-
ObjectCopy: Can't copy: type handler has accessors
4-
ObjectCopy: Can't copy: type handler has accessors
2+
ObjectCopy: Can't copy: from obj does not have copyable type handler
3+
ObjectCopy: Can't copy: from obj does not have copyable type handler
4+
ObjectCopy: Can't copy: from obj does not have copyable type handler
55
ObjectCopy: Can't copy: Prototypes don't match
6-
ObjectCopy: Can't copy: from obj has non-enumerable properties
6+
ObjectCopy: Can't copy: from obj does not have copyable type handler
77
ObjectCopy succeeded
88
ObjectCopy succeeded
99
ObjectCopy: Can't copy: to obj has object array
10-
ObjectCopy: Can't copy: Don't have PathTypeHandler
10+
ObjectCopy: Can't copy: from obj does not have copyable type handler
1111
pass

‎test/es6/weakmap_functionality.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,28 @@ var tests = [
501501
}
502502
},
503503

504+
{
505+
name: "WeakMap internal property data is not copied by Object.assign",
506+
body: function () {
507+
var key1 = {};
508+
var key2 = {};
509+
var map = new WeakMap();
510+
511+
map.set(key1, 1);
512+
map.delete(Object.assign(key2, key1));
513+
assert.isFalse(map.has(key2));
514+
515+
key1 = {};
516+
key2 = {};
517+
map = new WeakMap();
518+
519+
map.set(key1, 1);
520+
key1.a = 1;
521+
map.delete(Object.assign(key2, key1));
522+
assert.isFalse(map.has(key2));
523+
}
524+
}
525+
504526
];
505527

506528
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

0 commit comments

Comments
 (0)