Enforce that IP is saved for all JSFunction calls

Summary:
Add an assert in the runtime that if there is a CodeBlock for the
current stack frame, then the IP that is saved must be inside it.

Reviewed By: tmikov

Differential Revision: D64502107

fbshipit-source-id: 3c6bb72b8fb4d82232875b128a5bf5aeeeafec25

include/hermes/VM/Runtime.h

@@ -1418,6 +1418,10 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
   uint32_t noRJSLevel_{0};
 
   friend class NoRJSScope;
+
+  /// If the function currently at the top of the stack is a JSFunction, assert
+  /// that the given IP falls within it.
+  void assertTopCodeBlockContainsIP(const inst::Inst *ip) const;
 #endif
 
  public:
@@ -1428,6 +1432,9 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
   /// return to the interpreter at a different IP. This allows things external
   /// to the interpreter loop to affect the flow of bytecode execution.
   inline void setCurrentIP(const inst::Inst *ip) {
+#ifdef HERMES_SLOW_DEBUG
+    assertTopCodeBlockContainsIP(ip);
+#endif
     currentIP_ = ip;
   }
 
@@ -1439,6 +1446,9 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
     assert(
         (uintptr_t)currentIP_ != kInvalidCurrentIP &&
         "Current IP unknown - this probably means a CAPTURE_IP_* is missing in the interpreter.");
+#ifdef HERMES_SLOW_DEBUG
+    assertTopCodeBlockContainsIP(currentIP_);
+#endif
     return currentIP_;
   }
 

lib/VM/JSError.cpp

@@ -480,6 +480,9 @@ ExecutionStatus JSError::recordStackTrace(
       codeBlock = prev->getCalleeCodeBlock();
       locals = prev->getSHLocals();
     }
+    // Assert that if the caller is a JSFunction, the IP was saved. But to be
+    // defensive, we still check it below.
+    assert(!codeBlock || savedIP);
     if (codeBlock && savedIP) {
       stack->emplace_back(
           BytecodeStackTraceInfo(codeBlock, codeBlock->getOffsetOf(savedIP)));

lib/VM/Runtime.cpp

@@ -798,6 +798,14 @@ void Runtime::removeRuntimeModule(RuntimeModule *rm) {
 }
 
 #ifndef NDEBUG
+void Runtime::assertTopCodeBlockContainsIP(const inst::Inst *ip) const {
+  // Check that if the topmost function is currently a JSFunction, then the IP
+  // is in that function.
+  if (currentFrame_ != StackFramePtr(registerStackStart_))
+    if (auto *codeBlock = currentFrame_.getCalleeCodeBlock())
+      assert(codeBlock->contains(ip) && "IP not in CodeBlock");
+}
+
 void Runtime::printArrayCensus(llvh::raw_ostream &os) {
   // Do array capacity histogram.
   // Map from array size to number of arrays that are that size.

unittests/VMRuntime/InterpreterTest.cpp

@@ -204,7 +204,7 @@ TEST_F(InterpreterTest, SimpleSmokeTest) {
     ScopedNativeCallFrame frame(
         runtime,
         0,
-        HermesValue::encodeUndefinedValue(),
+        HermesValue::encodeNativePointer(codeBlock),
         HermesValue::encodeUndefinedValue(),
         HermesValue::encodeUndefinedValue());
     ASSERT_FALSE(frame.overflowed());
@@ -380,7 +380,7 @@ TEST_F(InterpreterTest, RecursiveFactorialTest) {
       ScopedNativeCallFrame newFrame(
           runtime,
           1,
-          HermesValue::encodeUndefinedValue(),
+          HermesValue::encodeNativePointer(codeBlock),
           HermesValue::encodeUndefinedValue(),
           HermesValue::encodeUndefinedValue());
       ASSERT_FALSE(newFrame.overflowed());