Skip to content

Commit 2dc56f0

Browse files
Mark query_captures function as unsafe
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 <[email protected]>
1 parent 3623bf1 commit 2dc56f0

File tree

1 file changed

+20
-14
lines changed

1 file changed

+20
-14
lines changed

helix-core/src/syntax.rs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,8 @@ thread_local! {
10791079

10801080
/// Creates an iterator over the captures in a query within the given range,
10811081
/// re-using a cursor from the pool if available.
1082-
fn query_captures<'a, 'tree>(
1082+
/// SAFETY: The `QueryCaptures` must be droped before the `QueryCursor` is dropped.
1083+
unsafe fn query_captures<'a, 'tree>(
10831084
query: &'a Query,
10841085
root: Node<'tree>,
10851086
source: RopeSlice<'a>,
@@ -1094,10 +1095,11 @@ fn query_captures<'a, 'tree>(
10941095
highlighter.cursors.pop().unwrap_or_default()
10951096
});
10961097

1098+
// This is the unsafe line:
10971099
// The `captures` iterator borrows the `Tree` and the `QueryCursor`, which
10981100
// prevents them from being moved. But both of these values are really just
10991101
// pointers, so it's actually ok to move them.
1100-
let cursor_ref = unsafe { mem::transmute::<_, &'static mut QueryCursor>(&mut cursor) };
1102+
let cursor_ref = mem::transmute::<_, &'static mut QueryCursor>(&mut cursor);
11011103

11021104
// if reusing cursors & no range this resets to whole range
11031105
cursor_ref.set_byte_range(range.unwrap_or(0..usize::MAX));
@@ -1460,12 +1462,14 @@ impl Syntax {
14601462
.layers
14611463
.iter()
14621464
.filter_map(|(_, layer)| {
1463-
let (cursor, captures) = query_captures(
1464-
query_fn(&layer.config),
1465-
layer.tree().root_node(),
1466-
source,
1467-
range.clone(),
1468-
);
1465+
let (cursor, captures) = unsafe {
1466+
query_captures(
1467+
query_fn(&layer.config),
1468+
layer.tree().root_node(),
1469+
source,
1470+
range.clone(),
1471+
)
1472+
};
14691473
let mut captures = captures.peekable();
14701474

14711475
// If there aren't any captures for this layer, skip the layer.
@@ -1497,12 +1501,14 @@ impl Syntax {
14971501
.filter_map(|(_, layer)| {
14981502
// TODO: if range doesn't overlap layer range, skip it
14991503

1500-
let (cursor, captures) = query_captures(
1501-
&layer.config.query,
1502-
layer.tree().root_node(),
1503-
source,
1504-
range.clone(),
1505-
);
1504+
let (cursor, captures) = unsafe {
1505+
query_captures(
1506+
&layer.config.query,
1507+
layer.tree().root_node(),
1508+
source,
1509+
range.clone(),
1510+
)
1511+
};
15061512
let mut captures = captures.peekable();
15071513

15081514
// If there are no captures, skip the layer

0 commit comments

Comments
 (0)