Title: [90938] trunk
Revision
90938
Author
[email protected]
Date
2011-07-13 11:59:34 -0700 (Wed, 13 Jul 2011)

Log Message

https://bugs.webkit.org/show_bug.cgi?id=64424
Our direct eval behaviour deviates slightly from the spec.

Reviewed by Oliver Hunt.

Source/_javascript_Core: 

The ES5 spec defines a concept of 'Direct Call to Eval' (see section 15.1.2.1.1), where
behaviour will differ from that of an indirect call (e.g. " { eval: window.eval }.eval();"
or "var a = eval; a();" are indirect calls), particularly in non-strict scopes variables
may be introduced into the caller's environment.

ES5 direct calls are any call where the callee function is provided by a reference, a base
of that Reference is an EnvironmentRecord (this corresponds to all productions
"PrimaryExpression: Identifier", see 10.2.2.1 GetIdentifierReference), and where the name
of the reference is "eval". This means any _expression_ of the form "eval(...)", and that
calls the standard built in eval method from on the Global Object, is considered to be
direct.

In _javascript_Core we are currently overly restrictive. We also check that the
EnvironmentRecord that is the base of the reference is the Declaractive Environment Record
at the root of the scope chain, corresponding to the Global Object - an "eval(..)" statement
that hits a var eval in a nested scope is not considered to be direct. This behaviour does
not emanate from the spec, and is incorrect.

* interpreter/Interpreter.cpp:
(JSC::Interpreter::privateExecute):
    - Fixed direct eval check in op_call_eval.
* jit/JITStubs.cpp:
(JSC::DEFINE_STUB_FUNCTION):
    - Fixed direct eval check in op_call_eval.
* runtime/Executable.h:
(JSC::isHostFunction):
    - Added check for host function with specific NativeFunction.

LayoutTests: 

Correct expected results.

* fast/js/eval-keyword-vs-function-expected.txt:
* fast/js/eval-keyword-vs-function.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (90937 => 90938)


--- trunk/LayoutTests/ChangeLog	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/LayoutTests/ChangeLog	2011-07-13 18:59:34 UTC (rev 90938)
@@ -1,3 +1,15 @@
+2011-07-12  Gavin Barraclough  <[email protected]>
+
+        https://bugs.webkit.org/show_bug.cgi?id=64424
+        Our direct eval behaviour deviates slightly from the spec.
+
+        Reviewed by Oliver Hunt.
+
+        Correct expected results.
+
+        * fast/js/eval-keyword-vs-function-expected.txt:
+        * fast/js/eval-keyword-vs-function.html:
+
 2011-07-13  Abhishek Arya  <[email protected]>
 
         Tests that we do not crash when frame is blown away in a beforeload

Modified: trunk/LayoutTests/fast/js/eval-keyword-vs-function-expected.txt (90937 => 90938)


--- trunk/LayoutTests/fast/js/eval-keyword-vs-function-expected.txt	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/LayoutTests/fast/js/eval-keyword-vs-function-expected.txt	2011-07-13 18:59:34 UTC (rev 90938)
@@ -12,7 +12,7 @@
 PASS: window.eval("x") should be 0 and is.
 PASS: globalEval("x") should be 0 and is.
 PASS: localEval("x") should be 0 and is.
-PASS: (function() { var eval = window.eval; return eval("x"); })() should be 0 and is.
+PASS: (function() { var eval = window.eval; return eval("x"); })() should be 1 and is.
 
 ----- Variable Object: -----
 
@@ -20,7 +20,7 @@
 PASS: window.eval("var y; "y" in window") should be true and is.
 PASS: globalEval("var y; "y" in window") should be true and is.
 PASS: localEval("var y; "y" in window") should be true and is.
-PASS: (function() { var eval = window.eval; return eval("var y; "y" in window"); })() should be true and is.
+PASS: (function() { var eval = window.eval; return eval("var y; "y" in window"); })() should be false and is.
 
 ----- Scope Chain for Setters: -----
 
@@ -28,7 +28,7 @@
 PASS: window.eval("z = 2; window.z") should be 2 and is.
 PASS: globalEval("z = 3; window.z") should be 3 and is.
 PASS: localEval("z = 4; window.z") should be 4 and is.
