Skip to content

Commit 67956c6

Browse files
committed
Fix JS number rounding on x87
Disable 80-bits extended precision, use standard 64-bits precision. The extra precision affects rounding and is user-observable, meaning it causes test262 tests to fail. The rounding mode is a per-CPU (or, more accurately, per-FPU) property, and since quickjs is often used as a library, take care to tweak the FPU control word before sensitive operations, and restore it afterwards. Only affects x87. SSE is IEEE-754 conforming. Fixes: #1236
1 parent d368b44 commit 67956c6

File tree

4 files changed

+49
-6
lines changed

4 files changed

+49
-6
lines changed

.github/workflows/ci.yml

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ jobs:
5959
- { os: ubuntu-latest, configType: msan }
6060
- { os: ubuntu-latest, configType: tcc }
6161
- { os: ubuntu-latest, arch: x86 }
62+
- { os: ubuntu-latest, arch: i686 }
6263
- { os: ubuntu-latest, arch: riscv64 }
6364
- { os: ubuntu-latest, arch: s390x }
6465

@@ -72,6 +73,11 @@ jobs:
7273
with:
7374
submodules: true
7475

76+
- name: Install gcc-multilib
77+
if: ${{ matrix.config.configType == 'i686' }}
78+
run: |
79+
sudo apt update && sudo apt install -y gcc-multilib
80+
7581
- name: install TCC
7682
if: ${{ matrix.config.configType == 'tcc' }}
7783
run: |
@@ -117,7 +123,8 @@ jobs:
117123
elif [ "${{ matrix.config.configType }}" = "msan" ]; then
118124
echo "BUILD_TYPE=RelWithDebInfo" >> $GITHUB_ENV;
119125
echo "QJS_ENABLE_MSAN=ON" >> $GITHUB_ENV;
120-
echo "CC=clang" >> $GITHUB_ENV;
126+
elif [ "${{ matrix.config.configType }}" = "i686" ]; then
127+
echo "CMAKE_C_FLAGS=-m32" >> $GITHUB_ENV;
121128
fi
122129
123130
- name: build
@@ -128,7 +135,8 @@ jobs:
128135
BUILD_SHARED_LIBS=$BUILD_SHARED_LIBS \
129136
QJS_ENABLE_ASAN=$QJS_ENABLE_ASAN \
130137
QJS_ENABLE_UBSAN=$QJS_ENABLE_UBSAN \
131-
QJS_ENABLE_MSAN=$QJS_ENABLE_MSAN
138+
QJS_ENABLE_MSAN=$QJS_ENABLE_MSAN \
139+
CMAKE_C_FLAGS=$CMAKE_C_FLAGS
132140
133141
- name: stats
134142
if: ${{ matrix.config.configType != 'examples' }}

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,11 @@ fuzz:
5959
./fuzz
6060

6161
$(BUILD_DIR):
62-
cmake -B $(BUILD_DIR) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -DCMAKE_INSTALL_PREFIX=$(INSTALL_PREFIX)
62+
cmake \
63+
-B $(BUILD_DIR) \
64+
-DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \
65+
-DCMAKE_INSTALL_PREFIX=$(INSTALL_PREFIX) \
66+
-DCMAKE_C_FLAGS="$(CMAKE_C_FLAGS)"
6367

6468
$(QJS): $(BUILD_DIR)
6569
cmake --build $(BUILD_DIR) -j $(JOBS)

cutils.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,26 @@ int js_thread_join(js_thread_t thrd);
634634

635635
#endif /* !defined(EMSCRIPTEN) && !defined(__wasi__) */
636636

637+
// JS requires strict rounding behavior. Turn on 64-bits double precision
638+
// and disable x87 80-bits extended precision for intermediate floating-point
639+
// results.
640+
// 0x300 is extended precision, 0x200 is double precision.
641+
// Note that `*&cw` in the asm constraints looks redundant but isn't.
642+
#if defined(__i386__) && !defined(_MSC_VER)
643+
#define JS_X87_FPCW_SAVE_AND_ADJUST(cw) \
644+
unsigned short cw; \
645+
__asm__ __volatile__("fnstcw %0" : "=m"(*&cw)); \
646+
do { \
647+
unsigned short t = 0x200 | (cw & ~0x300); \
648+
__asm__ __volatile__("fldcw %0" : /*empty*/ : "m"(*&t)); \
649+
} while (0)
650+
#define JS_X87_FPCW_RESTORE(cw) \
651+
__asm__ __volatile__("fldcw %0" : /*empty*/ : "m"(*&cw))
652+
#else
653+
#define JS_X87_FPCW_SAVE_AND_ADJUST(cw)
654+
#define JS_X87_FPCW_RESTORE(cw)
655+
#endif
656+
637657
#ifdef __cplusplus
638658
} /* extern "C" { */
639659
#endif

