Title: [243420] trunk
Revision
243420
Author
mark....@apple.com
Date
2019-03-23 21:15:56 -0700 (Sat, 23 Mar 2019)

Log Message

Rolling out r243032 and r243071 because the fix is incorrect.
https://bugs.webkit.org/show_bug.cgi?id=195892
<rdar://problem/48981239>

Not reviewed.

JSTests:

* stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js: Removed.

Source/_javascript_Core:

The fix is incorrect: it relies on being able to determine liveness of an object
in an ObjectPropertyCondition based on the state of the object's MarkedBit.
However, there's no guarantee that GC has run and that the MarkedBit is already
set even if the object is live.  As a result, we may not re-install adaptive
watchpoints based on presumed dead objects which are actually live.

I'm rolling this out, and will implement a more comprehensive fix to handle
watchpoint liveness later.

* bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp:
(JSC::AdaptiveInferredPropertyValueWatchpointBase::fire):
* bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
* bytecode/ObjectPropertyCondition.cpp:
(JSC::ObjectPropertyCondition::dumpInContext const):
* bytecode/StructureStubClearingWatchpoint.cpp:
(JSC::StructureStubClearingWatchpoint::fireInternal):
* dfg/DFGAdaptiveStructureWatchpoint.cpp:
(JSC::DFG::AdaptiveStructureWatchpoint::fireInternal):
* runtime/StructureRareData.cpp:
(JSC::ObjectToStringAdaptiveStructureWatchpoint::fireInternal):

LayoutTests:

* platform/mac/TestExpectations:

Modified Paths

Removed Paths

Diff

Modified: trunk/JSTests/ChangeLog (243419 => 243420)


--- trunk/JSTests/ChangeLog	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/JSTests/ChangeLog	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,3 +1,13 @@
+2019-03-23  Mark Lam  <mark....@apple.com>
+
+        Rolling out r243032 and r243071 because the fix is incorrect.
+        https://bugs.webkit.org/show_bug.cgi?id=195892
+        <rdar://problem/48981239>
+
+        Not reviewed.
+
+        * stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js: Removed.
+
 2019-03-22  Mark Lam  <mark....@apple.com>
 
         Placate exception check validation in genericTypedArrayViewProtoFuncLastIndexOf().

Deleted: trunk/JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js (243419 => 243420)


--- trunk/JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,30 +0,0 @@
-//@ requireOptions("--forceEagerCompilation=true")
-
-// This test should not crash.
-
-let a;
-
-function bar() {
-    a / 1;
-    a = null;
-}
-
-function foo(s) {
-    try {
-        eval(s);
-    } catch (e) {
-        gc();
-        bar();
-        '' + e + 0;
-    }
-}
-
-foo('zz');
-foo('class A { y() {} }; a=new A; zz');
-foo('class C { y() {} }; gc();');
-
-class A {
-    y() {}
-}
-
-A.prototype.z = 0

Modified: trunk/LayoutTests/ChangeLog (243419 => 243420)


--- trunk/LayoutTests/ChangeLog	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/LayoutTests/ChangeLog	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,3 +1,13 @@
+2019-03-23  Mark Lam  <mark....@apple.com>
+
+        Rolling out r243032 and r243071 because the fix is incorrect.
+        https://bugs.webkit.org/show_bug.cgi?id=195892
+        <rdar://problem/48981239>
+
+        Not reviewed.
+
+        * platform/mac/TestExpectations:
+
 2019-03-23  Justin Fan  <justin_...@apple.com>
 
         [Web GPU] Prototype compute pipeline with MSL

Modified: trunk/LayoutTests/platform/mac/TestExpectations (243419 => 243420)


--- trunk/LayoutTests/platform/mac/TestExpectations	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1069,7 +1069,7 @@
 webkit.org/b/159684 [ Debug ] inspector/indexeddb/deleteDatabaseNamesWithSpace.html [ Pass Timeout ]
 webkit.org/b/153894 [ Debug ] inspector/model/color.html [ Pass Timeout ]
 webkit.org/b/148636 inspector/model/parse-script-syntax-tree.html [ Pass Timeout ]
-webkit.org/b/148636 inspector/model/remote-object.html [ Pass Timeout Failure ]
+webkit.org/b/148636 inspector/model/remote-object.html [ Pass Timeout ]
 webkit.org/b/167265 inspector/network/client-blocked-load.html [ Pass Timeout ]
 webkit.org/b/148636 inspector/page/main-frame-resource.html [ Pass Timeout ]
 webkit.org/b/168146 [ Debug ] inspector/protocol/inspector-backend-invocation-return-value.html [ Pass Timeout ]

