Changeset 273931 in webkit for trunk/Source/JavaScriptCore/bytecompiler

Timestamp:

Mar 4, 2021, 4:18:23 PM (4 years ago)

Author:

[email protected]

Message:

Don't trust parsing information to tell us if we've emitted op_call_eval
​https://bugs.webkit.org/show_bug.cgi?id=222694
rdar://74778016

Reviewed by Yusuke Suzuki.

JSTests:

• stress/eval-liveness-should-not-come-from-parser.js: Added.

(foo):

Source/JavaScriptCore:

In the DFG, op_call_eval can't be inlined. Not inlining is required for how
eval is currently implemented in the DFG. For CodeBlocks with eval in them,
the scope register is also alive everywhere.

When doing spread of arguments in eval, we end up emitting a call varargs
instead of a direct eval. This seems like a spec bug:
​https://bugs.webkit.org/show_bug.cgi?id=222671

However, this leads to something that had eval textually in it leading to
us reporting the scope register is always alive, even if op_call_eval isn't
in the bytecode stream. This leads to a validation error, since the DFG
isn't actually keeping this scope register alive everywhere, because
op_call_eval isn't in the bytecode stream.

This patch fixes this by having a bit indicating if op_call_eval is in
the bytecode stream or not.

• bytecode/BytecodeUseDef.h:

(JSC::computeUsesForBytecodeIndex):

• bytecode/CodeBlock.h:

(JSC::CodeBlock::usesCallEval const):
(JSC::CodeBlock::usesEval const): Deleted.

• bytecode/ExecutableInfo.h:

(JSC::ExecutableInfo::ExecutableInfo):
(JSC::ExecutableInfo::usesEval const): Deleted.

• bytecode/UnlinkedCodeBlock.cpp:

(JSC::UnlinkedCodeBlock::UnlinkedCodeBlock):

• bytecode/UnlinkedCodeBlock.h:

(JSC::UnlinkedCodeBlock::usesCallEval const):
(JSC::UnlinkedCodeBlock::setUsesCallEval):
(JSC::UnlinkedCodeBlock::usesEval const): Deleted.

• bytecode/UnlinkedCodeBlockGenerator.h:

(JSC::UnlinkedCodeBlockGenerator::usesCallEval const):
(JSC::UnlinkedCodeBlockGenerator::setUsesCallEval):
(JSC::UnlinkedCodeBlockGenerator::usesEval const): Deleted.

• bytecode/UnlinkedFunctionExecutable.cpp:

(JSC::generateUnlinkedFunctionCodeBlock):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::emitCall):
(JSC::BytecodeGenerator::isThisUsedInInnerArrowFunction):
(JSC::BytecodeGenerator::isNewTargetUsedInInnerArrowFunction):
(JSC::BytecodeGenerator::isSuperUsedInInnerArrowFunction):
(JSC::BytecodeGenerator::isSuperCallUsedInInnerArrowFunction):

• dfg/DFGGraph.h:
• runtime/CachedTypes.cpp:

(JSC::CachedCodeBlock::usesCallEval const):
(JSC::UnlinkedCodeBlock::UnlinkedCodeBlock):
(JSC::CachedCodeBlock<CodeBlockType>::encode):
(JSC::CachedCodeBlock::usesEval const): Deleted.

• runtime/CodeCache.cpp:

(JSC::generateUnlinkedCodeBlockImpl):

• runtime/EvalExecutable.h:

(JSC::EvalExecutable::executableInfo const): Deleted.

• runtime/ModuleProgramExecutable.h:
• runtime/ProgramExecutable.h:
• runtime/ScriptExecutable.h:

(JSC::ScriptExecutable::usesEval const): Deleted.

File:

• trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (10 diffs)

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -348 +348 @@
     , m_expressionTooDeep(false)
     , m_isBuiltinFunction(codeBlock->isBuiltinFunction())
-    , m_usesNonStrictEval(codeBlock->usesEval() && !ecmaMode.isStrict())
+    , m_usesNonStrictEval(functionNode->usesEval() && !ecmaMode.isStrict())
     // FIXME: We should be able to have tail call elimination with the profiler
     // enabled. This is currently not possible because the profiler expects
@@ -380 +380 @@
     SourceParseMode parseMode = codeBlock->parseMode();
 
-    bool containsArrowOrEvalButNotInArrowBlock = ((functionNode->usesArrowFunction() && functionNode->doAnyInnerArrowFunctionsUseAnyFeature()) || functionNode->usesEval()) && !m_codeBlock->isArrowFunction();
+    bool containsArrowOrEvalButNotInArrowBlock = ((functionNode->usesArrowFunction() && functionNode->doAnyInnerArrowFunctionsUseAnyFeature()) || usesEval()) && !m_codeBlock->isArrowFunction();
     bool shouldCaptureSomeOfTheThings = shouldEmitDebugHooks() || functionNode->needsActivation() || containsArrowOrEvalButNotInArrowBlock;
 
