emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Some issues with the tree-sitter branch


From: Eli Zaretskii
Subject: Re: Some issues with the tree-sitter branch
Date: Mon, 17 Oct 2022 09:48:49 +0300

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 16 Oct 2022 21:53:06 -0700
> Cc: emacs-devel@gnu.org
> 
> > . Several places assign EMACS_INT values to uint32_t variables with
> >   an explicit range check (and error signal in case of overflow).
> 
> I think you meant the lack of a range check? Like mentioned here:
> 
> +  /* FIXME: We should signal an error below if START_BYTE
> +     etc. overflow the 32-bit unsigned data type.  */

Yes, I added FIXME comments to make it easier to find those places.

> I added buffer size check at parser creation time, and used casts to uint32_t 
> liberally, assuming the values never overflows and, so we don’t need to 
> handle the error at a million places. But I should have added checks in 
> ts_after_chang and other places where buffer size could change. I’ll add 
> checks in ts_after_change and other places, and if the argumetns overflows 
> uint32, it will set a flag (say, buffer_too_large) in the parser object, and 
> next time any lisp function tries to use that parser, an buffer-too-large 
> error will be signaled. WDYT?

That sounds fine to me, but I think we also should do something when
some value that can be larger than UINT_MAX is passed to tree-sitter
functions, because doing so might cause tree-sitter do something for a
completely unrelated portion of the buffer.  At the very least add
eassert there, so that at least a build with --enable-checking will
detect such cases.

> I see that you fixed them, I’ll keep those in mind in the future. That’s a 
> lot of lines you need to change, sorry about that :-(

That was a low-hanging fruit that was easy to pluck ;-)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]