Changeset 245652 in webkit

Timestamp:

May 22, 2019 3:19:38 PM (5 years ago)

Author:

Tadeu Zagallo

Message:

JSTests:
llint_slow_path_get_by_id needs to hold the CodeBlock's to update the metadata's mode
​https://bugs.webkit.org/show_bug.cgi?id=198120
<rdar://problem/49668795>

Reviewed by Michael Saboff.

• stress/get-array-length-concurrently-change-mode.js: Added.

(main):

Source/JavaScriptCore:
llint_slow_path_get_by_id needs to hold the CodeBlock's lock to update the metadata's mode
​https://bugs.webkit.org/show_bug.cgi?id=198120
<rdar://problem/49668795>

Reviewed by Michael Saboff.

There are two places in llint_slow_path_get_by_id where we change the
metadata's mode without holding the CodeBlock's lock. This is an issue
when switching to and from ArrayLength mode, since other places can
either get a pointer to an array profile that will be overwritten or
an array profile that hasn't yet been initialized.

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/get-array-length-concurrently-change-mode.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/llint/LLIntSlowPaths.cpp (modified) (3 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-05-22  Tadeu Zagallo  <tzagallo@apple.com>
+
+        llint_slow_path_get_by_id needs to hold the CodeBlock's to update the metadata's mode
+        https://bugs.webkit.org/show_bug.cgi?id=198120
+        <rdar://problem/49668795>
+
+        Reviewed by Michael Saboff.
+
+        * stress/get-array-length-concurrently-change-mode.js: Added.
+        (main):
+
 2019-05-22  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-05-22  Tadeu Zagallo  <tzagallo@apple.com>
+
+        llint_slow_path_get_by_id needs to hold the CodeBlock's lock to update the metadata's mode
+        https://bugs.webkit.org/show_bug.cgi?id=198120
+        <rdar://problem/49668795>
+
+        Reviewed by Michael Saboff.
+
+        There are two places in llint_slow_path_get_by_id where we change the
+        metadata's mode without holding the CodeBlock's lock. This is an issue
+        when switching to and from ArrayLength mode, since other places can
+        either get a pointer to an array profile that will be overwritten or
+        an array profile that hasn't yet been initialized.
+
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+
 2019-05-22  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -804 +804 @@
         Structure* structure = baseCell->structure(vm);
         if (slot.isValue() && slot.slotBase() == baseValue) {
+            ConcurrentJSLocker locker(codeBlock->m_lock);
+
             // Start out by clearing out the old cache.
             metadata.m_mode = GetByIdMode::Default;
@@ -815 +817 @@
                 && !structure->needImpurePropertyWatchpoint()) {
                 vm.heap.writeBarrier(codeBlock);
-                
-                ConcurrentJSLocker locker(codeBlock->m_lock);
 
                 metadata.m_modeMetadata.defaultMode.structureID = structure->id();
@@ -830 +830 @@
         && isJSArray(baseValue)
         && ident == vm.propertyNames->length) {
+        ConcurrentJSLocker locker(codeBlock->m_lock);
         metadata.m_mode = GetByIdMode::ArrayLength;
         new (&metadata.m_modeMetadata.arrayLengthMode.arrayProfile) ArrayProfile(codeBlock->bytecodeOffset(pc));