CodeCache should check that the UnlinkedCodeBlock was successfully created before...
authortzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Apr 2019 08:54:19 +0000 (08:54 +0000)
committertzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Apr 2019 08:54:19 +0000 (08:54 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244241 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/bytecode-cache-syntax-error.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/CodeCache.cpp
Tools/ChangeLog
Tools/Scripts/jsc-stress-test-helpers/bytecode-cache-test-helper.sh
Tools/Scripts/run-jsc-stress-tests

index ec1aa6f..3d14bdf 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         r244079 logically broke shouldSpeculateInt52
diff --git a/JSTests/stress/bytecode-cache-syntax-error.js b/JSTests/stress/bytecode-cache-syntax-error.js
new file mode 100644 (file)
index 0000000..d8fea83
--- /dev/null
@@ -0,0 +1,9 @@
+//@ runBytecodeCacheNoAssertion
+
+try {
+    loadString('function(){}');
+    throw new Error('loadString should have thrown');
+} catch (err) {
+    if (err.message != 'Function statements must have a name.')
+        throw new Error('Unexpected exception');
+}
index 1242135..58bcb91 100644 (file)
@@ -1,3 +1,16 @@
+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>
 
         r244079 logically broke shouldSpeculateInt52
index 931348d..6ee965c 100644 (file)
@@ -80,12 +80,13 @@ UnlinkedCodeBlockType* CodeCache::getUnlinkedGlobalCodeBlock(VM& vm, ExecutableT
     VariableEnvironment variablesUnderTDZ;
     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;
 }
index 7561f3f..885217f 100644 (file)
@@ -1,3 +1,19 @@
+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>
 
         Update AudioSession route sharing policy
index 4f39e72..462fa22 100644 (file)
@@ -45,6 +45,8 @@ trap _trap_exit EXIT
 
 export JSC_diskCachePath=$diskCachePath
 mysys "$pathToVM" "$inputFile" "${extraOptions[@]}"
-export JSC_forceDiskCache=true
-mysys "$pathToVM" "$inputFile" "${extraOptions[@]}"
 
+if [ -z "$JSC_forceDiskCache" ]; then
+    export JSC_forceDiskCache=true
+fi
+mysys "$pathToVM" "$inputFile" "${extraOptions[@]}"
index 2abbb0b..7058583 100755 (executable)
@@ -656,13 +656,21 @@ def runDefault(*optionalTestSpecificOptions)
     run("default", *(FTL_OPTIONS + optionalTestSpecificOptions))
 end
 
-def runBytecodeCache(*optionalTestSpecificOptions)
+def runBytecodeCacheImpl(optionalTestSpecificOptions, *additionalEnv)
     unless $hostOS == "darwin"
         skip
         return
     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
 
 def runBigIntEnabled(*optionalTestSpecificOptions)