Merge branch 'fix/signDisplay' of github.com:dream-sports-labs/hermes…

… into fix/signDisplay

API/hermes/TracingRuntime.cpp

@@ -58,7 +58,29 @@ void TracingRuntime::replaceNondeterministicFuncs() {
           const jsi::Value *args,
           size_t count) {
         auto fun = args[0].getObject(*runtime_).getFunction(*runtime_);
-        return fun.call(*runtime_);
+        jsi::Value result;
+        if (count > 1 && args[1].isObject()) {
+          result = fun.callWithThis(*runtime_, args[1].asObject(*runtime_));
+        } else {
+          result = fun.call(*runtime_);
+        }
+
+        if (result.isString()) {
+          // Recreate the result string via the TracingRuntime, so the string
+          // appears in the resulting trace.
+          const std::string resultStr =
+              result.getString(*runtime_).utf8(*runtime_);
+          jsi::String tracedResult = jsi::String::createFromUtf8(rt, resultStr);
+          return jsi::Value(std::move(tracedResult));
+        } else {
+          // Other values must be primitives that will be included directly in
+          // the trace.
+          assert(
+              (result.isUndefined() || result.isNull() || result.isNumber() ||
+               result.isBool()) &&
+              "Result is a pointer");
+          return result;
+        }
       });
 
   // Below two host functions are for WeakRef hook.
@@ -179,6 +201,25 @@ void TracingRuntime::replaceNondeterministicFuncs() {
   }
   Date.now = nativeDateNow;
   globalThis.Date = Date;
+
+  const defineProperty = Object.defineProperty;
+  const realStackPropertyGetter = Object.getOwnPropertyDescriptor(Error.prototype, 'stack').get;
+  defineProperty(Error.prototype, 'stack', {
+    get: function() {
+      var stack = callUntraced(realStackPropertyGetter, this);
+      // The real getter stores the stack on the error object, meaning that
+      // the real getter (and this wrapper) will not be invoked again if the
+      // stack is accessed again during recording. Mimic that behavior here,
+      // so the getter is also not invoked again during replay.
+      defineProperty(this, 'stack', {
+        value: stack,
+        writable: true,
+        configurable: true
+      });
+      return stack;
+    },
+    configurable: true
+  });
 });
 )";
   global()

tools/hermes-parser/js/.flowconfig

@@ -27,7 +27,7 @@ component_syntax=true
 enums=true
 
 [version]
-^0.243.0
+^0.244.0
 
 [lints]
 untyped-type-import=error

tools/hermes-parser/js/package.json

@@ -17,7 +17,7 @@
     "eslint-plugin-flowtype": "^8.0.3",
     "eslint-plugin-jest": "^25.2.4",
     "eslint-plugin-prettier": "^4.2.1",
-    "flow-bin": "^0.243.0",
+    "flow-bin": "^0.244.0",
     "glob": "^8.0.3",
     "jest": "^29.2.2",
     "jest-specific-snapshot": "^5.0.0",

tools/hermes-parser/js/yarn.lock

@@ -3003,10 +3003,10 @@ flatted@^3.1.0:
   resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.2.2.tgz#64bfed5cb68fe3ca78b3eb214ad97b63bedce561"
   integrity sha512-JaTY/wtrcSyvXJl4IMFHPKyFur1sE9AUqc0QnhOaJ0CxHtAoIV8pYDzeEfAaNEtGkOfq4gr3LBFmdXW5mOQFnA==
 
-flow-bin@^0.243.0:
-  version "0.243.0"
-  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.243.0.tgz#f7d00b84bbcb16e23261031c15f955c76992970a"
-  integrity sha512-4OyCAkI26Bm8wGd1eNt+EBkHYAGB5PsGnykxLB0y7wK8wPitQi6pi3HsoNlk3yM6gyJsDqkRJrlkpnBB6WDgjQ==
+flow-bin@^0.244.0:
+  version "0.244.0"
+  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.244.0.tgz#ed1c2123d4fd0a49b0bb62ce5c732a2e19927601"
+  integrity sha512-v9LyZCV7ubZZUOttMy1W4ih9cYd2XalxoH7/g+EvPrX5P/I4brA0JINgjW4A3B3P0macrgg466TbvcZrB9o7KQ==
 
 flow-enums-runtime@^0.0.6:
   version "0.0.6"

