Unreviewed, roll out r228366 since it did not progress anything.
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Feb 2018 23:38:15 +0000 (23:38 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Feb 2018 23:38:15 +0000 (23:38 +0000)
JSTests:

* stress/gc-error-stack.js: Removed.
* stress/no-gc-error-stack.js: Removed.

Source/JavaScriptCore:

* heap/Heap.cpp:
(JSC::Heap::finalizeUnconditionalFinalizers):
* runtime/ErrorInstance.cpp:
(JSC::ErrorInstance::visitChildren):
(JSC::ErrorInstance::finalizeUnconditionally): Deleted.
* runtime/ErrorInstance.h:
(JSC::ErrorInstance::stackTrace):
(JSC::ErrorInstance::subspaceFor): Deleted.
* runtime/Exception.cpp:
(JSC::Exception::visitChildren):
(JSC::Exception::finalizeUnconditionally): Deleted.
* runtime/Exception.h:
* runtime/StackFrame.cpp:
(JSC::StackFrame::visitChildren):
(JSC::StackFrame::isFinalizationCandidate): Deleted.
(JSC::StackFrame::finalizeUnconditionally): Deleted.
* runtime/StackFrame.h:
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:

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

13 files changed:
JSTests/ChangeLog
JSTests/stress/gc-error-stack.js [deleted file]
JSTests/stress/no-gc-error-stack.js [deleted file]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/runtime/ErrorInstance.cpp
Source/JavaScriptCore/runtime/ErrorInstance.h
Source/JavaScriptCore/runtime/Exception.cpp
Source/JavaScriptCore/runtime/Exception.h
Source/JavaScriptCore/runtime/StackFrame.cpp
Source/JavaScriptCore/runtime/StackFrame.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h

index b05cce9..51abe2f 100644 (file)
@@ -1,3 +1,10 @@
+2018-02-15  Filip Pizlo  <fpizlo@apple.com>
+
+        Unreviewed, roll out r228366 since it did not progress anything.
+
+        * stress/gc-error-stack.js: Removed.
+        * stress/no-gc-error-stack.js: Removed.
+
 2018-02-15  Tomas Popela  <tpopela@redhat.com>
 
         Many stress tests fail with JIT disabled
diff --git a/JSTests/stress/gc-error-stack.js b/JSTests/stress/gc-error-stack.js
deleted file mode 100644 (file)
index 6fef59e..0000000
+++ /dev/null
@@ -1,18 +0,0 @@
-"use strict";
-
-var errors = [];
-
-for (let i = 0; i < 1000; ++i)
-    eval("(function foo" + i + "() { errors.push(new Error()); })();");
-
-gc();
-
-let didGCAtLeastOne = false;
-for (let error of errors) {
-    if (!error.stack.startsWith("foo"))
-        didGCAtLeastOne = true;
-}
-
-if (!didGCAtLeastOne)
-    throw new Error("Bad result: didGCAtLeastOne = " + didGCAtLeastOne);
-
diff --git a/JSTests/stress/no-gc-error-stack.js b/JSTests/stress/no-gc-error-stack.js
deleted file mode 100644 (file)
index 31eb803..0000000
+++ /dev/null
@@ -1,22 +0,0 @@
-//@ runDefault
-
-"use strict";
-
-var errors = [];
-
-for (let i = 0; i < 1000; ++i)
-    eval("(function foo" + i + "() { errors.push(new Error()); })();");
-
-for (let error of errors)
-    error.stack;
-
-gc();
-
-for (let error of errors) {
-    if (!error.stack.startsWith("foo")) {
-        print("ERROR: Stack does not begin with foo: " + error.stack);
-        throw new Error("fail");
-    }
-}
-
-
index 8aff7af..9e0208c 100644 (file)
@@ -1,3 +1,28 @@
+2018-02-15  Filip Pizlo  <fpizlo@apple.com>
+
+        Unreviewed, roll out r228366 since it did not progress anything.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::finalizeUnconditionalFinalizers):
+        * runtime/ErrorInstance.cpp:
+        (JSC::ErrorInstance::visitChildren):
+        (JSC::ErrorInstance::finalizeUnconditionally): Deleted.
+        * runtime/ErrorInstance.h:
+        (JSC::ErrorInstance::stackTrace):
+        (JSC::ErrorInstance::subspaceFor): Deleted.
+        * runtime/Exception.cpp:
+        (JSC::Exception::visitChildren):
+        (JSC::Exception::finalizeUnconditionally): Deleted.
+        * runtime/Exception.h:
+        * runtime/StackFrame.cpp:
+        (JSC::StackFrame::visitChildren):
+        (JSC::StackFrame::isFinalizationCandidate): Deleted.
+        (JSC::StackFrame::finalizeUnconditionally): Deleted.
+        * runtime/StackFrame.h:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+
 2018-02-15  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Remove monotonicallyIncreasingTime and currentTime
