andrey/helix-mirror
1
0
Fork 0
mirror of https://github.com/helix-editor/helix.git synced 2024-11-22 09:26:19 +04:00
Code Issues Projects Releases Wiki Activity

Mark query_captures function as unsafe

Browse Source 
It's easy to mistakenly use-after-free the cursor and captures iterator
here because of the transmute. Ideally this could be fixed upstream in
tree-sitter by introducing an API with lifetimes/types that reflect the
lifetimes of the underlying data.

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
This commit is contained in:
Michael Davis 2022-12-21 09:27:50 -06:00
parent 68f7b87519
commit 1a5eb55477
No known key found for this signature in database
1 changed files with 20 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
helix-core/src/syntax.rs
View File

@ -1079,7 +1079,8 @@ pub struct TsParser {
/// Creates an iterator over the captures in a query within the given range, /// Creates an iterator over the captures in a query within the given range,
/// re-using a cursor from the pool if available. /// re-using a cursor from the pool if available.
fn query_captures<'a, 'tree>( /// SAFETY: The `QueryCaptures` must be droped before the `QueryCursor` is dropped.
unsafe fn query_captures<'a, 'tree>(
query: &'a Query, query: &'a Query,
root: Node<'tree>, root: Node<'tree>,
source: RopeSlice<'a>, source: RopeSlice<'a>,
@ -1094,10 +1095,11 @@ fn query_captures<'a, 'tree>(
highlighter.cursors.pop().unwrap_or_default() highlighter.cursors.pop().unwrap_or_default()
}); });
// This is the unsafe line:
// The `captures` iterator borrows the `Tree` and the `QueryCursor`, which // The `captures` iterator borrows the `Tree` and the `QueryCursor`, which
// prevents them from being moved. But both of these values are really just // prevents them from being moved. But both of these values are really just
// pointers, so it's actually ok to move them. // pointers, so it's actually ok to move them.
let cursor_ref = unsafe { mem::transmute::<_, &'static mut QueryCursor>(&mut cursor) }; let cursor_ref = mem::transmute::<_, &'static mut QueryCursor>(&mut cursor);
// if reusing cursors & no range this resets to whole range // if reusing cursors & no range this resets to whole range
cursor_ref.set_byte_range(range.unwrap_or(0..usize::MAX)); cursor_ref.set_byte_range(range.unwrap_or(0..usize::MAX));
@ -1460,12 +1462,14 @@ fn query_iter<'a, F>(
.layers .layers
.iter() .iter()
.filter_map(|(_, layer)| { .filter_map(|(_, layer)| {
let (cursor, captures) = query_captures( let (cursor, captures) = unsafe {
query_fn(&layer.config), query_captures(
layer.tree().root_node(), query_fn(&layer.config),
source, layer.tree().root_node(),
range.clone(), source,
); range.clone(),
)
};
let mut captures = captures.peekable(); let mut captures = captures.peekable();
// If there aren't any captures for this layer, skip the layer. // If there aren't any captures for this layer, skip the layer.
@ -1497,12 +1501,14 @@ pub fn highlight_iter<'a>(
.filter_map(|(_, layer)| { .filter_map(|(_, layer)| {
// TODO: if range doesn't overlap layer range, skip it // TODO: if range doesn't overlap layer range, skip it
let (cursor, captures) = query_captures( let (cursor, captures) = unsafe {
&layer.config.query, query_captures(
layer.tree().root_node(), &layer.config.query,
source, layer.tree().root_node(),
range.clone(), source,
); range.clone(),
)
};
let mut captures = captures.peekable(); let mut captures = captures.peekable();
// If there are no captures, skip the layer // If there are no captures, skip the layer