unittests/API/SynthTraceTest.cpp

@@ -18,6 +18,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <hermes/CompileJS.h>
 #include <hermes/hermes.h>
 
 #include <memory>
@@ -1273,15 +1274,31 @@ TEST_F(SynthTraceEnvironmentTest, NonDeterministicFunctionNames) {
 /// @{
 struct SynthTraceReplayTest : public SynthTraceRuntimeTest {
   std::unique_ptr<jsi::Runtime> replayRt;
+  std::vector<std::string> sources;
+
+  jsi::Value evalCompiled(jsi::Runtime &rt, std::string source) {
+    std::string bytecode;
+    EXPECT_TRUE(hermes::compileJS(source, bytecode));
+    this->sources.push_back(std::move(source));
+    return rt.evaluateJavaScript(
+        std::unique_ptr<facebook::jsi::StringBuffer>(
+            new facebook::jsi::StringBuffer(bytecode)),
+        "");
+  }
 
   void replay() {
     traceRt.reset();
 
+    std::vector<std::unique_ptr<llvh::MemoryBuffer>> sources;
+    for (const std::string &source : this->sources) {
+      sources.emplace_back(llvh::MemoryBuffer::getMemBuffer(source));
+    }
+
     tracing::TraceInterpreter::ExecuteOptions options;
     options.useTraceConfig = true;
     auto [_, rt] = tracing::TraceInterpreter::execFromMemoryBuffer(
         llvh::MemoryBuffer::getMemBuffer(traceResult), // traceBuf
-        {}, // codeBufs
+        std::move(sources), // codeBufs
         options, // ExecuteOptions
         makeHermesRuntime);
     replayRt = std::move(rt);
@@ -2224,6 +2241,61 @@ TEST_F(NonDeterminismReplayTest, HermesInternalGetInstrumentedStatsTest) {
   }
 }
 
+// Test that traces replay deterministically, even if the error stack changes.
+TEST_F(NonDeterminismReplayTest, ErrorStackTest) {
+  constexpr auto source = R"(
+var stack;
+var refetchedStack;
+
+function main() {
+  function inlineable2() {
+    var error = new Error('test');
+    stack = error.stack;
+    refetchedStack = error.stack;
+  }
+
+  function inlineable1() {
+    inlineable2();
+    return 123;
+  }
+
+  return inlineable1();
+}  
+main();
+  )";
+
+  // Run with optimizations, producing a stack like:
+  // Error: test
+  //   at main (:6:18)
+  //   at global (:16:5)
+  evalCompiled(*traceRt, source);
+  auto stack = eval(*traceRt, "stack").asString(*traceRt).utf8(*traceRt);
+  auto refetchedStack =
+      eval(*traceRt, "refetchedStack").asString(*traceRt).utf8(*traceRt);
+  // Ensure the stack is correctly fetched after any caching triggered by the
+  // first fetch.
+  EXPECT_EQ(stack, refetchedStack);
+
+  // Run without optimizations, producing a stack like:
+  // Error: test
+  //   at inlineable2 (:6:18)
+  //   at inlineable1 (:10:16)
+  //   at main (:14:21)
+  //   at global (:16:5)
+  // which should be ignored and replaced by the stack captured in the trace.
+  replay();
+  auto replayedStack =
+      eval(*replayRt, "stack").asString(*replayRt).utf8(*replayRt);
+  auto replayedRefetchedStack =
+      eval(*replayRt, "refetchedStack").asString(*replayRt).utf8(*replayRt);
+  // Ensure the stack is correctly fetched after any caching triggered by the
+  // first fetch.
+  EXPECT_EQ(replayedStack, replayedRefetchedStack);
+
+  // Ensure the stack is replayed identically to the recording.
+  EXPECT_EQ(stack, replayedStack);
+}
+
 // Verify that jsi::Runtime::setExternalMemoryPressure() is properly traced and
 // replayed
 TEST(SynthTraceReplayTestNoFixture, ExternalMemoryTest) {