Skip to content

Commit

Permalink
Avoid UB when checking if float fits in int32
Browse files Browse the repository at this point in the history
  • Loading branch information
bnoordhuis committed Nov 8, 2023
1 parent 2f51cbc commit 0068db8
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 23 deletions.
45 changes: 43 additions & 2 deletions quickjs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
}
Expand Down Expand Up @@ -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 */
}
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 1 addition & 21 deletions quickjs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions tests/test_builtin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0068db8

Please sign in to comment.