-    bool shouldCaptureAllOfTheThings = shouldEmitDebugHooks() || codeBlock->usesEval();
-    bool needsArguments = ((functionNode->usesArguments() && !codeBlock->isArrowFunction()) || codeBlock->usesEval() || (functionNode->usesArrowFunction() && !codeBlock->isArrowFunction() && isArgumentsUsedInInnerArrowFunction())) && parseMode != SourceParseMode::ClassFieldInitializerMode;
+    bool shouldCaptureAllOfTheThings = shouldEmitDebugHooks() || usesEval();
+    bool needsArguments = ((functionNode->usesArguments() && !codeBlock->isArrowFunction()) || usesEval() || (functionNode->usesArrowFunction() && !codeBlock->isArrowFunction() && isArgumentsUsedInInnerArrowFunction())) && parseMode != SourceParseMode::ClassFieldInitializerMode;
 
     if (isGeneratorOrAsyncFunctionBodyParseMode(parseMode)) {
@@ -446 +446 @@
         ASSERT(parseMode != SourceParseMode::GeneratorBodyMode);
         ASSERT(!isAsyncFunctionBodyParseMode(parseMode));
-        bool isDynamicScope = functionNameScopeIsDynamic(codeBlock->usesEval(), ecmaMode.isStrict());
+        bool isDynamicScope = functionNameScopeIsDynamic(usesEval(), ecmaMode.isStrict());
         bool isFunctionNameCaptured = captures(functionNode->ident().impl());
         bool markAsCaptured = isDynamicScope || isFunctionNameCaptured;
@@ -643 +643 @@
 
 
-    if (functionNode->needsNewTargetRegisterForThisScope() || isNewTargetUsedInInnerArrowFunction() || codeBlock->usesEval())
+    if (functionNode->needsNewTargetRegisterForThisScope() || isNewTargetUsedInInnerArrowFunction() || usesEval())
         m_newTargetRegister = addVar();
 
@@ -725 +725 @@
                 case ConstructorKind::None: {
                     bool shouldEmitToThis = false;
-                    if (functionNode->usesThis() || codeBlock->usesEval() || m_scopeNode->doAnyInnerArrowFunctionsUseThis() || m_scopeNode->doAnyInnerArrowFunctionsUseEval())
+                    if (functionNode->usesThis() || usesEval() || m_scopeNode->doAnyInnerArrowFunctionsUseThis() || m_scopeNode->doAnyInnerArrowFunctionsUseEval())
                         shouldEmitToThis = true;
                     else if ((functionNode->usesSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty()) && !ecmaMode.isStrict()) {
@@ -856 +856 @@
     , m_expressionTooDeep(false)
     , m_isBuiltinFunction(false)
-    , m_usesNonStrictEval(codeBlock->usesEval() && !ecmaMode.isStrict())
+    , m_usesNonStrictEval(evalNode->usesEval() && !ecmaMode.isStrict())
     , m_inTailPosition(false)
     , m_needsToUpdateArrowFunctionContext(evalNode->usesArrowFunction() || evalNode->usesEval())
@@ -932 +932 @@
     moduleEnvironmentSymbolTable->setScopeType(SymbolTable::ScopeType::LexicalScope);
 
-    bool shouldCaptureAllOfTheThings = shouldEmitDebugHooks() || codeBlock->usesEval();
+    bool shouldCaptureAllOfTheThings = shouldEmitDebugHooks() || usesEval();
     if (shouldCaptureAllOfTheThings)
         moduleProgramNode->varDeclarations().markAllVariablesAsCaptured();
@@ -3451 +3451 @@
     ASSERT(dst);
     ASSERT(dst != ignoredResult());
-    if constexpr (opcodeID == op_call_eval)
+    if constexpr (opcodeID == op_call_eval) {
+        m_codeBlock->setUsesCallEval();
         CallOp::emit(this, dst, func, callArguments.argumentCountIncludingThis(), callArguments.stackOffset(), ecmaMode());
-    else
+    } else
         CallOp::emit(this, dst, func, callArguments.argumentCountIncludingThis(), callArguments.stackOffset());
     
@@ -4633 +4634 @@
 bool BytecodeGenerator::isThisUsedInInnerArrowFunction()
 {
-    return m_scopeNode->doAnyInnerArrowFunctionsUseThis() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || m_codeBlock->usesEval();
+    return m_scopeNode->doAnyInnerArrowFunctionsUseThis() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || usesEval();
 }
     
@@ -4643 +4644 @@
 bool BytecodeGenerator::isNewTargetUsedInInnerArrowFunction()
 {
-    return m_scopeNode->doAnyInnerArrowFunctionsUseNewTarget() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || m_codeBlock->usesEval();
+    return m_scopeNode->doAnyInnerArrowFunctionsUseNewTarget() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || usesEval();
 }
 
 bool BytecodeGenerator::isSuperUsedInInnerArrowFunction()
 {
-    return m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || m_codeBlock->usesEval();
+    return m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || usesEval();
 }
 
 bool BytecodeGenerator::isSuperCallUsedInInnerArrowFunction()
 {
-    return m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || m_codeBlock->usesEval();
+    return m_scopeNode->doAnyInnerArrowFunctionsUseSuperCall() || m_scopeNode->doAnyInnerArrowFunctionsUseEval() || usesEval();
 }