Modified: trunk/Source/_javascript_Core/ChangeLog (243419 => 243420)


--- trunk/Source/_javascript_Core/ChangeLog	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,3 +1,33 @@
+2019-03-23  Mark Lam  <mark....@apple.com>
+
+        Rolling out r243032 and r243071 because the fix is incorrect.
+        https://bugs.webkit.org/show_bug.cgi?id=195892
+        <rdar://problem/48981239>
+
+        Not reviewed.
+
+        The fix is incorrect: it relies on being able to determine liveness of an object
+        in an ObjectPropertyCondition based on the state of the object's MarkedBit.
+        However, there's no guarantee that GC has run and that the MarkedBit is already
+        set even if the object is live.  As a result, we may not re-install adaptive
+        watchpoints based on presumed dead objects which are actually live.
+
+        I'm rolling this out, and will implement a more comprehensive fix to handle
+        watchpoint liveness later.
+
+        * bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp:
+        (JSC::AdaptiveInferredPropertyValueWatchpointBase::fire):
+        * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
+        (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
+        * bytecode/ObjectPropertyCondition.cpp:
+        (JSC::ObjectPropertyCondition::dumpInContext const):
+        * bytecode/StructureStubClearingWatchpoint.cpp:
+        (JSC::StructureStubClearingWatchpoint::fireInternal):
+        * dfg/DFGAdaptiveStructureWatchpoint.cpp:
+        (JSC::DFG::AdaptiveStructureWatchpoint::fireInternal):
+        * runtime/StructureRareData.cpp:
+        (JSC::ObjectToStringAdaptiveStructureWatchpoint::fireInternal):
+
 2019-03-23  Keith Miller  <keith_mil...@apple.com>
 
         Refactor clz/ctz and fix getLSBSet.

Modified: trunk/Source/_javascript_Core/bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-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
@@ -62,9 +62,7 @@
     if (!isValid())
         return;
 
-    // FIXME: The m_key.isStillLive() check should not be needed if this watchpoint was removed
-    // when m_key's m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (m_key.isStillLive() && m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
         install(vm);
         return;
     }

Modified: trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -49,9 +49,7 @@
 
 void LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal(VM& vm, const FireDetail&)
 {
-    // FIXME: The m_key.isStillLive() check should not be needed if this watchpoint was removed
-    // when m_key's m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (m_key.isStillLive() && m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
         install(vm);
         return;
     }

Modified: trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -37,14 +37,7 @@
         out.print("<invalid>");
         return;
     }
-
-    // FIXME: The m_key.isStillLive() check should not be needed if the watchpoint using this
-    // condition was removed when m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (!isStillLive()) {
-        out.print("<not live>");
-        return;
-    }
-
+    
     out.print("<", inContext(JSValue(m_object), context), ": ", inContext(m_condition, context), ">");
 }
 

Modified: trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,9 +36,7 @@
 
 void StructureStubClearingWatchpoint::fireInternal(VM& vm, const FireDetail&)
 {
-    // FIXME: The m_key.isStillLive() check should not be needed if this watchpoint was removed
-    // when m_key's m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (!m_key.isStillLive() || !m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    if (!m_key || !m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
         // This will implicitly cause my own demise: stub reset removes all watchpoints.
         // That works, because deleting a watchpoint removes it from the set's list, and
         // the set's list traversal for firing is robust against the set changing.

Modified: trunk/Source/_javascript_Core/dfg/DFGAdaptiveStructureWatchpoint.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/dfg/DFGAdaptiveStructureWatchpoint.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/dfg/DFGAdaptiveStructureWatchpoint.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,9 +52,7 @@
 
 void AdaptiveStructureWatchpoint::fireInternal(VM& vm, const FireDetail& detail)
 {
-    // FIXME: The m_key.isStillLive() check should not be needed if this watchpoint was removed
-    // when m_key's m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (m_key.isStillLive() && m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
         install(vm);
         return;
     }

Modified: trunk/Source/_javascript_Core/runtime/StructureRareData.cpp (243419 => 243420)


--- trunk/Source/_javascript_Core/runtime/StructureRareData.cpp	2019-03-24 01:35:13 UTC (rev 243419)
+++ trunk/Source/_javascript_Core/runtime/StructureRareData.cpp	2019-03-24 04:15:56 UTC (rev 243420)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-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
@@ -191,9 +191,7 @@
     if (!m_structureRareData->isLive())
         return;
 
-    // FIXME: The m_key.isStillLive() check should not be needed if this watchpoint was removed
-    // when m_key's m_object died. https://bugs.webkit.org/show_bug.cgi?id=195829
-    if (m_key.isStillLive() && m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
         install(vm);
         return;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to