Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (279812 => 279813)
--- trunk/Source/_javascript_Core/ChangeLog 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-07-11 01:27:40 UTC (rev 279813)
@@ -1,3 +1,47 @@
+2021-07-10 Saam Barati <[email protected]>
+
+ Continue to consult InlineAccess's Structure even after switching to a stub IC
+ https://bugs.webkit.org/show_bug.cgi?id=227785
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch fixes a crash in: stress/class-subclassing-function.js
+
+ The bug is this:
+ 1. We initialize a StructureStubInfo to be an inline self access doing a load based on structure S.
+ 2. We transition to being a PolymorphicAccess based StructureStubInfo. But, we haven't
+ generated code yet. We're in the buffered state. So we are still running the inline access
+ from (1). But the StructureStubInfo thinks it's a "Stub".
+ 3. S is collected
+ 4. We continue to run code from (1), because when we finalize the IC during GC, it
+ doesn't think it's an inline access.
+
+ The fix is to always track the structure S that we used when generating the inline
+ access, and to only stop tracking it once we've generated code for the Stub.
+
+ * bytecode/AccessCase.cpp:
+ (JSC::AccessCase::fromStructureStubInfo):
+ (JSC::AccessCase::propagateTransitions const):
+ * bytecode/AccessCase.h:
+ * bytecode/GetByStatus.cpp:
+ (JSC::GetByStatus::computeForStubInfoWithoutExitSiteFeedback):
+ * bytecode/InByStatus.cpp:
+ (JSC::InByStatus::computeForStubInfoWithoutExitSiteFeedback):
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::PolymorphicAccess::propagateTransitions const):
+ * bytecode/PolymorphicAccess.h:
+ * bytecode/PutByIdStatus.cpp:
+ (JSC::PutByIdStatus::computeForStubInfo):
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::initGetByIdSelf):
+ (JSC::StructureStubInfo::initPutByIdReplace):
+ (JSC::StructureStubInfo::initInByIdSelf):
+ (JSC::StructureStubInfo::addAccessCase):
+ (JSC::StructureStubInfo::reset):
+ (JSC::StructureStubInfo::visitWeakReferences):
+ (JSC::StructureStubInfo::propagateTransitions):
+ * bytecode/StructureStubInfo.h:
+
2021-07-10 Yusuke Suzuki <[email protected]>
[JSC] Workaround test262.report bug by making $ properties enumerable
Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/AccessCase.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -155,15 +155,15 @@
switch (stubInfo.cacheType()) {
case CacheType::GetByIdSelf:
RELEASE_ASSERT(stubInfo.hasConstantIdentifier);
- return ProxyableAccessCase::create(vm, owner, Load, identifier, stubInfo.u.byIdSelf.offset, stubInfo.u.byIdSelf.baseObjectStructure.get());
+ return ProxyableAccessCase::create(vm, owner, Load, identifier, stubInfo.u.byIdSelf.offset, stubInfo.inlineAccessBaseStructure.get());
case CacheType::PutByIdReplace:
RELEASE_ASSERT(stubInfo.hasConstantIdentifier);
- return AccessCase::create(vm, owner, Replace, identifier, stubInfo.u.byIdSelf.offset, stubInfo.u.byIdSelf.baseObjectStructure.get());
+ return AccessCase::create(vm, owner, Replace, identifier, stubInfo.u.byIdSelf.offset, stubInfo.inlineAccessBaseStructure.get());
case CacheType::InByIdSelf:
RELEASE_ASSERT(stubInfo.hasConstantIdentifier);
- return AccessCase::create(vm, owner, InHit, identifier, stubInfo.u.byIdSelf.offset, stubInfo.u.byIdSelf.baseObjectStructure.get());
+ return AccessCase::create(vm, owner, InHit, identifier, stubInfo.u.byIdSelf.offset, stubInfo.inlineAccessBaseStructure.get());
case CacheType::ArrayLength:
RELEASE_ASSERT(stubInfo.hasConstantIdentifier);
@@ -758,16 +758,14 @@
}
template<typename Visitor>
-bool AccessCase::propagateTransitions(Visitor& visitor) const
+void AccessCase::propagateTransitions(Visitor& visitor) const
{
- bool result = true;
-
if (m_structure)
- result &= m_structure->markIfCheap(visitor);
+ m_structure->markIfCheap(visitor);
if (m_polyProtoAccessChain) {
for (StructureID structureID : m_polyProtoAccessChain->chain())
- result &= visitor.vm().getStructure(structureID)->markIfCheap(visitor);
+ visitor.vm().getStructure(structureID)->markIfCheap(visitor);
}
switch (m_type) {
@@ -775,18 +773,14 @@
case Delete:
if (visitor.isMarked(m_structure->previousID()))
visitor.appendUnbarriered(m_structure.get());
- else
- result = false;
break;
default:
break;
}
-
- return result;
}
-template bool AccessCase::propagateTransitions(AbstractSlotVisitor&) const;
-template bool AccessCase::propagateTransitions(SlotVisitor&) const;
+template void AccessCase::propagateTransitions(AbstractSlotVisitor&) const;
+template void AccessCase::propagateTransitions(SlotVisitor&) const;
template<typename Visitor>
Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.h (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/AccessCase.h 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.h 2021-07-11 01:27:40 UTC (rev 279813)
@@ -300,7 +300,7 @@
DECLARE_VISIT_AGGREGATE_WITH_MODIFIER(const);
bool visitWeak(VM&) const;
- template<typename Visitor> bool propagateTransitions(Visitor&) const;
+ template<typename Visitor> void propagateTransitions(Visitor&) const;
// FIXME: This only exists because of how AccessCase puts post-generation things into itself.
// https://bugs.webkit.org/show_bug.cgi?id=156456
Modified: trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -213,7 +213,7 @@
return GetByStatus(NoInformation);
case CacheType::GetByIdSelf: {
- Structure* structure = stubInfo->u.byIdSelf.baseObjectStructure.get();
+ Structure* structure = stubInfo->inlineAccessBaseStructure.get();
if (structure->takesSlowPathInDFGForImpureProperty())
return GetByStatus(JSC::slowVersion(summary), *stubInfo);
CacheableIdentifier identifier = stubInfo->identifier();
Modified: trunk/Source/_javascript_Core/bytecode/InByStatus.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/InByStatus.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/InByStatus.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -139,7 +139,7 @@
return InByStatus(NoInformation);
case CacheType::InByIdSelf: {
- Structure* structure = stubInfo->u.byIdSelf.baseObjectStructure.get();
+ Structure* structure = stubInfo->inlineAccessBaseStructure.get();
if (structure->takesSlowPathInDFGForImpureProperty())
return InByStatus(TakesSlowPath);
CacheableIdentifier identifier = stubInfo->identifier();
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -371,16 +371,14 @@
}
template<typename Visitor>
-bool PolymorphicAccess::propagateTransitions(Visitor& visitor) const
+void PolymorphicAccess::propagateTransitions(Visitor& visitor) const
{
- bool result = true;
for (unsigned i = 0; i < size(); ++i)
- result &= at(i).propagateTransitions(visitor);
- return result;
+ at(i).propagateTransitions(visitor);
}
-template bool PolymorphicAccess::propagateTransitions(AbstractSlotVisitor&) const;
-template bool PolymorphicAccess::propagateTransitions(SlotVisitor&) const;
+template void PolymorphicAccess::propagateTransitions(AbstractSlotVisitor&) const;
+template void PolymorphicAccess::propagateTransitions(SlotVisitor&) const;
template<typename Visitor>
void PolymorphicAccess::visitAggregateImpl(Visitor& visitor)
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2021-07-11 01:27:40 UTC (rev 279813)
@@ -160,7 +160,7 @@
// This returns true if it has marked everything it will ever marked. This can be used as an
// optimization to then avoid calling this method again during the fixpoint.
- template<typename Visitor> bool propagateTransitions(Visitor&) const;
+ template<typename Visitor> void propagateTransitions(Visitor&) const;
void aboutToDie();
Modified: trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/PutByIdStatus.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -147,10 +147,10 @@
case CacheType::PutByIdReplace: {
PropertyOffset offset =
- stubInfo->u.byIdSelf.baseObjectStructure->getConcurrently(uid);
+ stubInfo->inlineAccessBaseStructure->getConcurrently(uid);
if (isValidOffset(offset)) {
return PutByIdVariant::replace(
- stubInfo->u.byIdSelf.baseObjectStructure.get(), offset);
+ stubInfo->inlineAccessBaseStructure.get(), offset);
}
return PutByIdStatus(JSC::slowVersion(summary));
}
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2021-07-11 01:27:40 UTC (rev 279813)
@@ -59,7 +59,7 @@
{
}
-void StructureStubInfo::initGetByIdSelf(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* baseObjectStructure, PropertyOffset offset, CacheableIdentifier identifier)
+void StructureStubInfo::initGetByIdSelf(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* inlineAccessBaseStructure, PropertyOffset offset, CacheableIdentifier identifier)
{
ASSERT(hasConstantIdentifier);
setCacheType(locker, CacheType::GetByIdSelf);
@@ -66,8 +66,7 @@
m_identifier = identifier;
codeBlock->vm().heap.writeBarrier(codeBlock);
- u.byIdSelf.baseObjectStructure.set(
- codeBlock->vm(), codeBlock, baseObjectStructure);
+ this->inlineAccessBaseStructure.set(codeBlock->vm(), codeBlock, inlineAccessBaseStructure);
u.byIdSelf.offset = offset;
}
@@ -81,25 +80,23 @@
setCacheType(locker, CacheType::StringLength);
}
-void StructureStubInfo::initPutByIdReplace(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* baseObjectStructure, PropertyOffset offset, CacheableIdentifier identifier)
+void StructureStubInfo::initPutByIdReplace(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* inlineAccessBaseStructure, PropertyOffset offset, CacheableIdentifier identifier)
{
setCacheType(locker, CacheType::PutByIdReplace);
m_identifier = identifier;
codeBlock->vm().heap.writeBarrier(codeBlock);
- u.byIdSelf.baseObjectStructure.set(
- codeBlock->vm(), codeBlock, baseObjectStructure);
+ this->inlineAccessBaseStructure.set(codeBlock->vm(), codeBlock, inlineAccessBaseStructure);
u.byIdSelf.offset = offset;
}
-void StructureStubInfo::initInByIdSelf(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* baseObjectStructure, PropertyOffset offset, CacheableIdentifier identifier)
+void StructureStubInfo::initInByIdSelf(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock, Structure* inlineAccessBaseStructure, PropertyOffset offset, CacheableIdentifier identifier)
{
setCacheType(locker, CacheType::InByIdSelf);
m_identifier = identifier;
codeBlock->vm().heap.writeBarrier(codeBlock);
- u.byIdSelf.baseObjectStructure.set(
- codeBlock->vm(), codeBlock, baseObjectStructure);
+ this->inlineAccessBaseStructure.set(codeBlock->vm(), codeBlock, inlineAccessBaseStructure);
u.byIdSelf.offset = offset;
}
@@ -228,6 +225,15 @@
if (!result.generatedSomeCode())
return result;
+
+ // When we first transition to becoming a Stub, we might still be running the inline
+ // access code. That's because when we first transition to becoming a Stub, we may
+ // be buffered, and we have not yet generated any code. Once the Stub finally generates
+ // code, we're no longer running the inline access code, so we can then clear out
+ // inlineAccessBaseStructure. The reason we don't clear inlineAccessBaseStructure while
+ // we're buffered is because we rely on it to reset during GC if inlineAccessBaseStructure
+ // is collected.
+ inlineAccessBaseStructure.clear();
// If we generated some code then we don't want to attempt to repatch in the future until we
// gather enough cases.
@@ -241,6 +247,7 @@
void StructureStubInfo::reset(const ConcurrentJSLockerBase& locker, CodeBlock* codeBlock)
{
clearBufferedStructures();
+ inlineAccessBaseStructure.clear();
if (m_cacheType == CacheType::Unset)
return;
@@ -346,20 +353,14 @@
});
}
- switch (m_cacheType) {
- case CacheType::GetByIdSelf:
- case CacheType::PutByIdReplace:
- case CacheType::InByIdSelf:
- if (vm.heap.isMarked(u.byIdSelf.baseObjectStructure.get()))
- return;
- break;
- case CacheType::Stub:
- if (u.stub->visitWeak(vm))
- return;
- break;
- default:
+ bool isValid = true;
+ if (inlineAccessBaseStructure)
+ isValid &= vm.heap.isMarked(inlineAccessBaseStructure.get());
+ if (m_cacheType == CacheType::Stub)
+ isValid &= u.stub->visitWeak(vm);
+
+ if (isValid)
return;
- }
reset(locker, codeBlock);
resetByGC = true;
@@ -366,27 +367,17 @@
}
template<typename Visitor>
-bool StructureStubInfo::propagateTransitions(Visitor& visitor)
+void StructureStubInfo::propagateTransitions(Visitor& visitor)
{
- switch (m_cacheType) {
- case CacheType::Unset:
- case CacheType::ArrayLength:
- case CacheType::StringLength:
- return true;
- case CacheType::GetByIdSelf:
- case CacheType::PutByIdReplace:
- case CacheType::InByIdSelf:
- return u.byIdSelf.baseObjectStructure->markIfCheap(visitor);
- case CacheType::Stub:
- return u.stub->propagateTransitions(visitor);
- }
-
- RELEASE_ASSERT_NOT_REACHED();
- return true;
+ if (inlineAccessBaseStructure)
+ inlineAccessBaseStructure->markIfCheap(visitor);
+
+ if (m_cacheType == CacheType::Stub)
+ u.stub->propagateTransitions(visitor);
}
-template bool StructureStubInfo::propagateTransitions(AbstractSlotVisitor&);
-template bool StructureStubInfo::propagateTransitions(SlotVisitor&);
+template void StructureStubInfo::propagateTransitions(AbstractSlotVisitor&);
+template void StructureStubInfo::propagateTransitions(SlotVisitor&);
StubInfoSummary StructureStubInfo::summary(VM& vm) const
{
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (279812 => 279813)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2021-07-11 01:27:31 UTC (rev 279812)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2021-07-11 01:27:40 UTC (rev 279813)
@@ -84,11 +84,11 @@
StructureStubInfo(AccessType, CodeOrigin);
~StructureStubInfo();
- void initGetByIdSelf(const ConcurrentJSLockerBase&, CodeBlock*, Structure* baseObjectStructure, PropertyOffset, CacheableIdentifier);
+ void initGetByIdSelf(const ConcurrentJSLockerBase&, CodeBlock*, Structure* inlineAccessBaseStructure, PropertyOffset, CacheableIdentifier);
void initArrayLength(const ConcurrentJSLockerBase&);
void initStringLength(const ConcurrentJSLockerBase&);
- void initPutByIdReplace(const ConcurrentJSLockerBase&, CodeBlock*, Structure* baseObjectStructure, PropertyOffset, CacheableIdentifier);
- void initInByIdSelf(const ConcurrentJSLockerBase&, CodeBlock*, Structure* baseObjectStructure, PropertyOffset, CacheableIdentifier);
+ void initPutByIdReplace(const ConcurrentJSLockerBase&, CodeBlock*, Structure* inlineAccessBaseStructure, PropertyOffset, CacheableIdentifier);
+ void initInByIdSelf(const ConcurrentJSLockerBase&, CodeBlock*, Structure* inlineAccessBaseStructure, PropertyOffset, CacheableIdentifier);
AccessGenerationResult addAccessCase(const GCSafeConcurrentJSLocker&, JSGlobalObject*, CodeBlock*, ECMAMode, CacheableIdentifier, RefPtr<AccessCase>);
@@ -104,7 +104,7 @@
void visitWeakReferences(const ConcurrentJSLockerBase&, CodeBlock*);
// This returns true if it has marked everything that it will ever mark.
- template<typename Visitor> bool propagateTransitions(Visitor&);
+ template<typename Visitor> void propagateTransitions(Visitor&);
StubInfoSummary summary(VM&) const;
@@ -326,11 +326,11 @@
CodeOrigin codeOrigin;
union {
struct {
- WriteBarrierBase<Structure> baseObjectStructure;
PropertyOffset offset;
} byIdSelf;
PolymorphicAccess* stub;
} u;
+ WriteBarrier<Structure> inlineAccessBaseStructure;
private:
CacheableIdentifier m_identifier;
// Represents those structures that already have buffered AccessCases in the PolymorphicAccess.
