Title: [290869] trunk/Source
Revision
290869
Author
mark....@apple.com
Date
2022-03-05 10:55:21 -0800 (Sat, 05 Mar 2022)

Log Message

Fix broken SuspendExceptionScope and remove redundant VM::DeferExceptionScope.
https://bugs.webkit.org/show_bug.cgi?id=237441
<rdar://problem/89769627>

Reviewed by Yusuke Suzuki.

Source/_javascript_Core:

SuspendExceptionScope was meant to do exactly the same thing that VM::DeferExceptionScope
does, except that SuspendExceptionScope hasn't been updated to handle exception
handling via VMTraps bits.

This patch will fix SuspendExceptionScope to work like VM::DeferExceptionScope,
and remove the now redundant VM::DeferExceptionScope.  SuspendExceptionScope is
the better name here because the scope actually suspends any pending exception.
This is different from other Defer scopes where we prevent some new event from
arising and defer the event to a later time.

* interpreter/FrameTracers.h:
(JSC::SuspendExceptionScope::SuspendExceptionScope):
(JSC::SuspendExceptionScope::~SuspendExceptionScope):
* interpreter/Interpreter.cpp:
(JSC::UnwindFunctor::notifyDebuggerOfUnwinding):
* runtime/TypeProfilerLog.cpp:
(JSC::TypeProfilerLog::processLogEntries):
* runtime/VM.h:
(JSC::VM::restorePreviousException): Deleted.
(JSC::VM::DeferExceptionScope::DeferExceptionScope): Deleted.
(JSC::VM::DeferExceptionScope::~DeferExceptionScope): Deleted.

Source/WebCore:

* inspector/InspectorFrontendAPIDispatcher.cpp:
(WebCore::InspectorFrontendAPIDispatcher::evaluateExpression):
* inspector/InspectorFrontendHost.cpp:
(WebCore::InspectorFrontendHost::evaluateScriptInExtensionTab):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (290868 => 290869)


--- trunk/Source/_javascript_Core/ChangeLog	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,3 +1,33 @@
+2022-03-05  Mark Lam  <mark....@apple.com>
+
+        Fix broken SuspendExceptionScope and remove redundant VM::DeferExceptionScope.
+        https://bugs.webkit.org/show_bug.cgi?id=237441
+        <rdar://problem/89769627>
+
+        Reviewed by Yusuke Suzuki.
+
+        SuspendExceptionScope was meant to do exactly the same thing that VM::DeferExceptionScope
+        does, except that SuspendExceptionScope hasn't been updated to handle exception
+        handling via VMTraps bits.
+
+        This patch will fix SuspendExceptionScope to work like VM::DeferExceptionScope,
+        and remove the now redundant VM::DeferExceptionScope.  SuspendExceptionScope is
+        the better name here because the scope actually suspends any pending exception.
+        This is different from other Defer scopes where we prevent some new event from
+        arising and defer the event to a later time.
+
+        * interpreter/FrameTracers.h:
+        (JSC::SuspendExceptionScope::SuspendExceptionScope):
+        (JSC::SuspendExceptionScope::~SuspendExceptionScope):
+        * interpreter/Interpreter.cpp:
+        (JSC::UnwindFunctor::notifyDebuggerOfUnwinding):
+        * runtime/TypeProfilerLog.cpp:
+        (JSC::TypeProfilerLog::processLogEntries):
+        * runtime/VM.h:
+        (JSC::VM::restorePreviousException): Deleted.
+        (JSC::VM::DeferExceptionScope::DeferExceptionScope): Deleted.
+        (JSC::VM::DeferExceptionScope::~DeferExceptionScope): Deleted.
+
 2022-03-04  Angelos Oikonomopoulos  <ange...@igalia.com>
 
         [JSC] Improve reuse of known register values on ARMv7

Modified: trunk/Source/_javascript_Core/interpreter/FrameTracers.h (290868 => 290869)


