Skip to content

Commit

Permalink
fix(es/minifier): Fix panic in bitwise logic and incorrect values (#9258
Browse files Browse the repository at this point in the history 
)

**Description:**

This PR fixes all issues listed in #9256 as well as the following:

`1.7976931348623157e+308 << 1.7976931348623157e+308` - swc currently
does not transform this and leaves it as-is

Anything involving low floating-point numbers, like `5e-324`, such as
`5e-324 >> 5e-324`, `5e-324 >> 0` etc - swc currently panics;
https://play.swc.rs/?version=1.6.7&code=H4sIAAAAAAAAA0vTME3VNTYyUbCzUzDQBACkm%2Ft7DgAAAA%3D%3D&config=H4sIAAAAAAAAA32UO3LjMAyG%2B5zCozrFjostcoDtcgYOTYIyvXxoCNCxJuO7L0TJj40hdRI%2B%2FAAJgPh%2B2%2B26E5ruY%2FfNn%2Fwz6IJQ7v9swTGRvrClAxM1muIH6t5v9IQTcjogNNN1Jh3p0gM1Fe5%2F7feLogs5I9wUiy365N34nNPkOBRAfLKxlUPWCInwf%2F3CSv6aAJX6bD%2FkHECnDaI0Kp8IeihSYJND0AOCOusiRJlOqovHLKWYYCWwaih5EHmynnxOnPOVWtBWmWxBQL6AIX8GSca5WJaQryfcp2ELh9r3rc8%2F1HDWoWoScsKltYRPK0Q9Zo%2BkXE1SCWe4UoMZLsX9qfROFaBa0qvulH1a6clfAK5A0IhJR5DiNg%2FH87SmdptKnxyPLI0C5%2FmWbpmg56Iq751Q2akyUMhL3Sxgq4GpskY6zoJXyofeggLneFaE0PjlyRylpDQOkJ0AuL%2FaSVM1A3V%2FhSt8ehAb%2BA%2FfkuQBWzyipuM6xTEecthIEIGO2W44cCsor%2BPCW%2BIyrPOaLPBogBVdKjbwugT4AVBWoe3Ll9ng58ERVR%2Fy4bEmFofrfQ9HnfrHe59X8dvi0MVsa4PLkp%2F6O6%2Fm393D6baF7wfvPH7elC3p9R%2BoYzQdMAYAAA%3D%3D

This PR also fixes `shiftCount` being incorrect and incorrect conversion
between `f64` and `i32`/`u32`. A new API in swc_ecma_utils has been
added, `to_js_int32` and `to_js_uint32` that allows external callers to
convert `f64`s to `i32` and `u32` so they can perform bitwise operations
themselves if they're trying to replicate JavaScript behaviour.

I also believe the updated bit shifting logic should be accessible
externally via swc_ecma_utils, but unsure about this.



**Related issue:**

 - Closes #9256

---------

Co-authored-by: magic-akari <[email protected]>
  • Loading branch information