-PASS: (function() { var eval = window.eval; return eval("z = 5; window.z"); })() should be 5 and is.
+PASS: (function() { var eval = window.eval; return eval("z = 5; window.z"); })() should be 4 and is.
 
 ----- This Object: -----
 

Modified: trunk/LayoutTests/fast/js/eval-keyword-vs-function.html (90937 => 90938)


--- trunk/LayoutTests/fast/js/eval-keyword-vs-function.html	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/LayoutTests/fast/js/eval-keyword-vs-function.html	2011-07-13 18:59:34 UTC (rev 90938)
@@ -43,7 +43,10 @@
     shouldBe('window.eval("x")', window.eval("x"), 0);
     shouldBe('globalEval("x")', globalEval("x"), 0);
     shouldBe('localEval("x")', localEval("x"), 0);
-    shouldBe('(function() { var eval = window.eval; return eval("x"); })()', (function() { var eval = window.eval; return eval("x"); })(), 0);
+
+    // Per ES5 15.1.2.11 & 10.2.2.1 any reference that hits an enviromment record (i.e. does not have a base)
+    // and has a reference name of "eval" is treated as a direct eval - the assignment to a var makes no difference.
+    shouldBe('(function() { var eval = window.eval; return eval("x"); })()', (function() { var eval = window.eval; return eval("x"); })(), 1);
 }
 
 function testVarY()
@@ -62,7 +65,9 @@
     shouldBe('localEval("var y; \"y\" in window")', localEval("var y; \"y\" in window"), true);
     delete window.y;
     
-    shouldBe('(function() { var eval = window.eval; return eval("var y; \"y\" in window"); })()', (function() { var eval = window.eval; return eval("var y; \"y\" in window"); })(), true);
+    // Per ES5 15.1.2.11 & 10.2.2.1 any reference that hits an enviromment record (i.e. does not have a base)
+    // and has a reference name of "eval" is treated as a direct eval - the assignment to a var makes no difference.
+    shouldBe('(function() { var eval = window.eval; return eval("var y; \"y\" in window"); })()', (function() { var eval = window.eval; return eval("var y; \"y\" in window"); })(), false);
 }
 
 function testSetZ()
@@ -79,7 +84,9 @@
 
     shouldBe('localEval("z = 4; window.z")', localEval("z = 4; window.z"), 4);
 
-    shouldBe('(function() { var eval = window.eval; return eval("z = 5; window.z"); })()', (function() { var eval = window.eval; return eval("z = 5; window.z"); })(), 5);
+    // Per ES5 15.1.2.11 & 10.2.2.1 any reference that hits an enviromment record (i.e. does not have a base)
+    // and has a reference name of "eval" is treated as a direct eval - the assignment to a var makes no difference.
+    shouldBe('(function() { var eval = window.eval; return eval("z = 5; window.z"); })()', (function() { var eval = window.eval; return eval("z = 5; window.z"); })(), 4);
 }
 
 function testThis()

Modified: trunk/Source/_javascript_Core/ChangeLog (90937 => 90938)


--- trunk/Source/_javascript_Core/ChangeLog	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-07-13 18:59:34 UTC (rev 90938)
@@ -1,3 +1,38 @@
+2011-07-12  Gavin Barraclough  <[email protected]>
+
+        https://bugs.webkit.org/show_bug.cgi?id=64424
+        Our direct eval behaviour deviates slightly from the spec.
+
+        Reviewed by Oliver Hunt.
+
+        The ES5 spec defines a concept of 'Direct Call to Eval' (see section 15.1.2.1.1), where
+        behaviour will differ from that of an indirect call (e.g. " { eval: window.eval }.eval();"
+        or "var a = eval; a();" are indirect calls), particularly in non-strict scopes variables
+        may be introduced into the caller's environment.
+
+        ES5 direct calls are any call where the callee function is provided by a reference, a base
+        of that Reference is an EnvironmentRecord (this corresponds to all productions
+        "PrimaryExpression: Identifier", see 10.2.2.1 GetIdentifierReference), and where the name
+        of the reference is "eval". This means any _expression_ of the form "eval(...)", and that
+        calls the standard built in eval method from on the Global Object, is considered to be
+        direct.
+
+        In _javascript_Core we are currently overly restrictive. We also check that the
+        EnvironmentRecord that is the base of the reference is the Declaractive Environment Record
+        at the root of the scope chain, corresponding to the Global Object - an "eval(..)" statement
+        that hits a var eval in a nested scope is not considered to be direct. This behaviour does
+        not emanate from the spec, and is incorrect.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::privateExecute):
+            - Fixed direct eval check in op_call_eval.
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+            - Fixed direct eval check in op_call_eval.
+        * runtime/Executable.h:
+        (JSC::isHostFunction):
+            - Added check for host function with specific NativeFunction.
+
 2011-07-13  Ademar de Souza Reis Jr.  <[email protected]>
 
         Reviewed by Andreas Kling.

Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (90937 => 90938)


