From f2f2153a04a0876ac8c146b44549b580600e3197 Mon Sep 17 00:00:00 2001 From: George Roman <30772943+georgeroman@users.noreply.github.com> Date: Sun, 18 Oct 2020 16:32:44 +0300 Subject: [PATCH] Fix some panics related to BigInt operations (#884) * Fix some panics related to BigInt operations * Address review comments * Address review comments --- boa/src/builtins/bigint/mod.rs | 21 +++++-- boa/src/builtins/bigint/operations.rs | 91 ++++++++++++--------------- boa/src/builtins/bigint/tests.rs | 35 +++++++++++ boa/src/value/operations.rs | 52 ++++++++++----- 4 files changed, 126 insertions(+), 73 deletions(-) diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index 2ad5f61014..94570ba253 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -173,9 +173,17 @@ impl BigInt { pub(crate) fn as_int_n(_this: &Value, args: &[Value], ctx: &mut Context) -> Result { let (modulo, bits) = Self::calculate_as_uint_n(args, ctx)?; - if bits > 0 && modulo >= BigInt::from(2).pow(&BigInt::from(bits as i64 - 1)) { + if bits > 0 + && modulo + >= BigInt::from(2) + .pow(&BigInt::from(bits as i64 - 1)) + .expect("the exponent must be positive") + { Ok(Value::from( - modulo - BigInt::from(2).pow(&BigInt::from(bits as i64)), + modulo + - BigInt::from(2) + .pow(&BigInt::from(bits as i64)) + .expect("the exponent must be positive"), )) } else { Ok(Value::from(modulo)) @@ -214,10 +222,11 @@ impl BigInt { let bigint = bigint_arg.to_bigint(ctx)?; Ok(( - bigint - .as_inner() - .clone() - .mod_floor(&BigInt::from(2).pow(&BigInt::from(bits as i64))), + bigint.as_inner().clone().mod_floor( + &BigInt::from(2) + .pow(&BigInt::from(bits as i64)) + .expect("the exponent must be positive"), + ), bits, )) } diff --git a/boa/src/builtins/bigint/operations.rs b/boa/src/builtins/bigint/operations.rs index 56f28cd164..7edbc898c1 100644 --- a/boa/src/builtins/bigint/operations.rs +++ b/boa/src/builtins/bigint/operations.rs @@ -7,15 +7,46 @@ use super::BigInt; impl BigInt { #[inline] - pub fn pow(self, other: &Self) -> Self { - Self( - self.0.pow( - other - .0 - .to_biguint() - .expect("RangeError: \"BigInt negative exponent\""), - ), - ) + pub fn pow(self, other: &Self) -> Result { + Ok(Self(self.0.pow( + other.0.to_biguint().ok_or("BigInt negative exponent")?, + ))) + } + + #[inline] + pub fn shift_right(mut self, other: Self) -> Result { + use std::ops::ShlAssign; + use std::ops::ShrAssign; + + if let Some(n) = other.0.to_i32() { + if n > 0 { + self.0.shr_assign(n as usize) + } else { + self.0.shl_assign(n.abs() as usize) + } + + Ok(self) + } else { + Err("Maximum BigInt size exceeded".into()) + } + } + + #[inline] + pub fn shift_left(mut self, other: Self) -> Result { + use std::ops::ShlAssign; + use std::ops::ShrAssign; + + if let Some(n) = other.0.to_i32() { + if n > 0 { + self.0.shl_assign(n as usize) + } else { + self.0.shr_assign(n.abs() as usize) + } + + Ok(self) + } else { + Err("Maximum BigInt size exceeded".into()) + } } /// Floored integer modulo. @@ -55,48 +86,6 @@ impl_bigint_operator!(BitAnd, bitand, BitAndAssign, bitand_assign); impl_bigint_operator!(BitOr, bitor, BitOrAssign, bitor_assign); impl_bigint_operator!(BitXor, bitxor, BitXorAssign, bitxor_assign); -impl std::ops::Shr for BigInt { - type Output = Self; - - fn shr(mut self, other: Self) -> Self::Output { - use std::ops::ShlAssign; - use std::ops::ShrAssign; - - if let Some(n) = other.0.to_i32() { - if n > 0 { - self.0.shr_assign(n as usize) - } else { - self.0.shl_assign(n.abs() as usize) - } - - return self; - } - - panic!("RangeError: Maximum BigInt size exceeded"); - } -} - -impl std::ops::Shl for BigInt { - type Output = Self; - - fn shl(mut self, other: Self) -> Self::Output { - use std::ops::ShlAssign; - use std::ops::ShrAssign; - - if let Some(n) = other.0.to_i32() { - if n > 0 { - self.0.shl_assign(n as usize) - } else { - self.0.shr_assign(n.abs() as usize) - } - - return self; - } - - panic!("RangeError: Maximum BigInt size exceeded"); - } -} - impl std::ops::Neg for BigInt { type Output = Self; diff --git a/boa/src/builtins/bigint/tests.rs b/boa/src/builtins/bigint/tests.rs index 5746b46731..06f4e726b2 100644 --- a/boa/src/builtins/bigint/tests.rs +++ b/boa/src/builtins/bigint/tests.rs @@ -202,6 +202,41 @@ fn pow() { ); } +#[test] +fn pow_negative_exponent() { + let mut engine = Context::new(); + + assert_throws(&mut engine, "10n ** (-10n)", "RangeError"); +} + +#[test] +fn shl() { + let mut engine = Context::new(); + + assert_eq!(forward(&mut engine, "8n << 2n"), "32n"); +} + +#[test] +fn shl_out_of_range() { + let mut engine = Context::new(); + + assert_throws(&mut engine, "1000n << 1000000000000000n", "RangeError"); +} + +#[test] +fn shr() { + let mut engine = Context::new(); + + assert_eq!(forward(&mut engine, "8n >> 2n"), "2n"); +} + +#[test] +fn shr_out_of_range() { + let mut engine = Context::new(); + + assert_throws(&mut engine, "1000n >> 1000000000000000n", "RangeError"); +} + #[test] fn to_string() { let mut engine = Context::new(); diff --git a/boa/src/value/operations.rs b/boa/src/value/operations.rs index 89f27a5e01..6bfa8aa371 100644 --- a/boa/src/value/operations.rs +++ b/boa/src/value/operations.rs @@ -176,14 +176,22 @@ impl Value { (Self::Integer(x), Self::Rational(y)) => Self::rational(f64::from(*x).powf(*y)), (Self::Rational(x), Self::Integer(y)) => Self::rational(x.powi(*y)), - (Self::BigInt(ref a), Self::BigInt(ref b)) => Self::bigint(a.as_inner().clone().pow(b)), + (Self::BigInt(ref a), Self::BigInt(ref b)) => Self::bigint( + a.as_inner() + .clone() + .pow(b) + .map_err(|msg| ctx.construct_range_error(msg))?, + ), // Slow path: (_, _) => match (self.to_numeric(ctx)?, other.to_numeric(ctx)?) { (Numeric::Number(a), Numeric::Number(b)) => Self::rational(a.powf(b)), - (Numeric::BigInt(ref a), Numeric::BigInt(ref b)) => { - Self::bigint(a.as_inner().clone().pow(b)) - } + (Numeric::BigInt(ref a), Numeric::BigInt(ref b)) => Self::bigint( + a.as_inner() + .clone() + .pow(b) + .map_err(|msg| ctx.construct_range_error(msg))?, + ), (_, _) => { return ctx.throw_type_error( "cannot mix BigInt and other types, use explicit conversions", @@ -304,18 +312,24 @@ impl Value { Self::integer(f64_to_int32(*x).wrapping_shl(*y as u32)) } - (Self::BigInt(ref a), Self::BigInt(ref b)) => { - Self::bigint(a.as_inner().clone() << b.as_inner().clone()) - } + (Self::BigInt(ref a), Self::BigInt(ref b)) => Self::bigint( + a.as_inner() + .clone() + .shift_left(b.as_inner().clone()) + .map_err(|msg| ctx.construct_range_error(&msg))?, + ), // Slow path: (_, _) => match (self.to_numeric(ctx)?, other.to_numeric(ctx)?) { (Numeric::Number(x), Numeric::Number(y)) => { Self::integer(f64_to_int32(x).wrapping_shl(f64_to_uint32(y))) } - (Numeric::BigInt(ref x), Numeric::BigInt(ref y)) => { - Self::bigint(x.as_inner().clone() << y.as_inner().clone()) - } + (Numeric::BigInt(ref x), Numeric::BigInt(ref y)) => Self::bigint( + x.as_inner() + .clone() + .shift_left(y.as_inner().clone()) + .map_err(|msg| ctx.construct_range_error(&msg))?, + ), (_, _) => { return ctx.throw_type_error( "cannot mix BigInt and other types, use explicit conversions", @@ -340,18 +354,24 @@ impl Value { Self::integer(f64_to_int32(*x).wrapping_shr(*y as u32)) } - (Self::BigInt(ref a), Self::BigInt(ref b)) => { - Self::bigint(a.as_inner().clone() >> b.as_inner().clone()) - } + (Self::BigInt(ref a), Self::BigInt(ref b)) => Self::bigint( + a.as_inner() + .clone() + .shift_right(b.as_inner().clone()) + .map_err(|msg| ctx.construct_range_error(&msg))?, + ), // Slow path: (_, _) => match (self.to_numeric(ctx)?, other.to_numeric(ctx)?) { (Numeric::Number(x), Numeric::Number(y)) => { Self::integer(f64_to_int32(x).wrapping_shr(f64_to_uint32(y))) } - (Numeric::BigInt(ref x), Numeric::BigInt(ref y)) => { - Self::bigint(x.as_inner().clone() >> y.as_inner().clone()) - } + (Numeric::BigInt(ref x), Numeric::BigInt(ref y)) => Self::bigint( + x.as_inner() + .clone() + .shift_right(y.as_inner().clone()) + .map_err(|msg| ctx.construct_range_error(&msg))?, + ), (_, _) => { return ctx.throw_type_error( "cannot mix BigInt and other types, use explicit conversions",