Skip to content

Force evaluation order in set_date_fields #268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 22, 2024
Merged

Force evaluation order in set_date_fields #268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8349a0f
Force evaluation order in `set_date_fields`
chqrlie Feb 18, 2024
9ae3aa1
Rewrite `set_date_fields` to match the ECMA specification
chqrlie Feb 21, 2024
027bb81
Disable date precision tests on win32 and cygwin targets
chqrlie Feb 21, 2024
4c2f2c5
Rewrite `set_date_fields` to match the ECMA specification
chqrlie Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 55 additions & 24 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46764,7 +46764,7 @@ static __exception int get_date_fields(JSContext *ctx, JSValue obj,
return FALSE; /* NaN */
d = 0; /* initialize all fields to 0 */
} else {
d = dval;
d = dval; /* assuming -8.64e15 <= dval <= -8.64e15 */
if (is_local) {
tz = -getTimezoneOffset(d);
d += tz * 60000;
Expand Down Expand Up @@ -46810,33 +46810,63 @@ static double time_clip(double t) {
return NAN;
}

/* The spec mandates the use of 'double' and it fixes the order
/* The spec mandates the use of 'double' and it specifies the order
of the operations */
static double set_date_fields(double fields[], int is_local) {
int64_t y;
double days, d, h, m1;
int i, m, md;

m1 = fields[1];
m = fmod(m1, 12);
if (m < 0)
m += 12;
y = (int64_t)(fields[0] + floor(m1 / 12));
days = days_from_year(y);

for(i = 0; i < m; i++) {
md = month_days[i];
double y, m, dt, ym, mn, day, h, s, milli, time, tv;
int yi, mi, i;
int64_t days;
volatile double temp; /* enforce evaluation order */

/* emulate 21.4.1.15 MakeDay ( year, month, date ) */
y = fields[0];
m = fields[1];
dt = fields[2];
ym = y + floor(m / 12);
mn = fmod(m, 12);
if (mn < 0)
mn += 12;
if (ym < -271821 || ym > 275760)
return NAN;

yi = ym;
mi = mn;
days = days_from_year(yi);
for(i = 0; i < mi; i++) {
days += month_days[i];
if (i == 1)
md += days_in_year(y) - 365;
days += md;
days += days_in_year(yi) - 365;
}
day = days + dt - 1;

/* emulate 21.4.1.14 MakeTime ( hour, min, sec, ms ) */
h = fields[3];
m = fields[4];
s = fields[5];
milli = fields[6];
/* Use a volatile intermediary variable to ensure order of evaluation
* as specified in ECMA. This fixes a test262 error on
* test262/test/built-ins/Date/UTC/fp-evaluation-order.js.
* Without the volatile qualifier, the compile can generate code
* that performs the computation in a different order or with instructions
* that produce a different result such as FMA (float multiply and add).
*/
time = h * 3600000;
time += (temp = m * 60000);
time += (temp = s * 1000);
time += milli;

/* emulate 21.4.1.16 MakeDate ( day, time ) */
tv = (temp = day * 86400000) + time; /* prevent generation of FMA */
if (!isfinite(tv))
return NAN;

/* adjust for local time and clip */
if (is_local) {
int64_t ti = tv < INT64_MIN ? INT64_MIN : tv >= 0x1p63 ? INT64_MAX : (int64_t)tv;
tv += getTimezoneOffset(ti) * 60000;
}
days += fields[2] - 1;
h = fields[3] * 3600000 + fields[4] * 60000 +
fields[5] * 1000 + fields[6];
d = days * 86400000 + h;
if (is_local)
d += getTimezoneOffset(d) * 60000;
return time_clip(d);
return time_clip(tv);
}

static JSValue get_date_field(JSContext *ctx, JSValue this_val,
Expand Down Expand Up @@ -47446,6 +47476,7 @@ static JSValue js_date_getTimezoneOffset(JSContext *ctx, JSValue this_val,
if (isnan(v))
return JS_NAN;
else
/* assuming -8.64e15 <= v <= -8.64e15 */
return JS_NewInt64(ctx, getTimezoneOffset((int64_t)trunc(v)));
}

Expand Down
74 changes: 58 additions & 16 deletions tests/test_builtin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,22 @@ function assert(actual, expected, message) {
if (arguments.length == 1)
expected = true;

if (actual === expected)
return;

if (actual !== null && expected !== null
&& typeof actual == 'object' && typeof expected == 'object'
&& actual.toString() === expected.toString())
return;

if (typeof actual === typeof expected) {
if (actual === expected) {
if (actual !== 0 || (1 / actual) === (1 / expected))
return;
}
if (typeof actual === 'number') {
if (isNaN(actual) && isNaN(expected))
return true;
}
if (typeof actual === 'object') {
if (actual !== null && expected !== null
&& actual.constructor === expected.constructor
&& actual.toString() === expected.toString())
return;
}
}
throw Error("assertion failed: got |" + actual + "|" +
", expected |" + expected + "|" +
(message ? " (" + message + ")" : ""));
Expand Down Expand Up @@ -594,20 +602,54 @@ function test_date()
assert(d.toISOString(), "2017-09-22T18:10:11.091Z");
a = Date.parse(d.toISOString());
assert((new Date(a)).toISOString(), d.toISOString());
s = new Date("2020-01-01T01:01:01.123Z").toISOString();
assert(s, "2020-01-01T01:01:01.123Z");
// implementation defined behavior
s = new Date("2020-01-01T01:01:01.1Z").toISOString();
assert(s == "2020-01-01T01:01:01.100Z");
assert(s, "2020-01-01T01:01:01.100Z");
s = new Date("2020-01-01T01:01:01.12Z").toISOString();
assert(s == "2020-01-01T01:01:01.120Z");
s = new Date("2020-01-01T01:01:01.123Z").toISOString();
assert(s == "2020-01-01T01:01:01.123Z");
assert(s, "2020-01-01T01:01:01.120Z");
s = new Date("2020-01-01T01:01:01.1234Z").toISOString();
assert(s == "2020-01-01T01:01:01.123Z");
assert(s, "2020-01-01T01:01:01.123Z");
s = new Date("2020-01-01T01:01:01.12345Z").toISOString();
assert(s == "2020-01-01T01:01:01.123Z");
assert(s, "2020-01-01T01:01:01.123Z");
s = new Date("2020-01-01T01:01:01.1235Z").toISOString();
assert(s == "2020-01-01T01:01:01.124Z");
assert(s == "2020-01-01T01:01:01.124Z" || // QuickJS
s == "2020-01-01T01:01:01.123Z"); // nodeJS
s = new Date("2020-01-01T01:01:01.9999Z").toISOString();
assert(s == "2020-01-01T01:01:02.000Z");
assert(s == "2020-01-01T01:01:02.000Z" || // QuickJS
s == "2020-01-01T01:01:01.999Z"); // nodeJS

assert(Date.UTC(2017), 1483228800000);
assert(Date.UTC(2017, 9), 1506816000000);
assert(Date.UTC(2017, 9, 22), 1508630400000);
assert(Date.UTC(2017, 9, 22, 18), 1508695200000);
assert(Date.UTC(2017, 9, 22, 18, 10), 1508695800000);
assert(Date.UTC(2017, 9, 22, 18, 10, 11), 1508695811000);
assert(Date.UTC(2017, 9, 22, 18, 10, 11, 91), 1508695811091);

assert(Date.UTC(NaN), NaN);
assert(Date.UTC(2017, NaN), NaN);
assert(Date.UTC(2017, 9, NaN), NaN);
assert(Date.UTC(2017, 9, 22, NaN), NaN);
assert(Date.UTC(2017, 9, 22, 18, NaN), NaN);
assert(Date.UTC(2017, 9, 22, 18, 10, NaN), NaN);
assert(Date.UTC(2017, 9, 22, 18, 10, 11, NaN), NaN);
assert(Date.UTC(2017, 9, 22, 18, 10, 11, 91, NaN), 1508695811091);

// TODO: Fix rounding errors on Windows/Cygwin.
if (!['win32', 'cygwin'].includes(os.platform)) {
// from test262/test/built-ins/Date/UTC/fp-evaluation-order.js
assert(Date.UTC(1970, 0, 1, 80063993375, 29, 1, -288230376151711740), 29312,
'order of operations / precision in MakeTime');
assert(Date.UTC(1970, 0, 213503982336, 0, 0, 0, -18446744073709552000), 34447360,
'precision in MakeDate');
}
//assert(Date.UTC(2017 - 1e9, 9 + 12e9), 1506816000000); // node fails this
assert(Date.UTC(2017, 9, 22 - 1e10, 18 + 24e10), 1508695200000);
assert(Date.UTC(2017, 9, 22, 18 - 1e10, 10 + 60e10), 1508695800000);
assert(Date.UTC(2017, 9, 22, 18, 10 - 1e10, 11 + 60e10), 1508695811000);
assert(Date.UTC(2017, 9, 22, 18, 10, 11 - 1e12, 91 + 1000e12), 1508695811091);
}

function test_regexp()
Expand Down