@levi-nz @magic-akari
levi-nz and magic-akari authored Jul 16, 2024
1 parent 5a218f7 commit baeb9e2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 46 deletions.
41 changes: 4 additions & 37 deletions crates/swc_ecma_transforms_optimization/src/simplify/expr/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,13 +633,6 @@ impl SimplifyExpr {

// Bit shift operations
op!("<<") | op!(">>") | op!(">>>") => {
/// Uses a method for treating a double as 32bits that is
/// equivalent to how JavaScript would convert a
/// number before applying a bit operation.
fn js_convert_double_to_bits(d: f64) -> i32 {
((d.floor() as i64) & 0xffff_ffff) as i32
}

fn try_fold_shift(
ctx: &ExprCtx,
op: BinaryOp,
Expand All @@ -654,38 +647,12 @@ impl SimplifyExpr {
(Known(lv), Known(rv)) => (lv, rv),
_ => unreachable!(),
};

// only the lower 5 bits are used when shifting, so don't do anything
// if the shift amount is outside [0,32)
if !(0.0..32.0).contains(&rv) {
return Unknown;
}

let rv_int = rv as i32;
if rv_int as f64 != rv {
unimplemented!("error reporting: FRACTIONAL_BITWISE_OPERAND")
// report(FRACTIONAL_BITWISE_OPERAND, right.span());
// return n;
}

if lv.floor() != lv {
unimplemented!("error reporting: FRACTIONAL_BITWISE_OPERAND")
// report(FRACTIONAL_BITWISE_OPERAND, left.span());
// return n;
}

let bits = js_convert_double_to_bits(lv);
let (lv, rv) = (JsNumber::from(lv), JsNumber::from(rv));

Known(match op {
op!("<<") => (bits << rv_int) as f64,
op!(">>") => (bits >> rv_int) as f64,
op!(">>>") => {
let res = bits as u32 >> rv_int as u32;
// JavaScript always treats the result of >>> as unsigned.
// We must force Java to do the same here.
// unimplemented!(">>> (Zerofill rshift)")
res as f64
}
op!("<<") => *(lv << rv),
op!(">>") => *(lv >> rv),
op!(">>>") => *(lv.unsigned_shr(rv)),

_ => unreachable!("Unknown bit operator {:?}", op),
})
Expand Down
27 changes: 24 additions & 3 deletions crates/swc_ecma_transforms_optimization/src/simplify/expr/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,27 @@ fn test_fold_bitwise_op2() {
fold("x = 12 | NaN", "x=12");
}

#[test]
fn test_issue_9256() {
// Returns -2 prior to fix (Number.MAX_VALUE)
fold("1.7976931348623157e+308 << 1", "0");

// Isn't changed prior to fix
fold("1.7976931348623157e+308 << 1.7976931348623157e+308", "0");
fold("1.7976931348623157e+308 >> 1.7976931348623157e+308", "0");

// Panics prior to fix (Number.MIN_VALUE)
fold("5e-324 >> 5e-324", "0");
fold("5e-324 << 5e-324", "0");
fold("5e-324 << 0", "0");
fold("0 << 5e-324", "0");

// Wasn't broken prior, used to ensure overflows are handled correctly
fold("1 << 31", "-2147483648");
fold("-8 >> 2", "-2");
fold("-8 >>> 2", "1073741822");
}

#[test]
#[ignore]
fn test_folding_mix_types_early() {
Expand Down Expand Up @@ -726,9 +747,9 @@ fn test_fold_bit_shifts() {

fold("x = 0xffffffff << 0", "x = -1");
fold("x = 0xffffffff << 4", "x = -16");
fold_same("1 << 32");
fold_same("1 << -1");
fold_same("1 >> 32");
fold("1 << 32", "1");
fold("1 << -1", "-2147483648");
fold("1 >> 32", "1");
}

#[test]
Expand Down
10 changes: 4 additions & 6 deletions crates/swc_ecma_utils/src/number.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ impl std::ops::Deref for JsNumber {
impl JsNumber {
// https://tc39.es/ecma262/#sec-toint32
fn as_int32(&self) -> i32 {
if !self.0.is_finite() {
return 0;
}

self.0.trunc() as i32
self.as_uint32() as i32
}

// https://tc39.es/ecma262/#sec-touint32
Expand All @@ -49,7 +45,8 @@ impl JsNumber {
return 0;
}

self.0.trunc() as u32
// pow(2, 32) = 4294967296
self.0.trunc().rem_euclid(4294967296.0) as u32
}
}

Expand Down Expand Up @@ -211,6 +208,7 @@ mod test_js_number {
assert_eq!(JsNumber(-0.0).as_uint32(), 0);
assert_eq!(JsNumber(f64::INFINITY).as_uint32(), 0);
assert_eq!(JsNumber(f64::NEG_INFINITY).as_uint32(), 0);
assert_eq!(JsNumber(-8.0).as_uint32(), 4294967288);
}

#[test]
Expand Down

0 comments on commit baeb9e2

Please sign in to comment.