Changeset 202689 in webkit for trunk/Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp

Timestamp:

Jun 30, 2016, 11:13:26 AM (9 years ago)

Author:

[email protected]

Message:

Generators violate bytecode liveness validation
​https://bugs.webkit.org/show_bug.cgi?id=159279

Reviewed by Yusuke Suzuki.
PerformanceTests:

Add Basic to our test harness.

Also made some cosmetic changes to the benchmark harness.

• ES6SampleBench/Basic/basic-tests.yaml: Added.
• ES6SampleBench/Basic/stress-test.js: Added.

(preciseTime):

• ES6SampleBench/driver.js:

(Driver):
(Driver.prototype.start):
(Driver.prototype.reportError):

• ES6SampleBench/glue.js:
• ES6SampleBench/index.html:

Source/JavaScriptCore:

Fix a liveness bug found by Basic. The problem is that resume's intended liveness rule is:
"live-in is just the token argument", but the liveness analysis thought that the rule was
"live-in is live-out minus defs plus live-at-catch". Clearly these two rules are quite
different. The way this sort of worked before is that we would define the defs of resume
as being equal to our prediction of what the live-outs would be. We did this in the hope
that we would subtract all live-outs. But, this misses the live-at-catch part. So, this
change adds another hack to neutralize live-at-catch.

This would make a lot more sense if we wrote a new liveness analysis that was just for
generator conversion. It could reuse BytecodeUseDef but otherwise it would be a new thing.
It would be easy to write crazy rules for save/resume in such an analysis, especially if
that analysis rewrote the bytecode. We could then just have an op_yield that is a no-op.
We would just record the live-outs of op_yield and use that for rewriting the code in terms
of a switch statement.

• bytecode/BytecodeLivenessAnalysis.cpp:

(JSC::stepOverInstruction):
(JSC::BytecodeLivenessAnalysis::dumpResults):

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::dumpBytecode):

Tools:

Add Basic to our test harness.

• Scripts/run-javascriptcore-tests:

(runJSCStressTests):

File:

• trunk/Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp

@@ -133 +133 @@
     // exception handler block to be included in the live-in of this particular bytecode.
     if (HandlerInfo* handler = codeBlock->handlerForBytecodeOffset(bytecodeOffset)) {
-        BytecodeBasicBlock* handlerBlock = findBasicBlockWithLeaderOffset(basicBlocks, handler->target);
-        ASSERT(handlerBlock);
-        handlerBlock->in().forEachSetBit(use);
+        // FIXME: This resume check should not be needed.
+        // https://bugs.webkit.org/show_bug.cgi?id=159281
+        Interpreter* interpreter = codeBlock->vm()->interpreter;
+        Instruction* instructionsBegin = codeBlock->instructions().begin();
+        Instruction* instruction = &instructionsBegin[bytecodeOffset];
+        OpcodeID opcodeID = interpreter->getOpcodeID(instruction->u.opcode);
+        if (opcodeID != op_resume) {
+            BytecodeBasicBlock* handlerBlock = findBasicBlockWithLeaderOffset(basicBlocks, handler->target);
+            ASSERT(handlerBlock);
+            handlerBlock->in().forEachSetBit(use);
+        }
     }
 }
@@ -290 +298 @@
 void BytecodeLivenessAnalysis::dumpResults()
 {
+    dataLog("\nDumping bytecode liveness for ", *m_codeBlock, ":\n");
     Interpreter* interpreter = m_codeBlock->vm()->interpreter;
     Instruction* instructionsBegin = m_codeBlock->instructions().begin();