- Revision
- 223161
- Author
- sbar...@apple.com
- Date
- 2017-10-10 17:53:59 -0700 (Tue, 10 Oct 2017)
Log Message
Prototype structure transition should be a deferred transition
https://bugs.webkit.org/show_bug.cgi?id=177734
Reviewed by Keith Miller.
Absence ObjectPropertyConditions work by verifying both that the Structure
does not have a particular property and that its prototype has
remained constant. However, the prototype transition was firing
the transition watchpoint before setting the object's structure.
This meant that isValid for Absence would never return false because
the prototype changed. Clearly this is wrong. The reason this didn't
break OPCs in general is that we'd also check if we could still watch
the OPC. In this case, we can't still watch it because we're inspecting
a structure with an invalidated transition watchpoint. To fix
this weird quirk of the code, I'm making it so that doing a prototype
transition uses the DeferredStructureTransitionWatchpointFire machinery.
This patch also fixes some dead code that I left in regarding
poly proto in OPC.
* bytecode/PropertyCondition.cpp:
(JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
* runtime/JSObject.cpp:
(JSC::JSObject::setPrototypeDirect):
* runtime/Structure.cpp:
(JSC::Structure::changePrototypeTransition):
* runtime/Structure.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (223160 => 223161)
--- trunk/Source/_javascript_Core/ChangeLog 2017-10-11 00:37:49 UTC (rev 223160)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-10-11 00:53:59 UTC (rev 223161)
@@ -1,3 +1,33 @@
+2017-10-10 Saam Barati <sbar...@apple.com>
+
+ Prototype structure transition should be a deferred transition
+ https://bugs.webkit.org/show_bug.cgi?id=177734
+
+ Reviewed by Keith Miller.
+
+ Absence ObjectPropertyConditions work by verifying both that the Structure
+ does not have a particular property and that its prototype has
+ remained constant. However, the prototype transition was firing
+ the transition watchpoint before setting the object's structure.
+ This meant that isValid for Absence would never return false because
+ the prototype changed. Clearly this is wrong. The reason this didn't
+ break OPCs in general is that we'd also check if we could still watch
+ the OPC. In this case, we can't still watch it because we're inspecting
+ a structure with an invalidated transition watchpoint. To fix
+ this weird quirk of the code, I'm making it so that doing a prototype
+ transition uses the DeferredStructureTransitionWatchpointFire machinery.
+
+ This patch also fixes some dead code that I left in regarding
+ poly proto in OPC.
+
+ * bytecode/PropertyCondition.cpp:
+ (JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::setPrototypeDirect):
+ * runtime/Structure.cpp:
+ (JSC::Structure::changePrototypeTransition):
+ * runtime/Structure.h:
+
2017-10-10 Robin Morisset <rmoris...@apple.com>
Avoid allocating useless landingBlocks in DFGByteCodeParser::handleInlining()
Modified: trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp (223160 => 223161)
--- trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp 2017-10-11 00:37:49 UTC (rev 223160)
+++ trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp 2017-10-11 00:53:59 UTC (rev 223161)
@@ -122,15 +122,7 @@
return false;
}
- JSObject* currentPrototype;
- if (structure->hasMonoProto())
- currentPrototype = structure->storedPrototypeObject();
- else {
- RELEASE_ASSERT(base);
- currentPrototype = jsDynamicCast<JSObject*>(*structure->vm(), base->getPrototypeDirect());
- }
-
- if (currentPrototype != prototype()) {
+ if (structure->storedPrototypeObject() != prototype()) {
if (PropertyConditionInternal::verbose) {
dataLog(
"Invalid because the prototype is ", structure->storedPrototype(), " even though "
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (223160 => 223161)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2017-10-11 00:37:49 UTC (rev 223160)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2017-10-11 00:53:59 UTC (rev 223161)
@@ -1634,7 +1634,8 @@
prototype.asCell()->didBecomePrototype();
if (structure(vm)->hasMonoProto()) {
- Structure* newStructure = Structure::changePrototypeTransition(vm, structure(vm), prototype);
+ DeferredStructureTransitionWatchpointFire deferred;
+ Structure* newStructure = Structure::changePrototypeTransition(vm, structure(vm), prototype, deferred);
setStructure(vm, newStructure);
} else
putDirect(vm, structure(vm)->polyProtoOffset(), prototype);
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (223160 => 223161)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2017-10-11 00:37:49 UTC (rev 223160)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2017-10-11 00:53:59 UTC (rev 223161)
@@ -545,12 +545,12 @@
return transition;
}
-Structure* Structure::changePrototypeTransition(VM& vm, Structure* structure, JSValue prototype)
+Structure* Structure::changePrototypeTransition(VM& vm, Structure* structure, JSValue prototype, DeferredStructureTransitionWatchpointFire& deferred)
{
ASSERT(prototype.isObject() || prototype.isNull());
DeferGC deferGC(vm.heap);
- Structure* transition = create(vm, structure);
+ Structure* transition = create(vm, structure, &deferred);
transition->m_prototype.set(vm, transition, prototype);
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (223160 => 223161)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2017-10-11 00:37:49 UTC (rev 223160)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2017-10-11 00:53:59 UTC (rev 223161)
@@ -181,7 +181,7 @@
static Structure* addPropertyTransitionToExistingStructureConcurrently(Structure*, UniquedStringImpl* uid, unsigned attributes, PropertyOffset&);
JS_EXPORT_PRIVATE static Structure* addPropertyTransitionToExistingStructure(Structure*, PropertyName, unsigned attributes, PropertyOffset&);
static Structure* removePropertyTransition(VM&, Structure*, PropertyName, PropertyOffset&);
- static Structure* changePrototypeTransition(VM&, Structure*, JSValue prototype);
+ static Structure* changePrototypeTransition(VM&, Structure*, JSValue prototype, DeferredStructureTransitionWatchpointFire&);
JS_EXPORT_PRIVATE static Structure* attributeChangeTransition(VM&, Structure*, PropertyName, unsigned attributes);
JS_EXPORT_PRIVATE static Structure* toCacheableDictionaryTransition(VM&, Structure*, DeferredStructureTransitionWatchpointFire* = nullptr);
static Structure* toUncacheableDictionaryTransition(VM&, Structure*);
