Direct call api functions (arm implementation)

src/arm/macro-assembler-arm.cc

Index: src/arm/macro-assembler-arm.cc
===================================================================
--- src/arm/macro-assembler-arm.cc	(revision 6551)
+++ src/arm/macro-assembler-arm.cc	(working copy)
@@ -632,11 +632,7 @@
 }
 
 
-void MacroAssembler::EnterExitFrame(bool save_doubles) {
-  // Compute the argv pointer in a callee-saved register.
-  add(r6, sp, Operand(r0, LSL, kPointerSizeLog2));
-  sub(r6, r6, Operand(kPointerSize));
-
+void MacroAssembler::EnterExitFrame(bool save_doubles, int stack_space) {
   // Setup the frame structure on the stack.
   ASSERT_EQ(2 * kPointerSize, ExitFrameConstants::kCallerSPDisplacement);
   ASSERT_EQ(1 * kPointerSize, ExitFrameConstants::kCallerPCOffset);
@@ -658,10 +654,6 @@
   mov(ip, Operand(ExternalReference(Top::k_context_address)));
   str(cp, MemOperand(ip));
 
-  // Setup argc and the builtin function in callee-saved registers.
-  mov(r4, Operand(r0));
-  mov(r5, Operand(r1));
-
   // Optionally save all double registers.
   if (save_doubles) {
     sub(sp, sp, Operand(DwVfpRegister::kNumRegisters * kDoubleSize));
@@ -678,7 +670,7 @@
   // Reserve place for the return address and align the frame preparing for

[Søren Thygesen Gjesse, 2011/02/02 13:24:38]

Please update this comment.

[Zaheer, 2011/02/03 07:27:31]

Done.

   // calling the runtime function.
   const int frame_alignment = MacroAssembler::ActivationFrameAlignment();
-  sub(sp, sp, Operand(kPointerSize));
+  sub(sp, sp, Operand((stack_space + 1) * kPointerSize));
   if (frame_alignment > 0) {
     ASSERT(IsPowerOf2(frame_alignment));
     and_(sp, sp, Operand(-frame_alignment));
@@ -1486,6 +1478,114 @@
 }
 
 
+MaybeObject* MacroAssembler::TryTailCallStub(CodeStub* stub, Condition cond) {
+  ASSERT(allow_stub_calls());  // stub calls are not allowed in some stubs

[Søren Thygesen Gjesse, 2011/02/02 13:24:38]

Start comment with uppercase letter end and with period.

[Zaheer, 2011/02/03 07:27:31]

Done.

+  Object* result;
+  { MaybeObject* maybe_result = stub->TryGetCode();
+    if (!maybe_result->ToObject(&result)) return maybe_result;
+  }
+  Jump(stub->GetCode(), RelocInfo::CODE_TARGET, cond);
+  return result;
+}
+
+static int AddressOffset(ExternalReference ref0, ExternalReference ref1) {
+  int64_t offset = (ref0.address() - ref1.address());

[antonm, 2011/02/02 13:56:28]

should have noticed that earlier, sorry.  I think ARM is 32-bit platform (or at
least we only support 32-bit ARM), so you don't need to be that paranoid about
offsets:   return ref0.address() - ref1.address() should be enough.

[Zaheer, 2011/02/03 07:27:31]

sorry copy/paste mistake from x64. Done.

+  // Check that fits into int.
+  ASSERT(static_cast<int>(offset) == offset);
+  return static_cast<int>(offset);
+}
+
+
+MaybeObject* MacroAssembler::TryCallApiFunctionAndReturn(
+    ApiFunction* function, int stack_space) {
+  ExternalReference next_address =
+      ExternalReference::handle_scope_next_address();
+  const int kNextOffset = 0;
+  const int kLimitOffset = AddressOffset(
+      ExternalReference::handle_scope_limit_address(),
+      next_address);
+  const int kLevelOffset = AddressOffset(
+      ExternalReference::handle_scope_level_address(),
+      next_address);
+
+  // Allocate HandleScope in callee-save registers.
+  mov(r7, Operand(next_address));
+  ldr(r4, MemOperand(r7, kNextOffset));
+  ldr(r5, MemOperand(r7, kLimitOffset));
+  ldr(r6, MemOperand(r7, kLevelOffset));
+  add(r6, r6, Operand(1));
+  str(r6, MemOperand(r7, kLevelOffset));
+
+  // Native call returns to the DirectCEntry stub which redirects to the
+  // return address pushed on stack (could have moved after GC).

[Søren Thygesen Gjesse, 2011/02/02 13:24:38]

As far as I can see this relies on DirectCEntryStub itself never moving. It is
the same assumption we have for the CEntryStub (and the RegExpCEntryStub I
think). Please add a comment on this.

For the CEntryStub we have been safe so far as it is generated quite early (with
crankshaft this is actually not the case for the variant that saves doubles).
How about this will it be generated early, or can a test case where this
actually moves be crafted?

[antonm, 2011/02/02 13:56:28]

Søren, yes, that's exactly the reason we call goes indirectly via stub.

[Søren Thygesen Gjesse, 2011/02/02 14:23:50]

Sure, and as discussed offline having DirectCEntryStub created in
Heap::CreateFixedStubs will in practice ensure that DirectCEntryStub itself will
not move even though there is no explicit code in the GC to ensure that.

[Zaheer, 2011/02/03 07:27:31]

Added comment

+  DirectCEntryStub stub;

[Søren Thygesen Gjesse, 2011/02/02 13:24:38]

I thing the calling of this stub should be factored out, maybe to a method on
the stub

  stub.GenerateCall(ref)

as this relies on the exact code generated by the stub.

[Zaheer, 2011/02/03 07:27:31]

Done. passed function as a parameter instead of ref.

+  mov(lr, Operand(reinterpret_cast<intptr_t>(stub.GetCode().location()),
+                  RelocInfo::CODE_TARGET));
+
+  // Push return address (accessible to GC through exit frame pc).
+  ExternalReference ref =
+      ExternalReference(function, ExternalReference::DIRECT_CALL);
+  mov(r2, Operand(reinterpret_cast<intptr_t>(ref.address())));
+  add(ip, pc, Operand(4));
+  str(ip, MemOperand(sp, 0));
+  Jump(r2);  // Call the api function.
+
+  Label promote_scheduled_exception;
+  Label delete_allocated_handles;
+  Label leave_exit_frame;
+
+  // If result is non-zero, dereference to get the result value
+  // otherwise set it to undefined.
+  cmp(r0, Operand(0));
+  LoadRoot(r0, Heap::kUndefinedValueRootIndex, eq);
+  ldr(r0, MemOperand(r0), ne);
+
+  // No more valid handles (the result handle was the last one). Restore
+  // previous handle scope.
+  str(r4, MemOperand(r7, kNextOffset));
+  if (FLAG_debug_code) {
+    ldr(r1, MemOperand(r7, kLevelOffset));
+    cmp(r1, r6);
+    Check(eq, "Unexpected level after return from api call");
+  }
+  sub(r6, r6, Operand(1));
+  str(r6, MemOperand(r7, kLevelOffset));
+  ldr(ip, MemOperand(r7, kLimitOffset));
+  cmp(r5, ip);
+  b(ne, &delete_allocated_handles);
+
+  // Check if the function scheduled an exception.
+  bind(&leave_exit_frame);
+  LoadRoot(r4, Heap::kTheHoleValueRootIndex);
+  mov(ip, Operand(ExternalReference(Top::k_pending_exception_address)));

[antonm, 2011/02/02 13:56:28]

that should be scheduled_exception, not pending.

(scheduled exceptions are thrown by embedder's code, pending by v8 and JS)

and if this version passes the tests, that sucks.  May I ask you to add a test
case which reveals the bug?  You should use v8::ThrowException in API callback

[Zaheer, 2011/02/03 07:27:31]

Thanks for catching and the explanation, i wasnt aware of the difference and
none of the unit tests/browsing showed up the problem.
Added a test.

+  ldr(r5, MemOperand(ip));
+  cmp(r4, r5);
+  b(ne, &promote_scheduled_exception);
+
+  // LeaveExitFrame expects unwind space to be in r4.
+  mov(r4, Operand(stack_space));
+  LeaveExitFrame(false);
+
+  bind(&promote_scheduled_exception);
+  MaybeObject* result = TryTailCallExternalReference(
+      ExternalReference(Runtime::kPromoteScheduledException), 0, 1);
+  if (result->IsFailure()) {
+    return result;
+  }
+
+  // HandleScope limit has changed. Delete allocated extensions.
+  bind(&delete_allocated_handles);
+  str(r5, MemOperand(r7, kLimitOffset));
+  mov(r4, r0);
+  PrepareCallCFunction(0, r5);
+  CallCFunction(ExternalReference::delete_handle_scope_extensions(), 0);
+  mov(r0, r4);
+  jmp(&leave_exit_frame);
+
+  return result;
+}
+
+
 void MacroAssembler::IllegalOperation(int num_arguments) {
   if (num_arguments > 0) {
     add(sp, sp, Operand(num_arguments * kPointerSize));
@@ -1738,6 +1838,15 @@
   JumpToExternalReference(ext);
 }
 
+MaybeObject* MacroAssembler::TryTailCallExternalReference(
+    const ExternalReference& ext, int num_arguments, int result_size) {
+  // TODO(1236192): Most runtime routines don't need the number of
+  // arguments passed in because it is constant. At some point we
+  // should remove this need and make the runtime routine entry code
+  // smarter.
+  mov(r0, Operand(num_arguments));
+  return TryJumpToExternalReference(ext);
+}
 
 void MacroAssembler::TailCallRuntime(Runtime::FunctionId fid,
                                      int num_arguments,
@@ -1756,6 +1865,16 @@
   Jump(stub.GetCode(), RelocInfo::CODE_TARGET);
 }
 
+MaybeObject* MacroAssembler::TryJumpToExternalReference(
+    const ExternalReference& builtin) {
+#if defined(__thumb__)
+  // Thumb mode builtin.
+  ASSERT((reinterpret_cast<intptr_t>(builtin.address()) & 1) == 1);
+#endif
+  mov(r1, Operand(builtin));
+  CEntryStub stub(1);
+  return TryTailCallStub(&stub);
+}
 
 void MacroAssembler::InvokeBuiltin(Builtins::JavaScript id,
                                    InvokeJSFlags flags,