diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a97bf3ba..c6b40a38f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,8 @@ Fixes to bugs found via fuzzing * Fixed crash when parsing combo-chaining expressions such as `(a.b).c`. * Fixed crash when calling functions that have `Dynamic` parameters with more than 16 parameters. * Fixed crash when indexing into an empty array with negative index. -* Indexing into an array with a negative index that is larger than the length of the array now returns an out-of-bounds error (similar to positive indices) instead of defaulting to the first element. +* Indexing into an array with a negative index that is larger than the length of the array now throws an out-of-bounds error (similar to positive indices) instead of defaulting to the first element. +* Fixed edge-case crash in timestamp functions. Enhancements ------------ diff --git a/Cargo.toml b/Cargo.toml index fa867df0f..a7f447f7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ categories = ["no-std", "embedded", "wasm", "parser-implementations"] [dependencies] smallvec = { version = "1.7.0", default-features = false, features = ["union", "const_new", "const_generics"] } +thin-vec = { version = "0.2.13", default-features = false } ahash = { version = "0.8.2", default-features = false, features = ["compile-time-rng"] } num-traits = { version = "0.2.0", default-features = false } once_cell = { version = "1.7.0", default-features = false, features = ["race"] } @@ -68,6 +69,8 @@ internals = [] debugging = ["internals"] ## Features and dependencies required by `bin` tools: `decimal`, `metadata`, `serde`, `debugging` and [`rustyline`](https://crates.io/crates/rustyline). bin-features = ["decimal", "metadata", "serde", "debugging", "rustyline"] +## Enable fuzzing via the [`arbitrary`](https://crates.io/crates/arbitrary) crate. +fuzz = ["arbitrary", "rust_decimal/rust-fuzz"] #! ### System Configuration Features diff --git a/README.md b/README.md index f106d36f0..eec27d94e 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Standard features * Built-in support for most common [data types](https://rhai.rs/book/language/values-and-types.html) including booleans, [integers](https://rhai.rs/book/language/numbers.html), [floating-point numbers](https://rhai.rs/book/language/numbers.html) (including [`Decimal`](https://crates.io/crates/rust_decimal)), [strings](https://rhai.rs/book/language/strings-chars.html), [Unicode characters](https://rhai.rs/book/language/strings-chars.html), [arrays](https://rhai.rs/book/language/arrays.html) (including packed [byte arrays](https://rhai.rs/book/language/blobs.html)) and [object maps](https://rhai.rs/book/language/object-maps.html). * Easily [call a script-defined function](https://rhai.rs/book/engine/call-fn.html) from Rust. * Relatively little `unsafe` code (yes there are some for performance reasons). -* Few dependencies - currently only [`smallvec`](https://crates.io/crates/smallvec), [`num-traits`](https://crates.io/crates/num-traits), [`once_cell`](https://crates.io/crates/once_cell), [`ahash`](https://crates.io/crates/ahash), [`bitflags`](https://crates.io/crates/bitflags) and [`smartstring`](https://crates.io/crates/smartstring). +* Few dependencies - currently only [`smallvec`](https://crates.io/crates/smallvec), [`thin-vec`](https://crates.io/crates/thin-vec), [`num-traits`](https://crates.io/crates/num-traits), [`once_cell`](https://crates.io/crates/once_cell), [`ahash`](https://crates.io/crates/ahash), [`bitflags`](https://crates.io/crates/bitflags) and [`smartstring`](https://crates.io/crates/smartstring). * Re-entrant scripting engine can be made `Send + Sync` (via the `sync` feature). * Compile once to [AST](https://rhai.rs/book/engine/compile.html) form for repeated evaluations. * Scripts are [optimized](https://rhai.rs/book/engine/optimize) (useful for template-based machine-generated scripts). diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 57bf2c83f..e2045de6c 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -10,7 +10,7 @@ cargo-fuzz = true [dependencies] arbitrary = { version = "1.3.2", features = ["derive"] } libfuzzer-sys = "0.4" -rhai = { path = "..", features = ["arbitrary"] } +rhai = { path = "..", features = ["fuzz", "decimal", "metadata", "debugging"] } # Prevent this from interfering with workspaces [workspace] diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 1c19938f7..a1d121bbf 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -5,8 +5,8 @@ use crate::engine::KEYWORD_FN_PTR; use crate::tokenizer::Token; use crate::types::dynamic::Union; use crate::{ - calc_fn_hash, Dynamic, FnArgsVec, FnPtr, Identifier, ImmutableString, Position, SmartString, - StaticVec, INT, + calc_fn_hash, Dynamic, FnPtr, Identifier, ImmutableString, Position, SmartString, StaticVec, + INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -19,6 +19,7 @@ use std::{ mem, num::{NonZeroU8, NonZeroUsize}, }; +use thin_vec::ThinVec; /// _(internals)_ A binary expression. /// Exported under the `internals` feature only. @@ -266,9 +267,9 @@ pub enum Expr { /// [String][ImmutableString] constant. StringConstant(ImmutableString, Position), /// An interpolated [string][ImmutableString]. - InterpolatedString(Box>, Position), + InterpolatedString(ThinVec, Position), /// [ expr, ... ] - Array(Box>, Position), + Array(ThinVec, Position), /// #{ name:expr, ... } Map( Box<(StaticVec<(Ident, Expr)>, BTreeMap)>, diff --git a/src/ast/namespace.rs b/src/ast/namespace.rs index 3721fb81f..3d7c50204 100644 --- a/src/ast/namespace.rs +++ b/src/ast/namespace.rs @@ -66,25 +66,6 @@ impl fmt::Display for Namespace { } } -impl From> for Namespace { - #[inline] - fn from(mut path: Vec) -> Self { - path.shrink_to_fit(); - Self { - index: None, - path: path.into(), - } - } -} - -impl From> for Namespace { - #[inline] - fn from(mut path: StaticVec) -> Self { - path.shrink_to_fit(); - Self { index: None, path } - } -} - impl Namespace { /// Constant for no namespace. pub const NONE: Self = Self { diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 3220eb98b..cc23136d3 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -359,7 +359,8 @@ impl Engine { #[cfg(not(feature = "no_object"))] (Expr::Property(..), ChainType::Dotting) => (), #[cfg(not(feature = "no_object"))] - (Expr::Property(..), ..) => { + #[cfg(not(feature = "no_index"))] + (Expr::Property(..), ChainType::Indexing) => { unreachable!("unexpected Expr::Property for indexing") } // Short-circuit for indexing with literal: {expr}[1] diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 6ff0a7b99..dcb175cf4 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -73,7 +73,7 @@ pub fn search_scope_only<'s>( { let val: Dynamic = crate::FnPtr { name: v.3.clone(), - curry: Vec::new(), + curry: thin_vec::ThinVec::new(), environ: None, fn_def: Some(fn_def.clone()), } diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index 656511959..501348e6b 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -25,14 +25,14 @@ pub type SharedGlobalConstants = pub struct GlobalRuntimeState { /// Names of imported [modules][crate::Module]. #[cfg(not(feature = "no_module"))] - imports: Vec, + imports: thin_vec::ThinVec, /// Stack of imported [modules][crate::Module]. #[cfg(not(feature = "no_module"))] - modules: Vec, + modules: thin_vec::ThinVec, /// The current stack of loaded [modules][crate::Module] containing script-defined functions. #[cfg(not(feature = "no_function"))] - pub lib: Vec, + pub lib: thin_vec::ThinVec, /// Source of the current context. /// /// No source if the string is empty. @@ -82,11 +82,11 @@ impl GlobalRuntimeState { pub fn new(engine: &Engine) -> Self { Self { #[cfg(not(feature = "no_module"))] - imports: Vec::new(), + imports: thin_vec::ThinVec::new(), #[cfg(not(feature = "no_module"))] - modules: Vec::new(), + modules: thin_vec::ThinVec::new(), #[cfg(not(feature = "no_function"))] - lib: Vec::new(), + lib: thin_vec::ThinVec::new(), source: None, num_operations: 0, #[cfg(not(feature = "no_module"))] @@ -176,7 +176,7 @@ impl GlobalRuntimeState { /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] #[inline] - pub(crate) fn iter_imports_raw( + pub fn iter_imports_raw( &self, ) -> impl Iterator { self.imports.iter().rev().zip(self.modules.iter().rev()) diff --git a/src/func/callable_function.rs b/src/func/callable_function.rs index 387121a49..94a7ca204 100644 --- a/src/func/callable_function.rs +++ b/src/func/callable_function.rs @@ -21,7 +21,7 @@ pub struct EncapsulatedEnviron { pub lib: crate::SharedModule, /// Imported [modules][crate::Module]. #[cfg(not(feature = "no_module"))] - pub imports: Vec<(crate::ImmutableString, crate::SharedModule)>, + pub imports: thin_vec::ThinVec<(crate::ImmutableString, crate::SharedModule)>, /// Globally-defined constants. #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_function"))] diff --git a/src/module/mod.rs b/src/module/mod.rs index 370bae0c2..5a741424f 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2195,7 +2195,7 @@ impl Module { let mut module = Self::new(); // Extra modules left become sub-modules - let mut imports = Vec::new(); + let mut imports = thin_vec::ThinVec::new(); if result.is_ok() { global diff --git a/src/optimizer.rs b/src/optimizer.rs index 2ded69ef1..088d9ce76 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -80,7 +80,7 @@ impl<'a> OptimizerState<'a> { #[cfg(not(feature = "no_function"))] { - _global.lib = _lib.to_vec(); + _global.lib = _lib.into(); } Self { diff --git a/src/packages/arithmetic.rs b/src/packages/arithmetic.rs index c6a13a179..05f7edca7 100644 --- a/src/packages/arithmetic.rs +++ b/src/packages/arithmetic.rs @@ -381,7 +381,7 @@ mod f32_functions { "Number raised to too large an index: {x} ** {y}" ))) } else { - #[allow(clippy::cast_possible_truncation)] + #[allow(clippy::cast_possible_truncation, clippy::unnecessary_cast)] Ok(x.powi(y as i32)) } } diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 6ce404b75..73376393e 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -5,7 +5,6 @@ use crate::engine::OP_EQUALS; use crate::eval::{calc_index, calc_offset_len}; use crate::module::ModuleFlags; use crate::plugin::*; - use crate::{ def_package, Array, Dynamic, ExclusiveRange, FnPtr, InclusiveRange, NativeCallContext, Position, RhaiResultOf, ERR, INT, MAX_USIZE_INT, @@ -13,6 +12,7 @@ use crate::{ #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{any::TypeId, cmp::Ordering, mem}; +use thin_vec::ThinVec; def_package! { /// Package of basic array utilities. @@ -1272,7 +1272,7 @@ pub mod array_functions { pub fn dedup(ctx: NativeCallContext, array: &mut Array) { let comparer = FnPtr { name: ctx.engine().get_interned_string(OP_EQUALS), - curry: Vec::new(), + curry: ThinVec::new(), environ: None, #[cfg(not(feature = "no_function"))] fn_def: None, diff --git a/src/packages/time_basic.rs b/src/packages/time_basic.rs index abbd8e7b7..7248619f6 100644 --- a/src/packages/time_basic.rs +++ b/src/packages/time_basic.rs @@ -189,41 +189,49 @@ mod time_functions { } } - #[allow(clippy::cast_sign_loss)] + #[inline(always)] + fn add_inner(timestamp: Instant, seconds: u64) -> Option { + if cfg!(not(feature = "unchecked")) { + timestamp.checked_add(Duration::from_secs(seconds)) + } else { + Some(timestamp + Duration::from_secs(seconds)) + } + } + #[inline] fn add_impl(timestamp: Instant, seconds: INT) -> RhaiResultOf { if seconds < 0 { - return subtract_impl(timestamp, -seconds); + subtract_inner(timestamp, seconds.unsigned_abs() as u64) + } else { + #[allow(clippy::cast_sign_loss)] + add_inner(timestamp, seconds as u64) } + .ok_or_else(|| { + make_arithmetic_err(format!( + "Timestamp overflow when adding {seconds} second(s)" + )) + }) + } + #[inline(always)] + fn subtract_inner(timestamp: Instant, seconds: u64) -> Option { if cfg!(not(feature = "unchecked")) { - timestamp - .checked_add(Duration::from_secs(seconds as u64)) - .ok_or_else(|| { - make_arithmetic_err(format!( - "Timestamp overflow when adding {seconds} second(s)" - )) - }) + timestamp.checked_sub(Duration::from_secs(seconds)) } else { - Ok(timestamp + Duration::from_secs(seconds as u64)) + Some(timestamp - Duration::from_secs(seconds)) } } - #[allow(clippy::cast_sign_loss)] + #[inline] fn subtract_impl(timestamp: Instant, seconds: INT) -> RhaiResultOf { if seconds < 0 { - return add_impl(timestamp, -seconds); - } - if cfg!(not(feature = "unchecked")) { - timestamp - .checked_sub(Duration::from_secs(seconds as u64)) - .ok_or_else(|| { - make_arithmetic_err(format!( - "Timestamp overflow when subtracting {seconds} second(s)" - )) - }) + add_inner(timestamp, seconds.unsigned_abs() as u64) } else { - Ok(timestamp - .checked_sub(Duration::from_secs(seconds as u64)) - .unwrap()) + #[allow(clippy::cast_sign_loss)] + subtract_inner(timestamp, seconds as u64) } + .ok_or_else(|| { + make_arithmetic_err(format!( + "Timestamp overflow when subtracting {seconds} second(s)" + )) + }) } /// Add the specified number of `seconds` to the timestamp and return it as a new timestamp. diff --git a/src/parser.rs b/src/parser.rs index 86b4e5e5f..a9da6d097 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -31,6 +31,7 @@ use std::{ hash::{Hash, Hasher}, num::{NonZeroU8, NonZeroUsize}, }; +use thin_vec::ThinVec; pub type ParseResult = Result; @@ -956,7 +957,7 @@ impl Engine { // [ ... settings.pos = eat_token(input, &Token::LeftBracket); - let mut array = FnArgsVec::new_const(); + let mut array = ThinVec::new(); loop { const MISSING_RBRACKET: &str = "to end this array literal"; @@ -1010,7 +1011,7 @@ impl Engine { array.shrink_to_fit(); - Ok(Expr::Array(array.into(), settings.pos)) + Ok(Expr::Array(array, settings.pos)) } /// Parse a map literal. @@ -1543,7 +1544,7 @@ impl Engine { // Interpolated string Token::InterpolatedString(..) => { - let mut segments = FnArgsVec::new_const(); + let mut segments = ThinVec::new(); let settings = settings.level_up()?; match input.next().unwrap() { @@ -1601,7 +1602,7 @@ impl Engine { Expr::StringConstant(state.get_interned_string(""), settings.pos) } else { segments.shrink_to_fit(); - Expr::InterpolatedString(segments.into(), settings.pos) + Expr::InterpolatedString(segments, settings.pos) } } @@ -3883,7 +3884,7 @@ impl Engine { let fn_ptr = crate::FnPtr { name: fn_name, - curry: Vec::new(), + curry: ThinVec::new(), environ: None, #[cfg(not(feature = "no_function"))] fn_def: Some(script.clone()), diff --git a/src/serde/ser.rs b/src/serde/ser.rs index d76975368..14cc2fee7 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -15,7 +15,7 @@ use num_traits::FromPrimitive; /// Serializer for [`Dynamic`][crate::Dynamic]. pub struct DynamicSerializer { /// Buffer to hold a temporary key. - key: Identifier, + _key: Identifier, /// Buffer to hold a temporary value. value: Dynamic, } @@ -25,7 +25,7 @@ impl DynamicSerializer { #[must_use] pub const fn new(value: Dynamic) -> Self { Self { - key: Identifier::new_const(), + _key: Identifier::new_const(), value, } } @@ -570,7 +570,7 @@ impl SerializeMap for DynamicSerializer { #[cfg(not(feature = "no_object"))] { let key = _key.serialize(&mut *self)?; - self.key = key + self._key = key .into_immutable_string() .map_err(|typ| { ERR::ErrorMismatchDataType("string".into(), typ.into(), Position::NONE) @@ -590,7 +590,7 @@ impl SerializeMap for DynamicSerializer { fn serialize_value(&mut self, _value: &T) -> RhaiResultOf<()> { #[cfg(not(feature = "no_object"))] { - let key = std::mem::take(&mut self.key); + let key = std::mem::take(&mut self._key); let value = _value.serialize(&mut *self)?; let map = self.value.downcast_mut::().unwrap(); map.insert(key, value); diff --git a/src/tests.rs b/src/tests.rs index 053d6688a..eee1680db 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -42,10 +42,10 @@ fn check_struct_sizes() { return; } - assert_eq!(size_of::(), 72); + assert_eq!(size_of::(), 24); assert_eq!( size_of::(), - 48 - if cfg!(feature = "no_function") { + 32 - if cfg!(feature = "no_function") { WORD_SIZE } else { 0 diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index 8c73a7fd4..5d26bb3ae 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -18,13 +18,14 @@ use std::{ mem, ops::{Index, IndexMut}, }; +use thin_vec::ThinVec; /// A general function pointer, which may carry additional (i.e. curried) argument values /// to be passed onto a function during a call. #[derive(Clone)] pub struct FnPtr { pub(crate) name: ImmutableString, - pub(crate) curry: Vec, + pub(crate) curry: ThinVec, pub(crate) environ: Option>, #[cfg(not(feature = "no_function"))] pub(crate) fn_def: Option>, @@ -469,7 +470,7 @@ impl TryFrom for FnPtr { if is_valid_function_name(&value) { Ok(Self { name: value, - curry: Vec::new(), + curry: ThinVec::new(), environ: None, #[cfg(not(feature = "no_function"))] fn_def: None, @@ -492,7 +493,7 @@ impl>> From for FnPtr { Self { name: fn_def.name.clone(), - curry: Vec::new(), + curry: ThinVec::new(), environ: None, fn_def: Some(fn_def), } diff --git a/src/types/scope.rs b/src/types/scope.rs index 46334ea1e..d94067736 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -9,6 +9,7 @@ use std::{ iter::{Extend, FromIterator}, marker::PhantomData, }; +use thin_vec::ThinVec; /// Minimum number of entries in the [`Scope`] to avoid reallocations. pub const MIN_SCOPE_ENTRIES: usize = 8; @@ -61,14 +62,14 @@ pub const MIN_SCOPE_ENTRIES: usize = 8; #[derive(Debug, Hash, Default)] pub struct Scope<'a> { /// Current value of the entry. - values: Vec, + values: ThinVec, /// Name of the entry. - names: Vec, + names: ThinVec, /// Aliases of the entry. /// /// This `Vec` is not filled until needed because aliases are used rarely /// (only for `export` statements). - aliases: Vec>, + aliases: ThinVec>, /// Phantom to keep the lifetime parameter in order not to break existing code. dummy: PhantomData<&'a ()>, } @@ -176,11 +177,11 @@ impl Scope<'_> { /// ``` #[inline(always)] #[must_use] - pub const fn new() -> Self { + pub fn new() -> Self { Self { - values: Vec::new(), - names: Vec::new(), - aliases: Vec::new(), + values: ThinVec::new(), + names: ThinVec::new(), + aliases: ThinVec::new(), dummy: PhantomData, } } @@ -200,9 +201,9 @@ impl Scope<'_> { #[must_use] pub fn with_capacity(capacity: usize) -> Self { Self { - values: Vec::with_capacity(capacity), - names: Vec::with_capacity(capacity), - aliases: Vec::new(), + values: ThinVec::with_capacity(capacity), + names: ThinVec::with_capacity(capacity), + aliases: ThinVec::new(), dummy: PhantomData, } } diff --git a/tests/time.rs b/tests/time.rs index c2269417b..d10258d21 100644 --- a/tests/time.rs +++ b/tests/time.rs @@ -1,9 +1,7 @@ #![cfg(not(feature = "no_time"))] use rhai::Engine; - #[cfg(not(feature = "no_float"))] use rhai::FLOAT; - #[cfg(feature = "no_float")] use rhai::INT; @@ -118,4 +116,8 @@ fn test_timestamp_op() { .unwrap(), 42 ); + + // Check edge case for crashes + #[cfg(not(feature = "unchecked"))] + let _ = engine.run("timestamp()-24>>-60"); }