index dd1e573..3408ec1 100644 (file)
@@ -579,8 +579,6 @@ void Heap::finalizeMarkedUnconditionalFinalizers(CellSet& cellSet)
 
 void Heap::finalizeUnconditionalFinalizers()
 {
-    finalizeMarkedUnconditionalFinalizers<ErrorInstance>(vm()->errorInstancesWithFinalizers);
-    finalizeMarkedUnconditionalFinalizers<Exception>(vm()->exceptionsWithFinalizers);
     finalizeMarkedUnconditionalFinalizers<InferredType>(vm()->inferredTypesWithFinalizers);
     finalizeMarkedUnconditionalFinalizers<InferredValue>(vm()->inferredValuesWithFinalizers);
     vm()->forEachCodeBlockSpace(
index 8f418ae..c172428 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003-2017 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -23,7 +23,6 @@
 
 #include "CodeBlock.h"
 #include "InlineCallFrame.h"
-#include "IsoCellSetInlines.h"
 #include "JSScope.h"
 #include "JSCInlines.h"
 #include "ParseInt.h"
@@ -234,33 +233,13 @@ void ErrorInstance::visitChildren(JSCell* cell, SlotVisitor& visitor)
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     Base::visitChildren(thisObject, visitor);
 
-    bool isFinalizationCandidate = false;
     {
         auto locker = holdLock(thisObject->cellLock());
         if (thisObject->m_stackTrace) {
-            for (StackFrame& frame : *thisObject->m_stackTrace) {
-                if (frame.isFinalizationCandidate()) {
-                    isFinalizationCandidate = true;
-                    break;
-                }
-            }
+            for (StackFrame& frame : *thisObject->m_stackTrace)
+                frame.visitChildren(visitor);
         }
     }
-    if (isFinalizationCandidate)
-        visitor.vm().errorInstancesWithFinalizers.add(thisObject);
-}
-
-void ErrorInstance::finalizeUnconditionally(VM& vm)
-{
-    {
-        auto locker = holdLock(cellLock());
-        if (m_stackTrace) {
-            for (StackFrame& frame : *m_stackTrace)
-                frame.finalizeUnconditionally(vm);
-        }
-    }
-    
-    vm.errorInstancesWithFinalizers.remove(this);
 }
 
 bool ErrorInstance::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
index b70af4f..56b3ae2 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2008-2018 Apple Inc. All rights reserved.
+ *  Copyright (C) 2008-2017 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
 
 namespace JSC {
 
-// FIXME: This should be final, but isn't because of bizarre (and mostly wrong) things done in
-// WebAssembly.
-// https://bugs.webkit.org/show_bug.cgi?id=182649
 class ErrorInstance : public JSDestructibleObject {
 public:
-    template<typename CellType>
-    static IsoSubspace* subspaceFor(VM& vm)
-    {
-        return &vm.errorInstanceSpace;
-    }
-
     typedef JSDestructibleObject Base;
     const static unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetPropertyNames;
 
@@ -77,8 +68,6 @@ public:
     JS_EXPORT_PRIVATE String sanitizedToString(ExecState*);
     
     Vector<StackFrame>* stackTrace() { return m_stackTrace.get(); }
-    
-    void finalizeUnconditionally(VM&);
 
     bool materializeErrorInfoIfNeeded(VM&);
     bool materializeErrorInfoIfNeeded(VM&, PropertyName);
index 0526750..bc989ef 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2018 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
@@ -27,7 +27,6 @@
 #include "Exception.h"
 
 #include "Interpreter.h"
-#include "IsoCellSetInlines.h"
 #include "JSCInlines.h"
 
 namespace JSC {
@@ -58,24 +57,9 @@ void Exception::visitChildren(JSCell* cell, SlotVisitor& visitor)
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     Base::visitChildren(thisObject, visitor);
 
-    bool isFinalizationCandidate = false;
     visitor.append(thisObject->m_value);
-    for (StackFrame& frame : thisObject->m_stack) {
-        if (frame.isFinalizationCandidate()) {
-            isFinalizationCandidate = true;
-            break;
-        }
-    }
-    if (isFinalizationCandidate)
-        visitor.vm().exceptionsWithFinalizers.add(thisObject);
-}
-
-void Exception::finalizeUnconditionally(VM& vm)
-{
-    for (StackFrame& frame : m_stack)
-        frame.finalizeUnconditionally(vm);
-    
-    vm.exceptionsWithFinalizers.remove(this);
+    for (StackFrame& frame : thisObject->m_stack)
+        frame.visitChildren(visitor);
 }
 
 Exception::Exception(VM& vm)
index cf5342b..64d0a0a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2018 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
 
 namespace JSC {
     
-class Exception final : public JSDestructibleObject {
+class Exception : public JSDestructibleObject {
 public:
-    template<typename CellType>
-    static IsoSubspace* subspaceFor(VM& vm)
-    {
-        return &vm.exceptionSpace;
-    }
-
     typedef JSDestructibleObject Base;
     static const unsigned StructureFlags = StructureIsImmortal | Base::StructureFlags;
 
@@ -66,8 +60,6 @@ public:
     void setDidNotifyInspectorOfThrow() { m_didNotifyInspectorOfThrow = true; }
 
     ~Exception();
-    
-    void finalizeUnconditionally(VM&);
 
 private:
     Exception(VM&);
index 15aefca..9f2d60d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2018 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
@@ -142,27 +142,12 @@ String StackFrame::toString(VM& vm) const
     return traceBuild.toString().impl();
 }
 
-bool StackFrame::isFinalizationCandidate()
+void StackFrame::visitChildren(SlotVisitor& visitor)
 {
-    if (m_callee && !Heap::isMarked(m_callee.get()))
-        return true;
-    if (m_codeBlock && !Heap::isMarked(m_codeBlock.get()))
-        return true;
-    return false;
-}
-
-void StackFrame::finalizeUnconditionally(VM&)
-{
-    // FIXME: We should do something smarter. For example, if this happens, we could stringify the
-    // whole stack trace. The main shortcoming is that that requires doing operations that are not
-    // currently legal during finalization. We could make this work by giving JSC a proper "second
-    // chance" finalizer infrastructure. Or maybe there's an even easier way.
-    // https://bugs.webkit.org/show_bug.cgi?id=182650
-    
-    if (m_callee && !Heap::isMarked(m_callee.get()))
-        m_callee.clear();
-    if (m_codeBlock && !Heap::isMarked(m_codeBlock.get()))
-        m_codeBlock.clear();
+    if (m_callee)
+        visitor.append(m_callee);
+    if (m_codeBlock)
+        visitor.append(m_codeBlock);
 }
 
 } // namespace JSC
index 038290f..2877723 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2018 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
@@ -56,8 +56,7 @@ public:
         return m_bytecodeOffset;
     }
     
-    bool isFinalizationCandidate();
-    void finalizeUnconditionally(VM&);
+    void visitChildren(SlotVisitor&);
 
 private:
     WriteBarrier<JSCell> m_callee { };
index d1f0281..d80d050 100644 (file)
@@ -260,8 +260,6 @@ VM::VM(VMType vmType, HeapType heapType)
     , boundFunctionSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), JSBoundFunction)
     , customGetterSetterFunctionSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), JSCustomGetterSetterFunction)
     , directEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), DirectEvalExecutable)
