From: Yuan Fu <casouri@gmail.com>
Date: Sat, 29 Jun 2024 07:16:36 +0000 (-0700)
Subject: Fix treesit crash (bug#71681)
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=54ad478bad32ec34c5eea9a912195f446f72f571;p=emacs.git

Fix treesit crash (bug#71681)

To reproduce the problem:

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

Emacs: 6f2036243f2 (2024-06-23, latest master)
Tree-sitter: 3da7deed (2024-06-08, version 0.22.6)

The immediate cause of the crash is that tree-sitter accessed a node's
tree, but the tree is already deleted.  Commenting out the
ts_tree_delete line in treesit_ensure_parsed can "fix" the crash.

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.

* src/treesit.c (treesit_record_change): Don't bump parser timestamp.
(treesit_sync_visible_region): Don't bump parser timestamp.
(Ftreesit_parser_set_included_ranges): Don't bump parser timestamp.
(treesit_ensure_parsed): Bump parser timestamp.
(Ftreesit_query_capture): Add node check.

(cherry picked from commit 8819e5a45d503b62c38e6310ceda9266606cd6d1)
---

diff --git a/src/treesit.c b/src/treesit.c
index 54b16eb1bb3..f0c786e921d 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -846,7 +846,6 @@ treesit_record_change (ptrdiff_t start_byte, ptrdiff_t old_end_byte,
 	  treesit_tree_edit_1 (tree, start_offset, old_end_offset,
 			       new_end_offset);
 	  XTS_PARSER (lisp_parser)->need_reparse = true;
-	  XTS_PARSER (lisp_parser)->timestamp++;
 
 	  /* VISIBLE_BEG/END records tree-sitter's range of view in
 	     the buffer.  We need to adjust them when tree-sitter's
@@ -948,10 +947,7 @@ treesit_sync_visible_region (Lisp_Object parser)
      this function is called), we need to reparse.  */
   if (visible_beg != BUF_BEGV_BYTE (buffer)
       || visible_end != BUF_ZV_BYTE (buffer))
-    {
-      XTS_PARSER (parser)->need_reparse = true;
-      XTS_PARSER (parser)->timestamp++;
-    }
+    XTS_PARSER (parser)->need_reparse = true;
 
   /* Before we parse or set ranges, catch up with the narrowing
      situation.  We change visible_beg and visible_end to match
@@ -1090,6 +1086,7 @@ treesit_ensure_parsed (Lisp_Object parser)
 
   XTS_PARSER (parser)->tree = new_tree;
   XTS_PARSER (parser)->need_reparse = false;
+  XTS_PARSER (parser)->timestamp++;
 
   /* After-change functions should run at the very end, most crucially
      after need_reparse is set to false, this way if the function
@@ -1725,7 +1722,6 @@ buffer.  */)
 	      ranges);
 
   XTS_PARSER (parser)->need_reparse = true;
-  XTS_PARSER (parser)->timestamp++;
   return Qnil;
 }
 
@@ -2923,11 +2919,10 @@ be completely in the region.
 
 If NODE-ONLY is non-nil, return a list of nodes.
 
-Besides a node, NODE can also be a parser, in which case the root node
-of that parser is used.
-NODE can also be a language symbol, in which case the root node of a
-parser for that language is used.  If such a parser doesn't exist, it
-is created.
+Besides a node, NODE can be a parser, in which case the root node of
+that parser is used.  NODE can also be a language symbol, in which case
+the root node of a parser for that language is used.  If such a parser
+doesn't exist, it is created.
 
 Signal `treesit-query-error' if QUERY is malformed or something else
 goes wrong.  You can use `treesit-query-validate' to validate and debug
@@ -2941,8 +2936,13 @@ the query.  */)
 
   treesit_initialize ();
 
-  /* Resolve NODE into an actual node.  */
+  /* Resolve NODE into an actual node, signals if node not
+     up-to-date.  */
   Lisp_Object lisp_node = treesit_resolve_node (node);
+  /* As of right now, the node returned by treesit_resolve_node always
+     passes treesit_check_node; but it might not be true in the future,
+     so adding the line below just to be safe.  */
+  treesit_check_node (node);
 
   /* Extract C values from Lisp objects.  */
   TSNode treesit_node = XTS_NODE (lisp_node)->node;
@@ -2970,8 +2970,8 @@ the query.  */)
 				 &signal_symbol, &signal_data))
     xsignal (signal_symbol, signal_data);
 
-  /* WARN: After this point, free TREESIT_QUERY and CURSOR before every
-     signal and return if NEEDS_TO_FREE_QUERY_AND_CURSOR is true.  */
+  /* WARN: After this point, if NEEDS_TO_FREE_QUERY_AND_CURSOR is true,
+     free TREESIT_QUERY and CURSOR before every signal and return.  */
 
   /* Set query range.  */
   if (!NILP (beg) && !NILP (end))