From fb20f7fc7524b037e4c6f410e6f4fcca77082866 Mon Sep 17 00:00:00 2001 From: "mmaly@chromium.org" Date: Tue, 22 Feb 2011 00:39:21 +0000 Subject: [PATCH] CallIC and KeyedCallIC not wrapping this for strict mode functions. Fix CallIC and KeyedCallIC to correctly use Handle. Review URL: http://codereview.chromium.org/6523052 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6874 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 15 +-- src/ia32/stub-cache-ia32.cc | 15 +-- src/ic.cc | 93 ++++++++++--------- src/ic.h | 2 +- src/x64/stub-cache-x64.cc | 15 +-- test/mjsunit/strict-mode.js | 217 +++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 296 insertions(+), 61 deletions(-) diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 675fdf4..e623ea1 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -2332,8 +2332,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case STRING_CHECK: - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { // Check that the object is a two-byte string or a symbol. @@ -2348,8 +2349,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case NUMBER_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; @@ -2369,8 +2371,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, } case BOOLEAN_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index fabc275..51cc46a 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -2204,8 +2204,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case STRING_CHECK: - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { // Check that the object is a string or a symbol. @@ -2220,8 +2221,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case NUMBER_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; @@ -2241,8 +2243,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, } case BOOLEAN_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; diff --git a/src/ic.cc b/src/ic.cc index 968b45d..7482830 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -435,16 +435,25 @@ Object* CallICBase::TryCallAsFunction(Object* object) { } -void CallICBase::ReceiverToObject(Handle object) { - HandleScope scope; - Handle receiver(object); +void CallICBase::ReceiverToObjectIfRequired(Handle callee, + Handle object) { + if (callee->IsJSFunction()) { + Handle function = Handle::cast(callee); + if (function->shared()->strict_mode() || function->IsBuiltin()) { + // Do not wrap receiver for strict mode functions or for builtins. + return; + } + } - // Change the receiver to the result of calling ToObject on it. - const int argc = this->target()->arguments_count(); - StackFrameLocator locator; - JavaScriptFrame* frame = locator.FindJavaScriptFrame(0); - int index = frame->ComputeExpressionsCount() - (argc + 1); - frame->SetExpression(index, *Factory::ToObject(object)); + // And only wrap string, number or boolean. + if (object->IsString() || object->IsNumber() || object->IsBoolean()) { + // Change the receiver to the result of calling ToObject on it. + const int argc = this->target()->arguments_count(); + StackFrameLocator locator; + JavaScriptFrame* frame = locator.FindJavaScriptFrame(0); + int index = frame->ComputeExpressionsCount() - (argc + 1); + frame->SetExpression(index, *Factory::ToObject(object)); + } } @@ -458,10 +467,6 @@ MaybeObject* CallICBase::LoadFunction(State state, return TypeError("non_object_property_call", object, name); } - if (object->IsString() || object->IsNumber() || object->IsBoolean()) { - ReceiverToObject(object); - } - // Check if the name is trivially convertible to an index and get // the element if so. uint32_t index; @@ -505,6 +510,7 @@ MaybeObject* CallICBase::LoadFunction(State state, object->GetProperty(*object, &lookup, *name, &attr); if (!maybe_result->ToObject(&result)) return maybe_result; } + if (lookup.type() == INTERCEPTOR) { // If the object does not have the requested property, check which // exception we need to throw. @@ -516,31 +522,37 @@ MaybeObject* CallICBase::LoadFunction(State state, } } - ASSERT(result != Heap::the_hole_value()); + ASSERT(!result->IsTheHole()); - if (result->IsJSFunction()) { + HandleScope scope; + // Wrap result in a handle because ReceiverToObjectIfRequired may allocate + // new object and cause GC. + Handle result_handle(result); + // Make receiver an object if the callee requires it. Strict mode or builtin + // functions do not wrap the receiver, non-strict functions and objects + // called as functions do. + ReceiverToObjectIfRequired(result_handle, object); + + if (result_handle->IsJSFunction()) { #ifdef ENABLE_DEBUGGER_SUPPORT // Handle stepping into a function if step into is active. if (Debug::StepInActive()) { // Protect the result in a handle as the debugger can allocate and might // cause GC. - HandleScope scope; - Handle function(JSFunction::cast(result)); + Handle function(JSFunction::cast(*result_handle)); Debug::HandleStepIn(function, object, fp(), false); return *function; } #endif - return result; + return *result_handle; } // Try to find a suitable function delegate for the object at hand. - result = TryCallAsFunction(result); - MaybeObject* answer = result; - if (!result->IsJSFunction()) { - answer = TypeError("property_not_function", object, name); - } - return answer; + result_handle = Handle(TryCallAsFunction(*result_handle)); + if (result_handle->IsJSFunction()) return *result_handle; + + return TypeError("property_not_function", object, name); } @@ -565,8 +577,8 @@ bool CallICBase::TryUpdateExtraICState(LookupResult* lookup, case kStringCharAt: if (object->IsString()) { String* string = String::cast(*object); - // Check that there's the right wrapper in the receiver slot. - ASSERT(string == JSValue::cast(args[0])->value()); + // Check there's the right string value or wrapper in the receiver slot. + ASSERT(string == args[0] || string == JSValue::cast(args[0])->value()); // If we're in the default (fastest) state and the index is // out of bounds, update the state to record this fact. if (*extra_ic_state == DEFAULT_STRING_STUB && @@ -775,10 +787,6 @@ MaybeObject* KeyedCallIC::LoadFunction(State state, return TypeError("non_object_property_call", object, key); } - if (object->IsString() || object->IsNumber() || object->IsBoolean()) { - ReceiverToObject(object); - } - if (FLAG_use_ic && state != MEGAMORPHIC && !object->IsAccessCheckNeeded()) { int argc = target()->arguments_count(); InLoopFlag in_loop = target()->ic_in_loop(); @@ -793,17 +801,20 @@ MaybeObject* KeyedCallIC::LoadFunction(State state, #endif } } - Object* result; - { MaybeObject* maybe_result = Runtime::GetObjectProperty(object, key); - if (!maybe_result->ToObject(&result)) return maybe_result; - } - if (result->IsJSFunction()) return result; - result = TryCallAsFunction(result); - MaybeObject* answer = result; - if (!result->IsJSFunction()) { - answer = TypeError("property_not_function", object, key); - } - return answer; + + HandleScope scope; + Handle result = GetProperty(object, key); + + // Make receiver an object if the callee requires it. Strict mode or builtin + // functions do not wrap the receiver, non-strict functions and objects + // called as functions do. + ReceiverToObjectIfRequired(result, object); + + if (result->IsJSFunction()) return *result; + result = Handle(TryCallAsFunction(*result)); + if (result->IsJSFunction()) return *result; + + return TypeError("property_not_function", object, key); } diff --git a/src/ic.h b/src/ic.h index 3b10d06..96838c7 100644 --- a/src/ic.h +++ b/src/ic.h @@ -224,7 +224,7 @@ class CallICBase: public IC { // Otherwise, it returns the undefined value. Object* TryCallAsFunction(Object* object); - void ReceiverToObject(Handle object); + void ReceiverToObjectIfRequired(Handle callee, Handle object); static void Clear(Address address, Code* target); friend class IC; diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index 973fece..c27e1b8 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -2060,8 +2060,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case STRING_CHECK: - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { // Check that the object is a two-byte string or a symbol. @@ -2076,8 +2077,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, break; case NUMBER_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; @@ -2096,8 +2098,9 @@ MaybeObject* CallStubCompiler::CompileCallConstant(Object* object, } case BOOLEAN_CHECK: { - if (!function->IsBuiltin()) { - // Calling non-builtins with a value as receiver requires boxing. + if (!function->IsBuiltin() && !function_info->strict_mode()) { + // Calling non-strict non-builtins with a value as the receiver + // requires boxing. __ jmp(&miss); } else { Label fast; diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js index fbba64e..bcd6e90 100644 --- a/test/mjsunit/strict-mode.js +++ b/test/mjsunit/strict-mode.js @@ -438,7 +438,7 @@ repeat(10, function() { testAssignToUndefined(false); }); })(); // Not transforming this in Function.call and Function.apply. -(function testThisTransform() { +(function testThisTransformCallApply() { function non_strict() { return this; } @@ -478,3 +478,218 @@ repeat(10, function() { testAssignToUndefined(false); }); assertEquals(typeof strict.apply("Hello"), "string"); assertTrue(strict.apply(object) === object); })(); + +(function testThisTransform() { + try { + function strict() { + "use strict"; + return typeof(this); + } + function nonstrict() { + return typeof(this); + } + + // Concat to avoid symbol. + var strict_name = "str" + "ict"; + var nonstrict_name = "non" + "str" + "ict"; + var strict_number = 17; + var nonstrict_number = 19; + var strict_name_get = "str" + "ict" + "get"; + var nonstrict_name_get = "non" + "str" + "ict" + "get" + var strict_number_get = 23; + var nonstrict_number_get = 29; + + function install(t) { + t.prototype.strict = strict; + t.prototype.nonstrict = nonstrict; + t.prototype[strict_number] = strict; + t.prototype[nonstrict_number] = nonstrict; + Object.defineProperty(t.prototype, strict_name_get, + { get: function() { return strict; }, + configurable: true }); + Object.defineProperty(t.prototype, nonstrict_name_get, + { get: function() { return nonstrict; }, + configurable: true }); + Object.defineProperty(t.prototype, strict_number_get, + { get: function() { return strict; }, + configurable: true }); + Object.defineProperty(t.prototype, nonstrict_number_get, + { get: function() { return nonstrict; }, + configurable: true }); + } + + function cleanup(t) { + delete t.prototype.strict; + delete t.prototype.nonstrict; + delete t.prototype[strict_number]; + delete t.prototype[nonstrict_number]; + delete t.prototype[strict_name_get]; + delete t.prototype[nonstrict_name_get]; + delete t.prototype[strict_number_get]; + delete t.prototype[nonstrict_number_get]; + } + + // Set up fakes + install(String); + install(Number); + install(Boolean) + + function callStrict(o) { + return o.strict(); + } + function callNonStrict(o) { + return o.nonstrict(); + } + function callKeyedStrict(o) { + return o[strict_name](); + } + function callKeyedNonStrict(o) { + return o[nonstrict_name](); + } + function callIndexedStrict(o) { + return o[strict_number](); + } + function callIndexedNonStrict(o) { + return o[nonstrict_number](); + } + function callStrictGet(o) { + return o.strictget(); + } + function callNonStrictGet(o) { + return o.nonstrictget(); + } + function callKeyedStrictGet(o) { + return o[strict_name_get](); + } + function callKeyedNonStrictGet(o) { + return o[nonstrict_name_get](); + } + function callIndexedStrictGet(o) { + return o[strict_number_get](); + } + function callIndexedNonStrictGet(o) { + return o[nonstrict_number_get](); + } + + for (var i = 0; i < 10; i ++) { + assertEquals(("hello").strict(), "string"); + assertEquals(("hello").nonstrict(), "object"); + assertEquals(("hello")[strict_name](), "string"); + assertEquals(("hello")[nonstrict_name](), "object"); + assertEquals(("hello")[strict_number](), "string"); + assertEquals(("hello")[nonstrict_number](), "object"); + + assertEquals((10 + i).strict(), "number"); + assertEquals((10 + i).nonstrict(), "object"); + assertEquals((10 + i)[strict_name](), "number"); + assertEquals((10 + i)[nonstrict_name](), "object"); + assertEquals((10 + i)[strict_number](), "number"); + assertEquals((10 + i)[nonstrict_number](), "object"); + + assertEquals((true).strict(), "boolean"); + assertEquals((true).nonstrict(), "object"); + assertEquals((true)[strict_name](), "boolean"); + assertEquals((true)[nonstrict_name](), "object"); + assertEquals((true)[strict_number](), "boolean"); + assertEquals((true)[nonstrict_number](), "object"); + + assertEquals((false).strict(), "boolean"); + assertEquals((false).nonstrict(), "object"); + assertEquals((false)[strict_name](), "boolean"); + assertEquals((false)[nonstrict_name](), "object"); + assertEquals((false)[strict_number](), "boolean"); + assertEquals((false)[nonstrict_number](), "object"); + + assertEquals(callStrict("howdy"), "string"); + assertEquals(callNonStrict("howdy"), "object"); + assertEquals(callKeyedStrict("howdy"), "string"); + assertEquals(callKeyedNonStrict("howdy"), "object"); + assertEquals(callIndexedStrict("howdy"), "string"); + assertEquals(callIndexedNonStrict("howdy"), "object"); + + assertEquals(callStrict(17 + i), "number"); + assertEquals(callNonStrict(17 + i), "object"); + assertEquals(callKeyedStrict(17 + i), "number"); + assertEquals(callKeyedNonStrict(17 + i), "object"); + assertEquals(callIndexedStrict(17 + i), "number"); + assertEquals(callIndexedNonStrict(17 + i), "object"); + + assertEquals(callStrict(true), "boolean"); + assertEquals(callNonStrict(true), "object"); + assertEquals(callKeyedStrict(true), "boolean"); + assertEquals(callKeyedNonStrict(true), "object"); + assertEquals(callIndexedStrict(true), "boolean"); + assertEquals(callIndexedNonStrict(true), "object"); + + assertEquals(callStrict(false), "boolean"); + assertEquals(callNonStrict(false), "object"); + assertEquals(callKeyedStrict(false), "boolean"); + assertEquals(callKeyedNonStrict(false), "object"); + assertEquals(callIndexedStrict(false), "boolean"); + assertEquals(callIndexedNonStrict(false), "object"); + + // All of the above, with getters + assertEquals(("hello").strictget(), "string"); + assertEquals(("hello").nonstrictget(), "object"); + assertEquals(("hello")[strict_name_get](), "string"); + assertEquals(("hello")[nonstrict_name_get](), "object"); + assertEquals(("hello")[strict_number_get](), "string"); + assertEquals(("hello")[nonstrict_number_get](), "object"); + + assertEquals((10 + i).strictget(), "number"); + assertEquals((10 + i).nonstrictget(), "object"); + assertEquals((10 + i)[strict_name_get](), "number"); + assertEquals((10 + i)[nonstrict_name_get](), "object"); + assertEquals((10 + i)[strict_number_get](), "number"); + assertEquals((10 + i)[nonstrict_number_get](), "object"); + + assertEquals((true).strictget(), "boolean"); + assertEquals((true).nonstrictget(), "object"); + assertEquals((true)[strict_name_get](), "boolean"); + assertEquals((true)[nonstrict_name_get](), "object"); + assertEquals((true)[strict_number_get](), "boolean"); + assertEquals((true)[nonstrict_number_get](), "object"); + + assertEquals((false).strictget(), "boolean"); + assertEquals((false).nonstrictget(), "object"); + assertEquals((false)[strict_name_get](), "boolean"); + assertEquals((false)[nonstrict_name_get](), "object"); + assertEquals((false)[strict_number_get](), "boolean"); + assertEquals((false)[nonstrict_number_get](), "object"); + + assertEquals(callStrictGet("howdy"), "string"); + assertEquals(callNonStrictGet("howdy"), "object"); + assertEquals(callKeyedStrictGet("howdy"), "string"); + assertEquals(callKeyedNonStrictGet("howdy"), "object"); + assertEquals(callIndexedStrictGet("howdy"), "string"); + assertEquals(callIndexedNonStrictGet("howdy"), "object"); + + assertEquals(callStrictGet(17 + i), "number"); + assertEquals(callNonStrictGet(17 + i), "object"); + assertEquals(callKeyedStrictGet(17 + i), "number"); + assertEquals(callKeyedNonStrictGet(17 + i), "object"); + assertEquals(callIndexedStrictGet(17 + i), "number"); + assertEquals(callIndexedNonStrictGet(17 + i), "object"); + + assertEquals(callStrictGet(true), "boolean"); + assertEquals(callNonStrictGet(true), "object"); + assertEquals(callKeyedStrictGet(true), "boolean"); + assertEquals(callKeyedNonStrictGet(true), "object"); + assertEquals(callIndexedStrictGet(true), "boolean"); + assertEquals(callIndexedNonStrictGet(true), "object"); + + assertEquals(callStrictGet(false), "boolean"); + assertEquals(callNonStrictGet(false), "object"); + assertEquals(callKeyedStrictGet(false), "boolean"); + assertEquals(callKeyedNonStrictGet(false), "object"); + assertEquals(callIndexedStrictGet(false), "boolean"); + assertEquals(callIndexedNonStrictGet(false), "object"); + + } + } finally { + // Cleanup + cleanup(String); + cleanup(Number); + cleanup(Boolean); + } +})(); -- 2.7.4