bug#71681: 29.3.50; tree-sitter crash

[Yuan Fu]

> On Jun 25, 2024, at 11:04 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Jun 24, 2024, at 12:46 AM, Yuan Fu <casouri@gmail.com> wrote:
>> 
>> 
>> 
>>> On Jun 23, 2024, at 10:38 AM, Juri Linkov <juri@linkov.net> wrote:
>>> 
>>>>> Evaluating this expression causes a crash:
>>>>> 
>>>>> (progn
>>>>> (find-file (expand-file-name "src/treesit.c" installation-directory))
>>>>> (c-ts-mode)
>>>>> (font-lock-ensure 63209 63387))
>>>>> 
>>>>> in latest master, but not in latest emacs-29 (only in 5-months old 
>>>>> emacs-29).
>>>>> 
>>>>> If this is not reproducible, I could provide more details.
>>>>> 
>>>>> libtree-sitter is at the latest version.
>>>> 
>>>> Hmm, I can’t reproduce with latest master and libtree-sitter. Maybe you 
>>>> can send me the exact commits that you used?
>>>> 
>>>> Here’s mine:
>>>> 
>>>> Emacs: 72f2b01e318
>>>> Tree-sitter: 6ec478c1
>>> 
>>> Probably reproducibility depends on the content of the src/treesit.c file.
>>> Then the most reliable way to reproduce it is this:
>>> 
>>> 0. emacs -Q
>>> 1. eval: (add-to-list 'major-mode-remap-alist '(c-mode . c-ts-mode))
>>> 2. C-x v L
>>> 3. in the *vc-change-log* buffer move point to the commit 20af58d3a13
>>> 4. type D
>>> 5. crash caused by diff-font-lock-syntax fontification that uses treesit
>>> 
>>> The numbers in (font-lock-ensure 63209 63387) above were extracted
>>> from diff hunk boundaries that might be different when the file was edited.
>> 
>> I reproduce it once with the first set of commits you provided, but for some 
>> reason couldn’t reproduce it again. I’m sure it’s something wrong that I 
>> did. I’ll report back when I make progress. TBH it seems like something 
>> wrong with tree-sitter itself, but I’ll make sure to figure out what’s the 
>> problem exactly.
>> 
>> Yuan
> 
> Ok, I can reproduce it now. Looking into it…

Finally figured out why. It’s not tree-sitter’s problem, but ours. I reduced 
the crash to a signal and pushed the fix to emacs-30. Next I’ll make sure the 
signal is properly handled. Below quoting the commit message:

The immediate cause of the crash is that tree-sitter accessed a node's
tree, but the tree is already deleted.

What happended, I think, is this:

1. Buffer modified, parser->need_reparse set to true,
parser->timestamp incremented.
2. A node is created from the parser, this node has the old tree but
the _new_ timestamp (bad!).
3. Parser re-parses (treesit_ensure_parsed), new tree created, old
tree deleted.
4. Ftreesit_query_capture accessed the old node, and the old tree,
crash.

We shouldn't bump the parser timestamp when we set
parser->need_reparse to true; instead, we should bump the timestamp
when we actually reparsed and created a new tree.

Yuan