Changeset 275969 in webkit

Timestamp:

Apr 14, 2021, 2:50:30 PM (4 years ago)

Author:

mark.lam@apple.com

Message:

Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormalized().
​https://bugs.webkit.org/show_bug.cgi?id=224565
rdar://76645980

Reviewed by Yusuke Suzuki.

JSTests:

• stress/suppress-TerrminationException-in-ASSERT-in-HashMapImpl-addNormalized.js: Added.

Source/JavaScriptCore:

HashMapImpl::addNormalized() has an ASSERT that calls jsMapHash(), which can
potentially throw exceptions. As a result, it has a RETURN_IF_EXCEPTION which
provides an opportunity to handle traps and throw a TerminationException. This
in turn causes the ASSERT to fail.

To fix this, we do:

1. Introduce VMTraps::DeferAction, which gives us DeferForAWhile and DeferUntilEndOfScope.

2. Templatize the DeferTermination RAII object on VMTraps::DeferAction. Introduce DeferTerrminationForAWhile, which is DeferTermination<VMTraps::DeferAction::DeferForAWhile>. DeferForAWhile means that the deferScope will not throw the TerminationException on exit. Instead, it will re-set the NeedTermination bit in the traps, and let the next trap check handle it.

3. Introduce DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE (and friends) which creates a DeferTerrminationForAWhile scope before doing an ASSERT_WITH_MESSAGE.

4. Use DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE instead in HashMapImpl::addNormalized().

• runtime/DeferTermination.h:

(JSC::DeferTermination::DeferTermination):
(JSC::DeferTermination::~DeferTermination):

• runtime/ExceptionHelpers.h:
• runtime/HashMapImpl.h:

(JSC::HashMapImpl::addNormalized):

• runtime/VMTraps.cpp:

(JSC::VMTraps::deferTermination):
(JSC::VMTraps::undoDeferTermination):

• runtime/VMTraps.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/suppress-TerrminationException-in-ASSERT-in-HashMapImpl-addNormalized.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/DeferTermination.h (modified) (3 diffs)
• Source/JavaScriptCore/runtime/ExceptionHelpers.h (modified) (2 diffs)
• Source/JavaScriptCore/runtime/HashMapImpl.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/VMTraps.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/VMTraps.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-04-14  Mark Lam  <mark.lam@apple.com>
+
+        Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormalized().
+        https://bugs.webkit.org/show_bug.cgi?id=224565
+        rdar://76645980
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/suppress-TerrminationException-in-ASSERT-in-HashMapImpl-addNormalized.js: Added.
+
 2021-04-14  Guillaume Emont  <guijemont@igalia.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-04-14  Mark Lam  <mark.lam@apple.com>
+
+        Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormalized().
+        https://bugs.webkit.org/show_bug.cgi?id=224565
+        rdar://76645980
+
+        Reviewed by Yusuke Suzuki.
+
+        HashMapImpl::addNormalized() has an ASSERT that calls jsMapHash(), which can
+        potentially throw exceptions.  As a result, it has a RETURN_IF_EXCEPTION which
+        provides an opportunity to handle traps and throw a TerminationException.  This
+        in turn causes the ASSERT to fail.
+
+        To fix this, we do:
+
+        1. Introduce VMTraps::DeferAction, which gives us DeferForAWhile and DeferUntilEndOfScope.
+
+        2. Templatize the DeferTermination RAII object on VMTraps::DeferAction.
+           Introduce DeferTerrminationForAWhile, which is DeferTermination<VMTraps::DeferAction::DeferForAWhile>.
+           DeferForAWhile means that the deferScope will not throw the TerminationException
+           on exit.  Instead, it will re-set the NeedTermination bit in the traps, and let
+           the next trap check handle it.
+
+        3. Introduce DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE (and friends) which creates
+           a DeferTerrminationForAWhile scope before doing an ASSERT_WITH_MESSAGE.
+
+        4. Use DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE instead in HashMapImpl::addNormalized().
+
+        * runtime/DeferTermination.h:
+        (JSC::DeferTermination::DeferTermination):
+        (JSC::DeferTermination::~DeferTermination):
+        * runtime/ExceptionHelpers.h:
+        * runtime/HashMapImpl.h:
+        (JSC::HashMapImpl::addNormalized):
+        * runtime/VMTraps.cpp:
+        (JSC::VMTraps::deferTermination):
+        (JSC::VMTraps::undoDeferTermination):
+        * runtime/VMTraps.h:
+
 2021-04-13  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/runtime/DeferTermination.h

@@ -32 +32 @@
 namespace JSC {
 
+template<VMTraps::DeferAction deferAction = VMTraps::DeferAction::DeferUntilEndOfScope>
 class DeferTermination {
     WTF_MAKE_NONCOPYABLE(DeferTermination);
@@ -39 +40 @@
         : m_vm(vm)
     {
-        m_vm.traps().deferTermination();
+        m_vm.traps().deferTermination(deferAction);
     }
     
     ~DeferTermination()
     {
-        m_vm.traps().undoDeferTermination();
+        m_vm.traps().undoDeferTermination(deferAction);
     }
 
@@ -51 +52 @@
 };
 
+using DeferTerminationForAWhile = DeferTermination<VMTraps::DeferAction::DeferForAWhile>;
+
 } // namespace JSC

trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.h

@@ -29 +29 @@
 #pragma once
 
+#include "DeferTermination.h"
 #include "ErrorInstance.h"
 #include "Exception.h"
@@ -61 +62 @@
 JS_EXPORT_PRIVATE Exception* throwStackOverflowError(JSGlobalObject*, ThrowScope&);
 
+#if ASSERT_ENABLED
+
+#define DEFER_TERMINATION_AND_ASSERT(vm, assertion, ...) do { \
+        JSC::DeferTerminationForAWhile deferScope(vm); \
+        ASSERT(assertion, __VA_ARGS__); \
+    } while (false)
+
+#define DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE(vm, assertion, ...) do { \
+        JSC::DeferTerminationForAWhile deferScope(vm); \
+        ASSERT_WITH_MESSAGE(assertion, __VA_ARGS__); \
+    } while (false)
+
+#else
+
+#define DEFER_TERMINATION_AND_ASSERT(vm, assertion, ...) UNUSED_PARAM(vm)
+#define DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE(vm, assertion, ...) UNUSED_PARAM(vm)
+
+#endif // ASSERT_ENABLED
+
+#define DEFER_TERMINATION_AND_RELEASE_ASSERT(vm, assertion, ...) do { \
+        JSC::DeferTerminationForAWhile deferScope(vm); \
+        RELEASE_ASSERT(assertion, __VA_ARGS__); \
+    } while (false)
+
+#define DEFER_TERMINATION_AND_RELEASE_ASSERT_WITH_MESSAGE(vm, assertion, ...) do { \
+        JSC::DeferTerminationForAWhile deferScope(vm); \
+        RELEASE_ASSERT_WITH_MESSAGE(assertion, __VA_ARGS__); \
+    } while (false)
+
 } // namespace JSC

trunk/Source/JavaScriptCore/runtime/HashMapImpl.h

@@ -497 +497 @@
     ALWAYS_INLINE HashMapBucketType* addNormalized(JSGlobalObject* globalObject, JSValue key, JSValue value, uint32_t hash)
     {
+        VM& vm = getVM(globalObject);
         ASSERT_WITH_MESSAGE(normalizeMapKey(key) == key, "We expect normalized values flowing into this function.");
-        ASSERT_WITH_MESSAGE(jsMapHash(globalObject, getVM(globalObject), key) == hash, "We expect hash value is what we expect.");
-
-        auto* bucket = addNormalizedInternal(getVM(globalObject), key, value, hash, [&] (HashMapBucketType* bucket) {
+        DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE(vm, jsMapHash(globalObject, getVM(globalObject), key) == hash, "We expect hash value is what we expect.");
+
+        auto* bucket = addNormalizedInternal(vm, key, value, hash, [&] (HashMapBucketType* bucket) {
             return !isDeleted(bucket) && areKeysEqual(globalObject, key, bucket->key());
         });

trunk/Source/JavaScriptCore/runtime/VMTraps.cpp

@@ -416 +416 @@
 }
 
-void VMTraps::deferTermination()
+void VMTraps::deferTermination(DeferAction)
 {
     auto locker = holdLock(*m_lock);
@@ -430 +430 @@
 }
 
-void VMTraps::undoDeferTermination()
+void VMTraps::undoDeferTermination(DeferAction action)
 {
     auto locker = holdLock(*m_lock);
@@ -436 +436 @@
     if (--m_deferTerminationCount == 0) {
         VM& vm = this->vm();
-        if (m_suspendedTerminationException || vm.terminationInProgress())
-            vm.setException(vm.terminationException());
+        if (m_suspendedTerminationException
+            || ((action == DeferAction::DeferUntilEndOfScope) && vm.terminationInProgress()))
+            vm.throwTerminationException();
+        else if ((action == DeferAction::DeferForAWhile) && vm.terminationInProgress())
+            setTrapBit(NeedTermination); // Let the next trap check handle it.
     }
 }

trunk/Source/JavaScriptCore/runtime/VMTraps.h

@@ -192 +192 @@
     void* trapBitsAddress() { return &m_trapBits; }
 
+    enum class DeferAction {
+        DeferForAWhile,
+        DeferUntilEndOfScope
+    };
+
     bool isDeferringTermination() const { return m_deferTerminationCount; }
-    JS_EXPORT_PRIVATE void deferTermination();
-    JS_EXPORT_PRIVATE void undoDeferTermination();
+    JS_EXPORT_PRIVATE void deferTermination(DeferAction);
+    JS_EXPORT_PRIVATE void undoDeferTermination(DeferAction);
 
     void notifyGrabAllLocks()