--- trunk/Source/_javascript_Core/interpreter/FrameTracers.h	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/_javascript_Core/interpreter/FrameTracers.h	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,20 +35,25 @@
 
 class SuspendExceptionScope {
 public:
-    SuspendExceptionScope(VM* vm)
+    SuspendExceptionScope(VM& vm)
         : m_vm(vm)
+        , m_exceptionWasSet(vm.m_exception)
+        , m_savedException(vm.m_exception, nullptr)
+        , m_savedLastException(vm.m_lastException, nullptr)
     {
-        auto scope = DECLARE_CATCH_SCOPE(*vm);
-        oldException = scope.exception();
-        scope.clearException();
+        if (m_exceptionWasSet)
+            m_vm.traps().clearTrapBit(VMTraps::NeedExceptionHandling);
     }
     ~SuspendExceptionScope()
     {
-        m_vm->restorePreviousException(oldException);
+        if (m_exceptionWasSet)
+            m_vm.traps().setTrapBit(VMTraps::NeedExceptionHandling);
     }
 private:
-    Exception* oldException;
-    VM* m_vm;
+    VM& m_vm;
+    bool m_exceptionWasSet;
+    SetForScope<Exception*> m_savedException;
+    SetForScope<Exception*> m_savedLastException;
 };
 
 class TopCallFrameSetter {

Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (290868 => 290869)


--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2022-03-05 18:55:21 UTC (rev 290869)
@@ -650,7 +650,7 @@
         DeferTermination deferScope(vm);
         auto catchScope = DECLARE_CATCH_SCOPE(vm);
 
-        SuspendExceptionScope scope(&vm);
+        SuspendExceptionScope scope(vm);
         if (callFrame->isAnyWasmCallee()
             || (callFrame->callee().isCell() && callFrame->callee().asCell()->inherits<JSFunction>(vm)))
             debugger->unwindEvent(callFrame);

Modified: trunk/Source/_javascript_Core/runtime/TypeProfilerLog.cpp (290868 => 290869)


--- trunk/Source/_javascript_Core/runtime/TypeProfilerLog.cpp	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/_javascript_Core/runtime/TypeProfilerLog.cpp	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,6 +29,7 @@
 #include "config.h"
 #include "TypeProfilerLog.h"
 
+#include "FrameTracers.h"
 #include "JSCJSValueInlines.h"
 #include "TypeLocation.h"
 
@@ -58,9 +59,9 @@
     // We need to do this because this code will call into calculatedDisplayName.
     // calculatedDisplayName will clear any exception it sees (because it thinks
     // it's a stack overflow). We may be called when an exception was already
-    // thrown, so we don't want calcualtedDisplayName to clear that exception that
+    // thrown, so we don't want calculatedDisplayName to clear that exception that
     // was thrown before we even got here.
-    VM::DeferExceptionScope deferExceptionScope(vm);
+    SuspendExceptionScope suspendExceptionScope(vm);
 
     MonotonicTime before { };
     if (TypeProfilerLogInternal::verbose) {

Modified: trunk/Source/_javascript_Core/runtime/VM.h (290868 => 290869)


--- trunk/Source/_javascript_Core/runtime/VM.h	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/_javascript_Core/runtime/VM.h	2022-03-05 18:55:21 UTC (rev 290869)
@@ -612,8 +612,6 @@
         return OBJECT_OFFSETOF(VM, heap) + OBJECT_OFFSETOF(Heap, m_mutatorShouldBeFenced);
     }
 
-    void restorePreviousException(Exception* exception) { setException(exception); }
-
     void clearLastException() { m_lastException = nullptr; }
 
     CallFrame** addressOfCallFrameForCatch() { return &callFrameForCatch; }
@@ -838,31 +836,6 @@
 
     static void setCrashOnVMCreation(bool);
 
-    class DeferExceptionScope {
-    public:
-        DeferExceptionScope(VM& vm)
-            : m_vm(vm)
-            , m_exceptionWasSet(vm.m_exception)
-            , m_savedException(vm.m_exception, nullptr)
-            , m_savedLastException(vm.m_lastException, nullptr)
-        {
-            if (m_exceptionWasSet)
-                m_vm.traps().clearTrapBit(VMTraps::NeedExceptionHandling);
-        }
-
-        ~DeferExceptionScope()
-        {
-            if (m_exceptionWasSet)
-                m_vm.traps().setTrapBit(VMTraps::NeedExceptionHandling);
-        }
-
-    private:
-        VM& m_vm;
-        bool m_exceptionWasSet;
-        SetForScope<Exception*> m_savedException;
-        SetForScope<Exception*> m_savedLastException;
-    };
-
     void addLoopHintExecutionCounter(const JSInstruction*);
     uintptr_t* getLoopHintExecutionCounter(const JSInstruction*);
     void removeLoopHintExecutionCounter(const JSInstruction*);
@@ -887,8 +860,6 @@
 #endif
 
 private:
-    friend class LLIntOffsetsExtractor;
-
     VM(VMType, HeapType, WTF::RunLoop* = nullptr, bool* success = nullptr);
     static VM*& sharedInstanceInternal();
     void createNativeThunk();
@@ -1017,12 +988,13 @@
     VM* m_prev; // Required by DoublyLinkedListNode.
     VM* m_next; // Required by DoublyLinkedListNode.
 
-    // Friends for exception checking purpose only.
     friend class Heap;
-    friend class CatchScope;
-    friend class ExceptionScope;
+    friend class CatchScope; // Friend for exception checking purpose only.
+    friend class ExceptionScope; // Friend for exception checking purpose only.
     friend class JSDollarVMHelper;
-    friend class ThrowScope;
+    friend class LLIntOffsetsExtractor;
+    friend class SuspendExceptionScope;
+    friend class ThrowScope; // Friend for exception checking purpose only.
     friend class VMTraps;
     friend class WTF::DoublyLinkedListNode<VM>;
 };

