From a8a4a5c6794634b606e636801c3f0758d899246a Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Sun, 21 Jun 2026 07:55:19 -0700 Subject: [PATCH] fix: Crash on constraint application fixes https://github.com/dmtrKovalenko/fff/issues/618 --- crates/fff-core/src/constraints.rs | 70 +++++++++++++------ crates/fff-core/src/grep.rs | 20 ++++-- crates/fff-core/src/score.rs | 4 +- crates/fff-core/src/types.rs | 10 +++ .../grep_overflow_path_constraint_segfault.rs | 57 +++++++++++++++ 5 files changed, 135 insertions(+), 26 deletions(-) create mode 100644 crates/fff-core/tests/grep_overflow_path_constraint_segfault.rs diff --git a/crates/fff-core/src/constraints.rs b/crates/fff-core/src/constraints.rs index d93ac70b..a7da1b97 100644 --- a/crates/fff-core/src/constraints.rs +++ b/crates/fff-core/src/constraints.rs @@ -37,6 +37,7 @@ pub(crate) trait Constrainable { fn write_file_name(&self, arena: ArenaPtr, out: &mut String); fn git_status(&self) -> Option; fn write_relative_path(&self, arena: ArenaPtr, out: &mut String); + fn is_overflow(&self) -> bool; } /// Windows stores paths with `\\`; `/` comes from user queries. @@ -162,13 +163,14 @@ pub fn path_contains_segment(path: &str, segment: &str) -> bool { pub(crate) fn apply_constraints<'a, T: Constrainable + Sync>( items: &'a [T], constraints: &[Constraint<'_>], - arena: ArenaPtr, + base_arena: ArenaPtr, + overflow_arena: ArenaPtr, ) -> Option> { if constraints.is_empty() { return None; } - let plan = ConstraintPlan::build(constraints, items, arena); - Some(plan.run(items, arena)) + let plan = ConstraintPlan::build(constraints, items, base_arena, overflow_arena); + Some(plan.run(items, base_arena, overflow_arena)) } #[cfg(feature = "zlob")] @@ -216,7 +218,8 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> { pub(crate) fn build( constraints: &'c [Constraint<'q>], items: &[T], - arena: ArenaPtr, + base_arena: ArenaPtr, + overflow_arena: ArenaPtr, ) -> Self { let mut extensions = SmallVec::new(); let mut rest = SmallVec::new(); @@ -227,7 +230,8 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> { } } let has_pre_filter = !extensions.is_empty() || rest.iter().any(|&c| !is_glob_node(c)); - let glob = build_glob_strategy(&rest, has_pre_filter, items, arena); + let glob = build_glob_strategy(&rest, has_pre_filter, items, base_arena, overflow_arena); + Self { extensions, rest, @@ -235,14 +239,20 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> { } } - fn run<'a, T: Constrainable + Sync>(&self, items: &'a [T], arena: ArenaPtr) -> Vec<&'a T> { + fn run<'a, T: Constrainable + Sync>( + &self, + items: &'a [T], + base_arean: ArenaPtr, + overflow_arena: ArenaPtr, + ) -> Vec<&'a T> { if items.len() >= PAR_THRESHOLD { use rayon::prelude::*; items .par_iter() .enumerate() .map_init(ConstraintsBuffers::new, |scratch, (i, item)| { - self.matches(item, i, arena, scratch).then_some(item) + self.matches(item, i, base_arean, overflow_arena, scratch) + .then_some(item) }) .flatten() .collect() @@ -251,7 +261,10 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> { items .iter() .enumerate() - .filter_map(|(i, item)| self.matches(item, i, arena, &mut scratch).then_some(item)) + .filter_map(|(i, item)| { + self.matches(item, i, base_arean, overflow_arena, &mut scratch) + .then_some(item) + }) .collect() } } @@ -261,9 +274,16 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> { &self, item: &T, index: usize, - arena: ArenaPtr, + base_arena: ArenaPtr, + overflow_arena: ArenaPtr, scratch: &mut ConstraintsBuffers, ) -> bool { + let arena = if item.is_overflow() { + overflow_arena + } else { + base_arena + }; + if !self.passes_extensions(item, arena, scratch) { return false; } @@ -442,6 +462,7 @@ fn build_glob_strategy( has_pre_filter: bool, items: &[T], arena: ArenaPtr, + overflow_arena: ArenaPtr, ) -> GlobStrategy { if !contains_glob(rest) { return GlobStrategy::None; @@ -449,7 +470,7 @@ fn build_glob_strategy( if has_pre_filter { return GlobStrategy::Inline(compile_globs(rest)); } - let buf = PathBuffer::collect(items, arena); + let buf = PathBuffer::collect(items, arena, overflow_arena); let path_refs = buf.as_strs(); GlobStrategy::Prepass(precompute_masks(rest, &path_refs)) } @@ -477,13 +498,18 @@ struct PathBuffer { } impl PathBuffer { - fn collect(items: &[T], arena: ArenaPtr) -> Self { + fn collect(items: &[T], arena: ArenaPtr, overflow_arena: ArenaPtr) -> Self { let mut bytes = Vec::::new(); let mut offsets = Vec::with_capacity(items.len()); let mut tmp = String::with_capacity(64); for item in items { + let item_arena = if item.is_overflow() { + overflow_arena + } else { + arena + }; let start = bytes.len(); - item.write_relative_path(arena, &mut tmp); + item.write_relative_path(item_arena, &mut tmp); bytes.extend_from_slice(tmp.as_bytes()); #[cfg(windows)] for b in &mut bytes[start..] { @@ -607,6 +633,10 @@ mod tests { fn git_status(&self) -> Option { None } + + fn is_overflow(&self) -> bool { + false + } } #[test] @@ -830,13 +860,13 @@ mod tests { let mismatch = [Constraint::FilePath("트.c")]; let exact_items = [item.clone()]; - let exact_matches = - apply_constraints(&exact_items, &exact, arena_ptr).expect("constraints applied"); + let exact_matches = apply_constraints(&exact_items, &exact, arena_ptr, arena_ptr) + .expect("constraints applied"); assert_eq!(exact_matches.len(), 1); let mismatch_items = [item]; - let mismatch_matches = - apply_constraints(&mismatch_items, &mismatch, arena_ptr).expect("constraints applied"); + let mismatch_matches = apply_constraints(&mismatch_items, &mismatch, arena_ptr, arena_ptr) + .expect("constraints applied"); assert!(mismatch_matches.is_empty()); } @@ -888,7 +918,7 @@ mod tests { // Not(Glob("**/*.rs")) should exclude .rs files let constraints = vec![Constraint::Not(Box::new(Constraint::Glob("**/*.rs")))]; - let result = apply_constraints(&items, &constraints, arena_ptr).unwrap(); + let result = apply_constraints(&items, &constraints, arena_ptr, arena_ptr).unwrap(); let paths: Vec<&str> = result.iter().map(|i| i.relative_path).collect(); assert!( !paths.contains(&"src/main.rs"), @@ -926,7 +956,7 @@ mod tests { ]; let mixed = vec![Constraint::Extension("rs"), Constraint::Glob("src/**")]; - let mixed_paths: Vec<&str> = apply_constraints(&items, &mixed, arena_ptr) + let mixed_paths: Vec<&str> = apply_constraints(&items, &mixed, arena_ptr, arena_ptr) .unwrap() .iter() .map(|i| i.relative_path) @@ -934,7 +964,7 @@ mod tests { assert_eq!(mixed_paths, vec!["src/main.rs"]); let pure_glob = vec![Constraint::Glob("src/**")]; - let glob_paths: Vec<&str> = apply_constraints(&items, &pure_glob, arena_ptr) + let glob_paths: Vec<&str> = apply_constraints(&items, &pure_glob, arena_ptr, arena_ptr) .unwrap() .iter() .map(|i| i.relative_path) @@ -968,7 +998,7 @@ mod tests { Constraint::Extension("rs"), Constraint::Not(Box::new(Constraint::Glob("vendor/**"))), ]; - let paths: Vec<&str> = apply_constraints(&items, &constraints, arena_ptr) + let paths: Vec<&str> = apply_constraints(&items, &constraints, arena_ptr, arena_ptr) .unwrap() .iter() .map(|i| i.relative_path) diff --git a/crates/fff-core/src/grep.rs b/crates/fff-core/src/grep.rs index 5c73fa5b..da07688e 100644 --- a/crates/fff-core/src/grep.rs +++ b/crates/fff-core/src/grep.rs @@ -1018,6 +1018,7 @@ pub(crate) fn multi_grep_search<'a>( base_file_count, options, arena, + overflow_arena, ); // If constraints yielded 0 files and we had FilePath constraints, @@ -1032,6 +1033,7 @@ pub(crate) fn multi_grep_search<'a>( base_file_count, options, arena, + overflow_arena, ); files_to_search = retry_files; filtered_file_count = retry_count; @@ -1440,12 +1442,18 @@ fn prefilter_files<'a>( base_count: usize, options: &GrepSearchOptions, arena: crate::simd_path::ArenaPtr, + overflow_arena: crate::simd_path::ArenaPtr, ) -> (Vec<&'a FileItem>, usize) { let max_file_size = options.max_file_size; let plan = if constraints.is_empty() { None } else { - Some(ConstraintPlan::build(constraints, files, arena)) + Some(ConstraintPlan::build( + constraints, + files, + arena, + overflow_arena, + )) }; let mut scratch = ConstraintsBuffers::new(); @@ -1482,7 +1490,7 @@ fn prefilter_files<'a>( continue; } if let Some(plan) = plan.as_ref() - && !plan.matches(f, file_idx, arena, &mut scratch) + && !plan.matches(f, file_idx, arena, overflow_arena, &mut scratch) { continue; } @@ -1514,7 +1522,7 @@ fn prefilter_files<'a>( continue; } if let Some(ref p) = plan - && !p.matches(f, boundary + offset, arena, &mut scratch) + && !p.matches(f, boundary + offset, arena, overflow_arena, &mut scratch) { continue; } @@ -1533,7 +1541,7 @@ fn prefilter_files<'a>( continue; } if let Some(ref p) = plan - && !p.matches(f, idx, arena, &mut scratch) + && !p.matches(f, idx, arena, overflow_arena, &mut scratch) { continue; } @@ -2047,6 +2055,7 @@ pub(crate) fn grep_search<'a>( base_count, options, arena, + overflow_arena, ); if files_to_search.is_empty() @@ -2060,6 +2069,7 @@ pub(crate) fn grep_search<'a>( base_count, options, arena, + overflow_arena, ); files_to_search = retry_files; @@ -2179,6 +2189,7 @@ pub(crate) fn grep_search<'a>( bigram_boundary, options, arena, + overflow_arena, ); if files_to_search.is_empty() @@ -2191,6 +2202,7 @@ pub(crate) fn grep_search<'a>( bigram_boundary, options, arena, + overflow_arena, ); files_to_search = retry_files; filtered_file_count = retry_count; diff --git a/crates/fff-core/src/score.rs b/crates/fff-core/src/score.rs index 918ca9d6..96544e87 100644 --- a/crates/fff-core/src/score.rs +++ b/crates/fff-core/src/score.rs @@ -276,7 +276,7 @@ pub(crate) fn fuzzy_match_and_score_dirs<'a>( let working_dirs: Vec<&DirItem> = if parsed_query.constraints.is_empty() { dirs.iter().collect() } else { - match apply_constraints(dirs, &parsed_query.constraints, arena) { + match apply_constraints(dirs, &parsed_query.constraints, arena, arena) { Some(filtered) if !filtered.is_empty() => filtered, Some(_) => return (vec![], vec![], 0), None => dirs.iter().collect(), @@ -488,7 +488,7 @@ fn match_and_score_in_arena<'a>( let working_files: FileItems<'a> = if parsed.constraints.is_empty() { FileItems::All(files) } else { - match apply_constraints(files, &parsed.constraints, arena) { + match apply_constraints(files, &parsed.constraints, arena, arena) { Some(filtered) if !filtered.is_empty() => FileItems::Filtered(filtered), Some(_) => { return vec![]; diff --git a/crates/fff-core/src/types.rs b/crates/fff-core/src/types.rs index 2720e6be..d603591a 100644 --- a/crates/fff-core/src/types.rs +++ b/crates/fff-core/src/types.rs @@ -205,6 +205,11 @@ impl Constrainable for DirItem { fn git_status(&self) -> Option { None } + + #[inline] + fn is_overflow(&self) -> bool { + DirItem::is_overflow(self) + } } #[derive(Debug)] @@ -756,6 +761,11 @@ impl Constrainable for FileItem { fn git_status(&self) -> Option { self.git_status } + + #[inline] + fn is_overflow(&self) -> bool { + FileItem::is_overflow(self) + } } #[derive(Debug, Clone, Default)] diff --git a/crates/fff-core/tests/grep_overflow_path_constraint_segfault.rs b/crates/fff-core/tests/grep_overflow_path_constraint_segfault.rs new file mode 100644 index 00000000..0cad79c3 --- /dev/null +++ b/crates/fff-core/tests/grep_overflow_path_constraint_segfault.rs @@ -0,0 +1,57 @@ +use std::fs; + +use fff_search::file_picker::FilePicker; +use fff_search::grep::{GrepMode, GrepSearchOptions}; +use fff_search::{AiGrepConfig, FFFQuery, FilePickerOptions}; +use tempfile::TempDir; + +// bug pinning https://github.com/dmtrKovalenko/fff/issues/618 +#[test] +fn grep_path_constraint_on_overflow_file_does_not_segfault() { + let tmp = TempDir::new().expect("tempdir"); + let base = tmp.path(); + let spec_dir = base.join("specs"); + fs::create_dir_all(&spec_dir).expect("mkdir specs"); + + let file = spec_dir.join("annotation-plan.md"); + fs::write(&file, "dependency\n").expect("write test file"); + + // Intentionally do NOT call `collect_files()`. This leaves the base path + // arena unset/null. `handle_create_or_modify` then adds the file as an + // overflow file whose path chunks live in the overflow arena. + let mut picker = FilePicker::new(FilePickerOptions { + base_path: base.to_string_lossy().to_string(), + enable_mmap_cache: false, + watch: false, + enable_home_dir_scanning: true, + ..Default::default() + }) + .expect("create picker"); + + let is_overflow = picker + .handle_create_or_modify(&file) + .expect("add overflow file") + .is_overflow(); + assert!(is_overflow, "file must be added to the overflow arena"); + + // Mirrors the `ffgrep({ pattern: "dependency", path: "specs/annotation-plan.md" })` + // call that crashed: in AI grep mode the path token becomes a FilePath + // constraint and `dependency` becomes the grep text. + let parsed = FFFQuery::parse("specs/annotation-plan.md dependency", AiGrepConfig); + + let opts = GrepSearchOptions { + mode: GrepMode::PlainText, + page_limit: 20, + smart_case: true, + ..Default::default() + }; + + // original issue: + // Debug builds typically abort with Rust's unsafe precondition check at + // simd_path.rs: ChunkedString::write_to_string. Release builds may SIGSEGV + // in memmove from an address like 0x30 (null arena + chunk_index * 16). + let result = picker.grep(&parsed, &opts); + + assert_eq!(result.files.len(), 1, "the overflow file should match"); + assert_eq!(result.matches.len(), 1, "`dependency` should match once"); +}