From 2dcb4d2c59da14df5706788d281b9da2b8d58313 Mon Sep 17 00:00:00 2001 From: "kasperl@chromium.org" Date: Mon, 27 Oct 2008 14:12:02 +0000 Subject: [PATCH] Fix issue 120 by patching the on-stack receiver in the IC stubs for calls just before invoking the target function instead of doing it before resolving the function. Review URL: http://codereview.chromium.org/8192 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@607 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/codegen-arm.cc | 7 +-- src/codegen-ia32.cc | 7 +-- src/ic-arm.cc | 115 +++++++++++++++++++++++++++-------------- src/ic-ia32.cc | 96 +++++++++++++++++++++++----------- src/stub-cache-arm.cc | 28 ++++++---- src/stub-cache-ia32.cc | 26 +++++++--- 6 files changed, 191 insertions(+), 88 deletions(-) diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc index c0b0e2ab98..4b1ada14f0 100644 --- a/src/codegen-arm.cc +++ b/src/codegen-arm.cc @@ -2240,9 +2240,10 @@ void CodeGenerator::VisitCall(Call* node) { __ mov(r0, Operand(var->name())); __ push(r0); - // TODO(120): Use global object for function lookup and inline - // cache, and use global proxy as 'this' for invocation. - LoadGlobalReceiver(r0); + // Pass the global object as the receiver and let the IC stub + // patch the stack to use the global proxy as 'this' in the + // invoked function. + LoadGlobal(); // Load the arguments. for (int i = 0; i < args->length(); i++) Load(args->at(i)); diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc index a92866c5f9..27655870db 100644 --- a/src/codegen-ia32.cc +++ b/src/codegen-ia32.cc @@ -2660,9 +2660,10 @@ void CodeGenerator::VisitCall(Call* node) { // Push the name of the function and the receiver onto the stack. frame_->Push(Immediate(var->name())); - // TODO(120): Use global object for function lookup and inline - // cache, and use global proxy as 'this' for invocation. - LoadGlobalReceiver(eax); + // Pass the global object as the receiver and let the IC stub + // patch the stack to use the global proxy as 'this' in the + // invoked function. + LoadGlobal(); // Load the arguments. for (int i = 0; i < args->length(); i++) { diff --git a/src/ic-arm.cc b/src/ic-arm.cc index 7063d6e7f3..c29caf125b 100644 --- a/src/ic-arm.cc +++ b/src/ic-arm.cc @@ -44,8 +44,7 @@ namespace v8 { namespace internal { // Helper function used from LoadIC/CallIC GenerateNormal. static void GenerateDictionaryLoad(MacroAssembler* masm, - Label* done_label, - Label* miss_label, + Label* miss, Register t0, Register t1) { // Register use: @@ -61,6 +60,8 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, // // r2 - holds the name of the property and is unchanges. + Label done; + // Check for the absence of an interceptor. // Load the map into t0. __ ldr(t0, FieldMemOperand(t1, JSObject::kMapOffset)); @@ -68,14 +69,14 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, __ ldr(t0, FieldMemOperand(t1, Map::kInstanceAttributesOffset)); __ tst(t0, Operand(1 << (Map::kHasNamedInterceptor + (3 * 8)))); // Jump to miss if the interceptor bit is set. - __ b(ne, miss_label); + __ b(ne, miss); // Check that the properties array is a dictionary. __ ldr(t0, FieldMemOperand(t1, JSObject::kPropertiesOffset)); __ ldr(r3, FieldMemOperand(t0, HeapObject::kMapOffset)); __ cmp(r3, Operand(Factory::hash_table_map())); - __ b(ne, miss_label); + __ b(ne, miss); // Compute the capacity mask. const int kCapacityOffset = @@ -107,17 +108,17 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, __ ldr(ip, FieldMemOperand(t1, kElementsStartOffset)); __ cmp(r2, Operand(ip)); if (i != kProbes - 1) { - __ b(eq, done_label); + __ b(eq, &done); } else { - __ b(ne, miss_label); + __ b(ne, miss); } } // Check that the value is a normal property. - __ bind(done_label); // t1 == t0 + 4*index + __ bind(&done); // t1 == t0 + 4*index __ ldr(r3, FieldMemOperand(t1, kElementsStartOffset + 2 * kPointerSize)); __ tst(r3, Operand(PropertyDetails::TypeField::mask() << kSmiTagSize)); - __ b(ne, miss_label); + __ b(ne, miss); // Get the value at the masked, scaled index and return. __ ldr(t1, FieldMemOperand(t1, kElementsStartOffset + 1 * kPointerSize)); @@ -273,12 +274,42 @@ void CallIC::GenerateMegamorphic(MacroAssembler* masm, int argc) { } +static void GenerateNormalHelper(MacroAssembler* masm, + int argc, + bool is_global_object, + Label* miss) { + // Search dictionary - put result in register r1. + GenerateDictionaryLoad(masm, miss, r0, r1); + + // Check that the value isn't a smi. + __ tst(r1, Operand(kSmiTagMask)); + __ b(eq, miss); + + // Check that the value is a JSFunction. + __ ldr(r0, FieldMemOperand(r1, HeapObject::kMapOffset)); + __ ldrb(r0, FieldMemOperand(r0, Map::kInstanceTypeOffset)); + __ cmp(r0, Operand(JS_FUNCTION_TYPE)); + __ b(ne, miss); + + // Patch the receiver with the global proxy if necessary. + if (is_global_object) { + __ ldr(r2, MemOperand(sp, argc * kPointerSize)); + __ ldr(r2, FieldMemOperand(r2, GlobalObject::kGlobalReceiverOffset)); + __ str(r2, MemOperand(sp, argc * kPointerSize)); + } + + // Invoke the function. + ParameterCount actual(argc); + __ InvokeFunction(r1, actual, JUMP_FUNCTION); +} + + void CallIC::GenerateNormal(MacroAssembler* masm, int argc) { // ----------- S t a t e ------------- // -- lr: return address // ----------------------------------- - Label miss, probe, done, global; + Label miss, global_object, non_global_object; // Get the receiver of the function from the stack into r1. __ ldr(r1, MemOperand(sp, argc * kPointerSize)); @@ -298,35 +329,28 @@ void CallIC::GenerateNormal(MacroAssembler* masm, int argc) { // If this assert fails, we have to check upper bound too. ASSERT(LAST_TYPE == JS_FUNCTION_TYPE); - // Check for access to global proxy. + // Check for access to global object. + __ cmp(r0, Operand(JS_GLOBAL_OBJECT_TYPE)); + __ b(eq, &global_object); + __ cmp(r0, Operand(JS_BUILTINS_OBJECT_TYPE)); + __ b(ne, &non_global_object); + + // Accessing global object: Load and invoke. + __ bind(&global_object); + GenerateNormalHelper(masm, argc, true, &miss); + + // Accessing non-global object: Check for access to global proxy. + Label global_proxy, invoke; + __ bind(&non_global_object); __ cmp(r0, Operand(JS_GLOBAL_PROXY_TYPE)); - __ b(eq, &global); - - // Search the dictionary placing the result in r1. - __ bind(&probe); - GenerateDictionaryLoad(masm, &done, &miss, r0, r1); - - // Check that the value isn't a smi. - __ tst(r1, Operand(kSmiTagMask)); - __ b(eq, &miss); - - // Check that the value is a JSFunction. - __ ldr(r0, FieldMemOperand(r1, HeapObject::kMapOffset)); - __ ldrb(r0, FieldMemOperand(r0, Map::kInstanceTypeOffset)); - __ cmp(r0, Operand(JS_FUNCTION_TYPE)); - __ b(ne, &miss); - - // TODO(120): Check for access to global object. Needs patching of - // receiver but no security check. - - // Invoke the function. - ParameterCount actual(argc); - __ InvokeFunction(r1, actual, JUMP_FUNCTION); + __ b(eq, &global_proxy); + __ bind(&invoke); + GenerateNormalHelper(masm, argc, false, &miss); // Global object access: Check access rights. - __ bind(&global); + __ bind(&global_proxy); __ CheckAccessGlobalProxy(r1, r0, &miss); - __ b(&probe); + __ b(&invoke); // Cache miss: Jump to runtime. __ bind(&miss); @@ -362,11 +386,26 @@ void CallIC::Generate(MacroAssembler* masm, __ mov(r1, Operand(r0)); __ LeaveInternalFrame(); - // TODO(120): Check for access to to global object. Needs patching - // of receiver but no security check. + // Check if the receiver is a global object of some sort. + Label invoke, global; + __ ldr(r2, MemOperand(sp, argc * kPointerSize)); // receiver + __ tst(r2, Operand(kSmiTagMask)); + __ b(eq, &invoke); + __ ldr(r3, FieldMemOperand(r2, HeapObject::kMapOffset)); + __ ldrb(r3, FieldMemOperand(r3, Map::kInstanceTypeOffset)); + __ cmp(r3, Operand(JS_GLOBAL_OBJECT_TYPE)); + __ b(eq, &global); + __ cmp(r3, Operand(JS_BUILTINS_OBJECT_TYPE)); + __ b(ne, &invoke); + + // Patch the receiver on the stack. + __ bind(&global); + __ ldr(r2, FieldMemOperand(r2, GlobalObject::kGlobalReceiverOffset)); + __ str(r2, MemOperand(sp, argc * kPointerSize)); // Invoke the function. ParameterCount actual(argc); + __ bind(&invoke); __ InvokeFunction(r1, actual, JUMP_FUNCTION); } @@ -398,7 +437,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { // -- [sp] : receiver // ----------------------------------- - Label miss, probe, done, global; + Label miss, probe, global; __ ldr(r0, MemOperand(sp, 0)); // Check that the receiver isn't a smi. @@ -418,7 +457,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { __ b(eq, &global); __ bind(&probe); - GenerateDictionaryLoad(masm, &done, &miss, r1, r0); + GenerateDictionaryLoad(masm, &miss, r1, r0); __ Ret(); // Global object access: Check access rights. diff --git a/src/ic-ia32.cc b/src/ic-ia32.cc index eca1e188de..82eb14d9d3 100644 --- a/src/ic-ia32.cc +++ b/src/ic-ia32.cc @@ -438,11 +438,42 @@ void CallIC::GenerateMegamorphic(MacroAssembler* masm, int argc) { } +static void GenerateNormalHelper(MacroAssembler* masm, + int argc, + bool is_global_object, + Label* miss) { + // Search dictionary - put result in register edx. + GenerateDictionaryLoad(masm, miss, eax, edx, ebx, ecx); + + // Move the result to register edi and check that it isn't a smi. + __ mov(edi, Operand(edx)); + __ test(edx, Immediate(kSmiTagMask)); + __ j(zero, miss, not_taken); + + // Check that the value is a JavaScript function. + __ mov(edx, FieldOperand(edx, HeapObject::kMapOffset)); + __ movzx_b(edx, FieldOperand(edx, Map::kInstanceTypeOffset)); + __ cmp(edx, JS_FUNCTION_TYPE); + __ j(not_equal, miss, not_taken); + + // Patch the receiver with the global proxy if necessary. + if (is_global_object) { + __ mov(edx, Operand(esp, (argc + 1) * kPointerSize)); + __ mov(edx, FieldOperand(edx, GlobalObject::kGlobalReceiverOffset)); + __ mov(Operand(esp, (argc + 1) * kPointerSize), edx); + } + + // Invoke the function. + ParameterCount actual(argc); + __ InvokeFunction(edi, actual, JUMP_FUNCTION); +} + + void CallIC::GenerateNormal(MacroAssembler* masm, int argc) { // ----------- S t a t e ------------- // ----------------------------------- - Label miss, probe, global; + Label miss, global_object, non_global_object; // Get the receiver of the function from the stack; 1 ~ return address. __ mov(edx, Operand(esp, (argc + 1) * kPointerSize)); @@ -462,36 +493,28 @@ void CallIC::GenerateNormal(MacroAssembler* masm, int argc) { // If this assert fails, we have to check upper bound too. ASSERT(LAST_TYPE == JS_FUNCTION_TYPE); - // Check for access to global proxy. + // Check for access to global object. + __ cmp(eax, JS_GLOBAL_OBJECT_TYPE); + __ j(equal, &global_object); + __ cmp(eax, JS_BUILTINS_OBJECT_TYPE); + __ j(not_equal, &non_global_object); + + // Accessing global object: Load and invoke. + __ bind(&global_object); + GenerateNormalHelper(masm, argc, true, &miss); + + // Accessing non-global object: Check for access to global proxy. + Label global_proxy, invoke; + __ bind(&non_global_object); __ cmp(eax, JS_GLOBAL_PROXY_TYPE); - __ j(equal, &global, not_taken); - - // Search the dictionary placing the result in edx. - __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, eax, edx, ebx, ecx); - - // Move the result to register edi and check that it isn't a smi. - __ mov(edi, Operand(edx)); - __ test(edx, Immediate(kSmiTagMask)); - __ j(zero, &miss, not_taken); - - // Check that the value is a JavaScript function. - __ mov(edx, FieldOperand(edx, HeapObject::kMapOffset)); - __ movzx_b(edx, FieldOperand(edx, Map::kInstanceTypeOffset)); - __ cmp(edx, JS_FUNCTION_TYPE); - __ j(not_equal, &miss, not_taken); - - // TODO(120): Check for access to global object. Needs patching of - // receiver but no security check. - - // Invoke the function. - ParameterCount actual(argc); - __ InvokeFunction(edi, actual, JUMP_FUNCTION); + __ j(equal, &global_proxy, not_taken); + __ bind(&invoke); + GenerateNormalHelper(masm, argc, false, &miss); // Global object proxy access: Check access rights. - __ bind(&global); + __ bind(&global_proxy); __ CheckAccessGlobalProxy(edx, eax, &miss); - __ jmp(&probe); + __ jmp(&invoke); // Cache miss: Jump to runtime. __ bind(&miss); @@ -528,11 +551,26 @@ void CallIC::Generate(MacroAssembler* masm, __ mov(Operand(edi), eax); __ LeaveInternalFrame(); - // TODO(120): Check for access to to global object. Needs patching - // of receiver but no security check. + // Check if the receiver is a global object of some sort. + Label invoke, global; + __ mov(edx, Operand(esp, (argc + 1) * kPointerSize)); // receiver + __ test(edx, Immediate(kSmiTagMask)); + __ j(zero, &invoke, not_taken); + __ mov(ecx, FieldOperand(edx, HeapObject::kMapOffset)); + __ movzx_b(ecx, FieldOperand(ecx, Map::kInstanceTypeOffset)); + __ cmp(ecx, JS_GLOBAL_OBJECT_TYPE); + __ j(equal, &global); + __ cmp(ecx, JS_BUILTINS_OBJECT_TYPE); + __ j(not_equal, &invoke); + + // Patch the receiver on the stack. + __ bind(&global); + __ mov(edx, FieldOperand(edx, GlobalObject::kGlobalReceiverOffset)); + __ mov(Operand(esp, (argc + 1) * kPointerSize), edx); // Invoke the function. ParameterCount actual(argc); + __ bind(&invoke); __ InvokeFunction(edi, actual, JUMP_FUNCTION); } diff --git a/src/stub-cache-arm.cc b/src/stub-cache-arm.cc index 5a3f994d49..2ef657e2ca 100644 --- a/src/stub-cache-arm.cc +++ b/src/stub-cache-arm.cc @@ -220,15 +220,15 @@ Object* CallStubCompiler::CompileCallField(Object* object, const int argc = arguments().immediate(); - // Get the receiver of the function from the stack into r1. - __ ldr(r1, MemOperand(sp, argc * kPointerSize)); + // Get the receiver of the function from the stack into r0. + __ ldr(r0, MemOperand(sp, argc * kPointerSize)); // Check that the receiver isn't a smi. - __ tst(r1, Operand(kSmiTagMask)); + __ tst(r0, Operand(kSmiTagMask)); __ b(eq, &miss); // Do the right check and compute the holder register. Register reg = - __ CheckMaps(JSObject::cast(object), r1, holder, r3, r2, &miss); + __ CheckMaps(JSObject::cast(object), r0, holder, r3, r2, &miss); GenerateFastPropertyLoad(masm(), r1, reg, holder, index); // Check that the function really is a function. @@ -240,8 +240,11 @@ Object* CallStubCompiler::CompileCallField(Object* object, __ cmp(r2, Operand(JS_FUNCTION_TYPE)); __ b(ne, &miss); + // Patch the receiver on the stack with the global proxy if + // necessary. if (object->IsGlobalObject()) { - // TODO(120): Patch receiver with the global proxy. + __ ldr(r3, FieldMemOperand(r0, GlobalObject::kGlobalReceiverOffset)); + __ str(r3, MemOperand(sp, argc * kPointerSize)); } // Invoke the function. @@ -278,10 +281,21 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, __ b(eq, &miss); } + // Make sure that it's okay not to patch the on stack receiver + // unless we're doing a receiver map check. + ASSERT(!object->IsGlobalObject() || check == RECEIVER_MAP_CHECK); + switch (check) { case RECEIVER_MAP_CHECK: // Check that the maps haven't changed. __ CheckMaps(JSObject::cast(object), r1, holder, r3, r2, &miss); + + // Patch the receiver on the stack with the global proxy if + // necessary. + if (object->IsGlobalObject()) { + __ ldr(r3, FieldMemOperand(r1, GlobalObject::kGlobalReceiverOffset)); + __ str(r3, MemOperand(sp, argc * kPointerSize)); + } break; case STRING_CHECK: @@ -353,10 +367,6 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, __ mov(r1, Operand(Handle(function))); __ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset)); - if (object->IsGlobalObject()) { - // TODO(120): Patch receiver with the global proxy. - } - // Jump to the cached code (tail call). Handle code(function->code()); ParameterCount expected(function->shared()->formal_parameter_count()); diff --git a/src/stub-cache-ia32.cc b/src/stub-cache-ia32.cc index 67c4e315d0..da3d8858bb 100644 --- a/src/stub-cache-ia32.cc +++ b/src/stub-cache-ia32.cc @@ -480,8 +480,11 @@ Object* CallStubCompiler::CompileCallField(Object* object, __ cmp(ebx, JS_FUNCTION_TYPE); __ j(not_equal, &miss, not_taken); + // Patch the receiver on the stack with the global proxy if + // necessary. if (object->IsGlobalObject()) { - // TODO(120): Patch receiver with the global proxy. + __ mov(edx, FieldOperand(edx, GlobalObject::kGlobalReceiverOffset)); + __ mov(Operand(esp, (argc + 1) * kPointerSize), edx); } // Invoke the function. @@ -517,10 +520,21 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, __ j(zero, &miss, not_taken); } + // Make sure that it's okay not to patch the on stack receiver + // unless we're doing a receiver map check. + ASSERT(!object->IsGlobalObject() || check == RECEIVER_MAP_CHECK); + switch (check) { case RECEIVER_MAP_CHECK: // Check that the maps haven't changed. __ CheckMaps(JSObject::cast(object), edx, holder, ebx, ecx, &miss); + + // Patch the receiver on the stack with the global proxy if + // necessary. + if (object->IsGlobalObject()) { + __ mov(edx, FieldOperand(edx, GlobalObject::kGlobalReceiverOffset)); + __ mov(Operand(esp, (argc + 1) * kPointerSize), edx); + } break; case STRING_CHECK: @@ -592,10 +606,6 @@ Object* CallStubCompiler::CompileCallConstant(Object* object, __ mov(Operand(edi), Immediate(Handle(function))); __ mov(esi, FieldOperand(edi, JSFunction::kContextOffset)); - if (object->IsGlobalObject()) { - // TODO(120): Patch receiver with the global proxy. - } - // Jump to the cached code (tail call). Handle code(function->code()); ParameterCount expected(function->shared()->formal_parameter_count()); @@ -626,6 +636,7 @@ Object* CallStubCompiler::CompileCallInterceptor(Object* object, // Get the receiver from the stack. __ mov(edx, Operand(esp, (argc + 1) * kPointerSize)); + // Check that the receiver isn't a smi. __ test(edx, Immediate(kSmiTagMask)); __ j(zero, &miss, not_taken); @@ -666,8 +677,11 @@ Object* CallStubCompiler::CompileCallInterceptor(Object* object, __ cmp(ebx, JS_FUNCTION_TYPE); __ j(not_equal, &miss, not_taken); + // Patch the receiver on the stack with the global proxy if + // necessary. if (object->IsGlobalObject()) { - // TODO(120): Patch receiver with the global proxy. + __ mov(edx, FieldOperand(edx, GlobalObject::kGlobalReceiverOffset)); + __ mov(Operand(esp, (argc + 1) * kPointerSize), edx); } // Invoke the function.