Changeset 257728 in webkit
- Timestamp:
- Mar 2, 2020 12:58:22 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r257605 r257728 1 2020-03-02 Justin Michaud <justin_michaud@apple.com> 2 3 Delete by val caching does not keep the subscript alive 4 https://bugs.webkit.org/show_bug.cgi?id=208393 5 6 Reviewed by Yusuke Suzuki. 7 8 * stress/delete-property-ic-stress.js: Added. 9 1 10 2020-02-27 Justin Michaud <justin_michaud@apple.com> 2 11 -
trunk/Source/JavaScriptCore/ChangeLog
r257721 r257728 1 2020-03-02 Justin Michaud <justin_michaud@apple.com> 2 3 Delete by val caching does not keep the subscript alive 4 https://bugs.webkit.org/show_bug.cgi?id=208393 5 6 Reviewed by Yusuke Suzuki. 7 8 Before, the provided test case crashed with asan because we did not keep deleteByVal 9 subscripts alive. This patch changes CacheableIdentifier to make this mistake harder 10 to make again, by making the constructor calls more explicit when CacheableIdentifier 11 will not keep an Identifier alive. 12 13 * jit/JITOperations.cpp: 14 * jit/Repatch.cpp: 15 (JSC::tryCachePutByID): 16 (JSC::tryCacheDeleteBy): 17 (JSC::repatchDeleteBy): 18 (JSC::tryCacheInByID): 19 (JSC::tryCacheInstanceOf): 20 (JSC::tryCacheDelBy): Deleted. 21 (JSC::repatchDelBy): Deleted. 22 * jit/Repatch.h: 23 * runtime/CacheableIdentifier.h: 24 * runtime/CacheableIdentifierInlines.h: 25 (JSC::CacheableIdentifier::createFromIdentifierOwnedByCodeBlock): 26 (JSC::CacheableIdentifier::createFromCell): 27 1 28 2020-03-02 Paulo Matos <pmatos@igalia.com> 2 29 -
trunk/Source/JavaScriptCore/jit/JITOperations.cpp
r257399 r257728 215 215 CodeBlock* codeBlock = callFrame->codeBlock(); 216 216 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull()) && !slot.isTaintedByOpaqueObject() && (slot.isCacheableValue() || slot.isCacheableGetter() || slot.isUnset())) 217 repatchGetBy(globalObject, codeBlock, baseValue, ident, slot, *stubInfo, GetByKind::Try);217 repatchGetBy(globalObject, codeBlock, baseValue, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), slot, *stubInfo, GetByKind::Try); 218 218 219 219 return JSValue::encode(slot.getPureResult()); … … 271 271 CodeBlock* codeBlock = callFrame->codeBlock(); 272 272 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull())) 273 repatchGetBy(globalObject, codeBlock, baseValue, ident, slot, *stubInfo, GetByKind::Direct);273 repatchGetBy(globalObject, codeBlock, baseValue, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), slot, *stubInfo, GetByKind::Direct); 274 274 275 275 RELEASE_AND_RETURN(scope, JSValue::encode(found ? slot.getValue(globalObject, ident) : jsUndefined())); … … 331 331 CodeBlock* codeBlock = callFrame->codeBlock(); 332 332 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull())) 333 repatchGetBy(globalObject, codeBlock, baseValue, ident, slot, *stubInfo, GetByKind::Normal);333 repatchGetBy(globalObject, codeBlock, baseValue, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), slot, *stubInfo, GetByKind::Normal); 334 334 return found ? slot.getValue(globalObject, ident) : jsUndefined(); 335 335 })); … … 388 388 CodeBlock* codeBlock = callFrame->codeBlock(); 389 389 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull())) 390 repatchGetBy(globalObject, codeBlock, baseValue, ident, slot, *stubInfo, GetByKind::WithThis);390 repatchGetBy(globalObject, codeBlock, baseValue, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), slot, *stubInfo, GetByKind::WithThis); 391 391 return found ? slot.getValue(globalObject, ident) : jsUndefined(); 392 392 })); … … 2023 2023 2024 2024 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull(), propertyName.impl())) 2025 repatchGetBy(globalObject, codeBlock, baseValue, subscript.asCell(), slot, *stubInfo, GetByKind::NormalByVal);2025 repatchGetBy(globalObject, codeBlock, baseValue, CacheableIdentifier::createFromCell(subscript.asCell()), slot, *stubInfo, GetByKind::NormalByVal); 2026 2026 return found ? slot.getValue(globalObject, propertyName) : jsUndefined(); 2027 2027 })); … … 2173 2173 2174 2174 if (baseValue.isObject()) { 2175 const Identifier propertyName= Identifier::fromUid(vm, uid);2175 const Identifier ident = Identifier::fromUid(vm, uid); 2176 2176 RETURN_IF_EXCEPTION(scope, encodedJSValue()); 2177 2177 2178 if (!parseIndex( propertyName)) {2178 if (!parseIndex(ident)) { 2179 2179 CodeBlock* codeBlock = callFrame->codeBlock(); 2180 2180 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull())) 2181 repatchDel By(globalObject, codeBlock, slot, baseValue, oldStructure, propertyName, *stubInfo, DelByKind::Normal);2181 repatchDeleteBy(globalObject, codeBlock, slot, baseValue, oldStructure, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), *stubInfo, DelByKind::Normal); 2182 2182 } 2183 2183 } … … 2241 2241 CodeBlock* codeBlock = callFrame->codeBlock(); 2242 2242 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull())) 2243 repatchDel By(globalObject, codeBlock, slot, baseValue, oldStructure, propertyName, *stubInfo, DelByKind::NormalByVal);2243 repatchDeleteBy(globalObject, codeBlock, slot, baseValue, oldStructure, CacheableIdentifier::createFromCell(subscript.asCell()), *stubInfo, DelByKind::NormalByVal); 2244 2244 } 2245 2245 } -
trunk/Source/JavaScriptCore/jit/Repatch.cpp
r257605 r257728 593 593 } 594 594 595 newCase = AccessCase::create(vm, codeBlock, AccessCase::Replace, ident, slot.cachedOffset(), oldStructure);595 newCase = AccessCase::create(vm, codeBlock, AccessCase::Replace, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), slot.cachedOffset(), oldStructure); 596 596 } else { 597 597 ASSERT(slot.type() == PutPropertySlot::NewProperty); … … 642 642 } 643 643 644 newCase = AccessCase::createTransition(vm, codeBlock, ident, offset, oldStructure, newStructure, conditionSet, WTFMove(prototypeAccessChain));644 newCase = AccessCase::createTransition(vm, codeBlock, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), offset, oldStructure, newStructure, conditionSet, WTFMove(prototypeAccessChain)); 645 645 } 646 646 } else if (slot.isCacheableCustom() || slot.isCacheableSetter()) { … … 671 671 672 672 newCase = GetterSetterAccessCase::create( 673 vm, codeBlock, slot.isCustomAccessor() ? AccessCase::CustomAccessorSetter : AccessCase::CustomValueSetter, oldStructure, ident,673 vm, codeBlock, slot.isCustomAccessor() ? AccessCase::CustomAccessorSetter : AccessCase::CustomValueSetter, oldStructure, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), 674 674 invalidOffset, conditionSet, WTFMove(prototypeAccessChain), slot.customSetter(), slot.base() != baseValue ? slot.base() : nullptr); 675 675 } else { … … 708 708 709 709 newCase = GetterSetterAccessCase::create( 710 vm, codeBlock, AccessCase::Setter, oldStructure, ident, offset, conditionSet, WTFMove(prototypeAccessChain));710 vm, codeBlock, AccessCase::Setter, oldStructure, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), offset, conditionSet, WTFMove(prototypeAccessChain)); 711 711 } 712 712 } … … 714 714 LOG_IC((ICEvent::PutByIdAddAccessCase, oldStructure->classInfo(), ident, slot.base() == baseValue)); 715 715 716 result = stubInfo.addAccessCase(locker, codeBlock, ident, WTFMove(newCase));716 result = stubInfo.addAccessCase(locker, codeBlock, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), WTFMove(newCase)); 717 717 718 718 if (result.generatedSomeCode()) { … … 738 738 } 739 739 740 static InlineCacheAction tryCacheDel By(JSGlobalObject* globalObject, CodeBlock* codeBlock, DeletePropertySlot& slot, JSValue baseValue, Structure* oldStructure, const Identifier&propertyName, StructureStubInfo& stubInfo, DelByKind)740 static InlineCacheAction tryCacheDeleteBy(JSGlobalObject* globalObject, CodeBlock* codeBlock, DeletePropertySlot& slot, JSValue baseValue, Structure* oldStructure, CacheableIdentifier propertyName, StructureStubInfo& stubInfo, DelByKind) 741 741 { 742 742 VM& vm = globalObject->vm(); … … 760 760 if (slot.isDeleteHit()) { 761 761 PropertyOffset newOffset = invalidOffset; 762 Structure* newStructure = Structure::removePropertyTransitionFromExistingStructureConcurrently(oldStructure, propertyName , newOffset);762 Structure* newStructure = Structure::removePropertyTransitionFromExistingStructureConcurrently(oldStructure, propertyName.uid(), newOffset); 763 763 if (!newStructure) 764 764 return RetryCacheLater; … … 770 770 ASSERT(newStructure->isObject()); 771 771 ASSERT(isValidOffset(newOffset)); 772 newCase = AccessCase::createDelete(vm, codeBlock, CacheableIdentifier(propertyName), newOffset, oldStructure, newStructure);772 newCase = AccessCase::createDelete(vm, codeBlock, propertyName, newOffset, oldStructure, newStructure); 773 773 } else if (!codeBlock->isStrictMode()) { 774 774 if (slot.isNonconfigurable()) 775 newCase = AccessCase::create(vm, codeBlock, AccessCase::DeleteNonConfigurable, CacheableIdentifier(propertyName), invalidOffset, oldStructure, { }, nullptr);775 newCase = AccessCase::create(vm, codeBlock, AccessCase::DeleteNonConfigurable, propertyName, invalidOffset, oldStructure, { }, nullptr); 776 776 else 777 newCase = AccessCase::create(vm, codeBlock, AccessCase::DeleteMiss, CacheableIdentifier(propertyName), invalidOffset, oldStructure, { }, nullptr);777 newCase = AccessCase::create(vm, codeBlock, AccessCase::DeleteMiss, propertyName, invalidOffset, oldStructure, { }, nullptr); 778 778 } 779 779 … … 782 782 if (result.generatedSomeCode()) { 783 783 RELEASE_ASSERT(result.code()); 784 LOG_IC((ICEvent::DelByReplaceWithJump, oldStructure->classInfo(), propertyName));784 LOG_IC((ICEvent::DelByReplaceWithJump, oldStructure->classInfo(), Identifier::fromUid(vm, propertyName.uid()))); 785 785 InlineAccess::rewireStubAsJump(stubInfo, CodeLocationLabel<JITStubRoutinePtrTag>(result.code())); 786 786 } … … 792 792 } 793 793 794 void repatchDel By(JSGlobalObject* globalObject, CodeBlock* codeBlock, DeletePropertySlot& slot, JSValue baseValue, Structure* oldStructure, const Identifier&propertyName, StructureStubInfo& stubInfo, DelByKind kind)794 void repatchDeleteBy(JSGlobalObject* globalObject, CodeBlock* codeBlock, DeletePropertySlot& slot, JSValue baseValue, Structure* oldStructure, CacheableIdentifier propertyName, StructureStubInfo& stubInfo, DelByKind kind) 795 795 { 796 796 SuperSamplerScope superSamplerScope(false); 797 798 if (tryCacheDelBy(globalObject, codeBlock, slot, baseValue, oldStructure, propertyName, stubInfo, kind) == GiveUpOnCache) { 799 LOG_IC((ICEvent::DelByReplaceWithGeneric, baseValue.classInfoOrNull(globalObject->vm()), propertyName)); 797 VM& vm = globalObject->vm(); 798 799 if (tryCacheDeleteBy(globalObject, codeBlock, slot, baseValue, oldStructure, propertyName, stubInfo, kind) == GiveUpOnCache) { 800 LOG_IC((ICEvent::DelByReplaceWithGeneric, baseValue.classInfoOrNull(globalObject->vm()), Identifier::fromUid(vm, propertyName.uid()))); 800 801 if (kind == DelByKind::Normal) 801 802 ftlThunkAwareRepatchCall(codeBlock, stubInfo.slowPathCallLocation, operationDeleteByIdGeneric); … … 892 893 893 894 std::unique_ptr<AccessCase> newCase = AccessCase::create( 894 vm, codeBlock, wasFound ? AccessCase::InHit : AccessCase::InMiss, ident, wasFound ? slot.cachedOffset() : invalidOffset, structure, conditionSet, WTFMove(prototypeAccessChain));895 896 result = stubInfo.addAccessCase(locker, codeBlock, ident, WTFMove(newCase));895 vm, codeBlock, wasFound ? AccessCase::InHit : AccessCase::InMiss, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), wasFound ? slot.cachedOffset() : invalidOffset, structure, conditionSet, WTFMove(prototypeAccessChain)); 896 897 result = stubInfo.addAccessCase(locker, codeBlock, CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(ident), WTFMove(newCase)); 897 898 898 899 if (result.generatedSomeCode()) { … … 958 959 959 960 if (!newCase) 960 newCase = AccessCase::create(vm, codeBlock, AccessCase::InstanceOfGeneric, Identifier());961 newCase = AccessCase::create(vm, codeBlock, AccessCase::InstanceOfGeneric, CacheableIdentifier()); 961 962 962 963 LOG_IC((ICEvent::InstanceOfAddAccessCase, structure->classInfo(), Identifier())); -
trunk/Source/JavaScriptCore/jit/Repatch.h
r257399 r257728 50 50 void repatchGetBy(JSGlobalObject*, CodeBlock*, JSValue, CacheableIdentifier, const PropertySlot&, StructureStubInfo&, GetByKind); 51 51 void repatchPutByID(JSGlobalObject*, CodeBlock*, JSValue, Structure*, const Identifier&, const PutPropertySlot&, StructureStubInfo&, PutKind); 52 void repatchDel By(JSGlobalObject*, CodeBlock*, DeletePropertySlot&, JSValue, Structure*, const Identifier&, StructureStubInfo&, DelByKind);52 void repatchDeleteBy(JSGlobalObject*, CodeBlock*, DeletePropertySlot&, JSValue, Structure*, CacheableIdentifier, StructureStubInfo&, DelByKind); 53 53 void repatchInByID(JSGlobalObject*, CodeBlock*, JSObject*, const Identifier&, bool wasFound, const PropertySlot&, StructureStubInfo&); 54 54 void repatchInstanceOf(JSGlobalObject*, CodeBlock*, JSValue value, JSValue prototype, StructureStubInfo&, bool wasFound); -
trunk/Source/JavaScriptCore/runtime/CacheableIdentifier.h
r254464 r257728 47 47 public: 48 48 CacheableIdentifier() = default; 49 inline CacheableIdentifier(const Identifier&); 50 inline CacheableIdentifier(JSCell* identifier); 49 50 static inline CacheableIdentifier createFromCell(JSCell* identifier); 51 static inline CacheableIdentifier createFromIdentifierOwnedByCodeBlock(const Identifier&); 51 52 52 53 CacheableIdentifier(const CacheableIdentifier&) = default; … … 84 85 85 86 private: 87 explicit inline CacheableIdentifier(const Identifier&); 88 explicit inline CacheableIdentifier(JSCell* identifier); 89 86 90 inline void setCellBits(JSCell*); 87 91 inline void setUidBits(UniquedStringImpl*); -
trunk/Source/JavaScriptCore/runtime/CacheableIdentifierInlines.h
r257399 r257728 35 35 36 36 namespace JSC { 37 38 inline CacheableIdentifier CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(const Identifier& i) 39 { 40 return CacheableIdentifier(i); 41 } 42 43 inline CacheableIdentifier CacheableIdentifier::createFromCell(JSCell* i) 44 { 45 return CacheableIdentifier(i); 46 } 37 47 38 48 inline CacheableIdentifier::CacheableIdentifier(const Identifier& identifier)
Note: See TracChangeset
for help on using the changeset viewer.