Attachment 370398 Details for Bug 198120 – Patch

[patch] Patch

bug-198120-20190522173545.patch (text/plain), 4.32 KB, created by Tadeu Zagallo on 2019-05-22 08:35:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-22 08:35:46 PDT

Size: 4.32 KB

patch

obsolete

>Subversion Revision: 245619
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 1a77084e64e556f53a40130df0a827e0c22876bf..3d9a2250b83eff2971a82e1c4266e11586c13845 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,20 @@
>+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 NOBODY (OOPS!).
>+
>+        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-21  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         [PlayStation] Don't call fcntl.
>diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>index 2e90c701da1ec75b1822aa6d7b3be7c7426f25b7..503e60e2c63cd36e3e2f05d2375e9e74c5aca2f0 100644
>--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>@@ -803,6 +803,8 @@ LLINT_SLOW_PATH_DECL(slow_path_get_by_id)
>         JSCell* baseCell = baseValue.asCell();
>         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;
>             metadata.m_modeMetadata.defaultMode.structureID = 0;
>@@ -814,8 +816,6 @@ LLINT_SLOW_PATH_DECL(slow_path_get_by_id)
>             if (structure->propertyAccessesAreCacheable()
>                 && !structure->needImpurePropertyWatchpoint()) {
>                 vm.heap.writeBarrier(codeBlock);
>-                
>-                ConcurrentJSLocker locker(codeBlock->m_lock);
> 
>                 metadata.m_modeMetadata.defaultMode.structureID = structure->id();
>                 metadata.m_modeMetadata.defaultMode.cachedOffset = slot.cachedOffset();
>@@ -829,6 +829,7 @@ LLINT_SLOW_PATH_DECL(slow_path_get_by_id)
>     } else if (!LLINT_ALWAYS_ACCESS_SLOW
>         && 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));
>         metadata.m_modeMetadata.arrayLengthMode.arrayProfile.observeStructure(baseValue.asCell()->structure(vm));
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 4e4f88656ac84a3bc0bf726ebf35439d91e305ad..7faf221ced39b0920dd1ce3ca07c5d789f2f6645 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+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 NOBODY (OOPS!).
>+
>+        * stress/get-array-length-concurrently-change-mode.js: Added.
>+        (main):
>+
> 2019-05-20  Keith Miller  <keith_miller@apple.com>
> 
>         Cleanup Yarr regexp code around paren contexts.
>diff --git a/JSTests/stress/get-array-length-concurrently-change-mode.js b/JSTests/stress/get-array-length-concurrently-change-mode.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..06f4783826daafd6718c3f7a9c1aecda67e9e742
>--- /dev/null
>+++ b/JSTests/stress/get-array-length-concurrently-change-mode.js
>@@ -0,0 +1,19 @@
>+//@ requireOptions("--watchdog=10000", "--watchdog-exception-ok")
>+function main() {
>+    runString(`
>+        function bar(_a) {
>+          eval(_a);
>+          arguments.length = 0;
>+          var array = [
>+            arguments,
>+            [0]
>+          ];
>+          var result = 0;
>+          for (var i = 0; i < 1000000; ++i)
>+            result += array[i % array.length].length;
>+        }
>+        bar('bar()');
>+    `);
>+    main();
>+}
>+main();

Actions: View | Formatted Diff | Diff

Attachments on bug 198120: 370398