Fix bug restoring of JS entry sp so to fix profiler-related valgrind issues.

The profiler did not correctly identify when there was a JS frame
on the stack after an exception was thrown.

I also refactored the code to use PopTryHandler macro-instructions on all platforms

BUG=73722
TEST=No more valgrind errors related to v8::internal::StackFrameIterator.
Review URL: http://codereview.chromium.org/7019010

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

diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index 730677d62b1a4bb095a8dbc05773341f4a1cbcea..852974953aa19b44c9ecd1d2ee220f8135244c47 100644 (file)
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -3564,11 +3564,20 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
 
 #ifdef ENABLE_LOGGING_AND_PROFILING
   // If this is the outermost JS call, set js_entry_sp value.
+  Label non_outermost_js;
   ExternalReference js_entry_sp(Isolate::k_js_entry_sp_address, isolate);
   __ mov(r5, Operand(ExternalReference(js_entry_sp)));
   __ ldr(r6, MemOperand(r5));
-  __ cmp(r6, Operand(0, RelocInfo::NONE));
-  __ str(fp, MemOperand(r5), eq);
+  __ cmp(r6, Operand(0));
+  __ b(ne, &non_outermost_js);
+  __ str(fp, MemOperand(r5));
+  __ mov(ip, Operand(Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME)));
+  Label cont;
+  __ b(&cont);
+  __ bind(&non_outermost_js);
+  __ mov(ip, Operand(Smi::FromInt(StackFrame::INNER_JSENTRY_FRAME)));
+  __ bind(&cont);
+  __ push(ip);
 #endif
 
   // Call a faked try-block that does the invoke.
@@ -3626,27 +3635,22 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   __ mov(lr, Operand(pc));
   masm->add(pc, ip, Operand(Code::kHeaderSize - kHeapObjectTag));
 
-  // Unlink this frame from the handler chain. When reading the
-  // address of the next handler, there is no need to use the address
-  // displacement since the current stack pointer (sp) points directly
-  // to the stack handler.
-  __ ldr(r3, MemOperand(sp, StackHandlerConstants::kNextOffset));
-  __ mov(ip, Operand(ExternalReference(Isolate::k_handler_address, isolate)));
-  __ str(r3, MemOperand(ip));
-  // No need to restore registers
-  __ add(sp, sp, Operand(StackHandlerConstants::kSize));
+  // Unlink this frame from the handler chain.
+  __ PopTryHandler();
 
+  __ bind(&exit);  // r0 holds result
 #ifdef ENABLE_LOGGING_AND_PROFILING
-  // If current FP value is the same as js_entry_sp value, it means that
-  // the current function is the outermost.
+  // Check if the current stack frame is marked as the outermost JS frame.
+  Label non_outermost_js_2;
+  __ pop(r5);
+  __ cmp(r5, Operand(Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME)));
+  __ b(ne, &non_outermost_js_2);
+  __ mov(r6, Operand(0));
   __ mov(r5, Operand(ExternalReference(js_entry_sp)));
-  __ ldr(r6, MemOperand(r5));
-  __ cmp(fp, Operand(r6));
-  __ mov(r6, Operand(0, RelocInfo::NONE), LeaveCC, eq);
-  __ str(r6, MemOperand(r5), eq);
+  __ str(r6, MemOperand(r5));
+  __ bind(&non_outermost_js_2);
 #endif
 
