aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbfredl <bjorn.linse@gmail.com>2023-08-29 11:21:57 +0200
committerbfredl <bjorn.linse@gmail.com>2023-08-29 11:35:46 +0200
commit50a03c0e9975925e3198a2741c5b9fc0ad727e84 (patch)
tree60af1e770fe2ada5a4089318377264c80886dedd
parent6e45567b498ca8455aaf3628c10de997ac070ee1 (diff)
downloadrneovim-50a03c0e9975925e3198a2741c5b9fc0ad727e84.tar.gz
rneovim-50a03c0e9975925e3198a2741c5b9fc0ad727e84.tar.bz2
rneovim-50a03c0e9975925e3198a2741c5b9fc0ad727e84.zip
fix(treesitter): fix another TSNode:tree() double free
Unfortunately the gc=false objects can refer to a dangling tree if the gc=true tree was freed first. This reuses the same tree object as the node itself is keeping alive via the uservalue of the node userdata. (wrapped in a table due to lua 5.1 restrictions)
-rw-r--r--src/nvim/lua/treesitter.c22
-rw-r--r--test/functional/treesitter/node_spec.lua14
2 files changed, 25 insertions, 11 deletions
diff --git a/src/nvim/lua/treesitter.c b/src/nvim/lua/treesitter.c
index 39935f656f..7b308ef7b4 100644
--- a/src/nvim/lua/treesitter.c
+++ b/src/nvim/lua/treesitter.c
@@ -53,7 +53,6 @@ typedef struct {
typedef struct {
TSTree *tree;
- bool gc;
} TSLuaTree;
#ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -477,13 +476,12 @@ static int parser_parse(lua_State *L)
return luaL_error(L, "An error occurred when parsing.");
}
- // The new tree will be pushed to the stack, without copy, ownership is now to
- // the lua GC.
- // Old tree is still owned by the lua GC.
+ // The new tree will be pushed to the stack, without copy, ownership is now to the lua GC.
+ // Old tree is owned by lua GC since before
uint32_t n_ranges = 0;
TSRange *changed = old_tree ? ts_tree_get_changed_ranges(old_tree, new_tree, &n_ranges) : NULL;
- push_tree(L, new_tree, true); // [tree]
+ push_tree(L, new_tree); // [tree]
push_ranges(L, changed, n_ranges, include_bytes); // [tree, ranges]
@@ -509,7 +507,7 @@ static int tree_copy(lua_State *L)
}
TSTree *copy = ts_tree_copy(ud->tree);
- push_tree(L, copy, true); // [tree]
+ push_tree(L, copy); // [tree]
return 1;
}
@@ -779,8 +777,9 @@ static int parser_get_logger(lua_State *L)
/// push tree interface on lua stack.
///
-/// The tree is garbage collected if gc is true
-void push_tree(lua_State *L, TSTree *tree, bool gc)
+/// The tree is not copied. Ownership of the tree is transfered from c code to
+/// lua. if needed use ts_tree_copy() in the caller
+void push_tree(lua_State *L, TSTree *tree)
{
if (tree == NULL) {
lua_pushnil(L);
@@ -789,7 +788,6 @@ void push_tree(lua_State *L, TSTree *tree, bool gc)
TSLuaTree *ud = lua_newuserdata(L, sizeof(TSLuaTree)); // [udata]
ud->tree = tree;
- ud->gc = gc;
lua_getfield(L, LUA_REGISTRYINDEX, TS_META_TREE); // [udata, meta]
lua_setmetatable(L, -2); // [udata]
@@ -812,7 +810,7 @@ static TSLuaTree *tree_check(lua_State *L, int index)
static int tree_gc(lua_State *L)
{
TSLuaTree *ud = tree_check(L, 1);
- if (ud && ud->gc) {
+ if (ud) {
ts_tree_delete(ud->tree);
}
return 0;
@@ -1322,7 +1320,9 @@ static int node_tree(lua_State *L)
return 0;
}
- push_tree(L, (TSTree *)node.tree, false);
+ lua_getfenv(L, 1); // [udata, reftable]
+ lua_rawgeti(L, -1, 1); // [udata, reftable, tree_udata]
+
return 1;
}
diff --git a/test/functional/treesitter/node_spec.lua b/test/functional/treesitter/node_spec.lua
index 72508ba958..eef75d0e91 100644
--- a/test/functional/treesitter/node_spec.lua
+++ b/test/functional/treesitter/node_spec.lua
@@ -26,6 +26,20 @@ describe('treesitter node API', function()
assert_alive()
end)
+ it('double free tree 2', function()
+ exec_lua([[
+ parser = vim.treesitter.get_parser(0, "c")
+ local x = parser:parse()[1]:root():tree()
+ vim.api.nvim_buf_set_text(0, 0,0, 0,0, {'y'})
+ parser:parse()
+ vim.api.nvim_buf_set_text(0, 0,0, 0,1, {'z'})
+ parser:parse()
+ collectgarbage()
+ x:root()
+ ]])
+ assert_alive()
+ end)
+
it('can move between siblings', function()
insert([[
int main(int x, int y, int z) {