Need to check ObjectPropertyCondition liveness before accessing it when firing watchp...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Mar 2019 04:44:57 +0000 (04:44 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Mar 2019 04:44:57 +0000 (04:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195827
<rdar://problem/48845513>

Reviewed by Filip Pizlo.

JSTests:

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

Source/JavaScriptCore:

m_object in ObjectPropertyCondition may no longer be live by the time the watchpoint fires.

* 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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243032 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp
Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp
Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp
Source/JavaScriptCore/bytecode/StructureStubClearingWatchpoint.cpp
Source/JavaScriptCore/dfg/DFGAdaptiveStructureWatchpoint.cpp
Source/JavaScriptCore/runtime/StructureRareData.cpp

index 9f89ffc..adc0649 100644 (file)
@@ -1,3 +1,13 @@
+2019-03-15  Mark Lam  <mark.lam@apple.com>
+
+        Need to check ObjectPropertyCondition liveness before accessing it when firing watchpoints.
+        https://bugs.webkit.org/show_bug.cgi?id=195827
+        <rdar://problem/48845513>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js: Added.
+
 2019-03-15  Dominik Infuehr  <dinfuehr@igalia.com>
 
         [ARM,MIPS] Skip slow tests
diff --git a/JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js b/JSTests/stress/check-object-property-condition-liveness-before-accessing-it-when-watchpoints-fire.js
new file mode 100644 (file)
index 0000000..15af0c8
--- /dev/null
@@ -0,0 +1,30 @@
+//@ 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
index c4db71a..c48d140 100644 (file)
@@ -1,3 +1,26 @@
+2019-03-15  Mark Lam  <mark.lam@apple.com>
+
+        Need to check ObjectPropertyCondition liveness before accessing it when firing watchpoints.
+        https://bugs.webkit.org/show_bug.cgi?id=195827
+        <rdar://problem/48845513>
+
+        Reviewed by Filip Pizlo.
+
+        m_object in ObjectPropertyCondition may no longer be live by the time the watchpoint fires.
+
+        * 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-15  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Make more properties lazily-allocated in JSGlobalObject, including properties only used in JIT mode
index b26fbf6..b3727db 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2019 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,7 +62,9 @@ void AdaptiveInferredPropertyValueWatchpointBase::fire(VM& vm, const FireDetail&
     if (!isValid())
         return;
 
-    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    // 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)) {
         install(vm);
         return;
     }
index 6beddf9..a131fa1 100644 (file)
@@ -49,7 +49,9 @@ void LLIntPrototypeLoadAdaptiveStructureWatchpoint::install(VM& vm)
 
 void LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal(VM& vm, const FireDetail&)
 {
-    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    // 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)) {
         install(vm);
         return;
     }
index 3aad094..565bd5a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2019 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,7 +37,14 @@ void ObjectPropertyCondition::dumpInContext(PrintStream& out, DumpContext* conte
         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), ">");
 }
 
index 648b1e8..783f7ea 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012, 2015-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2019 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,7 +36,9 @@ namespace JSC {
 
 void StructureStubClearingWatchpoint::fireInternal(VM& vm, const FireDetail&)
 {
-    if (!m_key || !m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    // 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)) {
         // 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.
index d4a8b9a..8d6e194 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2019 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,7 +52,9 @@ void AdaptiveStructureWatchpoint::install(VM& vm)
 
 void AdaptiveStructureWatchpoint::fireInternal(VM& vm, const FireDetail& detail)
 {
-    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    // 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)) {
         install(vm);
         return;
     }
index d5606cd..4568932 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2019 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,7 +191,9 @@ void ObjectToStringAdaptiveStructureWatchpoint::fireInternal(VM& vm, const FireD
     if (!m_structureRareData->isLive())
         return;
 
-    if (m_key.isWatchable(PropertyCondition::EnsureWatchability)) {
+    // 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)) {
         install(vm);
         return;
     }