From: olehougaard Date: Fri, 21 Nov 2008 12:49:57 +0000 (+0000) Subject: Fixing the detection of aliased eval so that it is exact. X-Git-Tag: upstream/4.7.83~24979 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3877c91a195ad7e1acfd0a699bf85ab873fb1b2c;p=platform%2Fupstream%2Fv8.git Fixing the detection of aliased eval so that it is exact. Fixing the semantics of aliased eval so that it is conformant. Review URL: http://codereview.chromium.org/11563 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@819 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/ast.cc b/src/ast.cc index 2f21d92..b1c99aa 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -37,7 +37,7 @@ VariableProxySentinel VariableProxySentinel::this_proxy_(true); VariableProxySentinel VariableProxySentinel::identifier_proxy_(false); ValidLeftHandSideSentinel ValidLeftHandSideSentinel::instance_; Property Property::this_property_(VariableProxySentinel::this_proxy(), NULL, 0); -Call Call::sentinel_(NULL, NULL, false, 0); +Call Call::sentinel_(NULL, NULL, Call::ALIASED, 0); // ---------------------------------------------------------------------------- diff --git a/src/ast.h b/src/ast.h index b383f76..b06a383 100644 --- a/src/ast.h +++ b/src/ast.h @@ -864,13 +864,20 @@ class Property: public Expression { class Call: public Expression { public: + enum EvalType { + ALIASED, // Either not eval or an aliased eval. + POTENTIALLY_DIRECT // Looks like a direct eval at codegen time. + // Needs to be determined at runtime whether the + // eval is direct. + }; + Call(Expression* expression, ZoneList* arguments, - bool is_eval, + EvalType eval_type, int pos) : expression_(expression), arguments_(arguments), - is_eval_(is_eval), + eval_type_(eval_type), pos_(pos) { } virtual void Accept(Visitor* v); @@ -880,7 +887,7 @@ class Call: public Expression { Expression* expression() const { return expression_; } ZoneList* arguments() const { return arguments_; } - bool is_eval() { return is_eval_; } + EvalType eval_type() { return eval_type_; } int position() { return pos_; } static Call* sentinel() { return &sentinel_; } @@ -888,7 +895,7 @@ class Call: public Expression { private: Expression* expression_; ZoneList* arguments_; - bool is_eval_; + EvalType eval_type_; int pos_; static Call sentinel_; @@ -898,7 +905,7 @@ class Call: public Expression { class CallNew: public Call { public: CallNew(Expression* expression, ZoneList* arguments, int pos) - : Call(expression, arguments, false, pos) { } + : Call(expression, arguments, ALIASED, pos) { } virtual void Accept(Visitor* v); }; diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc index e5120ae..2184b61 100644 --- a/src/codegen-arm.cc +++ b/src/codegen-arm.cc @@ -2320,6 +2320,12 @@ void CodeGenerator::VisitCall(Call* node) { // is resolved in cache misses (this also holds for megamorphic calls). // ------------------------------------------------------------------------ + if (node->eval_type() == Call::POTENTIALLY_DIRECT) { + __ CallRuntime(Runtime::kSetInPotentiallyDirectEval, 0); + } else { + __ CallRuntime(Runtime::kClearInPotentiallyDirectEval, 0); + } + if (var != NULL && !var->is_this() && var->is_global()) { // ---------------------------------- // JavaScript example: 'foo(1, 2, 3)' // foo is global diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc index 230dc17..4b43e32 100644 --- a/src/codegen-ia32.cc +++ b/src/codegen-ia32.cc @@ -2690,6 +2690,12 @@ void CodeGenerator::VisitCall(Call* node) { // is resolved in cache misses (this also holds for megamorphic calls). // ------------------------------------------------------------------------ + if (node->eval_type() == Call::POTENTIALLY_DIRECT) { + __ CallRuntime(Runtime::kSetInPotentiallyDirectEval, 0); + } else { + __ CallRuntime(Runtime::kClearInPotentiallyDirectEval, 0); + } + if (var != NULL && !var->is_this() && var->is_global()) { // ---------------------------------- // JavaScript example: 'foo(1, 2, 3)' // foo is global diff --git a/src/debug-delay.js b/src/debug-delay.js index 5aa0334..c860c07 100644 --- a/src/debug-delay.js +++ b/src/debug-delay.js @@ -1117,7 +1117,7 @@ DebugCommandProcessor.prototype.scriptsCommandToJSONRequest_ = function(args) { DebugCommandProcessor.prototype.responseToText = function(json_response) { try { // Convert the JSON string to an object. - response = %CompileString('(' + json_response + ')', 0, false)(); + response = %CompileString('(' + json_response + ')', 0)(); if (!response.success) { return response.message; @@ -1323,7 +1323,7 @@ DebugCommandProcessor.prototype.processDebugJSONRequest = function(json_request, try { try { // Convert the JSON string to an object. - request = %CompileString('(' + json_request + ')', 0, false)(); + request = %CompileString('(' + json_request + ')', 0)(); // Create an initial response. response = this.createResponse(request); @@ -1776,7 +1776,7 @@ DebugCommandProcessor.prototype.scriptsRequest_ = function(request, response) { DebugCommandProcessor.prototype.isRunning = function(json_response) { try { // Convert the JSON string to an object. - response = %CompileString('(' + json_response + ')', 0, false)(); + response = %CompileString('(' + json_response + ')', 0)(); // Return whether VM should be running after this request. return response.running; diff --git a/src/parser.cc b/src/parser.cc index bcc439f..5941c0b 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -327,6 +327,8 @@ class ParserFactory BASE_EMBEDDED { return Handle(); } + + virtual Expression* NewProperty(Expression* obj, Expression* key, int pos) { if (obj == VariableProxySentinel::this_proxy()) { return Property::this_property(); @@ -337,7 +339,7 @@ class ParserFactory BASE_EMBEDDED { virtual Expression* NewCall(Expression* expression, ZoneList* arguments, - bool is_eval, int pos) { + Call::EvalType eval_type, int pos) { return Call::sentinel(); } @@ -387,8 +389,8 @@ class AstBuildingParserFactory : public ParserFactory { virtual Expression* NewCall(Expression* expression, ZoneList* arguments, - bool is_eval, int pos) { - return new Call(expression, arguments, is_eval, pos); + Call::EvalType eval_type, int pos) { + return new Call(expression, arguments, eval_type, pos); } virtual Statement* EmptyStatement() { @@ -2294,38 +2296,32 @@ Expression* Parser::ParseLeftHandSideExpression(bool* ok) { ZoneList* args = ParseArguments(CHECK_OK); // Keep track of eval() calls since they disable all local variable - // optimizations. We can ignore locally declared variables with - // name 'eval' since they override the global 'eval' function. We - // only need to look at unresolved variables (VariableProxies). - + // optimizations. + // The calls that need special treatment are the + // direct (i.e. not aliased) eval calls. These calls are all of the + // form eval(...) with no explicit receiver object where eval is not + // declared in the current scope chain. These calls are marked as + // potentially direct eval calls. Whether they are actually direct calls + // to eval is determined at run time. + + Call::EvalType eval_type = Call::ALIASED; if (!is_pre_parsing_) { - // We assume that only a function called 'eval' can be used - // to invoke the global eval() implementation. This permits - // for massive optimizations. VariableProxy* callee = result->AsVariableProxy(); if (callee != NULL && callee->IsVariable(Factory::eval_symbol())) { - // We do not allow direct calls to 'eval' in our internal - // JS files. Use builtin functions instead. - ASSERT(!Bootstrapper::IsActive()); - top_scope_->RecordEvalCall(); - } else { - // This is rather convoluted code to check if we're calling - // a function named 'eval' through a property access. If so, - // we mark it as a possible eval call (we don't know if the - // receiver will resolve to the global object or not), but - // we do not treat the call as an eval() call - we let the - // call get through to the JavaScript eval code defined in - // v8natives.js. - Property* property = result->AsProperty(); - if (property != NULL) { - Literal* key = property->key()->AsLiteral(); - if (key != NULL && - key->handle().is_identical_to(Factory::eval_symbol())) { - // We do not allow direct calls to 'eval' in our - // internal JS files. Use builtin functions instead. - ASSERT(!Bootstrapper::IsActive()); - top_scope_->RecordEvalCall(); - } + Handle name = callee->name(); + Variable* var = NULL; + for (Scope* scope = top_scope_; + scope != NULL; + scope = scope->outer_scope()) { + var = scope->Lookup(callee->name()); + if (var != NULL) break; + } + if (var == NULL) { + // We do not allow direct calls to 'eval' in our internal + // JS files. Use builtin functions instead. + ASSERT(!Bootstrapper::IsActive()); + top_scope_->RecordEvalCall(); + eval_type = Call::POTENTIALLY_DIRECT; } } } @@ -2342,7 +2338,7 @@ Expression* Parser::ParseLeftHandSideExpression(bool* ok) { if (is_eval && args->length() == 0) { result = NEW(Literal(Factory::undefined_value())); } else { - result = factory()->NewCall(result, args, is_eval, pos); + result = factory()->NewCall(result, args, eval_type, pos); } break; } diff --git a/src/runtime.cc b/src/runtime.cc index 71b5fed..0d369c8 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -3930,95 +3930,107 @@ static Object* Runtime_NumberIsFinite(Arguments args) { } -static Object* EvalContext() { - // The topmost JS frame belongs to the eval function which called - // the CompileString runtime function. We need to unwind one level - // to get to the caller of eval. - StackFrameLocator locator; - JavaScriptFrame* frame = locator.FindJavaScriptFrame(1); - - // TODO(900055): Right now we check if the caller of eval() supports - // eval to determine if it's an aliased eval or not. This may not be - // entirely correct in the unlikely case where a function uses both - // aliased and direct eval calls. - HandleScope scope; - if (!ScopeInfo<>::SupportsEval(frame->FindCode())) { - // Aliased eval: Evaluate in the global context of the eval - // function to support aliased, cross environment evals. - return *Top::global_context(); - } +static Object* Runtime_GlobalReceiver(Arguments args) { + ASSERT(args.length() == 1); + Object* global = args[0]; + if (!global->IsJSGlobalObject()) return Heap::null_value(); + return JSGlobalObject::cast(global)->global_receiver(); +} - // Fetch the caller context from the frame. - Handle caller(Context::cast(frame->context())); - // Check for eval() invocations that cross environments. Use the - // context from the stack if evaluating in current environment. - Handle target = Top::global_context(); - if (caller->global_context() == *target) return *caller; +static Object* Runtime_CompileString(Arguments args) { + HandleScope scope; + ASSERT(args.length() == 2); + CONVERT_ARG_CHECKED(String, source, 0); + CONVERT_ARG_CHECKED(Smi, line_offset, 1); - // Otherwise, use the global context from the other environment. - return *target; + // Compile source string. + Handle boilerplate = + Compiler::CompileEval(source, line_offset->value(), true); + if (boilerplate.is_null()) return Failure::Exception(); + Handle context(Top::context()->global_context()); + Handle fun = + Factory::NewFunctionFromBoilerplate(boilerplate, context); + return *fun; } -static Object* Runtime_EvalReceiver(Arguments args) { +static Object* Runtime_ExecDirectEval(Arguments args) { ASSERT(args.length() == 1); + + if (!args[0]->IsString()) return args[0]; + + Handle source = args.at(0); + + // Compute the eval context. + HandleScope scope; StackFrameLocator locator; JavaScriptFrame* frame = locator.FindJavaScriptFrame(1); - // Fetch the caller context from the frame. - Context* caller = Context::cast(frame->context()); + Handle context(Context::cast(frame->context())); + bool is_global = context->IsGlobalContext(); - // Check for eval() invocations that cross environments. Use the - // top frames receiver if evaluating in current environment. - Context* global_context = Top::context()->global()->global_context(); - if (caller->global_context() == global_context) { - return frame->receiver(); + // Compile source string. + Handle boilerplate = + Compiler::CompileEval(source, 0, is_global); + if (boilerplate.is_null()) return Failure::Exception(); + Handle fun = + Factory::NewFunctionFromBoilerplate(boilerplate, context); + + // Call generated function + Handle receiver(frame->receiver()); + bool has_pending_exception = false; + Handle result = + Execution::Call(fun, receiver, 0, NULL, &has_pending_exception); + if (has_pending_exception) { + ASSERT(Top::has_pending_exception()); + return Failure::Exception(); } + return *result; +} + - // Otherwise use the given argument (the global object of the - // receiving context). - return args[0]; +static Object* Runtime_SetInPotentiallyDirectEval(Arguments args) { + ASSERT(args.length() == 0); + Top::set_in_potentially_direct_eval(true); + return Heap::undefined_value(); } -static Object* Runtime_GlobalReceiver(Arguments args) { - ASSERT(args.length() == 1); - Object* global = args[0]; - if (!global->IsJSGlobalObject()) return Heap::null_value(); - return JSGlobalObject::cast(global)->global_receiver(); +static Object* Runtime_ClearInPotentiallyDirectEval(Arguments args) { + ASSERT(args.length() == 0); + Top::set_in_potentially_direct_eval(false); + return Heap::undefined_value(); } -static Object* Runtime_CompileString(Arguments args) { - HandleScope scope; - ASSERT(args.length() == 3); - CONVERT_ARG_CHECKED(String, source, 0); - CONVERT_ARG_CHECKED(Smi, line_offset, 1); - bool contextual = args[2]->IsTrue(); - RUNTIME_ASSERT(contextual || args[2]->IsFalse()); +static Object* Runtime_InDirectEval(Arguments args) { + ASSERT(args.length() == 0); - // Compute the eval context. - Handle context; - if (contextual) { - // Get eval context. May not be available if we are calling eval - // through an alias, and the corresponding frame doesn't have a - // proper eval context set up. - Object* eval_context = EvalContext(); - if (eval_context->IsFailure()) return eval_context; - context = Handle(Context::cast(eval_context)); - } else { - context = Handle(Top::context()->global_context()); + // No need to look further if the static analysis showed that this + // is aliased. + if (!Top::is_in_potentially_direct_eval()) { + return Heap::false_value(); } - - // Compile source string. - bool is_global = context->IsGlobalContext(); - Handle boilerplate = - Compiler::CompileEval(source, line_offset->value(), is_global); - if (boilerplate.is_null()) return Failure::Exception(); - Handle fun = - Factory::NewFunctionFromBoilerplate(boilerplate, context); - return *fun; + // Find where the 'eval' symbol is bound. It is unaliased only if + // it is bound in the global object. + HandleScope scope; + StackFrameLocator locator; + JavaScriptFrame* frame = locator.FindJavaScriptFrame(1); + Context* context = Context::cast(frame->context()); + int index; + PropertyAttributes attributes; + while (context && !context->IsGlobalContext()) { + context->Lookup(Factory::eval_symbol(), FOLLOW_PROTOTYPE_CHAIN, + &index, &attributes); + if (attributes != ABSENT) return Heap::false_value(); + if (context->is_function_context()) { + context = Context::cast(context->closure()->context()); + } else { + context = context->previous(); + } + } + return Heap::true_value(); } diff --git a/src/runtime.h b/src/runtime.h index 36a7180..d0c30d4 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -190,13 +190,16 @@ namespace v8 { namespace internal { F(NumberIsFinite, 1) \ \ /* Globals */ \ - F(CompileString, 3) \ + F(CompileString, 2) \ F(CompileScript, 4) \ F(GlobalPrint, 1) \ \ /* Eval */ \ - F(EvalReceiver, 1) \ F(GlobalReceiver, 1) \ + F(SetInPotentiallyDirectEval, 0) \ + F(ClearInPotentiallyDirectEval, 0) \ + F(InDirectEval, 0) \ + F(ExecDirectEval, 1) \ \ F(SetProperty, -1 /* 3 or 4 */) \ F(IgnoreAttributesAndSetProperty, -1 /* 3 or 4 */) \ diff --git a/src/top.cc b/src/top.cc index ace79a9..677ef83 100644 --- a/src/top.cc +++ b/src/top.cc @@ -102,6 +102,7 @@ void Top::InitializeThreadLocal() { clear_scheduled_exception(); thread_local_.save_context_ = NULL; thread_local_.catcher_ = NULL; + thread_local_.in_potentially_direct_eval_ = false; } diff --git a/src/top.h b/src/top.h index 431f05e..44f1c41 100644 --- a/src/top.h +++ b/src/top.h @@ -67,6 +67,10 @@ class ThreadLocalTop BASE_EMBEDDED { // Call back function to report unsafe JS accesses. v8::FailedAccessCheckCallback failed_access_check_callback_; + + // Flag for whether we are currently in a call that the static analysis + // marked as a potentially direct eval. + bool in_potentially_direct_eval_; }; #define TOP_ADDRESS_LIST(C) \ @@ -165,6 +169,14 @@ class Top { } static inline Address* handler_address() { return &thread_local_.handler_; } + static inline void set_in_potentially_direct_eval(bool b) { + thread_local_.in_potentially_direct_eval_ = b; + } + + static inline bool is_in_potentially_direct_eval() { + return thread_local_.in_potentially_direct_eval_; + } + // Generated code scratch locations. static void* formal_count_address() { return &thread_local_.formal_count_; } diff --git a/src/v8natives.js b/src/v8natives.js index f3c98a5..cedd2bb 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -101,7 +101,6 @@ function GlobalParseFloat(string) { return %StringParseFloat(ToString(string)); } - function GlobalEval(x) { if (!IS_STRING(x)) return x; @@ -110,10 +109,14 @@ function GlobalEval(x) { 'be the global object from which eval originated'); } - var f = %CompileString(x, 0, true); + if (%InDirectEval()) { + return %ExecDirectEval(x); + } + + var f = %CompileString(x, 0); if (!IS_FUNCTION(f)) return f; - return f.call(%EvalReceiver(this)); + return f.call(this); } @@ -121,7 +124,7 @@ function GlobalEval(x) { function GlobalExecScript(expr, lang) { // NOTE: We don't care about the character casing. if (!lang || /javascript/i.test(lang)) { - var f = %CompileString(ToString(expr), 0, false); + var f = %CompileString(ToString(expr), 0); f.call(%GlobalReceiver(global)); } return null; @@ -140,7 +143,7 @@ function SetupGlobal() { // ECMA-262 - 15.1.1.3. %SetProperty(global, "undefined", void 0, DONT_ENUM | DONT_DELETE); - + // Setup non-enumerable function on the global object. InstallFunctions(global, DONT_ENUM, $Array( "isNaN", GlobalIsNaN, @@ -521,7 +524,7 @@ function NewFunction(arg1) { // length == 1 // The call to SetNewFunctionAttributes will ensure the prototype // property of the resulting function is enumerable (ECMA262, 15.3.5.2). - var f = %CompileString(source, -1, false)(); + var f = %CompileString(source, -1)(); %FunctionSetName(f, "anonymous"); return %SetNewFunctionAttributes(f); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index e1549eb..3b6b594 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -4022,7 +4022,7 @@ THREADED_TEST(Eval) { v8::HandleScope scope; LocalContext current; - // Test that un-aliased eval uses local context. + // Test that un-aliased eval reads from local context. Local