--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2011-07-13 18:59:34 UTC (rev 90938)
@@ -4099,12 +4099,10 @@
         ASSERT(codeBlock->codeType() != FunctionCode || !codeBlock->needsFullScopeChain() || callFrame->r(codeBlock->activationRegister()).jsValue());
         JSValue funcVal = callFrame->r(func).jsValue();
 
-        Register* newCallFrame = callFrame->registers() + registerOffset;
-        Register* argv = newCallFrame - RegisterFile::CallFrameHeaderSize - argCount;
-        JSValue thisValue = argv[0].jsValue();
-        JSGlobalObject* globalObject = callFrame->scopeChain()->globalObject.get();
+        if (isHostFunction(callFrame->globalData(), funcVal, globalFuncEval)) {
+            Register* newCallFrame = callFrame->registers() + registerOffset;
+            Register* argv = newCallFrame - RegisterFile::CallFrameHeaderSize - argCount;
 
-        if (thisValue == globalObject && funcVal == globalObject->evalFunction()) {
             JSValue result = callEval(callFrame, registerFile, argv, argCount, registerOffset);
             if ((exceptionValue = globalData->exception))
                 goto vm_throw;

Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (90937 => 90938)


--- trunk/Source/_javascript_Core/jit/JITStubs.cpp	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp	2011-07-13 18:59:34 UTC (rev 90938)
@@ -3159,18 +3159,15 @@
     int registerOffset = stackFrame.args[1].int32();
     int argCount = stackFrame.args[2].int32();
 
+    if (!isHostFunction(callFrame->globalData(), funcVal, globalFuncEval))
+        return JSValue::encode(JSValue());
+
     Register* newCallFrame = callFrame->registers() + registerOffset;
     Register* argv = newCallFrame - RegisterFile::CallFrameHeaderSize - argCount;
-    JSValue baseValue = argv[0].jsValue();
-    JSGlobalObject* globalObject = callFrame->scopeChain()->globalObject.get();
 
-    if (baseValue == globalObject && funcVal == globalObject->evalFunction()) {
-        JSValue result = interpreter->callEval(callFrame, registerFile, argv, argCount, registerOffset);
-        CHECK_FOR_EXCEPTION_AT_END();
-        return JSValue::encode(result);
-    }
-
-    return JSValue::encode(JSValue());
+    JSValue result = interpreter->callEval(callFrame, registerFile, argv, argCount, registerOffset);
+    CHECK_FOR_EXCEPTION_AT_END();
+    return JSValue::encode(result);
 }
 
 DEFINE_STUB_FUNCTION(void*, op_throw)

Modified: trunk/Source/_javascript_Core/runtime/Executable.h (90937 => 90938)


--- trunk/Source/_javascript_Core/runtime/Executable.h	2011-07-13 18:56:37 UTC (rev 90937)
+++ trunk/Source/_javascript_Core/runtime/Executable.h	2011-07-13 18:59:34 UTC (rev 90938)
@@ -514,6 +514,15 @@
         ASSERT(isHostFunction());
         return static_cast<NativeExecutable*>(m_executable.get())->function();
     }
+
+    inline bool isHostFunction(JSGlobalData& globalData, JSValue value, NativeFunction nativeFunction)
+    {
+        JSFunction* function = static_cast<JSFunction*>(getJSFunction(globalData, value));
+        if (!function || !function->isHostFunction())
+            return false;
+        return function->nativeFunction() == nativeFunction;
+    }
+
 }
 
 #endif
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to