-    , errorInstanceSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), ErrorInstance)
-    , exceptionSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), Exception)
     , executableToCodeBlockEdgeSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), ExecutableToCodeBlockEdge)
     , functionExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), FunctionExecutable)
     , functionSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), JSFunction)
@@ -282,8 +280,6 @@ VM::VM(VMType vmType, HeapType heapType)
     , webAssemblyFunctionSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), WebAssemblyFunction)
     , webAssemblyWrapperFunctionSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), WebAssemblyWrapperFunction)
 #endif
-    , errorInstancesWithFinalizers(errorInstanceSpace)
-    , exceptionsWithFinalizers(exceptionSpace)
     , executableToCodeBlockEdgesWithConstraints(executableToCodeBlockEdgeSpace)
     , executableToCodeBlockEdgesWithFinalizers(executableToCodeBlockEdgeSpace)
     , inferredTypesWithFinalizers(inferredTypeSpace)
index 923bf32..55099a4 100644 (file)
@@ -342,8 +342,6 @@ public:
     IsoSubspace boundFunctionSpace;
     IsoSubspace customGetterSetterFunctionSpace;
     IsoSubspace directEvalExecutableSpace;
-    IsoSubspace errorInstanceSpace;
-    IsoSubspace exceptionSpace;
     IsoSubspace executableToCodeBlockEdgeSpace;
     IsoSubspace functionExecutableSpace;
     IsoSubspace functionSpace;
@@ -365,8 +363,6 @@ public:
     IsoSubspace webAssemblyWrapperFunctionSpace;
 #endif
     
-    IsoCellSet errorInstancesWithFinalizers;
-    IsoCellSet exceptionsWithFinalizers;
     IsoCellSet executableToCodeBlockEdgesWithConstraints;
     IsoCellSet executableToCodeBlockEdgesWithFinalizers;
     IsoCellSet inferredTypesWithFinalizers;