From 0068db8a113d0bdf18b5c6eed05002d4c494eccf Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 8 Nov 2023 19:03:53 +0100 Subject: [PATCH] Avoid UB when checking if float fits in int32 --- quickjs.c | 45 +++++++++++++++++++++++++++++++++++++++++-- quickjs.h | 22 +-------------------- tests/test_builtin.js | 4 ++++ 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/quickjs.c b/quickjs.c index b6a2df5..07cb57d 100644 --- a/quickjs.c +++ b/quickjs.c @@ -10797,7 +10797,8 @@ static int JS_ToInt64Free(JSContext *ctx, int64_t *pres, JSValue val) ret = v << ((e - 1023) - 52); /* take the sign into account */ if (u.u64 >> 63) - ret = -ret; + if (ret != INT64_MIN) + ret = -ret; } else { ret = 0; /* also handles NaN and +inf */ } @@ -10872,7 +10873,8 @@ static int JS_ToInt32Free(JSContext *ctx, int32_t *pres, JSValue val) ret = v >> 32; /* take the sign into account */ if (u.u64 >> 63) - ret = -ret; + if (ret != INT32_MIN) + ret = -ret; } else { ret = 0; /* also handles NaN and +inf */ } @@ -11968,6 +11970,45 @@ static double js_pow(double a, double b) } } +// Special care is taken to not invoke UB when checking if the result fits +// in an int32_t. Leans on the fact that the input is integral if the lower +// 52 bits of the equation 2**e * (f + 2**52) are zero. +static BOOL float_is_int32(double d) +{ + uint64_t u, m, e, f; + JSFloat64Union t; + + t.d = d; + u = t.u64; + + // special case -0 + m = 1ull << 63; + if (u == m) + return FALSE; + + e = (u >> 52) & 0x7FF; + if (e > 0) + e -= 1023; + + // too large, nan or inf? + if (e > 30) + return FALSE; + + // fractional or subnormal if low bits are non-zero + f = 0xFFFFFFFFFFFFFull & u; + m = 0xFFFFFFFFFFFFFull >> e; + return 0 == (f & m); +} + +JSValue JS_NewFloat64(JSContext *ctx, double d) +{ + if (float_is_int32(d)) { + return JS_MKVAL(JS_TAG_INT, (int32_t)d); + } else { + return __JS_NewFloat64(ctx, d); + } +} + #ifdef CONFIG_BIGNUM JSValue JS_NewBigInt64_1(JSContext *ctx, int64_t v) diff --git a/quickjs.h b/quickjs.h index c25f10c..0d2af44 100644 --- a/quickjs.h +++ b/quickjs.h @@ -539,30 +539,10 @@ static js_force_inline JSValue JS_NewUint32(JSContext *ctx, uint32_t val) return v; } +JSValue JS_NewFloat64(JSContext *ctx, double d); JSValue JS_NewBigInt64(JSContext *ctx, int64_t v); JSValue JS_NewBigUint64(JSContext *ctx, uint64_t v); -static js_force_inline JSValue JS_NewFloat64(JSContext *ctx, double d) -{ - JSValue v; - int32_t val; - union { - double d; - uint64_t u; - } u, t; - u.d = d; - val = (int32_t)d; - t.d = val; - /* -0 cannot be represented as integer, so we compare the bit - representation */ - if (u.u == t.u) { - v = JS_MKVAL(JS_TAG_INT, val); - } else { - v = __JS_NewFloat64(ctx, d); - } - return v; -} - static inline JS_BOOL JS_IsNumber(JSValueConst v) { int tag = JS_VALUE_GET_TAG(v); diff --git a/tests/test_builtin.js b/tests/test_builtin.js index 4ce2355..64f05f4 100644 --- a/tests/test_builtin.js +++ b/tests/test_builtin.js @@ -332,6 +332,10 @@ function test_number() assert(+" 123 ", 123); assert(+"0b111", 7); assert(+"0o123", 83); + assert(parseFloat("2147483647"), 2147483647); + assert(parseFloat("2147483648"), 2147483648); + assert(parseFloat("-2147483647"), -2147483647); + assert(parseFloat("-2147483648"), -2147483648); assert(parseFloat("0x1234"), 0); assert(parseFloat("Infinity"), Infinity); assert(parseFloat("-Infinity"), -Infinity);