Diff
Modified: branches/safari-613-branch/JSTests/ChangeLog (292996 => 292997)
--- branches/safari-613-branch/JSTests/ChangeLog 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/JSTests/ChangeLog 2022-04-19 00:50:09 UTC (rev 292997)
@@ -1,5 +1,71 @@
2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r292594. rdar://problem/91467003
+
+ [JSC] Fire structure transition watchpoint in Structure::finishCreation instead of Structure constructor
+ https://bugs.webkit.org/show_bug.cgi?id=238980
+
+ Reviewed by Saam Barati.
+
+ JSTests:
+
+ * stress/heap-allocation-in-did-structure-transition-watchpoint.js: Added.
+ (__isPropertyOfType):
+ (__getProperties):
+ (__getObjects):
+ (__getRandomObject):
+ (__getRandomProperty):
+ (__callGC):
+ (get var):
+
+ Source/_javascript_Core:
+
+ After https://github.com/WebKit/WebKit/commit/dc3a347474a183891f8e07966dc09e684d7a1d13 change,
+ we start using Structure::get in the main thread. However one of the difference between Structure::get and
+ Structure::getConcurrently is that it can allocate GC memory: PropertyTable can be materialized.
+
+ Structure constructor was firing structure transition watchpoint. And some of watchpoints were using
+ Structure::getConcurrently. That's fine before, but now, it becomes Structure::get. It is not OK since
+ we cannot allocate GC memory inside constructor of GC managed objects.
+
+ This patch split didTransitionFromThisStructure into didTransitionFromThisStructureWithoutFiringWatchpoint and
+ fireStructureTransitionWatchpoint. And firing watchpoints in Structure::finishCreation instead of Structure
+ constructor so that we can allocate GC memory while firing watchpoints.
+
+ * runtime/BrandedStructure.cpp:
+ (JSC::BrandedStructure::BrandedStructure):
+ (JSC::BrandedStructure::create):
+ * runtime/BrandedStructure.h:
+ * runtime/Structure.cpp:
+ (JSC::Structure::Structure):
+ (JSC::Structure::didTransitionFromThisStructureWithoutFiringWatchpoint const):
+ (JSC::Structure::fireStructureTransitionWatchpoint const):
+ (JSC::Structure::didTransitionFromThisStructure const):
+ * runtime/Structure.h:
+ (JSC::Structure::finishCreation):
+ * runtime/StructureInlines.h:
+ (JSC::Structure::create):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292594 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-04-07 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] Fire structure transition watchpoint in Structure::finishCreation instead of Structure constructor
+ https://bugs.webkit.org/show_bug.cgi?id=238980
+
+ Reviewed by Saam Barati.
+
+ * stress/heap-allocation-in-did-structure-transition-watchpoint.js: Added.
+ (__isPropertyOfType):
+ (__getProperties):
+ (__getObjects):
+ (__getRandomObject):
+ (__getRandomProperty):
+ (__callGC):
+ (get var):
+
+2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r292484. rdar://problem/89253391
[JSC] Substring resolving should check 8bit / 16bit again
Added: branches/safari-613-branch/JSTests/stress/heap-allocation-in-did-structure-transition-watchpoint.js (0 => 292997)
--- branches/safari-613-branch/JSTests/stress/heap-allocation-in-did-structure-transition-watchpoint.js (rev 0)
+++ branches/safari-613-branch/JSTests/stress/heap-allocation-in-did-structure-transition-watchpoint.js 2022-04-19 00:50:09 UTC (rev 292997)
@@ -0,0 +1,33 @@
+function __isPropertyOfType() {
+}
+function __getProperties(obj) {
+ let properties = [];
+ for (let name of Object.getOwnPropertyNames(obj)) {
+ properties.push();
+ }
+}
+function* __getObjects() {
+ let obj_names = __getProperties( 'object');
+}
+function __getRandomObject() {
+ for (let obj of __getObjects()) {
+ }
+}
+function __getRandomProperty() {
+}
+(function () {
+ __callGC = function () {
+ gc();
+ };
+})();
+ Array.prototype[2] = undefined;
+__getRandomObject(), {
+ get: function () {
+ }
+ };
+ var __v_18 = Function();
+ delete __v_18[__getRandomProperty()], __callGC();
+ for (var __v_26 = 0; __v_26 < 10; ++__v_26) {
+ Object.defineProperty(Array.prototype, __v_26, {
+ });
+ }
\ No newline at end of file
Modified: branches/safari-613-branch/Source/_javascript_Core/ChangeLog (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/ChangeLog 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/ChangeLog 2022-04-19 00:50:09 UTC (rev 292997)
@@ -1,5 +1,88 @@
2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r292594. rdar://problem/91467003
+
+ [JSC] Fire structure transition watchpoint in Structure::finishCreation instead of Structure constructor
+ https://bugs.webkit.org/show_bug.cgi?id=238980
+
+ Reviewed by Saam Barati.
+
+ JSTests:
+
+ * stress/heap-allocation-in-did-structure-transition-watchpoint.js: Added.
+ (__isPropertyOfType):
+ (__getProperties):
+ (__getObjects):
+ (__getRandomObject):
+ (__getRandomProperty):
+ (__callGC):
+ (get var):
+
+ Source/_javascript_Core:
+
+ After https://github.com/WebKit/WebKit/commit/dc3a347474a183891f8e07966dc09e684d7a1d13 change,
+ we start using Structure::get in the main thread. However one of the difference between Structure::get and
+ Structure::getConcurrently is that it can allocate GC memory: PropertyTable can be materialized.
+
+ Structure constructor was firing structure transition watchpoint. And some of watchpoints were using
+ Structure::getConcurrently. That's fine before, but now, it becomes Structure::get. It is not OK since
+ we cannot allocate GC memory inside constructor of GC managed objects.
+
+ This patch split didTransitionFromThisStructure into didTransitionFromThisStructureWithoutFiringWatchpoint and
+ fireStructureTransitionWatchpoint. And firing watchpoints in Structure::finishCreation instead of Structure
+ constructor so that we can allocate GC memory while firing watchpoints.
+
+ * runtime/BrandedStructure.cpp:
+ (JSC::BrandedStructure::BrandedStructure):
+ (JSC::BrandedStructure::create):
+ * runtime/BrandedStructure.h:
+ * runtime/Structure.cpp:
+ (JSC::Structure::Structure):
+ (JSC::Structure::didTransitionFromThisStructureWithoutFiringWatchpoint const):
+ (JSC::Structure::fireStructureTransitionWatchpoint const):
+ (JSC::Structure::didTransitionFromThisStructure const):
+ * runtime/Structure.h:
+ (JSC::Structure::finishCreation):
+ * runtime/StructureInlines.h:
+ (JSC::Structure::create):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292594 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-04-07 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] Fire structure transition watchpoint in Structure::finishCreation instead of Structure constructor
+ https://bugs.webkit.org/show_bug.cgi?id=238980
+
+ Reviewed by Saam Barati.
+
+ After https://github.com/WebKit/WebKit/commit/dc3a347474a183891f8e07966dc09e684d7a1d13 change,
+ we start using Structure::get in the main thread. However one of the difference between Structure::get and
+ Structure::getConcurrently is that it can allocate GC memory: PropertyTable can be materialized.
+
+ Structure constructor was firing structure transition watchpoint. And some of watchpoints were using
+ Structure::getConcurrently. That's fine before, but now, it becomes Structure::get. It is not OK since
+ we cannot allocate GC memory inside constructor of GC managed objects.
+
+ This patch split didTransitionFromThisStructure into didTransitionFromThisStructureWithoutFiringWatchpoint and
+ fireStructureTransitionWatchpoint. And firing watchpoints in Structure::finishCreation instead of Structure
+ constructor so that we can allocate GC memory while firing watchpoints.
+
+ * runtime/BrandedStructure.cpp:
+ (JSC::BrandedStructure::BrandedStructure):
+ (JSC::BrandedStructure::create):
+ * runtime/BrandedStructure.h:
+ * runtime/Structure.cpp:
+ (JSC::Structure::Structure):
+ (JSC::Structure::didTransitionFromThisStructureWithoutFiringWatchpoint const):
+ (JSC::Structure::fireStructureTransitionWatchpoint const):
+ (JSC::Structure::didTransitionFromThisStructure const):
+ * runtime/Structure.h:
+ (JSC::Structure::finishCreation):
+ * runtime/StructureInlines.h:
+ (JSC::Structure::create):
+
+2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r292484. rdar://problem/89253391
[JSC] Substring resolving should check 8bit / 16bit again
Modified: branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.cpp (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.cpp 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.cpp 2022-04-19 00:50:09 UTC (rev 292997)
@@ -31,8 +31,8 @@
namespace JSC {
-BrandedStructure::BrandedStructure(VM& vm, Structure* previous, UniquedStringImpl* brandUid, DeferredStructureTransitionWatchpointFire* deferred)
- : Structure(vm, previous, deferred)
+BrandedStructure::BrandedStructure(VM& vm, Structure* previous, UniquedStringImpl* brandUid)
+ : Structure(vm, previous)
, m_brand(brandUid)
{
if (previous->isBrandedStructure())
@@ -40,8 +40,8 @@
this->setIsBrandedStructure(true);
}
-BrandedStructure::BrandedStructure(VM& vm, BrandedStructure* previous, DeferredStructureTransitionWatchpointFire* deferred)
- : Structure(vm, previous, deferred)
+BrandedStructure::BrandedStructure(VM& vm, BrandedStructure* previous)
+ : Structure(vm, previous)
, m_brand(previous->m_brand)
, m_parentBrand(vm, this, previous->m_parentBrand.get(), WriteBarrier<BrandedStructure>::MayBeNull)
{
@@ -51,8 +51,8 @@
Structure* BrandedStructure::create(VM& vm, Structure* previous, UniquedStringImpl* brandUid, DeferredStructureTransitionWatchpointFire* deferred)
{
ASSERT(vm.structureStructure);
- BrandedStructure* newStructure = new (NotNull, allocateCell<BrandedStructure>(vm)) BrandedStructure(vm, previous, brandUid, deferred);
- newStructure->finishCreation(vm, previous);
+ BrandedStructure* newStructure = new (NotNull, allocateCell<BrandedStructure>(vm)) BrandedStructure(vm, previous, brandUid);
+ newStructure->finishCreation(vm, previous, deferred);
ASSERT(newStructure->type() == StructureType);
return newStructure;
}
Modified: branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.h (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.h 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/runtime/BrandedStructure.h 2022-04-19 00:50:09 UTC (rev 292997)
@@ -68,8 +68,8 @@
}
private:
- BrandedStructure(VM&, Structure*, UniquedStringImpl* brand, DeferredStructureTransitionWatchpointFire*);
- BrandedStructure(VM&, BrandedStructure*, DeferredStructureTransitionWatchpointFire*);
+ BrandedStructure(VM&, Structure*, UniquedStringImpl* brand);
+ BrandedStructure(VM&, BrandedStructure*);
static Structure* create(VM&, Structure*, UniquedStringImpl* brand, DeferredStructureTransitionWatchpointFire* = nullptr);
Modified: branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.cpp (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.cpp 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.cpp 2022-04-19 00:50:09 UTC (rev 292997)
@@ -272,7 +272,7 @@
ASSERT(!this->typeInfo().overridesGetCallData() || m_classInfo->methodTable.getCallData != &JSCell::getCallData);
}
-Structure::Structure(VM& vm, Structure* previous, DeferredStructureTransitionWatchpointFire* deferred)
+Structure::Structure(VM& vm, Structure* previous)
: JSCell(vm, vm.structureStructure.get())
, m_inlineCapacity(previous->m_inlineCapacity)
, m_bitField(0)
@@ -307,7 +307,9 @@
ASSERT(!previous->typeInfo().structureIsImmortal());
setPreviousID(vm, previous);
- previous->didTransitionFromThisStructure(deferred);
+ // Do not fire watchpoint inside Structure constructor since watchpoint can involve further heap allocations.
+ // We fire watchpoint separately in Structure::finishCreation.
+ previous->didTransitionFromThisStructureWithoutFiringWatchpoint();
// Copy this bit now, in case previous was being watched.
setTransitionWatchpointIsLikelyToBeFired(previous->transitionWatchpointIsLikelyToBeFired());
@@ -1244,7 +1246,7 @@
out.print("Structure transition from ", *m_structure);
}
-void Structure::didTransitionFromThisStructure(DeferredStructureTransitionWatchpointFire* deferred) const
+void Structure::didTransitionFromThisStructureWithoutFiringWatchpoint() const
{
// If the structure is being watched, and this is the kind of structure that the DFG would
// like to watch, then make sure to note for all future versions of this structure that it's
@@ -1251,7 +1253,10 @@
// unwise to watch it.
if (m_transitionWatchpointSet.isBeingWatched())
const_cast<Structure*>(this)->setTransitionWatchpointIsLikelyToBeFired(true);
+}
+void Structure::fireStructureTransitionWatchpoint(DeferredStructureTransitionWatchpointFire* deferred) const
+{
if (deferred) {
ASSERT(deferred->structure() == this);
m_transitionWatchpointSet.fireAll(vm(), deferred);
@@ -1259,6 +1264,12 @@
m_transitionWatchpointSet.fireAll(vm(), StructureFireDetail(this));
}
+void Structure::didTransitionFromThisStructure(DeferredStructureTransitionWatchpointFire* deferred) const
+{
+ didTransitionFromThisStructureWithoutFiringWatchpoint();
+ fireStructureTransitionWatchpoint(deferred);
+}
+
template<typename Visitor>
void Structure::visitChildrenImpl(JSCell* cell, Visitor& visitor)
{
Modified: branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.h (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.h 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/runtime/Structure.h 2022-04-19 00:50:09 UTC (rev 292997)
@@ -135,7 +135,7 @@
JS_EXPORT_PRIVATE static bool isValidPrototype(JSValue);
protected:
- void finishCreation(VM& vm, const Structure* previous)
+ void finishCreation(VM& vm, const Structure* previous, DeferredStructureTransitionWatchpointFire* deferred)
{
this->finishCreation(vm);
if (previous->hasRareData()) {
@@ -145,6 +145,7 @@
rareData()->setSharedPolyProtoWatchpoint(previousRareData->copySharedPolyProtoWatchpoint());
}
}
+ previous->fireStructureTransitionWatchpoint(deferred);
}
private:
@@ -651,6 +652,10 @@
m_transitionWatchpointSet.add(watchpoint);
}
+ void didTransitionFromThisStructureWithoutFiringWatchpoint() const;
+ void fireStructureTransitionWatchpoint(DeferredStructureTransitionWatchpointFire*) const;
+
+ // This function does the both didTransitionFromThisStructureWithoutFiringWatchpoint and fireStructureTransitionWatchpoint.
void didTransitionFromThisStructure(DeferredStructureTransitionWatchpointFire* = nullptr) const;
InlineWatchpointSet& transitionWatchpointSet() const
@@ -741,7 +746,7 @@
static_assert(s_bitWidthOfTransitionKind <= sizeof(TransitionKind) * 8);
protected:
- Structure(VM&, Structure*, DeferredStructureTransitionWatchpointFire*);
+ Structure(VM&, Structure*);
private:
friend class LLIntOffsetsExtractor;
Modified: branches/safari-613-branch/Source/_javascript_Core/runtime/StructureInlines.h (292996 => 292997)
--- branches/safari-613-branch/Source/_javascript_Core/runtime/StructureInlines.h 2022-04-19 00:50:05 UTC (rev 292996)
+++ branches/safari-613-branch/Source/_javascript_Core/runtime/StructureInlines.h 2022-04-19 00:50:09 UTC (rev 292997)
@@ -79,11 +79,10 @@
ASSERT(vm.structureStructure);
Structure* newStructure;
if (previous->isBrandedStructure())
- newStructure = new (NotNull, allocateCell<BrandedStructure>(vm)) BrandedStructure(vm, jsCast<BrandedStructure*>(previous), deferred);
+ newStructure = new (NotNull, allocateCell<BrandedStructure>(vm)) BrandedStructure(vm, jsCast<BrandedStructure*>(previous));
else
- newStructure = new (NotNull, allocateCell<Structure>(vm)) Structure(vm, previous, deferred);
-
- newStructure->finishCreation(vm, previous);
+ newStructure = new (NotNull, allocateCell<Structure>(vm)) Structure(vm, previous);
+ newStructure->finishCreation(vm, previous, deferred);
return newStructure;
}
