From b1530257ab7944aae5d9ea14841d8c0aae511c9e Mon Sep 17 00:00:00 2001 From: "danno@chromium.org" Date: Fri, 13 Jul 2012 16:33:27 +0000 Subject: [PATCH] Revert 12083: Implements a new API to set a function entry hook for profiling. TBR=mstarzinger@chromium.org Review URL: https://chromiumcodereview.appspot.com/10695206 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12084 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 40 ++------------- src/api.cc | 6 --- src/arm/code-stubs-arm.cc | 59 ---------------------- src/arm/full-codegen-arm.cc | 2 - src/arm/lithium-codegen-arm.cc | 2 - src/code-stubs.cc | 22 -------- src/code-stubs.h | 34 +------------ src/ia32/code-stubs-ia32.cc | 32 ------------ src/ia32/full-codegen-ia32.cc | 3 +- src/ia32/lithium-codegen-ia32.cc | 2 - src/x64/code-stubs-x64.cc | 62 ----------------------- src/x64/full-codegen-x64.cc | 2 - src/x64/lithium-codegen-x64.cc | 2 - test/cctest/test-api.cc | 105 --------------------------------------- 14 files changed, 6 insertions(+), 367 deletions(-) diff --git a/include/v8.h b/include/v8.h index 34995d0..18c5ba6 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2915,34 +2915,16 @@ typedef bool (*EntropySource)(unsigned char* buffer, size_t length); * resolving the location of a return address on the stack. Profilers that * change the return address on the stack can use this to resolve the stack * location to whereever the profiler stashed the original return address. - * - * \param return_addr_location points to a location on stack where a machine - * return address resides. - * \returns either return_addr_location, or else a pointer to the profiler's - * copy of the original return address. - * - * \note the resolver function must not cause garbage collection. + * When invoked, return_addr_location will point to a location on stack where + * a machine return address resides, this function should return either the + * same pointer, or a pointer to the profiler's copy of the original return + * address. */ typedef uintptr_t (*ReturnAddressLocationResolver)( uintptr_t return_addr_location); /** - * FunctionEntryHook is the type of the profile entry hook called at entry to - * any generated function when function-level profiling is enabled. - * - * \param function the address of the function that's being entered. - * \param return_addr_location points to a location on stack where the machine - * return address resides. This can be used to identify the caller of - * \p function, and/or modified to divert execution when \p function exits. - * - * \note the entry hook must not cause garbage collection. - */ -typedef void (*FunctionEntryHook)(uintptr_t function, - uintptr_t return_addr_location); - - -/** * Interface for iterating though all external resources in the heap. */ class V8EXPORT ExternalResourceVisitor { // NOLINT @@ -3203,20 +3185,6 @@ class V8EXPORT V8 { ReturnAddressLocationResolver return_address_resolver); /** - * Allows the host application to provide the address of a function that's - * invoked on entry to every V8-generated function. - * Note that \p entry_hook is invoked at the very start of each - * generated function. - * - * \param entry_hook a function that will be invoked on entry to every - * V8-generated function. - * \returns true on success on supported platforms, false on failure. - * \note Setting a new entry hook function when one is already active will - * fail. - */ - static bool SetFunctionEntryHook(FunctionEntryHook entry_hook); - - /** * Adjusts the amount of registered external memory. Used to give * V8 an indication of the amount of externally allocated memory * that is kept alive by JavaScript objects. V8 uses this to decide diff --git a/src/api.cc b/src/api.cc index d7037be..4b1a3a1 100644 --- a/src/api.cc +++ b/src/api.cc @@ -33,7 +33,6 @@ #include "../include/v8-profiler.h" #include "../include/v8-testing.h" #include "bootstrapper.h" -#include "code-stubs.h" #include "compiler.h" #include "conversions-inl.h" #include "counters.h" @@ -4233,11 +4232,6 @@ void v8::V8::SetReturnAddressLocationResolver( } -bool v8::V8::SetFunctionEntryHook(FunctionEntryHook entry_hook) { - return i::ProfileEntryHookStub::SetFunctionEntryHook(entry_hook); -} - - bool v8::V8::Dispose() { i::Isolate* isolate = i::Isolate::Current(); if (!ApiCheck(isolate != NULL && isolate->IsDefaultIsolate(), diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index e059c2f..169a032 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -7513,65 +7513,6 @@ void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { __ Ret(); } - -void ProfileEntryHookStub::MaybeCallEntryHook(MacroAssembler* masm) { - if (entry_hook_ != NULL) { - ProfileEntryHookStub stub; - __ push(lr); - __ CallStub(&stub); - __ pop(lr); - } -} - - -void ProfileEntryHookStub::Generate(MacroAssembler* masm) { - // The entry hook is a "push lr" instruction, followed by a call. - const int32_t kReturnAddressDistanceFromFunctionStart = - Assembler::kCallTargetAddressOffset + Assembler::kInstrSize; - - // Save live volatile registers. - __ Push(lr, r5, r1); - const int32_t kNumSavedRegs = 3; - - // Compute the function's address for the first argument. - __ sub(r0, lr, Operand(kReturnAddressDistanceFromFunctionStart)); - - // The caller's return address is above the saved temporaries. - // Grab that for the second argument to the hook. - __ add(r1, sp, Operand(kNumSavedRegs * kPointerSize)); - - // Align the stack if necessary. - int frame_alignment = masm->ActivationFrameAlignment(); - if (frame_alignment > kPointerSize) { - __ mov(r5, sp); - ASSERT(IsPowerOf2(frame_alignment)); - __ and_(sp, sp, Operand(-frame_alignment)); - } - -#if defined(V8_HOST_ARCH_ARM) - __ mov(ip, Operand(reinterpret_cast(&entry_hook_))); - __ ldr(ip, MemOperand(ip)); -#else - // Under the simulator we need to indirect the entry hook through a - // trampoline function at a known address. - Address trampoline_address = reinterpret_cast
( - reinterpret_cast(EntryHookTrampoline)); - ApiFunction dispatcher(trampoline_address); - __ mov(ip, Operand(ExternalReference(&dispatcher, - ExternalReference::BUILTIN_CALL, - masm->isolate()))); -#endif - __ Call(ip); - - // Restore the stack pointer if needed. - if (frame_alignment > kPointerSize) { - __ mov(sp, r5); - } - - __ Pop(lr, r5, r1); - __ Ret(); -} - #undef __ } } // namespace v8::internal diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 15e9d8b..aadff7a 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -134,8 +134,6 @@ void FullCodeGenerator::Generate() { SetFunctionPosition(function()); Comment cmnt(masm_, "[ function compiled by full code generator"); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 29588f4..fb687f7 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -127,8 +127,6 @@ void LCodeGen::Comment(const char* format, ...) { bool LCodeGen::GeneratePrologue() { ASSERT(is_generating()); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info_->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { diff --git a/src/code-stubs.cc b/src/code-stubs.cc index f672d33..8f31660 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -470,26 +470,4 @@ void ElementsTransitionAndStoreStub::Generate(MacroAssembler* masm) { KeyedStoreIC::GenerateRuntimeSetProperty(masm, strict_mode_); } - -FunctionEntryHook ProfileEntryHookStub::entry_hook_ = NULL; - - -void ProfileEntryHookStub::EntryHookTrampoline(intptr_t function, - intptr_t stack_pointer) { - if (entry_hook_ != NULL) - entry_hook_(function, stack_pointer); -} - - -bool ProfileEntryHookStub::SetFunctionEntryHook(FunctionEntryHook entry_hook) { - // We don't allow setting a new entry hook over one that's - // already active, as the hooks won't stack. - if (entry_hook != 0 && entry_hook_ != 0) - return false; - - entry_hook_ = entry_hook; - return true; -} - - } } // namespace v8::internal diff --git a/src/code-stubs.h b/src/code-stubs.h index f190632..5a076d2 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -73,8 +73,7 @@ namespace internal { V(DebuggerStatement) \ V(StringDictionaryLookup) \ V(ElementsTransitionAndStore) \ - V(StoreArrayLiteralElement) \ - V(ProfileEntryHook) + V(StoreArrayLiteralElement) // List of code stubs only used on ARM platforms. #ifdef V8_TARGET_ARCH_ARM @@ -1143,37 +1142,6 @@ class StoreArrayLiteralElementStub : public CodeStub { DISALLOW_COPY_AND_ASSIGN(StoreArrayLiteralElementStub); }; - -class ProfileEntryHookStub : public CodeStub { - public: - explicit ProfileEntryHookStub() {} - - // The profile entry hook function is not allowed to cause a GC. - virtual bool SometimesSetsUpAFrame() { return false; } - - // Generates a call to the entry hook if it's enabled. - static void MaybeCallEntryHook(MacroAssembler* masm); - - // Sets or unsets the entry hook function. Returns true on success, - // false on an attempt to replace a non-NULL entry hook with another - // non-NULL hook. - static bool SetFunctionEntryHook(FunctionEntryHook entry_hook); - - private: - static void EntryHookTrampoline(intptr_t function, - intptr_t stack_pointer); - - Major MajorKey() { return ProfileEntryHook; } - int MinorKey() { return 0; } - - void Generate(MacroAssembler* masm); - - // The current function entry hook. - static FunctionEntryHook entry_hook_; - - DISALLOW_COPY_AND_ASSIGN(ProfileEntryHookStub); -}; - } } // namespace v8::internal #endif // V8_CODE_STUBS_H_ diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 7be2b4f..afa3e1c 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -7474,38 +7474,6 @@ void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { __ ret(0); } - -void ProfileEntryHookStub::MaybeCallEntryHook(MacroAssembler* masm) { - if (entry_hook_ != NULL) { - ProfileEntryHookStub stub; - masm->CallStub(&stub); - } -} - - -void ProfileEntryHookStub::Generate(MacroAssembler* masm) { - // Ecx is the only volatile register we must save. - __ push(ecx); - - // Calculate and push the original stack pointer. - __ lea(eax, Operand(esp, kPointerSize)); - __ push(eax); - - // Calculate and push the function address. - __ mov(eax, Operand(eax, 0)); - __ sub(eax, Immediate(Assembler::kCallInstructionLength)); - __ push(eax); - - // Call the entry hook. - int32_t hook_location = reinterpret_cast(&entry_hook_); - __ call(Operand(hook_location, RelocInfo::NONE)); - __ add(esp, Immediate(2 * kPointerSize)); - - // Restore ecx. - __ pop(ecx); - __ ret(0); -} - #undef __ } } // namespace v8::internal diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 5daf940..2867d5e 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -123,8 +123,6 @@ void FullCodeGenerator::Generate() { SetFunctionPosition(function()); Comment cmnt(masm_, "[ function compiled by full code generator"); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { @@ -4552,6 +4550,7 @@ FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit( return previous_; } + #undef __ } } // namespace v8::internal diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 4abdc8b..6317a54 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -135,8 +135,6 @@ void LCodeGen::Comment(const char* format, ...) { bool LCodeGen::GeneratePrologue() { ASSERT(is_generating()); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info_->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 957f631..ecdb392 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -6420,68 +6420,6 @@ void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { __ ret(0); } - -void ProfileEntryHookStub::MaybeCallEntryHook(MacroAssembler* masm) { - if (entry_hook_ != NULL) { - ProfileEntryHookStub stub; - masm->CallStub(&stub); - } -} - - -void ProfileEntryHookStub::Generate(MacroAssembler* masm) { - // Save volatile registers. -#ifdef _WIN64 - const int kNumSavedRegisters = 1; - - __ push(rcx); -#else - const int kNumSavedRegisters = 3; - - __ push(rcx); - __ push(rdi); - __ push(rsi); -#endif - - // Calculate the original stack pointer and store it in the second arg. -#ifdef _WIN64 - __ lea(rdx, Operand(rsp, kNumSavedRegisters * kPointerSize)); -#else - __ lea(rsi, Operand(rsp, kNumSavedRegisters * kPointerSize)); -#endif - - // Calculate the function address to the first arg. -#ifdef _WIN64 - __ movq(rcx, Operand(rdx, 0)); - __ subq(rcx, Immediate(Assembler::kShortCallInstructionLength)); -#else - __ movq(rdi, Operand(rsi, 0)); - __ subq(rdi, Immediate(Assembler::kShortCallInstructionLength)); -#endif - - // Reserve stack for the first 4 args and align the stack. - __ movq(kScratchRegister, rsp); - __ subq(rsp, Immediate(4 * kPointerSize)); - int frame_alignment = OS::ActivationFrameAlignment(); - ASSERT(IsPowerOf2(frame_alignment)); - __ and_(rsp, Immediate(-frame_alignment)); - - // Call the entry hook. - int64_t hook_location = reinterpret_cast(&entry_hook_); - __ movq(rax, hook_location, RelocInfo::NONE); - __ call(Operand(rax, 0)); - __ movq(rsp, kScratchRegister); - - // Restore volatile regs. -#ifdef _WIN64 - __ pop(rcx); -#else - __ pop(rsi); - __ pop(rdi); - __ pop(rcx); -#endif -} - #undef __ } } // namespace v8::internal diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index b27412f..1259160 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -123,8 +123,6 @@ void FullCodeGenerator::Generate() { SetFunctionPosition(function()); Comment cmnt(masm_, "[ function compiled by full code generator"); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 461b632..68e4396 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -128,8 +128,6 @@ void LCodeGen::Comment(const char* format, ...) { bool LCodeGen::GeneratePrologue() { ASSERT(is_generating()); - ProfileEntryHookStub::MaybeCallEntryHook(masm_); - #ifdef DEBUG if (strlen(FLAG_stop_at) > 0 && info_->function()->name()->IsEqualTo(CStrVector(FLAG_stop_at))) { diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 57cffae..2296072 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -33,7 +33,6 @@ #include "isolate.h" #include "compilation-cache.h" #include "execution.h" -#include "objects.h" #include "snapshot.h" #include "platform.h" #include "utils.h" @@ -10874,110 +10873,6 @@ THREADED_TEST(NestedHandleScopeAndContexts) { } -static i::Handle* foo_ptr = NULL; -static int foo_count = 0; -static i::Handle* bar_ptr = NULL; -static int bar_count = 0; - - -static void entry_hook(uintptr_t function, - uintptr_t return_addr_location) { - i::Code* code = i::Code::GetCodeFromTargetAddress( - reinterpret_cast(function)); - CHECK(code != NULL); - - if (bar_ptr != NULL && code == (*bar_ptr)->code()) - ++bar_count; - - if (foo_ptr != NULL && code == (*foo_ptr)->code()) - ++foo_count; - - // TODO(siggi): Verify return_addr_location. -} - - -static void RunLoopInNewEnv() { - bar_ptr = NULL; - foo_ptr = NULL; - - v8::HandleScope outer; - v8::Persistent env = Context::New(); - env->Enter(); - - const char* script = - "function bar() {" - " var sum = 0;" - " for (i = 0; i < 100; ++i)" - " sum = foo(i);" - " return sum;" - "}" - "function foo(i) { return i * i; }"; - CompileRun(script); - i::Handle bar = - i::Handle::cast( - v8::Utils::OpenHandle(*env->Global()->Get(v8_str("bar")))); - ASSERT(*bar); - - i::Handle foo = - i::Handle::cast( - v8::Utils::OpenHandle(*env->Global()->Get(v8_str("foo")))); - ASSERT(*foo); - - bar_ptr = &bar; - foo_ptr = &foo; - - v8::Handle value = CompileRun("bar();"); - CHECK(value->IsNumber()); - CHECK_EQ(9801.0, v8::Number::Cast(*value)->Value()); - - // Test the optimized codegen path. - value = CompileRun("%OptimizeFunctionOnNextCall(foo);" - "bar();"); - CHECK(value->IsNumber()); - CHECK_EQ(9801.0, v8::Number::Cast(*value)->Value()); - - env->Exit(); -} - - -THREADED_TEST(SetFunctionEntryHook) { - i::FLAG_allow_natives_syntax = true; - - // Test setting and resetting the entry hook. - // Nulling it should always succeed. - CHECK(v8::V8::SetFunctionEntryHook(NULL)); - - CHECK(v8::V8::SetFunctionEntryHook(entry_hook)); - // Setting a hook while one's active should fail. - CHECK_EQ(false, v8::V8::SetFunctionEntryHook(entry_hook)); - - CHECK(v8::V8::SetFunctionEntryHook(NULL)); - - // Reset the entry count to zero and set the entry hook. - bar_count = 0; - foo_count = 0; - CHECK(v8::V8::SetFunctionEntryHook(entry_hook)); - RunLoopInNewEnv(); - - CHECK_EQ(2, bar_count); - CHECK_EQ(200, foo_count); - - // Clear the entry hook and count. - bar_count = 0; - foo_count = 0; - v8::V8::SetFunctionEntryHook(NULL); - - // Clear the compilation cache to make sure we don't reuse the - // functions from the previous invocation. - v8::internal::Isolate::Current()->compilation_cache()->Clear(); - - // Verify that entry hooking is now disabled. - RunLoopInNewEnv(); - CHECK(0u == bar_count); - CHECK(0u == foo_count); -} - - static int64_t cast(intptr_t x) { return static_cast(x); } -- 2.7.4