Title: [220012] trunk
- Revision
- 220012
- Author
- [email protected]
- Date
- 2017-07-28 13:29:09 -0700 (Fri, 28 Jul 2017)
Log Message
ObjectToStringAdaptiveStructureWatchpoint should not fire if it's dying imminently.
https://bugs.webkit.org/show_bug.cgi?id=174948
<rdar://problem/33495680>
Reviewed by Filip Pizlo.
JSTests:
* stress/regress-174948.js: Added.
Source/_javascript_Core:
ObjectToStringAdaptiveStructureWatchpoint is owned by StructureRareData. If its
owner StructureRareData is already known to be dead (in terms of GC liveness) but
hasn't been destructed yet (i.e. not swept by the GC yet), we should ignore all
requests to fire this watchpoint.
If the GC had the chance to sweep the StructureRareData, thereby destructing the
ObjectToStringAdaptiveStructureWatchpoint, it (the watchpoint) would have removed
itself from the WatchpointSet it was on. Hence, it would not have been fired.
But since the watchpoint hasn't been destructed yet, it still remains on the
WatchpointSet and needs to guard against being fired in this state. The fix is
to simply return early if its owner StructureRareData is not live. This has the
effect of the watchpoint fire being a no-op, which is equivalent to the watchpoint
not firing as we would expect.
This patch also removes some cargo cult copying of watchpoint code which
instantiates a StringFireDetail. In a few cases, that StringFireDetail is never
used. This patch removes these unnecessary instantiations.
* bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
* runtime/StructureRareData.cpp:
(JSC::ObjectToStringAdaptiveStructureWatchpoint::fireInternal):
(JSC::ObjectToStringAdaptiveInferredPropertyValueWatchpoint::handleFire):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (220011 => 220012)
--- trunk/JSTests/ChangeLog 2017-07-28 20:03:42 UTC (rev 220011)
+++ trunk/JSTests/ChangeLog 2017-07-28 20:29:09 UTC (rev 220012)
@@ -1,3 +1,13 @@
+2017-07-28 Mark Lam <[email protected]>
+
+ ObjectToStringAdaptiveStructureWatchpoint should not fire if it's dying imminently.
+ https://bugs.webkit.org/show_bug.cgi?id=174948
+ <rdar://problem/33495680>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/regress-174948.js: Added.
+
2017-07-28 Yusuke Suzuki <[email protected]>
ASSERTION FAILED: candidate->op() == PhantomCreateRest || candidate->op() == PhantomDirectArguments || candidate->op() == PhantomClonedArguments || candidate->op() == PhantomSpread || candidate->op() == PhantomNewArrayWithSpread
Added: trunk/JSTests/stress/regress-174948.js (0 => 220012)
--- trunk/JSTests/stress/regress-174948.js (rev 0)
+++ trunk/JSTests/stress/regress-174948.js 2017-07-28 20:29:09 UTC (rev 220012)
@@ -0,0 +1,5 @@
++new function() {};
+new function() {};
+edenGC();
+(function() {}).prototype[0] = 0;
+
Modified: trunk/Source/_javascript_Core/ChangeLog (220011 => 220012)
--- trunk/Source/_javascript_Core/ChangeLog 2017-07-28 20:03:42 UTC (rev 220011)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-07-28 20:29:09 UTC (rev 220012)
@@ -1,3 +1,36 @@
+2017-07-28 Mark Lam <[email protected]>
+
+ ObjectToStringAdaptiveStructureWatchpoint should not fire if it's dying imminently.
+ https://bugs.webkit.org/show_bug.cgi?id=174948
+ <rdar://problem/33495680>
+
+ Reviewed by Filip Pizlo.
+
+ ObjectToStringAdaptiveStructureWatchpoint is owned by StructureRareData. If its
+ owner StructureRareData is already known to be dead (in terms of GC liveness) but
+ hasn't been destructed yet (i.e. not swept by the GC yet), we should ignore all
+ requests to fire this watchpoint.
+
+ If the GC had the chance to sweep the StructureRareData, thereby destructing the
+ ObjectToStringAdaptiveStructureWatchpoint, it (the watchpoint) would have removed
+ itself from the WatchpointSet it was on. Hence, it would not have been fired.
+
+ But since the watchpoint hasn't been destructed yet, it still remains on the
+ WatchpointSet and needs to guard against being fired in this state. The fix is
+ to simply return early if its owner StructureRareData is not live. This has the
+ effect of the watchpoint fire being a no-op, which is equivalent to the watchpoint
+ not firing as we would expect.
+
+ This patch also removes some cargo cult copying of watchpoint code which
+ instantiates a StringFireDetail. In a few cases, that StringFireDetail is never
+ used. This patch removes these unnecessary instantiations.
+
+ * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
+ (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
+ * runtime/StructureRareData.cpp:
+ (JSC::ObjectToStringAdaptiveStructureWatchpoint::fireInternal):
+ (JSC::ObjectToStringAdaptiveInferredPropertyValueWatchpoint::handleFire):
+
2017-07-28 Yusuke Suzuki <[email protected]>
ASSERTION FAILED: candidate->op() == PhantomCreateRest || candidate->op() == PhantomDirectArguments || candidate->op() == PhantomClonedArguments || candidate->op() == PhantomSpread || candidate->op() == PhantomNewArrayWithSpread
Modified: trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp (220011 => 220012)
--- trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp 2017-07-28 20:03:42 UTC (rev 220011)
+++ trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp 2017-07-28 20:29:09 UTC (rev 220012)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -47,7 +47,7 @@
m_key.object()->structure()->addTransitionWatchpoint(this);
}
-void LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal(const FireDetail& detail)
+void LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal(const FireDetail&)
{
if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
install();
@@ -54,11 +54,6 @@
return;
}
- StringPrintStream out;
- out.print("ObjectToStringValue Adaptation of ", m_key, " failed: ", detail);
-
- StringFireDetail stringDetail(out.toCString().data());
-
CodeBlock::clearLLIntGetByIdCache(m_getByIdInstruction);
}
Modified: trunk/Source/_javascript_Core/runtime/StructureRareData.cpp (220011 => 220012)
--- trunk/Source/_javascript_Core/runtime/StructureRareData.cpp 2017-07-28 20:03:42 UTC (rev 220011)
+++ trunk/Source/_javascript_Core/runtime/StructureRareData.cpp 2017-07-28 20:29:09 UTC (rev 220012)
@@ -192,18 +192,16 @@
m_key.object()->structure()->addTransitionWatchpoint(this);
}
-void ObjectToStringAdaptiveStructureWatchpoint::fireInternal(const FireDetail& detail)
+void ObjectToStringAdaptiveStructureWatchpoint::fireInternal(const FireDetail&)
{
+ if (!m_structureRareData->isLive())
+ return;
+
if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
install();
return;
}
- StringPrintStream out;
- out.print("ObjectToStringValue Adaptation of ", m_key, " failed: ", detail);
-
- StringFireDetail stringDetail(out.toCString().data());
-
m_structureRareData->clearObjectToStringValue();
}
@@ -218,13 +216,8 @@
return m_structureRareData->isLive();
}
-void ObjectToStringAdaptiveInferredPropertyValueWatchpoint::handleFire(const FireDetail& detail)
+void ObjectToStringAdaptiveInferredPropertyValueWatchpoint::handleFire(const FireDetail&)
{
- StringPrintStream out;
- out.print("Adaptation of ", key(), " failed: ", detail);
-
- StringFireDetail stringDetail(out.toCString().data());
-
m_structureRareData->clearObjectToStringValue();
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
