bug#68954: [PATCH] Crasher in treesit_traverse_get_predicate

bug#68954: [PATCH] Crasher in treesit_traverse_get_predicate

[Dominique Quatravaux]

[-- Attachment #1.1: Type: text/plain, Size: 370 bytes --]

Because nothing guarantees or enforces that either Vtreesit_thing_settings,
or the cdrs of its elements, are cons cells, it is easy to construct values
for `treesit-thing-settings' that make Emacs crash (e.g., when calling
`prog-fill-reindent-defun' from any tree-sitter enabled mode).

The attached patch fixes that.

--
 Dominique Quatravaux
 dominique@quatravaux.org

[-- Attachment #1.2: Type: text/html, Size: 596 bytes --]

[-- Attachment #2: treesit.diff --]
[-- Type: application/octet-stream, Size: 618 bytes --]

--- treesit.c.orig	2024-02-06 18:01:23
+++ treesit.c	2024-02-06 18:01:41
@@ -3268,11 +3268,11 @@
 static Lisp_Object
 treesit_traverse_get_predicate (Lisp_Object thing, Lisp_Object language)
 {
-  Lisp_Object cons = assq_no_quit (language, Vtreesit_thing_settings);
+  Lisp_Object cons = assq_no_signal (language, Vtreesit_thing_settings);
   if (NILP (cons))
     return Qnil;
   Lisp_Object definitions = XCDR (cons);
-  Lisp_Object entry = assq_no_quit (thing, definitions);
+  Lisp_Object entry = assq_no_signal (thing, definitions);
   if (NILP (entry))
     return Qnil;
   /* ENTRY looks like (THING PRED).  */

bug#68954: Offending commit

[Dominique Quatravaux]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

https://github.com/emacs-mirror/emacs/commit/d005e685e1df7692085378633348db39a5190374
doesn't do what it says it does in the commit header, and is very likely
the one that introduced the crash.

--
 Dominique Quatravaux
 dominique@quatravaux.org

[-- Attachment #2: Type: text/html, Size: 544 bytes --]

bug#68954: Offending commit

[Yuan Fu]

> On Feb 6, 2024, at 9:15 AM, Dominique Quatravaux <dominique@quatravaux.org> wrote:
> 
> https://github.com/emacs-mirror/emacs/commit/d005e685e1df7692085378633348db39a5190374 doesn't do what it says it does in the commit header, and is very likely the one that introduced the crash.

Thanks! If you want to submit a patch I’m more than happy to apply it for you (you don’t need to sign the copyright assignment since it’s well under 15 LOC). Otherwise I can apply the change myself.

Also it’s a good idea to not change the email title, sometime that might break threading in email clients.

Yuan

bug#68954: Offending commit

[Dominique Quatravaux]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

Le mer. 7 févr. 2024 à 02:58, Yuan Fu <casouri@gmail.com> a écrit :



Thanks! If you want to submit a patch I’m more than happy to apply it for
you


I attached a patch to my first email already.

Also it’s a good idea to not change the email title, sometime that might
break threading in email clients.


Indeed, as can be seen right here with the patch! I'll be more careful next
time.

-- 
Dominique Quatravaux
dominique@quatravaux.org

[-- Attachment #2: Type: text/html, Size: 1192 bytes --]

bug#68954: Offending commit

[Yuan Fu]

> On Feb 6, 2024, at 11:24 PM, Dominique Quatravaux <dominique@quatravaux.org> wrote:
> 
> Le mer. 7 févr. 2024 à 02:58, Yuan Fu <casouri@gmail.com> a écrit :
> 
> 
> Thanks! If you want to submit a patch I’m more than happy to apply it for you
> 
> I attached a patch to my first email already.

My bad, I wasn’t clear enough. I meant a patch generated by git format-patch, which includes a commit message, etc. This way when I apply it it’ll show your as the author. If you don’t care about it, I can apply the diff myself and push it.

Btw, there’s a specific format for commit messages, you can check out the CONTRIBUTE file under repo root, in the “Commit messages” section.

Actually, here’s what I would write for your change, feel free to modify it as you like.

Fix treesit_traverse_get_predicate (bug#68954)

Commit d005e685e1df7692085378633348db39a5190374 should have replaced
assq_no_quit with assq_no_signal, but didn't, this commit fixes that.

* src/treesit.c (treesit_traverse_get_predicate): Replace assq_no_quit
with assq_no_signal.


Yuan

bug#68954: Offending commit

[Dominique Quatravaux]

[-- Attachment #1.1: Type: text/plain, Size: 385 bytes --]

Le jeu. 8 févr. 2024 à 02:35, Yuan Fu <casouri@gmail.com> a écrit :

>
>
> Actually, here’s what I would write for your change, feel free to modify
> it as you like.
>
>
Many thanks for the step-by-step instructions! Attached is what `git
format-patch` gave me, I hope that I got everything right.

Yours truly,

--
 Dominique Quatravaux
 dominique@quatravaux.org

[-- Attachment #1.2: Type: text/html, Size: 868 bytes --]

[-- Attachment #2: 0001-Fix-treesit_traverse_get_predicate-bug-68954.patch --]
[-- Type: application/octet-stream, Size: 1305 bytes --]

From 0eb6c02b3f16b4567053b7476039b0fbf6a0f55a Mon Sep 17 00:00:00 2001
From: Dominique Quatravaux <dominique.quatravaux@epfl.ch>
Date: Thu, 8 Feb 2024 10:19:10 +0100
Subject: [PATCH] Fix treesit_traverse_get_predicate (bug#68954)

Commit d005e685e1df7692085378633348db39a5190374 should have replaced
assq_no_quit with assq_no_signal, but didn't, this commit fixes that.

* src/treesit.c (treesit_traverse_get_predicate): Replace assq_no_quit
with assq_no_signal.

Copyright-paperwork-exempt: yes
---
 src/treesit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/treesit.c b/src/treesit.c
index 69b59fca111..aece3a7728e 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -3268,11 +3268,11 @@ treesit_traverse_child_helper (TSTreeCursor *cursor,
 static Lisp_Object
 treesit_traverse_get_predicate (Lisp_Object thing, Lisp_Object language)
 {
-  Lisp_Object cons = assq_no_quit (language, Vtreesit_thing_settings);
+  Lisp_Object cons = assq_no_signal (language, Vtreesit_thing_settings);
   if (NILP (cons))
     return Qnil;
   Lisp_Object definitions = XCDR (cons);
-  Lisp_Object entry = assq_no_quit (thing, definitions);
+  Lisp_Object entry = assq_no_signal (thing, definitions);
   if (NILP (entry))
     return Qnil;
   /* ENTRY looks like (THING PRED).  */
-- 
2.43.0

bug#68954: Offending commit

[Yuan Fu]

> On Feb 8, 2024, at 1:36 AM, Dominique Quatravaux <dominique@quatravaux.org> wrote:
> 
> 
> 
> Le jeu. 8 févr. 2024 à 02:35, Yuan Fu <casouri@gmail.com> a écrit :
> 
> 
> Actually, here’s what I would write for your change, feel free to modify it as you like.
> 
> 
> Many thanks for the step-by-step instructions! Attached is what `git format-patch` gave me, I hope that I got everything right.
> 
> Yours truly,
>  

Great! Merged to master. Closing this. I also recommend you start your copyright assignment process, in case you contribute more in the future. You can ask for the from and instructions on emacs-devel, or just CC Eli (eliz@gnu.org) in this thread and ask him to send you one.

Yuan