From a9ce5bb5ea4a55a94949b01691ab126bee75f286 Mon Sep 17 00:00:00 2001 From: "dcarney@chromium.org" Date: Mon, 10 Jun 2013 07:41:16 +0000 Subject: [PATCH] add a default value for return value R=svenpanne@chromium.org BUG= Review URL: https://codereview.chromium.org/16642003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15024 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 31 ++++++++++++++++++++----------- src/arguments.h | 8 ++++++++ src/arm/stub-cache-arm.cc | 20 ++++++++++++-------- src/ia32/stub-cache-ia32.cc | 17 +++++++++++------ src/x64/stub-cache-x64.cc | 19 +++++++++++-------- test/cctest/test-api.cc | 7 +++++++ 6 files changed, 69 insertions(+), 33 deletions(-) diff --git a/include/v8.h b/include/v8.h index 68f2de0..e99c6d9 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2852,6 +2852,7 @@ class ReturnValue { template friend class ReturnValue; template friend class FunctionCallbackInfo; template friend class PropertyCallbackInfo; + V8_INLINE(internal::Object* GetDefaultValue()); V8_INLINE(explicit ReturnValue(internal::Object** slot)); internal::Object** value_; }; @@ -2876,16 +2877,17 @@ class FunctionCallbackInfo { V8_INLINE(Isolate* GetIsolate() const); V8_INLINE(ReturnValue GetReturnValue() const); // This shouldn't be public, but the arm compiler needs it. - static const int kArgsLength = 5; + static const int kArgsLength = 6; protected: friend class internal::FunctionCallbackArguments; friend class internal::CustomArguments; static const int kReturnValueIndex = 0; - static const int kIsolateIndex = -1; - static const int kDataIndex = -2; - static const int kCalleeIndex = -3; - static const int kHolderIndex = -4; + static const int kReturnValueDefaultValueIndex = -1; + static const int kIsolateIndex = -2; + static const int kDataIndex = -3; + static const int kCalleeIndex = -4; + static const int kHolderIndex = -5; V8_INLINE(FunctionCallbackInfo(internal::Object** implicit_args, internal::Object** values, @@ -2920,7 +2922,7 @@ class PropertyCallbackInfo { V8_INLINE(Local Holder() const); V8_INLINE(ReturnValue GetReturnValue() const); // This shouldn't be public, but the arm compiler needs it. - static const int kArgsLength = 5; + static const int kArgsLength = 6; protected: friend class MacroAssembler; @@ -2930,7 +2932,8 @@ class PropertyCallbackInfo { static const int kHolderIndex = -1; static const int kDataIndex = -2; static const int kReturnValueIndex = -3; - static const int kIsolateIndex = -4; + static const int kReturnValueDefaultValueIndex = -4; + static const int kIsolateIndex = -5; V8_INLINE(PropertyCallbackInfo(internal::Object** args)) : args_(args) { } @@ -5658,7 +5661,7 @@ template void ReturnValue::Set(const Persistent& handle) { TYPE_CHECK(T, S); if (V8_UNLIKELY(handle.IsEmpty())) { - SetUndefined(); + *value_ = GetDefaultValue(); } else { *value_ = *reinterpret_cast(*handle); } @@ -5669,7 +5672,7 @@ template void ReturnValue::Set(const Handle handle) { TYPE_CHECK(T, S); if (V8_UNLIKELY(handle.IsEmpty())) { - SetUndefined(); + *value_ = GetDefaultValue(); } else { *value_ = *reinterpret_cast(*handle); } @@ -5728,8 +5731,14 @@ void ReturnValue::SetUndefined() { template Isolate* ReturnValue::GetIsolate() { - // Isolate is always the pointer below value_ on the stack. - return *reinterpret_cast(&value_[-1]); + // Isolate is always the pointer below the default value on the stack. + return *reinterpret_cast(&value_[-2]); +} + +template +internal::Object* ReturnValue::GetDefaultValue() { + // Default value is always the pointer below value_ on the stack. + return value_[-1]; } diff --git a/src/arguments.h b/src/arguments.h index e13ddc9..b2608e5 100644 --- a/src/arguments.h +++ b/src/arguments.h @@ -253,6 +253,10 @@ class PropertyCallbackArguments values[T::kHolderIndex] = holder; values[T::kDataIndex] = data; values[T::kIsolateIndex] = reinterpret_cast(isolate); + // Here the hole is set as default value. + // It cannot escape into js as it's remove in Call below. + values[T::kReturnValueDefaultValueIndex] = + isolate->heap()->the_hole_value(); values[T::kReturnValueIndex] = isolate->heap()->the_hole_value(); ASSERT(values[T::kHolderIndex]->IsHeapObject()); ASSERT(values[T::kIsolateIndex]->IsSmi()); @@ -313,6 +317,10 @@ class FunctionCallbackArguments values[T::kCalleeIndex] = callee; values[T::kHolderIndex] = holder; values[T::kIsolateIndex] = reinterpret_cast(isolate); + // Here the hole is set as default value. + // It cannot escape into js as it's remove in Call below. + values[T::kReturnValueDefaultValueIndex] = + isolate->heap()->the_hole_value(); values[T::kReturnValueIndex] = isolate->heap()->the_hole_value(); ASSERT(values[T::kCalleeIndex]->IsJSFunction()); ASSERT(values[T::kHolderIndex]->IsHeapObject()); diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 39300e3..3595b52 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -893,11 +893,12 @@ static void GenerateFastApiDirectCall(MacroAssembler* masm, // -- sp[4] : callee JS function // -- sp[8] : call data // -- sp[12] : isolate - // -- sp[16] : ReturnValue - // -- sp[20] : last JS argument + // -- sp[16] : ReturnValue default value + // -- sp[20] : ReturnValue + // -- sp[24] : last JS argument // -- ... - // -- sp[(argc + 4) * 4] : first JS argument - // -- sp[(argc + 5) * 4] : receiver + // -- sp[(argc + 5) * 4] : first JS argument + // -- sp[(argc + 6) * 4] : receiver // ----------------------------------- // Get the function and setup the context. Handle function = optimization.constant_function(); @@ -914,13 +915,14 @@ static void GenerateFastApiDirectCall(MacroAssembler* masm, __ Move(r6, call_data); } __ mov(r7, Operand(ExternalReference::isolate_address(masm->isolate()))); - // Store JS function, call data, isolate and ReturnValue. + // Store JS function, call data, isolate ReturnValue default and ReturnValue. __ stm(ib, sp, r5.bit() | r6.bit() | r7.bit()); __ LoadRoot(r5, Heap::kUndefinedValueRootIndex); __ str(r5, MemOperand(sp, 4 * kPointerSize)); + __ str(r5, MemOperand(sp, 5 * kPointerSize)); // Prepare arguments. - __ add(r2, sp, Operand(4 * kPointerSize)); + __ add(r2, sp, Operand(5 * kPointerSize)); // Allocate the v8::Arguments structure in the arguments' space since // it's not controlled by GC. @@ -1434,9 +1436,11 @@ void BaseLoadStubCompiler::GenerateLoadCallback( } __ Push(reg, scratch3()); __ LoadRoot(scratch3(), Heap::kUndefinedValueRootIndex); + __ mov(scratch4(), scratch3()); + __ Push(scratch3(), scratch4()); __ mov(scratch4(), Operand(ExternalReference::isolate_address(isolate()))); - __ Push(scratch3(), scratch4(), name()); + __ Push(scratch4(), name()); __ mov(r0, sp); // r0 = Handle const int kApiStackSpace = 1; @@ -1462,7 +1466,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( __ CallApiFunctionAndReturn(ref, kStackUnwindSpace, returns_handle, - 4); + 5); } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 691e9a6..3906623 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -469,11 +469,12 @@ static void GenerateFastApiCall(MacroAssembler* masm, // (first fast api call extra argument) // -- esp[12] : api call data // -- esp[16] : isolate - // -- esp[20] : ReturnValue - // -- esp[24] : last argument + // -- esp[20] : ReturnValue default value + // -- esp[24] : ReturnValue + // -- esp[28] : last argument // -- ... - // -- esp[(argc + 5) * 4] : first argument - // -- esp[(argc + 6) * 4] : receiver + // -- esp[(argc + 6) * 4] : first argument + // -- esp[(argc + 7) * 4] : receiver // ----------------------------------- // Get the function and setup the context. Handle function = optimization.constant_function(); @@ -495,9 +496,11 @@ static void GenerateFastApiCall(MacroAssembler* masm, Immediate(reinterpret_cast(masm->isolate()))); __ mov(Operand(esp, 5 * kPointerSize), masm->isolate()->factory()->undefined_value()); + __ mov(Operand(esp, 6 * kPointerSize), + masm->isolate()->factory()->undefined_value()); // Prepare arguments. - STATIC_ASSERT(kFastApiCallArguments == 5); + STATIC_ASSERT(kFastApiCallArguments == 6); __ lea(eax, Operand(esp, kFastApiCallArguments * kPointerSize)); const int kApiArgc = 1; // API function gets reference to the v8::Arguments. @@ -1387,6 +1390,8 @@ void BaseLoadStubCompiler::GenerateLoadCallback( __ push(Immediate(Handle(callback->data(), isolate()))); } __ push(Immediate(isolate()->factory()->undefined_value())); // ReturnValue + // ReturnValue default value + __ push(Immediate(isolate()->factory()->undefined_value())); __ push(Immediate(reinterpret_cast(isolate()))); // Save a pointer to where we pushed the arguments pointer. This will be @@ -1420,7 +1425,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( __ CallApiFunctionAndReturn(getter_address, kStackSpace, returns_handle, - 5); + 6); } diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index bc3df68..06d8f71 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -449,12 +449,13 @@ static void GenerateFastApiCall(MacroAssembler* masm, // (first fast api call extra argument) // -- rsp[24] : api call data // -- rsp[32] : isolate - // -- rsp[40] : ReturnValue + // -- rsp[40] : ReturnValue default value + // -- rsp[48] : ReturnValue // - // -- rsp[48] : last argument + // -- rsp[56] : last argument // -- ... - // -- rsp[(argc + 5) * 8] : first argument - // -- rsp[(argc + 6) * 8] : receiver + // -- rsp[(argc + 6) * 8] : first argument + // -- rsp[(argc + 7) * 8] : receiver // ----------------------------------- // Get the function and setup the context. Handle function = optimization.constant_function(); @@ -477,9 +478,10 @@ static void GenerateFastApiCall(MacroAssembler* masm, __ movq(Operand(rsp, 4 * kPointerSize), kScratchRegister); __ LoadRoot(kScratchRegister, Heap::kUndefinedValueRootIndex); __ movq(Operand(rsp, 5 * kPointerSize), kScratchRegister); + __ movq(Operand(rsp, 6 * kPointerSize), kScratchRegister); // Prepare arguments. - STATIC_ASSERT(kFastApiCallArguments == 5); + STATIC_ASSERT(kFastApiCallArguments == 6); __ lea(rbx, Operand(rsp, kFastApiCallArguments * kPointerSize)); // Function address is a foreign pointer outside V8's heap. @@ -1305,6 +1307,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( } __ LoadRoot(kScratchRegister, Heap::kUndefinedValueRootIndex); __ push(kScratchRegister); // return value + __ push(kScratchRegister); // return value default __ PushAddress(ExternalReference::isolate_address(isolate())); __ push(name()); // name // Save a pointer to where we pushed the arguments pointer. This will be @@ -1337,8 +1340,8 @@ void BaseLoadStubCompiler::GenerateLoadCallback( const int kArgStackSpace = 1; __ PrepareCallApiFunction(kArgStackSpace, returns_handle); - STATIC_ASSERT(PropertyCallbackArguments::kArgsLength == 5); - __ lea(rax, Operand(name_arg, 5 * kPointerSize)); + STATIC_ASSERT(PropertyCallbackArguments::kArgsLength == 6); + __ lea(rax, Operand(name_arg, 6 * kPointerSize)); // v8::AccessorInfo::args_. __ movq(StackSpaceOperand(0), rax); @@ -1350,7 +1353,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( __ CallApiFunctionAndReturn(getter_address, kStackSpace, returns_handle, - 4); + 5); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 10f8af1..a69fe90 100755 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -810,6 +810,13 @@ static void CheckReturnValue(const T& t) { CHECK_EQ(v8::Isolate::GetCurrent(), t.GetIsolate()); CHECK_EQ(t.GetIsolate(), rv.GetIsolate()); CHECK((*o)->IsTheHole() || (*o)->IsUndefined()); + // Verify reset + bool is_runtime = (*o)->IsTheHole(); + rv.Set(true); + CHECK(!(*o)->IsTheHole() && !(*o)->IsUndefined()); + rv.Set(v8::Handle()); + CHECK((*o)->IsTheHole() || (*o)->IsUndefined()); + CHECK_EQ(is_runtime, (*o)->IsTheHole()); } static v8::Handle handle_call(const v8::Arguments& args) { -- 2.7.4