Modified: trunk/Source/WebCore/ChangeLog (290868 => 290869)


--- trunk/Source/WebCore/ChangeLog	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/WebCore/ChangeLog	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,3 +1,16 @@
+2022-03-05  Mark Lam  <mark....@apple.com>
+
+        Fix broken SuspendExceptionScope and remove redundant VM::DeferExceptionScope.
+        https://bugs.webkit.org/show_bug.cgi?id=237441
+        <rdar://problem/89769627>
+
+        Reviewed by Yusuke Suzuki.
+
+        * inspector/InspectorFrontendAPIDispatcher.cpp:
+        (WebCore::InspectorFrontendAPIDispatcher::evaluateExpression):
+        * inspector/InspectorFrontendHost.cpp:
+        (WebCore::InspectorFrontendHost::evaluateScriptInExtensionTab):
+
 2022-03-05  Alan Bujtas  <za...@apple.com>
 
         [IFC][Integration] Do not bail out on IFC content with floats inside.

Modified: trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp (290868 => 290869)


--- trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -259,7 +259,7 @@
     ASSERT(!m_suspended);
     ASSERT(m_queuedEvaluations.isEmpty());
 
-    JSC::SuspendExceptionScope scope(&m_frontendPage->inspectorController().vm());
+    JSC::SuspendExceptionScope scope(m_frontendPage->inspectorController().vm());
     return m_frontendPage->mainFrame().script().evaluateInWorld(ScriptSourceCode(_expression_), mainThreadNormalWorld());
 }
 

Modified: trunk/Source/WebCore/inspector/InspectorFrontendHost.cpp (290868 => 290869)


--- trunk/Source/WebCore/inspector/InspectorFrontendHost.cpp	2022-03-05 17:18:30 UTC (rev 290868)
+++ trunk/Source/WebCore/inspector/InspectorFrontendHost.cpp	2022-03-05 18:55:21 UTC (rev 290869)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2007-2022 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Matt Lilek <web...@mattlilek.com>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -740,7 +740,7 @@
     if (!frameGlobalObject)
         return Exception { InvalidStateError, "Unable to find global object for <iframe>"_s };
 
-    JSC::SuspendExceptionScope scope(&frameGlobalObject->vm());
+    JSC::SuspendExceptionScope scope(frameGlobalObject->vm());
     ValueOrException result = frame->script().evaluateInWorld(ScriptSourceCode(scriptSource), mainThreadNormalWorld());
     
     if (!result)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to