Fix EntryHookStub on ia32 and x64.

These stubs were computing the return address location incorrectly.
Add testing for same.

R=danno@chromium.org

Review URL: https://codereview.chromium.org/15769017

Patch from Sigurður Ásgeirsson <siggi@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14962 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index 8cb4725..b08203b 100644 (file)
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -7766,14 +7766,16 @@ void ProfileEntryHookStub::MaybeCallEntryHook(MacroAssembler* masm) {
 
 void ProfileEntryHookStub::Generate(MacroAssembler* masm) {
   // Ecx is the only volatile register we must save.
+  const int kNumSavedRegisters = 1;
   __ push(ecx);
 
   // Calculate and push the original stack pointer.
-  __ lea(eax, Operand(esp, kPointerSize));
+  __ lea(eax, Operand(esp, (kNumSavedRegisters + 1) * kPointerSize));
   __ push(eax);
 
-  // Calculate and push the function address.
-  __ mov(eax, Operand(eax, 0));
+  // Retrieve our return address and use it to calculate the calling
+  // function's address.
+  __ mov(eax, Operand(esp, (kNumSavedRegisters + 1) * kPointerSize));
   __ sub(eax, Immediate(Assembler::kCallInstructionLength));
   __ push(eax);
 

diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index f5d38f3..92cc459 100644 (file)
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -6756,17 +6756,17 @@ void ProfileEntryHookStub::Generate(MacroAssembler* masm) {
 
   // Calculate the original stack pointer and store it in the second arg.
 #ifdef _WIN64
-  __ lea(rdx, Operand(rsp, kNumSavedRegisters * kPointerSize));
+  __ lea(rdx, Operand(rsp, (kNumSavedRegisters + 1) * kPointerSize));
 #else
-  __ lea(rsi, Operand(rsp, kNumSavedRegisters * kPointerSize));
+  __ lea(rsi, Operand(rsp, (kNumSavedRegisters + 1) * kPointerSize));
 #endif
 
   // Calculate the function address to the first arg.
 #ifdef _WIN64
-  __ movq(rcx, Operand(rdx, 0));
+  __ movq(rcx, Operand(rsp, kNumSavedRegisters * kPointerSize));
   __ subq(rcx, Immediate(Assembler::kShortCallInstructionLength));
 #else
-  __ movq(rdi, Operand(rsi, 0));
+  __ movq(rdi, Operand(rsp, kNumSavedRegisters * kPointerSize));
   __ subq(rdi, Immediate(Assembler::kShortCallInstructionLength));
 #endif
 

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
old mode 100644 (file)
new mode 100755 (executable)
index cb3a38e..c4687cc
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -12085,9 +12085,10 @@ THREADED_TEST(NestedHandleScopeAndContexts) {
 
 
 static i::Handle<i::JSFunction>* foo_ptr = NULL;
-static int foo_count = 0;
+static int foo_entry_count = 0;
 static i::Handle<i::JSFunction>* bar_ptr = NULL;
-static int bar_count = 0;
+static int bar_entry_count = 0;
+static int bar_caller_count = 0;
 
 
 static void entry_hook(uintptr_t function,
@@ -12097,14 +12098,21 @@ static void entry_hook(uintptr_t function,
   CHECK(code != NULL);
 
   if (bar_ptr != NULL && code == (*bar_ptr)->code())
-    ++bar_count;
+    ++bar_entry_count;
 
   if (foo_ptr != NULL && code == (*foo_ptr)->code())
-    ++foo_count;
+    ++foo_entry_count;
 
-  // TODO(siggi): Verify return_addr_location.
-  //     This can be done by capturing JitCodeEvents, but requires an ordered
-  //     collection.
+  // Let's check whether bar is the caller.
+  if (bar_ptr != NULL) {
+    const v8::internal::byte* caller =
+        *reinterpret_cast<v8::internal::byte**>(return_addr_location);
+
+    if ((*bar_ptr)->code()->instruction_start() <= caller &&
+        (*bar_ptr)->code()->instruction_end() > caller) {
+      ++bar_caller_count;
+    }
+  }
 }
 
 
@@ -12175,17 +12183,20 @@ TEST(SetFunctionEntryHook) {
   CHECK(v8::V8::SetFunctionEntryHook(NULL));
 
   // Reset the entry count to zero and set the entry hook.
-  bar_count = 0;
-  foo_count = 0;
+  bar_entry_count = 0;
+  bar_caller_count = 0;
+  foo_entry_count = 0;
   CHECK(v8::V8::SetFunctionEntryHook(entry_hook));
   RunLoopInNewEnv();
 
-  CHECK_EQ(2, bar_count);
-  CHECK_EQ(200, foo_count);
+  CHECK_EQ(2, bar_entry_count);
+  CHECK_EQ(200, bar_caller_count);
+  CHECK_EQ(200, foo_entry_count);
 
   // Clear the entry hook and count.
-  bar_count = 0;
-  foo_count = 0;
+  bar_entry_count = 0;
+  bar_caller_count = 0;
+  foo_entry_count = 0;
   v8::V8::SetFunctionEntryHook(NULL);
 
   // Clear the compilation cache to make sure we don't reuse the
@@ -12194,8 +12205,9 @@ TEST(SetFunctionEntryHook) {
 
   // Verify that entry hooking is now disabled.
   RunLoopInNewEnv();
-  CHECK_EQ(0u, bar_count);
-  CHECK_EQ(0u, foo_count);
+  CHECK_EQ(0u, bar_entry_count);
+  CHECK_EQ(0u, bar_caller_count);
+  CHECK_EQ(0u, foo_entry_count);
 }