-  __ bind(&exit);  // r0 holds result
   // Restore the top frame descriptors from the stack.
   __ pop(r3);
   __ mov(ip,

diff --git a/src/frames.h b/src/frames.h
index 27ba33913bcad3afe57964af8c66ebc0d06b828b..aa91026fbe44d4c497c81b74390694f72a53e406 100644 (file)
--- a/src/frames.h
+++ b/src/frames.h
@@ -152,6 +152,12 @@ class StackFrame BASE_EMBEDDED {
     NO_ID = 0
   };
 
+  // Used to mark the outermost JS entry frame.
+  enum JsFrameMarker {
+    INNER_JSENTRY_FRAME = 0,
+    OUTERMOST_JSENTRY_FRAME = 1
+  };
+
   struct State {
     State() : sp(NULL), fp(NULL), pc_address(NULL) { }
     Address sp;

diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index 6786d5288f148e0032bbac213211715c81819ca1..4bf73367fcc65e95a644530779f17b79046ee529 100644 (file)
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -4197,7 +4197,12 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   __ cmp(Operand::StaticVariable(js_entry_sp), Immediate(0));
   __ j(not_equal, &not_outermost_js);
   __ mov(Operand::StaticVariable(js_entry_sp), ebp);
+  __ push(Immediate(Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME)));
+  Label cont;
+  __ jmp(&cont);
   __ bind(&not_outermost_js);
+  __ push(Immediate(Smi::FromInt(StackFrame::INNER_JSENTRY_FRAME)));
+  __ bind(&cont);
 #endif
 
   // Call a faked try-block that does the invoke.
@@ -4243,23 +4248,19 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   __ call(Operand(edx));
 
   // Unlink this frame from the handler chain.
-  __ pop(Operand::StaticVariable(ExternalReference(
-      Isolate::k_handler_address,
-      masm->isolate())));
-  // Pop next_sp.
-  __ add(Operand(esp), Immediate(StackHandlerConstants::kSize - kPointerSize));
+  __ PopTryHandler();
 
+  __ bind(&exit);
 #ifdef ENABLE_LOGGING_AND_PROFILING
-  // If current EBP value is the same as js_entry_sp value, it means that
-  // the current function is the outermost.
-  __ cmp(ebp, Operand::StaticVariable(js_entry_sp));
+  // Check if the current stack frame is marked as the outermost JS frame.
+  __ pop(ebx);
+  __ cmp(Operand(ebx), Immediate(Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME)));
   __ j(not_equal, &not_outermost_js_2);
   __ mov(Operand::StaticVariable(js_entry_sp), Immediate(0));
   __ bind(&not_outermost_js_2);
 #endif
 
   // Restore the top frame descriptor from the stack.
-  __ bind(&exit);
   __ pop(Operand::StaticVariable(ExternalReference(
       Isolate::k_c_entry_fp_address,
       masm->isolate())));

diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index af9054bc8f599be2db16e772c28fd0cdfd0fc9d2..c72d7c36a7c6d33c0f1b8f38edb4f97993d8e0f5 100644 (file)
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -3270,9 +3270,14 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   __ Load(rax, js_entry_sp);
   __ testq(rax, rax);
   __ j(not_zero, &not_outermost_js);
+  __ Push(Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME));
   __ movq(rax, rbp);
   __ Store(js_entry_sp, rax);
+  Label cont;
+  __ jmp(&cont);
   __ bind(&not_outermost_js);
+  __ Push(Smi::FromInt(StackFrame::INNER_JSENTRY_FRAME));
+  __ bind(&cont);
 #endif
 
   // Call a faked try-block that does the invoke.
@@ -3314,25 +3319,20 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   __ call(kScratchRegister);
 
   // Unlink this frame from the handler chain.
-  Operand handler_operand =
-      masm->ExternalOperand(ExternalReference(Isolate::k_handler_address,
-                                              isolate));
-  __ pop(handler_operand);
-  // Pop next_sp.
-  __ addq(rsp, Immediate(StackHandlerConstants::kSize - kPointerSize));
+  __ PopTryHandler();
 
+  __ bind(&exit);
 #ifdef ENABLE_LOGGING_AND_PROFILING
-  // If current RBP value is the same as js_entry_sp value, it means that
-  // the current function is the outermost.
-  __ movq(kScratchRegister, js_entry_sp);
-  __ cmpq(rbp, Operand(kScratchRegister, 0));
+  // Check if the current stack frame is marked as the outermost JS frame.
+  __ pop(rbx);
+  __ Cmp(rbx, Smi::FromInt(StackFrame::OUTERMOST_JSENTRY_FRAME));
   __ j(not_equal, &not_outermost_js_2);
+  __ movq(kScratchRegister, js_entry_sp);
   __ movq(Operand(kScratchRegister, 0), Immediate(0));
   __ bind(&not_outermost_js_2);
 #endif
 
   // Restore the top frame descriptor from the stack.
-  __ bind(&exit);
   {
     Operand c_entry_fp_operand = masm->ExternalOperand(c_entry_fp);
     __ pop(c_entry_fp_operand);