quickjs.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17998,8 +17998,10 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
1799817998
sp[-2] = js_int32(r);
1799917999
sp--;
1800018000
} else if (JS_VALUE_IS_BOTH_FLOAT(op1, op2)) {
18001+
JS_X87_FPCW_SAVE_AND_ADJUST(fpcw);
1800118002
sp[-2] = js_float64(JS_VALUE_GET_FLOAT64(op1) +
1800218003
JS_VALUE_GET_FLOAT64(op2));
18004+
JS_X87_FPCW_RESTORE(fpcw);
1800318005
sp--;
1800418006
} else {
1800518007
add_slow:
@@ -18066,8 +18068,10 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
1806618068
sp[-2] = js_int32(r);
1806718069
sp--;
1806818070
} else if (JS_VALUE_IS_BOTH_FLOAT(op1, op2)) {
18071+
JS_X87_FPCW_SAVE_AND_ADJUST(fpcw);
1806918072
sp[-2] = js_float64(JS_VALUE_GET_FLOAT64(op1) -
1807018073
JS_VALUE_GET_FLOAT64(op2));
18074+
JS_X87_FPCW_RESTORE(fpcw);
1807118075
sp--;
1807218076
} else {
1807318077
goto binary_arith_slow;
@@ -18098,7 +18102,9 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
1809818102
sp[-2] = js_int32(r);
1809918103
sp--;
1810018104
} else if (JS_VALUE_IS_BOTH_FLOAT(op1, op2)) {
18105+
JS_X87_FPCW_SAVE_AND_ADJUST(fpcw);
1810118106
d = JS_VALUE_GET_FLOAT64(op1) * JS_VALUE_GET_FLOAT64(op2);
18107+
JS_X87_FPCW_RESTORE(fpcw);
1810218108
mul_fp_res:
1810318109
sp[-2] = js_float64(d);
1810418110
sp--;
@@ -52277,7 +52283,9 @@ static double set_date_fields(double fields[minimum_length(7)], int is_local) {
5227752283
int yi, mi, i;
5227852284
int64_t days;
5227952285
volatile double temp; /* enforce evaluation order */
52286+
double d = NAN;
5228052287

52288+
JS_X87_FPCW_SAVE_AND_ADJUST(fpcw);
5228152289
/* emulate 21.4.1.15 MakeDay ( year, month, date ) */
5228252290
y = fields[0];
5228352291
m = fields[1];
@@ -52287,7 +52295,7 @@ static double set_date_fields(double fields[minimum_length(7)], int is_local) {
5228752295
if (mn < 0)
5228852296
mn += 12;
5228952297
if (ym < -271821 || ym > 275760)
52290-
return NAN;
52298+
goto out;
5229152299

5229252300
yi = ym;
5229352301
mi = mn;
@@ -52319,14 +52327,17 @@ static double set_date_fields(double fields[minimum_length(7)], int is_local) {
5231952327
/* emulate 21.4.1.16 MakeDate ( day, time ) */
5232052328
tv = (temp = day * 86400000) + time; /* prevent generation of FMA */
5232152329
if (!isfinite(tv))
52322-
return NAN;
52330+
goto out;
5232352331

5232452332
/* adjust for local time and clip */
5232552333
if (is_local) {
5232652334
int64_t ti = tv < INT64_MIN ? INT64_MIN : tv >= 0x1p63 ? INT64_MAX : (int64_t)tv;
5232752335
tv += getTimezoneOffset(ti) * 60000;
5232852336
}
52329-
return time_clip(tv);
52337+
d = time_clip(tv);
52338+
out:
52339+
JS_X87_FPCW_RESTORE(fpcw);
52340+
return d;
5233052341
}
5233152342

5233252343
static JSValue get_date_field(JSContext *ctx, JSValueConst this_val,

0 commit comments

Comments
 (0)