- Revision
- 261668
- Author
- [email protected]
- Date
- 2020-05-13 19:15:33 -0700 (Wed, 13 May 2020)
Log Message
JSDOMWindowBase m_windowCloseWatchpoints must be Ref<>
https://bugs.webkit.org/show_bug.cgi?id=211844
Reviewed by Mark Lam.
Source/_javascript_Core:
* bytecode/Watchpoint.cpp:
(JSC::InlineWatchpointSet::inflateSlow):
* bytecode/Watchpoint.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::JSGlobalObject):
* runtime/Structure.cpp:
(JSC::Structure::ensurePropertyReplacementWatchpointSet):
* runtime/SymbolTable.cpp:
(JSC::SymbolTableEntry::prepareToWatch):
* runtime/VM.cpp:
(JSC::VM::ensureWatchpointSetForImpureProperty):
Source/WebCore:
JSDOMWindowBase::m_windowCloseWatchpoints is WatchpointSet, not Ref<WatchpointSet>. And it is passed to JSC IC layer via PropertySlot::setWatchpoint(...).
And ProxyableAccessCase can hold it as `RefPtr<WatchpointSet> m_additionalSet;`, this is wrong.
This patch hides WatchpointSet constructor behind `protected` to disallow non Ref<> WatchpointSet construction. We made it `protected` since InferredValueWatchpointSet
inherits WatchpointSet and uses this constructor.
Possibly, this does not matter: ProxyableAccessCase keeps Structure, which points to JSDOMWindowBase. So, it would not happen that ProxyableAccessCase has a wrong pointer
to WatchpointSet since JSDOMWindowBase is kept alive anyway. But avoid using RefCounted objects without RefCount allocation is better since this can cause bugs easily.
* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::JSDOMWindowBase):
(WebCore::JSDOMWindowBase::fireFrameClearedWatchpointsForWindow):
* bindings/js/JSDOMWindowBase.h:
* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlot):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (261667 => 261668)
--- trunk/Source/_javascript_Core/ChangeLog 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-05-14 02:15:33 UTC (rev 261668)
@@ -1,3 +1,22 @@
+2020-05-13 Yusuke Suzuki <[email protected]>
+
+ JSDOMWindowBase m_windowCloseWatchpoints must be Ref<>
+ https://bugs.webkit.org/show_bug.cgi?id=211844
+
+ Reviewed by Mark Lam.
+
+ * bytecode/Watchpoint.cpp:
+ (JSC::InlineWatchpointSet::inflateSlow):
+ * bytecode/Watchpoint.h:
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::JSGlobalObject):
+ * runtime/Structure.cpp:
+ (JSC::Structure::ensurePropertyReplacementWatchpointSet):
+ * runtime/SymbolTable.cpp:
+ (JSC::SymbolTableEntry::prepareToWatch):
+ * runtime/VM.cpp:
+ (JSC::VM::ensureWatchpointSetForImpureProperty):
+
2020-05-13 Caio Lima <[email protected]>
Making 32-bits JIT build without Unified Build system
Modified: trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp (261667 => 261668)
--- trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -183,7 +183,7 @@
{
ASSERT(isThin());
ASSERT(!isCompilationThread());
- WatchpointSet* fat = adoptRef(new WatchpointSet(decodeState(m_data))).leakRef();
+ WatchpointSet* fat = &WatchpointSet::create(decodeState(m_data)).leakRef();
WTF::storeStoreFence();
m_data = bitwise_cast<uintptr_t>(fat);
return fat;
Modified: trunk/Source/_javascript_Core/bytecode/Watchpoint.h (261667 => 261668)
--- trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2020-05-14 02:15:33 UTC (rev 261668)
@@ -186,8 +186,6 @@
friend class LLIntOffsetsExtractor;
friend class DeferredWatchpointFire;
public:
- JS_EXPORT_PRIVATE WatchpointSet(WatchpointState);
-
// FIXME: In many cases, it would be amazing if this *did* fire the watchpoints. I suspect that
// this might be hard to get right, but still, it might be awesome.
JS_EXPORT_PRIVATE ~WatchpointSet(); // Note that this will not fire any of the watchpoints; if you need to know when a WatchpointSet dies then you need a separate mechanism for this.
@@ -295,6 +293,9 @@
JS_EXPORT_PRIVATE void fireAllSlow(VM&, DeferredWatchpointFire* deferredWatchpoints); // Ditto.
JS_EXPORT_PRIVATE void fireAllSlow(VM&, const char* reason); // Ditto.
+protected:
+ JS_EXPORT_PRIVATE WatchpointSet(WatchpointState);
+
private:
void fireAllWatchpoints(VM&, const FireDetail&);
void take(WatchpointSet* other);
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (261667 => 261668)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -430,9 +430,9 @@
: Base(vm, structure, nullptr)
, m_vm(&vm)
, m_linkTimeConstants(numberOfLinkTimeConstants)
- , m_masqueradesAsUndefinedWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
- , m_havingABadTimeWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
- , m_varInjectionWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
+ , m_masqueradesAsUndefinedWatchpoint(WatchpointSet::create(IsWatched))
+ , m_havingABadTimeWatchpoint(WatchpointSet::create(IsWatched))
+ , m_varInjectionWatchpoint(WatchpointSet::create(IsWatched))
, m_weakRandom(Options::forceWeakRandomSeed() ? Options::forcedWeakRandomSeed() : static_cast<unsigned>(randomNumber() * (std::numeric_limits<unsigned>::max() + 1.0)))
, m_arrayIteratorProtocolWatchpointSet(IsWatched)
, m_mapIteratorProtocolWatchpointSet(IsWatched)
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (261667 => 261668)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -953,7 +953,7 @@
}
auto result = rareData->m_replacementWatchpointSets->add(offset, nullptr);
if (result.isNewEntry)
- result.iterator->value = adoptRef(new WatchpointSet(IsWatched));
+ result.iterator->value = WatchpointSet::create(IsWatched);
return result.iterator->value.get();
}
Modified: trunk/Source/_javascript_Core/runtime/SymbolTable.cpp (261667 => 261668)
--- trunk/Source/_javascript_Core/runtime/SymbolTable.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/runtime/SymbolTable.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -69,7 +69,7 @@
FatEntry* entry = inflate();
if (entry->m_watchpoints)
return;
- entry->m_watchpoints = adoptRef(new WatchpointSet(ClearWatchpoint));
+ entry->m_watchpoints = WatchpointSet::create(ClearWatchpoint);
}
SymbolTableEntry::FatEntry* SymbolTableEntry::inflateSlow()
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (261667 => 261668)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -1163,7 +1163,7 @@
{
auto result = m_impurePropertyWatchpointSets.add(propertyName, nullptr);
if (result.isNewEntry)
- result.iterator->value = adoptRef(new WatchpointSet(IsWatched));
+ result.iterator->value = WatchpointSet::create(IsWatched);
return result.iterator->value.get();
}
Modified: trunk/Source/WebCore/ChangeLog (261667 => 261668)
--- trunk/Source/WebCore/ChangeLog 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/WebCore/ChangeLog 2020-05-14 02:15:33 UTC (rev 261668)
@@ -1,3 +1,26 @@
+2020-05-13 Yusuke Suzuki <[email protected]>
+
+ JSDOMWindowBase m_windowCloseWatchpoints must be Ref<>
+ https://bugs.webkit.org/show_bug.cgi?id=211844
+
+ Reviewed by Mark Lam.
+
+ JSDOMWindowBase::m_windowCloseWatchpoints is WatchpointSet, not Ref<WatchpointSet>. And it is passed to JSC IC layer via PropertySlot::setWatchpoint(...).
+ And ProxyableAccessCase can hold it as `RefPtr<WatchpointSet> m_additionalSet;`, this is wrong.
+
+ This patch hides WatchpointSet constructor behind `protected` to disallow non Ref<> WatchpointSet construction. We made it `protected` since InferredValueWatchpointSet
+ inherits WatchpointSet and uses this constructor.
+
+ Possibly, this does not matter: ProxyableAccessCase keeps Structure, which points to JSDOMWindowBase. So, it would not happen that ProxyableAccessCase has a wrong pointer
+ to WatchpointSet since JSDOMWindowBase is kept alive anyway. But avoid using RefCounted objects without RefCount allocation is better since this can cause bugs easily.
+
+ * bindings/js/JSDOMWindowBase.cpp:
+ (WebCore::JSDOMWindowBase::JSDOMWindowBase):
+ (WebCore::JSDOMWindowBase::fireFrameClearedWatchpointsForWindow):
+ * bindings/js/JSDOMWindowBase.h:
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::getOwnPropertySlot):
+
2020-05-13 Jack Lee <[email protected]>
Nullptr crash in InsertParagraphSeparatorCommand::doApply when the canonical position is uneditable
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp (261667 => 261668)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -90,7 +90,7 @@
JSDOMWindowBase::JSDOMWindowBase(VM& vm, Structure* structure, RefPtr<DOMWindow>&& window, JSWindowProxy* proxy)
: JSDOMGlobalObject(vm, structure, proxy->world(), &s_globalObjectMethodTable)
- , m_windowCloseWatchpoints((window && window->frame()) ? IsWatched : IsInvalidated)
+ , m_windowCloseWatchpoints(WatchpointSet::create((window && window->frame()) ? IsWatched : IsInvalidated))
, m_wrapped(WTFMove(window))
, m_proxy(proxy)
{
@@ -296,7 +296,7 @@
if (!wrapper)
continue;
JSDOMWindowBase* jsWindow = JSC::jsCast<JSDOMWindowBase*>(wrapper);
- jsWindow->m_windowCloseWatchpoints.fireAll(vm, "Frame cleared");
+ jsWindow->m_windowCloseWatchpoints->fireAll(vm, "Frame cleared");
}
}
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.h (261667 => 261668)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.h 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.h 2020-05-14 02:15:33 UTC (rev 261668)
@@ -89,7 +89,7 @@
JSDOMWindowBase(JSC::VM&, JSC::Structure*, RefPtr<DOMWindow>&&, JSWindowProxy*);
void finishCreation(JSC::VM&, JSWindowProxy*);
- JSC::WatchpointSet m_windowCloseWatchpoints;
+ Ref<JSC::WatchpointSet> m_windowCloseWatchpoints;
private:
using ResponseCallback = WTF::Function<void(const char*, size_t)>;
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (261667 => 261668)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2020-05-14 01:09:19 UTC (rev 261667)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2020-05-14 02:15:33 UTC (rev 261668)
@@ -194,7 +194,7 @@
// FIXME: this needs more explanation.
// (Particularly, is it correct that this exists here but not in getOwnPropertySlotByIndex?)
- slot.setWatchpointSet(thisObject->m_windowCloseWatchpoints);
+ slot.setWatchpointSet(thisObject->m_windowCloseWatchpoints.get());
// (2) Regular own properties.
PropertySlot slotCopy = slot;
