Changeset 244241 in webkit

Timestamp:

Apr 13, 2019 1:54:19 AM (5 years ago)

Author:

Tadeu Zagallo

Message:

CodeCache should check that the UnlinkedCodeBlock was successfully created before caching it
​https://bugs.webkit.org/show_bug.cgi?id=196880

Reviewed by Yusuke Suzuki.

JSTests:

• stress/bytecode-cache-syntax-error.js: Added.

(catch):

Source/JavaScriptCore:

CodeCache should not tell the SourceProvider to cache the bytecode if it failed
to create the UnlinkedCodeBlock.

• runtime/CodeCache.cpp:

(JSC::CodeCache::getUnlinkedGlobalCodeBlock):

Tools:

Add a new function for bytecode cache tests that does not forceDiskCache
for the second run: runBytecodeCacheNoAssetion. This is necessary for the
test added in this patch, since the code is invalid and therefore won't be
cached. It should also be useful for tests that evaluate dynamically
generated code.

• Scripts/jsc-stress-test-helpers/bytecode-cache-test-helper.sh:
• Scripts/run-jsc-stress-tests:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/bytecode-cache-syntax-error.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/CodeCache.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/Scripts/jsc-stress-test-helpers/bytecode-cache-test-helper.sh (modified) (1 diff)
• Tools/Scripts/run-jsc-stress-tests (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-04-13  Tadeu Zagallo  <tzagallo@apple.com>
+
+        CodeCache should check that the UnlinkedCodeBlock was successfully created before caching it
+        https://bugs.webkit.org/show_bug.cgi?id=196880
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/bytecode-cache-syntax-error.js: Added.
+        (catch):
+
 2019-04-12  Saam barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-13  Tadeu Zagallo  <tzagallo@apple.com>
+
+        CodeCache should check that the UnlinkedCodeBlock was successfully created before caching it
+        https://bugs.webkit.org/show_bug.cgi?id=196880
+
+        Reviewed by Yusuke Suzuki.
+
+        CodeCache should not tell the SourceProvider to cache the bytecode if it failed
+        to create the UnlinkedCodeBlock.
+
+        * runtime/CodeCache.cpp:
+        (JSC::CodeCache::getUnlinkedGlobalCodeBlock):
+
 2019-04-12  Saam barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/runtime/CodeCache.cpp

@@ -81 +81 @@
     unlinkedCodeBlock = generateUnlinkedCodeBlock<UnlinkedCodeBlockType, ExecutableType>(vm, executable, source, strictMode, scriptMode, debuggerMode, error, evalContextType, &variablesUnderTDZ);
 
-    if (unlinkedCodeBlock && Options::useCodeCache())
+    if (unlinkedCodeBlock && Options::useCodeCache()) {
         m_sourceCode.addCache(key, SourceCodeValue(vm, unlinkedCodeBlock, m_sourceCode.age()));
 
-    key.source().provider().cacheBytecode([&] {
-        return encodeCodeBlock(vm, key, unlinkedCodeBlock);
-    });
+        key.source().provider().cacheBytecode([&] {
+            return encodeCodeBlock(vm, key, unlinkedCodeBlock);
+        });
+    }
 
     return unlinkedCodeBlock;

trunk/Tools/ChangeLog

@@ +1 @@
+2019-04-13  Tadeu Zagallo  <tzagallo@apple.com>
+
+        CodeCache should check that the UnlinkedCodeBlock was successfully created before caching it
+        https://bugs.webkit.org/show_bug.cgi?id=196880
+
+        Reviewed by Yusuke Suzuki.
+
+        Add a new function for bytecode cache tests that does not forceDiskCache
+        for the second run: runBytecodeCacheNoAssetion. This is necessary for the
+        test added in this patch, since the code is invalid and therefore won't be
+        cached. It should also be useful for tests that evaluate dynamically
+        generated code.
+
+        * Scripts/jsc-stress-test-helpers/bytecode-cache-test-helper.sh:
+        * Scripts/run-jsc-stress-tests:
+
 2019-04-12  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Tools/Scripts/jsc-stress-test-helpers/bytecode-cache-test-helper.sh

@@ -46 +46 @@
 export JSC_diskCachePath=$diskCachePath
 mysys "$pathToVM" "$inputFile" "${extraOptions[@]}"
-export JSC_forceDiskCache=true
+
+if [ -z "$JSC_forceDiskCache" ]; then
+    export JSC_forceDiskCache=true
+fi
 mysys "$pathToVM" "$inputFile" "${extraOptions[@]}"
-

trunk/Tools/Scripts/run-jsc-stress-tests

@@ -657 +657 @@
 end
 
-def runBytecodeCache(*optionalTestSpecificOptions)
+def runBytecodeCacheImpl(optionalTestSpecificOptions, *additionalEnv)
     unless $hostOS == "darwin"
         skip
@@ -663 +663 @@
     end
     options = BASE_OPTIONS + $testSpecificRequiredOptions + FTL_OPTIONS + optionalTestSpecificOptions
-    addRunCommand("bytecode-cache", ["sh", (pathToHelpers + "bytecode-cache-test-helper.sh").to_s, pathToVM.to_s, $benchmark.to_s] + options, silentOutputHandler, simpleErrorHandler)
+    addRunCommand("bytecode-cache", ["sh", (pathToHelpers + "bytecode-cache-test-helper.sh").to_s, pathToVM.to_s, $benchmark.to_s] + options, silentOutputHandler, simpleErrorHandler, *additionalEnv)
+end
+
+def runBytecodeCache(*optionalTestSpecificOptions)
+    runBytecodeCacheImpl(optionalTestSpecificOptions)
+end
+
+def runBytecodeCacheNoAssertion(*optionalTestSpecificOptions)
+    runBytecodeCacheImpl(optionalTestSpecificOptions, "JSC_forceDiskCache=false")
 end