fix: Crash on constraint application

fixes #618

Author: dmtrKovalenko

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<git2::Status>;
     fn write_relative_path(&self, arena: ArenaPtr, out: &mut String);
+    fn is_overflow(&self) -> bool;
 }
 
 /// Windows stores paths with `\\`; `/` comes from user queries.
@@ -163,12 +164,13 @@ pub(crate) fn apply_constraints<'a, T: Constrainable + Sync>(
     items: &'a [T],
     constraints: &[Constraint<'_>],
     arena: ArenaPtr,
+    overflow_arena: ArenaPtr,
 ) -> Option<Vec<&'a T>> {
     if constraints.is_empty() {
         return None;
     }
-    let plan = ConstraintPlan::build(constraints, items, arena);
-    Some(plan.run(items, arena))
+    let plan = ConstraintPlan::build(constraints, items, arena, overflow_arena);
+    Some(plan.run(items, arena, overflow_arena))
 }
 
 #[cfg(feature = "zlob")]
@@ -217,6 +219,7 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> {
         constraints: &'c [Constraint<'q>],
         items: &[T],
         arena: ArenaPtr,
+        overflow_arena: ArenaPtr,
     ) -> Self {
         let mut extensions = SmallVec::new();
         let mut rest = SmallVec::new();
@@ -227,22 +230,28 @@ 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, arena, overflow_arena);
         Self {
             extensions,
             rest,
             glob,
         }
     }
 
-    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],
+        arena: 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, arena, overflow_arena, scratch)
+                        .then_some(item)
                 })
                 .flatten()
                 .collect()
@@ -251,7 +260,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, arena, overflow_arena, &mut scratch)
+                        .then_some(item)
+                })
                 .collect()
         }
     }
@@ -262,8 +274,17 @@ impl<'q, 'c> ConstraintPlan<'q, 'c> {
         item: &T,
         index: usize,
         arena: ArenaPtr,
+        overflow_arena: ArenaPtr,
         scratch: &mut ConstraintsBuffers,
     ) -> bool {
+        // Overflow items' path chunks live in the overflow arena; reading them
+        // through the base arena dereferences a wrong (possibly null) pointer.
+        let arena = if item.is_overflow() {
+            overflow_arena
+        } else {
+            arena
+        };
+
         if !self.passes_extensions(item, arena, scratch) {
             return false;
         }
@@ -442,14 +463,15 @@ fn build_glob_strategy<T: Constrainable>(
     has_pre_filter: bool,
     items: &[T],
     arena: ArenaPtr,
+    overflow_arena: ArenaPtr,
 ) -> GlobStrategy {
     if !contains_glob(rest) {
         return GlobStrategy::None;
     }
     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 +499,18 @@ struct PathBuffer {
 }
 
 impl PathBuffer {
-    fn collect<T: Constrainable>(items: &[T], arena: ArenaPtr) -> Self {
+    fn collect<T: Constrainable>(items: &[T], arena: ArenaPtr, overflow_arena: ArenaPtr) -> Self {
         let mut bytes = Vec::<u8>::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 +634,10 @@ mod tests {
         fn git_status(&self) -> Option<git2::Status> {
             None
         }
+
+        fn is_overflow(&self) -> bool {
+            false
+        }
     }
 
     #[test]
@@ -830,13 +861,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 +919,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,15 +957,15 @@ 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)
             .collect();
         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 +999,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)

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;

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![];

crates/fff-core/src/types.rs

@@ -205,6 +205,11 @@ impl Constrainable for DirItem {
     fn git_status(&self) -> Option<git2::Status> {
         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<git2::Status> {
         self.git_status
     }
+
+    #[inline]
+    fn is_overflow(&self) -> bool {
+        FileItem::is_overflow(self)
+    }
 }
 
 #[derive(Debug, Clone, Default)]

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");
+}