Save caller IP before setting up callee frame

Summary:
Save the caller IP before we have created the callee frame. This allows
us to impose additional asserts in `getCurrentIP` subsequent diffs,
since we will now be guaranteed that it will produce an address within
the top CodeBlock on the stack (if any).

Reviewed By: tmikov

Differential Revision: D64737416

fbshipit-source-id: 335e585689f9226e14774359146abc38a2374d07

include/hermes/VM/Callable.h

@@ -710,8 +710,8 @@ class NativeFunction : public Callable {
           Runtime::StackOverflowKind::NativeStack);
     }
 
-    auto newFrame = runtime.setCurrentFrameToTopOfStack();
     runtime.saveCallerIPInStackFrame();
+    auto newFrame = runtime.setCurrentFrameToTopOfStack();
     // Allocate the "reserved" registers in the new frame.
     if (LLVM_UNLIKELY(!runtime.checkAndAllocStack(
             StackFrameLayout::CalleeExtraRegistersAtStart,

include/hermes/VM/Runtime.h

@@ -1243,12 +1243,13 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
   llvh::MutableArrayRef<PinnedHermesValue> registerStackAllocation_;
   PinnedHermesValue *registerStackStart_;
   PinnedHermesValue *registerStackEnd_;
+  /// Past-the-end pointer for the current frame. This points to the first
+  /// uninitialized element at the end of the stack.
   PinnedHermesValue *stackPointer_;
   /// Manages data to be used in the case of a crash.
   std::shared_ptr<CrashManager> crashMgr_;
-  /// Points to the last register in the callers frame. The current frame (the
-  /// callee frame) starts in the next register and continues up to and
-  /// including \c stackPointer_.
+  /// Points to the first register in the current frame. The current frame
+  /// continues up to \c stackPointer (exclusive).
   StackFramePtr currentFrame_{nullptr};
 
   /// Used to guard against stack overflow. Either uses real stack checking or
@@ -1455,18 +1456,20 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
 #endif
 
   /// Save the return address in the caller in the stack frame.
-  /// This needs to be called at the beginning of a function call, after the
-  /// stack frame is set up.
+  /// This needs to be called at the beginning of a function call, before the
+  /// stack frame is set up. \c stackPointer_ should be at the end of the caller
+  /// frame, pointing to the first register of the callee frame that is about to
+  /// be set up.
   void saveCallerIPInStackFrame() {
+    StackFramePtr newFrame(stackPointer_);
 #ifndef NDEBUG
     assert(
-        (!currentFrame_.getSavedIP() ||
+        (!newFrame.getSavedIP() ||
          ((uintptr_t)currentIP_ != kInvalidCurrentIP &&
-          currentFrame_.getSavedIP() == currentIP_)) &&
+          newFrame.getSavedIP() == currentIP_)) &&
         "The ip should either be null or already have the expected value");
 #endif
-    currentFrame_.getSavedIPRef() =
-        HermesValue::encodeNativePointer(getCurrentIP());
+    newFrame.getSavedIPRef() = HermesValue::encodeNativePointer(getCurrentIP());
   }
 
  private:

lib/VM/Interpreter.cpp

@@ -994,6 +994,11 @@ CallResult<HermesValue> Interpreter::interpretFunction(
       return JSFunction::_jittedCall(jitPtr, runtime);
     }
 
+    // If the interpreter was invoked indirectly from another JS function, the
+    // caller's IP may not have been saved to the stack frame. Ensure that it is
+    // correctly recorded.
+    runtime.saveCallerIPInStackFrame();
+
     // Check for invalid invocation. This is done before setting up the stack so
     // the exception appears to come from the call site.
     auto newFrame = StackFramePtr(runtime.getStackPointer());
@@ -1017,10 +1022,6 @@ CallResult<HermesValue> Interpreter::interpretFunction(
 
     // Advance the frame pointer.
     runtime.setCurrentFrame(newFrame);
-    // If the interpreter was invoked indirectly from another JS function, the
-    // caller's IP may not have been saved to the stack frame. Ensure that it is
-    // correctly recorded.
-    runtime.saveCallerIPInStackFrame();
     // Point frameRegs to the first register in the new frame.
     frameRegs = &newFrame.getFirstLocalRef();
     ip = (Inst const *)curCodeBlock->begin();