Merge pull request #792 from schungx/master

Fix timestamp crash.
rhaiscript · Dec 12, 2023 · 169af8e · 169af8e
2 parents f892361 + dfd31e0
commit 169af8e
Show file tree

Hide file tree

Showing 21 changed files with 91 additions and 91 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<FnArgsVec<Expr>>, Position),
+    InterpolatedString(ThinVec<Expr>, Position),
     /// [ expr, ... ]
-    Array(Box<FnArgsVec<Expr>>, Position),
+    Array(ThinVec<Expr>, Position),
     /// #{ name:expr, ... }
     Map(
         Box<(StaticVec<(Ident, Expr)>, BTreeMap<Identifier, Dynamic>)>,

diff --git a/src/ast/namespace.rs b/src/ast/namespace.rs
@@ -66,25 +66,6 @@ impl fmt::Display for Namespace {
     }
 }
 
-impl From<Vec<Ident>> for Namespace {
-    #[inline]
-    fn from(mut path: Vec<Ident>) -> Self {
-        path.shrink_to_fit();
-        Self {
-            index: None,
-            path: path.into(),
-        }
-    }
-}
-
-impl From<StaticVec<Ident>> for Namespace {
-    #[inline]
-    fn from(mut path: StaticVec<Ident>) -> 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
@@ -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
@@ -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
@@ -25,14 +25,14 @@ pub type SharedGlobalConstants =
 pub struct GlobalRuntimeState {
     /// Names of imported [modules][crate::Module].
     #[cfg(not(feature = "no_module"))]
-    imports: Vec<ImmutableString>,
+    imports: thin_vec::ThinVec<ImmutableString>,
     /// Stack of imported [modules][crate::Module].
     #[cfg(not(feature = "no_module"))]
-    modules: Vec<crate::SharedModule>,
+    modules: thin_vec::ThinVec<crate::SharedModule>,
 
     /// The current stack of loaded [modules][crate::Module] containing script-defined functions.
     #[cfg(not(feature = "no_function"))]
-    pub lib: Vec<crate::SharedModule>,
+    pub lib: thin_vec::ThinVec<crate::SharedModule>,
     /// 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<Item = (&ImmutableString, &crate::SharedModule)> {
         self.imports.iter().rev().zip(self.modules.iter().rev())

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -5,14 +5,14 @@ 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,
 };
 #[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
@@ -189,41 +189,49 @@ mod time_functions {
         }
     }
 
-    #[allow(clippy::cast_sign_loss)]
+    #[inline(always)]
+    fn add_inner(timestamp: Instant, seconds: u64) -> Option<Instant> {
+        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<Instant> {
         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<Instant> {
         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<Instant> {
         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
@@ -31,6 +31,7 @@ use std::{
     hash::{Hash, Hasher},
     num::{NonZeroU8, NonZeroUsize},
 };
+use thin_vec::ThinVec;
 
 pub type ParseResult<T> = Result<T, ParseError>;
 
@@ -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
@@ -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<T: ?Sized + Serialize>(&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::<crate::Map>().unwrap();
             map.insert(key, value);

diff --git a/src/tests.rs b/src/tests.rs
@@ -42,10 +42,10 @@ fn check_struct_sizes() {
         return;
     }
 
-    assert_eq!(size_of::<Scope>(), 72);
+    assert_eq!(size_of::<Scope>(), 24);
     assert_eq!(
         size_of::<FnPtr>(),
-        48 - if cfg!(feature = "no_function") {
+        32 - if cfg!(feature = "no_function") {
             WORD_SIZE
         } else {
             0