Assertion failure for direct eval in non-class method
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 May 2016 14:28:30 +0000 (14:28 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 May 2016 14:28:30 +0000 (14:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157138

Reviewed by Saam Barati.

This assertion was incorrect. In method definitions in object literals,
it can be sloppy mode, but its DerivedContextType may not be DerivedContextType::None.

* bytecode/EvalCodeCache.h:
(JSC::EvalCodeCache::CacheKey::CacheKey):
(JSC::EvalCodeCache::CacheKey::operator==):
(JSC::EvalCodeCache::CacheKey::Hash::equal):
(JSC::EvalCodeCache::tryGet):
(JSC::EvalCodeCache::getSlow):
* interpreter/Interpreter.cpp:
(JSC::eval):
* tests/stress/direct-eval-in-object-literal-methods.js: Added.
(shouldBe):
(throw.new.Error):
(shouldBe.Parent.prototype.l):
(shouldBe.Parent):
(shouldBe.Derived.prototype.m):
(shouldBe.Derived):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/EvalCodeCache.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/tests/stress/direct-eval-in-object-literal-methods.js [new file with mode: 0644]

index b60d9e2..f25920f 100644 (file)
@@ -1,3 +1,29 @@
+2016-05-13  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Assertion failure for direct eval in non-class method
+        https://bugs.webkit.org/show_bug.cgi?id=157138
+
+        Reviewed by Saam Barati.
+
+        This assertion was incorrect. In method definitions in object literals,
+        it can be sloppy mode, but its DerivedContextType may not be DerivedContextType::None.
+
+        * bytecode/EvalCodeCache.h:
+        (JSC::EvalCodeCache::CacheKey::CacheKey):
+        (JSC::EvalCodeCache::CacheKey::operator==):
+        (JSC::EvalCodeCache::CacheKey::Hash::equal):
+        (JSC::EvalCodeCache::tryGet):
+        (JSC::EvalCodeCache::getSlow):
+        * interpreter/Interpreter.cpp:
+        (JSC::eval):
+        * tests/stress/direct-eval-in-object-literal-methods.js: Added.
+        (shouldBe):
+        (throw.new.Error):
+        (shouldBe.Parent.prototype.l):
+        (shouldBe.Parent):
+        (shouldBe.Derived.prototype.m):
+        (shouldBe.Derived):
+
 2016-05-13  Skachkov Oleksandr  <gskachkov@gmail.com>
 
         Assertion failure for super() call in arrow function default parameters
index 8f97010..c461257 100644 (file)
@@ -46,9 +46,10 @@ namespace JSC {
     public:
         class CacheKey {
         public:
-            CacheKey(const String& source, bool isArrowFunctionContext)
+            CacheKey(const String& source, bool isArrowFunctionContext, DerivedContextType derivedContextType)
                 : m_source(source.impl())
                 , m_isArrowFunctionContext(isArrowFunctionContext)
+                , m_derivedContextType(derivedContextType)
             {
             }
 
@@ -65,7 +66,7 @@ namespace JSC {
 
             bool operator==(const CacheKey& other) const
             {
-                return m_source == other.m_source && m_isArrowFunctionContext == other.m_isArrowFunctionContext;
+                return m_source == other.m_source && m_isArrowFunctionContext == other.m_isArrowFunctionContext && m_derivedContextType == other.m_derivedContextType;
             }
 
             bool isHashTableDeletedValue() const { return m_source.isHashTableDeletedValue(); }
@@ -77,7 +78,7 @@ namespace JSC {
                 }
                 static bool equal(const CacheKey& lhs, const CacheKey& rhs)
                 {
-                    return StringHash::equal(lhs.m_source, rhs.m_source) && lhs.m_isArrowFunctionContext == rhs.m_isArrowFunctionContext;
+                    return StringHash::equal(lhs.m_source, rhs.m_source) && lhs.m_isArrowFunctionContext == rhs.m_isArrowFunctionContext && lhs.m_derivedContextType == rhs.m_derivedContextType;
                 }
                 static const bool safeToCompareToEmptyOrDeleted = false;
             };
@@ -87,13 +88,14 @@ namespace JSC {
         private:
             RefPtr<StringImpl> m_source;
             bool m_isArrowFunctionContext { false };
+            DerivedContextType m_derivedContextType { DerivedContextType::None };
         };
 
-        EvalExecutable* tryGet(bool inStrictContext, const String& evalSource, bool isArrowFunctionContext, JSScope* scope)
+        EvalExecutable* tryGet(bool inStrictContext, const String& evalSource, bool isArrowFunctionContext, DerivedContextType derivedContextType, JSScope* scope)
         {
             if (isCacheable(inStrictContext, evalSource, scope)) {
                 ASSERT(!inStrictContext);
-                return m_cacheMap.fastGet(CacheKey(evalSource, isArrowFunctionContext)).get();
+                return m_cacheMap.fastGet(CacheKey(evalSource, isArrowFunctionContext, derivedContextType)).get();
             }
             return nullptr;
         }
@@ -109,8 +111,7 @@ namespace JSC {
             if (isCacheable(inStrictContext, evalSource, scope) && m_cacheMap.size() < maxCacheEntries) {
                 ASSERT(!inStrictContext);
                 ASSERT_WITH_MESSAGE(thisTDZMode == ThisTDZMode::CheckIfNeeded, "Always CheckIfNeeded because the caching is enabled only in the sloppy mode.");
-                ASSERT_WITH_MESSAGE(derivedContextType == DerivedContextType::None, "derivedContextType is always None because class methods and class constructors are always evaluated as the strict code.");
-                m_cacheMap.set(CacheKey(evalSource, isArrowFunctionContext), WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
+                m_cacheMap.set(CacheKey(evalSource, isArrowFunctionContext, derivedContextType), WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
             }
             
             return evalExecutable;
index 47e3888..c2edbe7 100644 (file)
@@ -158,7 +158,15 @@ JSValue eval(CallFrame* callFrame)
     UnlinkedCodeBlock* callerUnlinkedCodeBlock = callerCodeBlock->unlinkedCodeBlock();
 
     bool isArrowFunctionContext = callerUnlinkedCodeBlock->isArrowFunction() || callerUnlinkedCodeBlock->isArrowFunctionContext();
-    EvalExecutable* eval = callerCodeBlock->evalCodeCache().tryGet(callerCodeBlock->isStrictMode(), programSource, isArrowFunctionContext, callerScopeChain);
+
+    DerivedContextType derivedContextType = callerUnlinkedCodeBlock->derivedContextType();
+    if (!isArrowFunctionContext && callerUnlinkedCodeBlock->isClassContext()) {
+        derivedContextType = callerUnlinkedCodeBlock->isConstructor()
+            ? DerivedContextType::DerivedConstructorContext
+            : DerivedContextType::DerivedMethodContext;
+    }
+
+    EvalExecutable* eval = callerCodeBlock->evalCodeCache().tryGet(callerCodeBlock->isStrictMode(), programSource, isArrowFunctionContext, derivedContextType, callerScopeChain);
 
     if (!eval) {
         if (!callerCodeBlock->isStrictMode()) {
@@ -180,14 +188,6 @@ JSValue eval(CallFrame* callFrame)
         if (callerUnlinkedCodeBlock->constructorKind() == ConstructorKind::Derived)
             thisTDZMode = ThisTDZMode::AlwaysCheck;
 
-        DerivedContextType derivedContextType = callerUnlinkedCodeBlock->derivedContextType();
-        
-        if (!isArrowFunctionContext && callerUnlinkedCodeBlock->isClassContext()) {
-            derivedContextType = callerUnlinkedCodeBlock->isConstructor()
-                ? DerivedContextType::DerivedConstructorContext
-                : DerivedContextType::DerivedMethodContext;
-        }
-        
         EvalContextType evalContextType;
         
         if (isFunctionParseMode(callerUnlinkedCodeBlock->parseMode()))
diff --git a/Source/JavaScriptCore/tests/stress/direct-eval-in-object-literal-methods.js b/Source/JavaScriptCore/tests/stress/direct-eval-in-object-literal-methods.js
new file mode 100644 (file)
index 0000000..8c62f87
--- /dev/null
@@ -0,0 +1,62 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+{
+    let object = {
+        n()
+        {
+            return 42;
+        }
+    };
+
+    let derived = {
+        m()
+        {
+            return eval("super.n()");
+        }
+    };
+    Object.setPrototypeOf(derived, object);
+    shouldBe(derived.m(), 42);
+    // Cached.
+    shouldBe(derived.m(), 42);
+}
+
+{
+    let object = {
+        l()
+        {
+            return 42;
+        }
+    };
+
+    let derived = {
+        m()
+        {
+            return eval("super.l()");
+        }
+    };
+    Object.setPrototypeOf(derived, object);
+    shouldBe(derived.m(), 42);
+    // Cached.
+    shouldBe(derived.m(), 42);
+
+    class Parent {
+        l()
+        {
+            return 55;
+        }
+    }
+
+    class Derived extends Parent {
+        m()
+        {
+            return eval("super.l()");
+        }
+    }
+    let instance = new Derived();
+    // Under the strict code, not cached.
+    shouldBe(instance.l(), 55);
+    shouldBe(instance.l(), 55);
+}