Modified: trunk/Source/_javascript_Core/ChangeLog (208403 => 208404)
--- trunk/Source/_javascript_Core/ChangeLog 2016-11-04 22:53:33 UTC (rev 208403)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-11-04 23:02:19 UTC (rev 208404)
@@ -1,3 +1,40 @@
+2016-11-02 Geoffrey Garen <[email protected]>
+
+ EvalCodeCache should not give up in strict mode and other cases
+ https://bugs.webkit.org/show_bug.cgi?id=164357
+
+ Reviewed by Michael Saboff.
+
+ EvalCodeCache gives up in non-trivial cases because generated eval code
+ can't soundly migrate from, for example, a let scope to a non-let scope.
+ The number of cases has grown over time.
+
+ Instead, let's cache eval code based on the location of the call to
+ eval(). That way, we never relocate the code, and it's sound to make
+ normal assumptions about our surrounding scope.
+
+ * bytecode/EvalCodeCache.h:
+ (JSC::EvalCodeCache::CacheKey::CacheKey): Use CallSiteIndex to uniquely
+ identify the location of our call to eval().
+
+ (JSC::EvalCodeCache::CacheKey::hash):
+ (JSC::EvalCodeCache::CacheKey::operator==):
+ (JSC::EvalCodeCache::CacheKey::Hash::equal): Use CallSiteIndex instead
+ of lots of other flags.
+
+ (JSC::EvalCodeCache::tryGet): No need to include details that are implied
+ by our CallSiteIndex.
+
+ (JSC::EvalCodeCache::getSlow): No need to skip caching in complex
+ situations. We promise we'll never relocate the cached code.
+
+ (JSC::EvalCodeCache::isCacheableScope): Deleted.
+ (JSC::EvalCodeCache::isCacheable): Deleted.
+
+ * interpreter/Interpreter.cpp:
+ (JSC::eval): Pass through a CallSiteIndex to uniquely identify this call
+ to eval().
+
2016-11-04 Keith Miller <[email protected]>
Add support for Wasm br_table
Modified: trunk/Source/_javascript_Core/bytecode/EvalCodeCache.h (208403 => 208404)
--- trunk/Source/_javascript_Core/bytecode/EvalCodeCache.h 2016-11-04 22:53:33 UTC (rev 208403)
+++ trunk/Source/_javascript_Core/bytecode/EvalCodeCache.h 2016-11-04 23:02:19 UTC (rev 208404)
@@ -47,9 +47,9 @@
// Specialized cache key (compared with SourceCodeKey) for eval code cache.
class CacheKey {
public:
- CacheKey(const String& source, DerivedContextType derivedContextType, EvalContextType evalContextType, bool isArrowFunctionContext)
+ CacheKey(const String& source, CallSiteIndex callSiteIndex)
: m_source(source.impl())
- , m_flags(SourceCodeType::EvalType, JSParserBuiltinMode::NotBuiltin, JSParserStrictMode::NotStrict, JSParserScriptMode::Classic, derivedContextType, evalContextType, isArrowFunctionContext)
+ , m_callSiteIndex(callSiteIndex)
{
}
@@ -60,13 +60,13 @@
CacheKey() = default;
- unsigned hash() const { return m_source->hash(); }
+ unsigned hash() const { return m_source->hash() ^ m_callSiteIndex.bits(); }
bool isEmptyValue() const { return !m_source; }
bool operator==(const CacheKey& other) const
{
- return m_source == other.m_source && m_flags == other.m_flags;
+ return m_callSiteIndex == other.m_callSiteIndex && m_source == other.m_source;
}
bool isHashTableDeletedValue() const { return m_source.isHashTableDeletedValue(); }
@@ -78,7 +78,7 @@
}
static bool equal(const CacheKey& lhs, const CacheKey& rhs)
{
- return StringHash::equal(lhs.m_source, rhs.m_source) && lhs.m_flags == rhs.m_flags;
+ return lhs == rhs;
}
static const bool safeToCompareToEmptyOrDeleted = false;
};
@@ -87,19 +87,15 @@
private:
RefPtr<StringImpl> m_source;
- SourceCodeFlags m_flags;
+ CallSiteIndex m_callSiteIndex;
};
- EvalExecutable* tryGet(bool inStrictContext, const String& evalSource, DerivedContextType derivedContextType, EvalContextType evalContextType, bool isArrowFunctionContext, JSScope* scope)
+ EvalExecutable* tryGet(const String& evalSource, CallSiteIndex callSiteIndex)
{
- if (isCacheable(inStrictContext, evalSource, scope)) {
- ASSERT(!inStrictContext);
- return m_cacheMap.fastGet(CacheKey(evalSource, derivedContextType, evalContextType, isArrowFunctionContext)).get();
- }
- return nullptr;
+ return m_cacheMap.fastGet(CacheKey(evalSource, callSiteIndex)).get();
}
- EvalExecutable* getSlow(ExecState* exec, JSCell* owner, bool inStrictContext, DerivedContextType derivedContextType, EvalContextType evalContextType, bool isArrowFunctionContext, const String& evalSource, JSScope* scope)
+ EvalExecutable* getSlow(ExecState* exec, JSCell* owner, const String& evalSource, CallSiteIndex callSiteIndex, bool inStrictContext, DerivedContextType derivedContextType, EvalContextType evalContextType, bool isArrowFunctionContext, JSScope* scope)
{
VariableEnvironment variablesUnderTDZ;
JSScope::collectVariablesUnderTDZ(scope, variablesUnderTDZ);
@@ -107,11 +103,9 @@
if (!evalExecutable)
return nullptr;
- if (isCacheable(inStrictContext, evalSource, scope) && m_cacheMap.size() < maxCacheEntries) {
- ASSERT(!inStrictContext);
- m_cacheMap.set(CacheKey(evalSource, derivedContextType, evalContextType, isArrowFunctionContext), WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
- }
-
+ if (m_cacheMap.size() < maxCacheEntries)
+ m_cacheMap.set(CacheKey(evalSource, callSiteIndex), WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
+
return evalExecutable;
}
@@ -125,19 +119,6 @@
}
private:
- ALWAYS_INLINE bool isCacheableScope(JSScope* scope)
- {
- return scope->isGlobalLexicalEnvironment() || scope->isFunctionNameScopeObject() || scope->isVarScope();
- }
-
- ALWAYS_INLINE bool isCacheable(bool inStrictContext, const String& evalSource, JSScope* scope)
- {
- // If eval() is called and it has access to a lexical scope, we can't soundly cache it.
- // If the eval() only has access to the "var" scope, then we can cache it.
- return !inStrictContext
- && static_cast<size_t>(evalSource.length()) < Options::maximumEvalCacheableSourceLength()
- && isCacheableScope(scope);
- }
static const int maxCacheEntries = 64;
typedef HashMap<CacheKey, WriteBarrier<EvalExecutable>, CacheKey::Hash, CacheKey::HashTraits> EvalCacheMap;
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (208403 => 208404)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-11-04 22:53:33 UTC (rev 208403)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-11-04 23:02:19 UTC (rev 208404)
@@ -109,6 +109,7 @@
RETURN_IF_EXCEPTION(scope, JSValue());
CallFrame* callerFrame = callFrame->callerFrame();
+ CallSiteIndex callerCallSiteIndex = callerFrame->callSiteIndex();
CodeBlock* callerCodeBlock = callerFrame->codeBlock();
JSScope* callerScopeChain = callerFrame->uncheckedR(callerCodeBlock->scopeRegister().offset()).Register::scope();
UnlinkedCodeBlock* callerUnlinkedCodeBlock = callerCodeBlock->unlinkedCodeBlock();
@@ -130,7 +131,7 @@
else
evalContextType = EvalContextType::None;
- EvalExecutable* eval = callerCodeBlock->evalCodeCache().tryGet(callerCodeBlock->isStrictMode(), programSource, derivedContextType, evalContextType, isArrowFunctionContext, callerScopeChain);
+ EvalExecutable* eval = callerCodeBlock->evalCodeCache().tryGet(programSource, callerCallSiteIndex);
if (!eval) {
if (!callerCodeBlock->isStrictMode()) {
if (programSource.is8Bit()) {
@@ -147,7 +148,7 @@
// If the literal parser bailed, it should not have thrown exceptions.
ASSERT(!scope.exception());
- eval = callerCodeBlock->evalCodeCache().getSlow(callFrame, callerCodeBlock, callerCodeBlock->isStrictMode(), derivedContextType, evalContextType, isArrowFunctionContext, programSource, callerScopeChain);
+ eval = callerCodeBlock->evalCodeCache().getSlow(callFrame, callerCodeBlock, programSource, callerCallSiteIndex, callerCodeBlock->isStrictMode(), derivedContextType, evalContextType, isArrowFunctionContext, callerScopeChain);
if (!eval)
return jsUndefined();
