Skip to content

Commit

Permalink
Merge pull request #786 from schungx/master
Browse files Browse the repository at this point in the history
Fix more fuzzing bugs.
  • Loading branch information
schungx committed Nov 27, 2023
2 parents 8b69f93 + f3b4918 commit ee6261b
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Bug fixes
* Fixed crash when parsing multi-segment interpolated string longer than maximum (found via fuzzing).
* Fixed crash when parsing unterminated comment (found via fuzzing).
* Fixed crash when parsing deeply-nested right-associated operators such as `**` (found via fuzzing).
* Fixed crash when parsing combo-chaining expressions such as `(a.b).c` (found via fuzzing).

Deprecated API's
----------------
Expand Down
4 changes: 2 additions & 2 deletions src/api/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::func::native::locked_write;
use crate::parser::{ParseSettingFlags, ParseState};
use crate::tokenizer::{lex_raw, Token};
use crate::types::StringsInterner;
use crate::{Engine, LexError, Map, OptimizationLevel, RhaiResultOf};
use crate::{Engine, LexError, Map, RhaiResultOf};
#[cfg(feature = "no_std")]
use std::prelude::v1::*;

Expand Down Expand Up @@ -134,7 +134,7 @@ impl Engine {
state,
|s| s.flags |= ParseSettingFlags::DISALLOW_UNQUOTED_MAP_PROPERTIES,
#[cfg(not(feature = "no_optimize"))]
OptimizationLevel::None,
crate::OptimizationLevel::None,
#[cfg(feature = "no_optimize")]
<_>::default(),
)?
Expand Down
24 changes: 15 additions & 9 deletions src/eval/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct FnResolutionCacheEntry {
#[derive(Debug, Clone, Default)]
pub struct FnResolutionCache {
/// Hash map containing cached functions.
pub map: StraightHashMap<Option<FnResolutionCacheEntry>>,
pub dict: StraightHashMap<Option<FnResolutionCacheEntry>>,
/// Bloom filter to avoid caching "one-hit wonders".
pub filter: BloomFilterU64,
}
Expand All @@ -34,7 +34,7 @@ impl FnResolutionCache {
#[inline(always)]
#[allow(dead_code)]
pub fn clear(&mut self) {
self.map.clear();
self.dict.clear();
self.filter.clear();
}
}
Expand All @@ -45,39 +45,45 @@ impl FnResolutionCache {
/// The following caches are contained inside this type:
/// * A stack of [function resolution caches][FnResolutionCache]
#[derive(Debug, Clone)]
pub struct Caches(StaticVec<FnResolutionCache>);
pub struct Caches {
fn_resolution: StaticVec<FnResolutionCache>,
}

impl Caches {
/// Create an empty [`Caches`].
#[inline(always)]
#[must_use]
pub const fn new() -> Self {
Self(StaticVec::new_const())
Self {
fn_resolution: StaticVec::new_const(),
}
}
/// Get the number of function resolution cache(s) in the stack.
#[inline(always)]
#[must_use]
pub fn fn_resolution_caches_len(&self) -> usize {
self.0.len()
self.fn_resolution.len()
}
/// Get a mutable reference to the current function resolution cache.
///
/// A new function resolution cache is pushed onto the stack if the stack is empty.
#[inline]
#[must_use]
pub fn fn_resolution_cache_mut(&mut self) -> &mut FnResolutionCache {
// Push a new function resolution cache if the stack is empty
if self.0.is_empty() {
if self.fn_resolution.is_empty() {
self.push_fn_resolution_cache();
}
self.0.last_mut().unwrap()
self.fn_resolution.last_mut().unwrap()
}
/// Push an empty function resolution cache onto the stack and make it current.
#[inline(always)]
pub fn push_fn_resolution_cache(&mut self) {
self.0.push(<_>::default());
self.fn_resolution.push(<_>::default());
}
/// Rewind the function resolution caches stack to a particular size.
#[inline(always)]
pub fn rewind_fn_resolution_caches(&mut self, len: usize) {
self.0.truncate(len);
self.fn_resolution.truncate(len);
}
}
4 changes: 2 additions & 2 deletions src/eval/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl Engine {
let this_ptr = this_ptr.as_deref_mut();

#[cfg(not(feature = "no_module"))]
let imports_len = global.num_imports();
let orig_imports_len = global.num_imports();

let result =
self.eval_stmt(global, caches, scope, this_ptr, stmt, restore_orig_state)?;
Expand All @@ -83,7 +83,7 @@ impl Engine {
// Without global functions, the extra modules never affect function resolution.
if global
.scan_imports_raw()
.skip(imports_len)
.skip(orig_imports_len)
.any(|(.., m)| m.contains_indexed_global_functions())
{
// Different scenarios where the cache must be cleared - notice that this is
Expand Down
6 changes: 4 additions & 2 deletions src/func/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Engine {

let cache = caches.fn_resolution_cache_mut();

match cache.map.entry(hash) {
match cache.dict.entry(hash) {
Entry::Occupied(entry) => entry.into_mut().as_ref(),
Entry::Vacant(entry) => {
let num_args = args.as_deref().map_or(0, FnCallArgs::len);
Expand All @@ -190,22 +190,24 @@ impl Engine {
let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic`

loop {
// First check scripted functions in the AST or embedded environments
#[cfg(not(feature = "no_function"))]
let func = _global
.lib
.iter()
.rev()
.chain(self.global_modules.iter())
.find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw())));
#[cfg(feature = "no_function")]
let func = None;

// Then check the global namespace
let func = func.or_else(|| {
self.global_modules
.iter()
.find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw())))
});

// Then check imported modules for global functions, then global sub-modules for global functions
#[cfg(not(feature = "no_module"))]
let func = func
.or_else(|| _global.get_qualified_fn(hash, true))
Expand Down
16 changes: 10 additions & 6 deletions src/func/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ impl Engine {
}

// Does a script-defined function exist?
///
/// # Note
///
/// If the scripted function is not found, this information is cached for future look-ups.
#[must_use]
pub(crate) fn has_script_fn(
&self,
Expand All @@ -219,27 +223,27 @@ impl Engine {
) -> bool {
let cache = caches.fn_resolution_cache_mut();

if let Some(result) = cache.map.get(&hash_script).map(Option::is_some) {
if let Some(result) = cache.dict.get(&hash_script).map(Option::is_some) {
return result;
}

// First check script-defined functions
let r = global.lib.iter().any(|m| m.contains_fn(hash_script))
let res = global.lib.iter().any(|m| m.contains_fn(hash_script))
// Then check the global namespace and packages
|| self.global_modules.iter().any(|m| m.contains_fn(hash_script));

#[cfg(not(feature = "no_module"))]
let r = r ||
let res = res ||
// Then check imported modules
global.contains_qualified_fn(hash_script)
// Then check sub-modules
|| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash_script));

if !r && !cache.filter.is_absent_and_set(hash_script) {
if !res && !cache.filter.is_absent_and_set(hash_script) {
// Do not cache "one-hit wonders"
cache.map.insert(hash_script, None);
cache.dict.insert(hash_script, None);
}

r
res
}
}
2 changes: 1 addition & 1 deletion src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ fn optimize_combo_chain(expr: &mut Expr) {
Expr::Dot(mut x, opt, pos) => match x.lhs.take() {
#[cfg(not(feature = "no_index"))]
Expr::Index(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Index),
Expr::Dot(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Index),
Expr::Dot(x2, opt2, pos2) => (x, opt, pos, x2, opt2, pos2, Expr::Dot, Expr::Dot),
_ => unreachable!("combo chain expected"),
},
_ => unreachable!("combo chain expected"),
Expand Down
2 changes: 2 additions & 0 deletions tests/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ fn test_map_indexing() {
}

assert_eq!(engine.eval::<INT>("let y = #{a: 1, b: 2, c: 3}; y.a = 5; y.a").unwrap(), 5);
assert_eq!(engine.eval::<INT>("let y = #{a: #{x:9, y:8, z:7}, b: 2, c: 3}; (y.a).z").unwrap(), 7);
assert_eq!(engine.eval::<INT>("let y = #{a: #{x:9, y:8, z:7}, b: 2, c: 3}; (y.a).z = 42; y.a.z").unwrap(), 42);

engine.run("let y = #{a: 1, b: 2, c: 3}; y.z").unwrap();

Expand Down

0 comments on commit ee6261b

Please sign in to comment.