Changeset 178692 in webkit for trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

Timestamp:

Jan 19, 2015, 8:47:55 PM (10 years ago)

Author:

[email protected]

Message:

Basic block start offsets should never be larger than end offsets in the control flow profiler
​https://bugs.webkit.org/show_bug.cgi?id=140377

Reviewed by Filip Pizlo.

The bytecode generator will emit code more than once for some AST nodes. For instance,
the finally block of TryNode will emit two code paths for its finally block: one for
the normal path, and another for the path where an exception is thrown in the catch block.

This repeated code emission of the same AST node previously broke how the control
flow profiler computed text ranges of basic blocks because when the same AST node
is emitted multiple times, there is a good chance that there are ranges that span
from the end offset of one of these duplicated nodes back to the start offset of
the same duplicated node. This caused a basic block range to report a larger start
offset than end offset. This was incorrect. Now, when this situation is encountered
while linking a CodeBlock, the faulty range in question is ignored.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::CodeBlock):
(JSC::CodeBlock::insertBasicBlockBoundariesForControlFlowProfiler):

• bytecode/CodeBlock.h:
• bytecompiler/NodesCodegen.cpp:

(JSC::ForInNode::emitMultiLoopBytecode):
(JSC::ForOfNode::emitBytecode):
(JSC::TryNode::emitBytecode):

• parser/Parser.cpp:

(JSC::Parser<LexerType>::parseConditionalExpression):

• runtime/ControlFlowProfiler.cpp:

(JSC::ControlFlowProfiler::ControlFlowProfiler):

• runtime/ControlFlowProfiler.h:

(JSC::ControlFlowProfiler::dummyBasicBlock):

File:

• trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -2111 +2111 @@
         this->emitLoopHeader(generator, propertyName.get());
 
+        generator.emitProfileControlFlow(m_statement->startOffset());
+
         generator.pushIndexedForInScope(local.get(), i.get());
         generator.emitNode(dst, m_statement);
         generator.popIndexedForInScope(local.get());
+
+        generator.emitProfileControlFlow(m_statement->endOffset());
 
         generator.emitLabel(scope->continueTarget());
@@ -2145 +2149 @@
         this->emitLoopHeader(generator, propertyName.get());
 
+        generator.emitProfileControlFlow(m_statement->startOffset());
+
         generator.pushStructureForInScope(local.get(), i.get(), propertyName.get(), structureEnumerator.get());
         generator.emitNode(dst, m_statement);
         generator.popStructureForInScope(local.get());
+
+        generator.emitProfileControlFlow(m_statement->endOffset());
 
         generator.emitLabel(scope->continueTarget());
@@ -2178 +2186 @@
         this->emitLoopHeader(generator, propertyName.get());
 
+        generator.emitProfileControlFlow(m_statement->startOffset());
+
         generator.emitNode(dst, m_statement);
 
@@ -2197 +2207 @@
     generator.emitDebugHook(WillExecuteStatement, firstLine(), startOffset(), lineStartOffset());
     generator.emitLabel(end.get());
+    generator.emitProfileControlFlow(m_statement->endOffset());
 }
 
@@ -2248 +2259 @@
             assignNode->bindings()->bindValue(generator, value);
         }
+        generator.emitProfileControlFlow(m_statement->startOffset());
         generator.emitNode(dst, m_statement);
     };
     generator.emitEnumeration(this, m_expr, extractor);
+    generator.emitProfileControlFlow(m_statement->endOffset());
 }
 
@@ -2609 +2622 @@
         RefPtr<Label> finallyEndLabel = generator.newLabel();
 
-        // FIXME: To the JS programmer, running the normal path is the same basic block as running the uncaught exception path.
-        // But, we generate two different code paths for this, but we shouldn't generate two op_profile_control_flows for these because they
-        // logically represent the same basic block.
-        // https://bugs.webkit.org/show_bug.cgi?id=139287
-        if (m_catchBlock)
-            generator.emitProfileControlFlow(m_catchBlock->endOffset());
-        else
-            generator.emitProfileControlFlow(m_tryBlock->endOffset());
+        int finallyStartOffset = m_catchBlock ? m_catchBlock->endOffset() : m_tryBlock->endOffset();
 
         // Normal path: run the finally code, and jump to the end.
+        generator.emitProfileControlFlow(finallyStartOffset);
         generator.emitNode(dst, m_finallyBlock);
+        generator.emitProfileControlFlow(m_finallyBlock->endOffset());
         generator.emitJump(finallyEndLabel.get());
 
         // Uncaught exception path: invoke the finally block, then re-throw the exception.
         RefPtr<RegisterID> tempExceptionRegister = generator.popTryAndEmitCatch(tryData, generator.newTemporary(), preFinallyLabel.get());
+        generator.emitProfileControlFlow(finallyStartOffset);
         generator.emitNode(dst, m_finallyBlock);
         generator.emitThrow(tempExceptionRegister.get());
 
         generator.emitLabel(finallyEndLabel.get());
-    } else 
+        generator.emitProfileControlFlow(m_finallyBlock->endOffset());
+    } else
         generator.emitProfileControlFlow(m_catchBlock->endOffset());