Harden the trace intepreter so it doesn't access records that don't e…

…xist.

Summary:
I (ddetlefs) created a trace.  At the time I did this, there was a problem with the trace fixup tool, so I fixed up the trace by hand, removing the incomplete last record.  When I ran it, the trace interpreter crashed.  I figured out that this was from accessing the records vector out of bounds.

While we have also fixed the fixup-tool to walk back to "last empty stack", we decided it would also be good to harden the TraceInterpreter against such crashes.  I by considering where "records" is accessed, I identified 3 such places.  This diff fixes them, in both the hermes and static_h copies of TraceInterpreter.cpp.

Reviewed By: mattbfb

Differential Revision: D61546894

fbshipit-source-id: c6a99b2631b16913b07956912914c3803f91a15c

API/hermes/TraceInterpreter.cpp

@@ -456,6 +456,14 @@ Function TraceInterpreter::createHostFunction(
           const Value *args,
           size_t count) mutable -> Value {
         try {
+          // Javascript execution has invoked this host function,
+          // expecting the trace to contain records for the execution of
+          // the function.  If the trace is truncated, those records
+          // may not exist -- return early in that case.
+          if (nextExecIndex_ >= trace_.records().size()) {
+            return Value::undefined();
+          }
+
           const auto &rec = trace_.records()[nextExecIndex_];
           const auto &ctnr =
               record_cast<const SynthTrace::CallToNativeRecord>(*rec);
@@ -494,6 +502,15 @@ Object TraceInterpreter::createHostObject(ObjectID objID) {
 
     Value get(Runtime &rt, const PropNameID &name) override {
       try {
+        // Javascript execution has invoked this host object getter,
+        // expecting the trace to contain records for the execution of
+        // the function.  If the trace is truncated, those records
+        // may not exist -- return early in that case.
+        if (interpreter_.nextExecIndex_ >=
+            interpreter_.trace_.records().size()) {
+          return Value::undefined();
+        }
+
         const auto &rec =
             interpreter_.trace_.records()[interpreter_.nextExecIndex_];
         const auto &gpnr =
@@ -517,6 +534,15 @@ Object TraceInterpreter::createHostObject(ObjectID objID) {
 
     void set(Runtime &rt, const PropNameID &name, const Value &value) override {
       try {
+        // Javascript execution has invoked this host object setter,
+        // expecting the trace to contain records for the execution of
+        // the function.  If the trace is truncated, those records
+        // may not exist -- return early in that case.
+        if (interpreter_.nextExecIndex_ >=
+            interpreter_.trace_.records().size()) {
+          return;
+        }
+
         const auto &rec =
             interpreter_.trace_.records()[interpreter_.nextExecIndex_];
         const auto &spnr =

API/hermes/synthtest/tests/TestFunctions.h

@@ -27,6 +27,9 @@
   F(nativePropertyNames)             \
   F(nativeSetsConstant)              \
   F(parseGCConfig)                   \
+  F(partialTraceHostFunction)        \
+  F(partialTraceHostObjectGet)       \
+  F(partialTraceHostObjectSet)       \
   F(surrogatePairString)
 
 #define TEST_FUNC_FORWARD_DECL(name) \

API/hermes/synthtest/tests/partial_trace_host_function.cpp

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "TestFunctions.h"
+
+namespace facebook {
+namespace hermes {
+namespace synthtest {
+
+const char *partialTraceHostFunctionTrace() {
+  return R"###(
+{
+  "globalObjID": 1,
+  "trace": [
+    {
+      "type": "BeginExecJSRecord",
+      "time": 0
+    },
+    {
+      "type": "EndExecJSRecord",
+      "retval": "undefined:",
+      "time": 0
+    },
+    {
+      "type": "CreatePropNameRecord",
+      "objID": 2,
+      "encoding": "ASCII",
+      "chars": "f"
+    },
+    {
+      "type": "GetPropertyRecord",
+      "time": 0,
+      "objID": 1,
+      "propID": "propIDTag:2",
+      "propName": "f",
+    },
+    {
+      "type": "ReturnToNativeRecord",
+      "time": 0,
+      "retval": "object:10"
+    },
+    {
+      "type": "CreatePropNameIDRecord",
+      "objID": 40,
+      "encoding": "ASCII",
+      "chars": "HostFunction1"
+    },
+    {
+      "type": "CreateHostFunctionRecord",
+      "time": 0,
+      "objID": 11,
+      "propNameID": 40,
+      "functionName": "HostFunction1"
+    },
+    {
+      "type": "CallFromNativeRecord",
+      "time": 0,
+      "functionID": 10,
+      "thisArg": "undefined:",
+      "args": ["object:11"]
+    }
+  ]
+}
+)###";
+}
+
+const char *partialTraceHostFunctionSource() {
+  return R"###(
+'use strict';
+
+(function(global) {
+  // callbacks execute f
+  // read the zeroth element of the return result,
+  // execute that as a function with no args,
+  // read the zeroth element of the return value and expect it to be false.
+  global.f = function(nativeFunc) {
+    nativeFunc();
+  };
+})(this);
+)###";
+}
+
+} // namespace synthtest
+} // namespace hermes
+} // namespace facebook

API/hermes/synthtest/tests/partial_trace_host_object_get.cpp

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "TestFunctions.h"
+
+namespace facebook {
+namespace hermes {
+namespace synthtest {
+
+const char *partialTraceHostObjectGetTrace() {
+  return R"###(
+{
+  "globalObjID": 1,
+  "trace": [
+    {
+      "type": "BeginExecJSRecord",
+      "time": 0
+    },
+    {
+      "type": "EndExecJSRecord",
+      "retval": "undefined:",
+      "time": 0
+    },
+    {
+      "type": "CreatePropNameRecord",
+      "objID": 2,
+      "encoding": "ASCII",
+      "chars": "f"
+    },
+    {
+      "type": "GetPropertyRecord",
+      "time": 0,
+      "objID": 1,
+      "propID": "propIDTag:2",
+      "propName": "f",
+    },
+    {
+      "type": "ReturnToNativeRecord",
+      "time": 0,
+      "retval": "object:10"
+    },
+    {
+      "type": "CreatePropNameIDRecord",
+      "objID": 40,
+      "encoding": "ASCII",
+      "chars": "HostFunction1"
+    },
+    {
+      "type": "CreateHostObjectRecord",
+      "time": 0,
+      "objID": 11,
+    },
+    {
+      "type": "CallFromNativeRecord",
+      "time": 0,
+      "functionID": 10,
+      "thisArg": "undefined:",
+      "args": ["object:11"]
+    }
+  ]
+}
+)###";
+}
+
+const char *partialTraceHostObjectGetSource() {
+  return R"###(
+'use strict';
+
+(function(global) {
+  // callbacks execute f
+  // read the zeroth element of the return result,
+  // execute that as a function with no args,
+  // read the zeroth element of the return value and expect it to be false.
+  global.f = function(hostObject) {
+    return hostObject.a;
+  };
+})(this);
+)###";
+}
+
+} // namespace synthtest
+} // namespace hermes
+} // namespace facebook

API/hermes/synthtest/tests/partial_trace_host_object_set.cpp

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "TestFunctions.h"
+
+namespace facebook {
+namespace hermes {
+namespace synthtest {
+
+const char *partialTraceHostObjectSetTrace() {
+  return R"###(
+{
+  "globalObjID": 1,
+  "trace": [
+    {
+      "type": "BeginExecJSRecord",
+      "time": 0
+    },
+    {
+      "type": "EndExecJSRecord",
+      "retval": "undefined:",
+      "time": 0
+    },
+    {
+      "type": "CreatePropNameRecord",
+      "objID": 2,
+      "encoding": "ASCII",
+      "chars": "f"
+    },
+    {
+      "type": "GetPropertyRecord",
+      "time": 0,
+      "objID": 1,
+      "propID": "propIDTag:2",
+      "propName": "f",
+    },
+    {
+      "type": "ReturnToNativeRecord",
+      "time": 0,
+      "retval": "object:10"
+    },
+    {
+      "type": "CreatePropNameIDRecord",
+      "objID": 40,
+      "encoding": "ASCII",
+      "chars": "HostFunction1"
+    },
+    {
+      "type": "CreateHostObjectRecord",
+      "time": 0,
+      "objID": 11,
+    },
+    {
+      "type": "CallFromNativeRecord",
+      "time": 0,
+      "functionID": 10,
+      "thisArg": "undefined:",
+      "args": ["object:11"]
+    }
+  ]
+}
+)###";
+}
+
+const char *partialTraceHostObjectSetSource() {
+  return R"###(
+'use strict';
+
+(function(global) {
+  // callbacks execute f
+  // read the zeroth element of the return result,
+  // execute that as a function with no args,
+  // read the zeroth element of the return value and expect it to be false.
+  global.f = function(hostObject) {
+    hostObject.a = 7;
+  };
+})(this);
+)###";
+}
+
+} // namespace synthtest
+} // namespace hermes
+} // namespace facebook