Changeset 244295 in webkit

Timestamp:

Apr 15, 2019 2:55:33 PM (5 years ago)

Author:

Tadeu Zagallo

Message:

Incremental bytecode cache should not append function updates when loaded from memory
​https://bugs.webkit.org/show_bug.cgi?id=196865

Reviewed by Filip Pizlo.

JSTests:

• stress/bytecode-cache-shared-code-block.js: Added.

(b):
(program):

Source/JavaScriptCore:

Function updates hold the assumption that a function can only be executed/cached
after its containing code block has already been cached. This assumptions does
not hold if the UnlinkedCodeBlock is loaded from memory by the CodeCache, since
we might have two independent SourceProviders executing different paths of the
code and causing the same UnlinkedCodeBlock to be modified in memory.
Use a RefPtr instead of Ref for m_cachedBytecode in ShellSourceProvider to distinguish
between a new, empty cache and a cache that was not loaded and therefore cannot be updated.

• jsc.cpp:

(ShellSourceProvider::ShellSourceProvider):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/bytecode-cache-shared-code-block.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jsc.cpp (modified) (6 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Incremental bytecode cache should not append function updates when loaded from memory
+        https://bugs.webkit.org/show_bug.cgi?id=196865
+
+        Reviewed by Filip Pizlo.
+
+        * stress/bytecode-cache-shared-code-block.js: Added.
+        (b):
+        (program):
+
 2019-04-13  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Incremental bytecode cache should not append function updates when loaded from memory
+        https://bugs.webkit.org/show_bug.cgi?id=196865
+
+        Reviewed by Filip Pizlo.
+
+        Function updates hold the assumption that a function can only be executed/cached
+        after its containing code block has already been cached. This assumptions does
+        not hold if the UnlinkedCodeBlock is loaded from memory by the CodeCache, since
+        we might have two independent SourceProviders executing different paths of the
+        code and causing the same UnlinkedCodeBlock to be modified in memory.
+        Use a RefPtr instead of Ref for m_cachedBytecode in ShellSourceProvider to distinguish
+        between a new, empty cache and a cache that was not loaded and therefore cannot be updated.
+
+        * jsc.cpp:
+        (ShellSourceProvider::ShellSourceProvider):
+
 2019-04-15  Saam barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/jsc.cpp

@@ -973 +973 @@
     RefPtr<CachedBytecode> cachedBytecode() const override
     {
-        if (!m_cachedBytecode->size())
+        if (!m_cachedBytecode)
             loadBytecode();
         return m_cachedBytecode.copyRef();
@@ -980 +980 @@
     void updateCache(const UnlinkedFunctionExecutable* executable, const SourceCode&, CodeSpecializationKind kind, const UnlinkedFunctionCodeBlock* codeBlock) const override
     {
-        if (!cacheEnabled())
+        if (!cacheEnabled() || !m_cachedBytecode)
             return;
         Ref<CachedBytecode> cachedBytecode = encodeFunctionCodeBlock(*executable->vm(), codeBlock);
@@ -990 +990 @@
         if (!cacheEnabled())
             return;
+        if (!m_cachedBytecode)
+            m_cachedBytecode = CachedBytecode::create();
         m_cachedBytecode->addGlobalUpdate(generator());
     }
@@ -996 +998 @@
     {
 #if OS(DARWIN)
-        if (!cacheEnabled() || !m_cachedBytecode->hasUpdates())
+        if (!cacheEnabled() || !m_cachedBytecode || !m_cachedBytecode->hasUpdates())
             return;
 
         auto clearBytecode = makeScopeExit([&] {
-            m_cachedBytecode = CachedBytecode::create();
+            m_cachedBytecode = nullptr;
         });
 
@@ -1076 +1078 @@
     ShellSourceProvider(const String& source, const SourceOrigin& sourceOrigin, URL&& url, const TextPosition& startPosition, SourceProviderSourceType sourceType)
         : StringSourceProvider(source, sourceOrigin, WTFMove(url), startPosition, sourceType)
-        , m_cachedBytecode(CachedBytecode::create())
-    {
-        loadBytecode();
+    {
     }
 
@@ -1087 +1087 @@
     }
 
-    mutable Ref<CachedBytecode> m_cachedBytecode;
+    mutable RefPtr<CachedBytecode> m_cachedBytecode;
 };