Skip to content

Commit

Permalink
Fix synth trace handling of Date constructor
Browse files Browse the repository at this point in the history
Summary:
The current implementation of the `Date` constructor in synth traces
invokes `Date` by obtaining it from the global object. However, this
can break valid JS code, since the `Date` property may have been
changed by the user. We should instead be invoking the real `Date`
constructor that was saved.

See the added test for an example of how the current setup might
break.

To fix this, we can change the `nativeDateCtor` function to act like
`Date.now()`. The result of that call can then be used to
deterministically reconstruct the `Date` object.

Original Author: neildhar
Original Reviewed By: fbmal7
Original Revision: D38897029
Original Git: 36ed60d

Reviewed By: neildhar

Differential Revision: D39736207

fbshipit-source-id: 6d3931ad665dcceabc1e07a98a10823b5564a212
  • Loading branch information
Tzvetan Mikov authored and neildhar committed Nov 22, 2022
1 parent e69655a commit b04ece4
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
31 changes: 9 additions & 22 deletions API/hermes/TracingRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ void TracingRuntime::setupDate() {
.asNumber();
jsi::Function *origDateFunc = saveFunction({"Date"});

jsi::Function nativeDateCtor = jsi::Function::createFromHostFunction(
// We can't effectively trace objects, so we instead trace the value of
// Date.now(), and use that to deterministically reconstruct the Date object
// during replay.
jsi::Function nativeDateNow = jsi::Function::createFromHostFunction(
*this,
jsi::PropNameID::forAscii(*this, "Date"),
lenProp,
Expand All @@ -119,24 +122,8 @@ void TracingRuntime::setupDate() {
const jsi::Value &thisVal,
const jsi::Value *args,
size_t count) {
auto ret = origDateFunc->callAsConstructor(*runtime_);
// We cannot return this value here, because the trace would be
// invalid. `new Date()` returns an object, so returning it would mean
// returning an object that has never been defined. Therefore, we trace
// reconstructing a new Date with the argument being the getTime() value
// from the Date object created in the untraced runtime. Conceptually,
// we are transforming calls to the no-arg Date constructor:
// var myDate = new Date();
// -->
// var tmp = new Date(); <-- this is untraced
// var arg = tmp.getTime(); <-- this is untraced
// var myDate = new Date(arg); <-- this is traced
auto obj = ret.asObject(*runtime_);
auto val = obj.getPropertyAsFunction(*runtime_, "getTime")
.callWithThis(*runtime_, obj);
return this->global()
.getPropertyAsFunction(*this, "Date")
.callAsConstructor(*this, val);
return origDateFunc->getPropertyAsFunction(*runtime_, "now")
.call(*runtime_);
});

jsi::Function nativeDateFunc = jsi::Function::createFromHostFunction(
Expand All @@ -159,12 +146,12 @@ void TracingRuntime::setupDate() {
});

auto code = R"(
(function(nativeDateCtor, nativeDateFunc){
(function(nativeDateNow, nativeDateFunc){
var DateReal = Date;
function DateJSReplacement(...args){
if (new.target){
if (arguments.length == 0){
return nativeDateCtor();
return new DateReal(nativeDateNow());
} else {
// calling new Date with arguments is deterministic
return new DateReal(...args);
Expand All @@ -185,7 +172,7 @@ void TracingRuntime::setupDate() {
.call(*this, code)
.asObject(*this)
.asFunction(*this)
.call(*this, {std::move(nativeDateCtor), std::move(nativeDateFunc)});
.call(*this, {std::move(nativeDateNow), std::move(nativeDateFunc)});
insertHostForwarder({"Date", "now"});
}

Expand Down
14 changes: 14 additions & 0 deletions unittests/API/SynthTraceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,20 @@ TEST_F(NonDeterminismReplayTest, DateNewWithArgsTest) {
EXPECT_EQ(dateTime, replayedTime);
}

TEST_F(NonDeterminismReplayTest, DateReplacedTest) {
eval(*traceRt, R"(
var oldDate = Date;
Date = undefined;
var x = new oldDate();
)");
auto traceTime = eval(*traceRt, "x.getTime()").asNumber();

replay();

auto replayedTime = eval(*replayRt, "x.getTime()").asNumber();
EXPECT_EQ(traceTime, replayedTime);
}

TEST_F(NonDeterminismReplayTest, MathRandomTest) {
eval(*traceRt, "var x = Math.random();");
auto randVal = eval(*traceRt, "x").asNumber();
Expand Down

0 comments on commit b04ece4

Please sign in to comment.