bug#67977: 30.0.50; tree-sitter: Emacs crashes when accessing treesit-nodes in a narrowed buffer

[Dmitry Gutov <dmitry@gutov.dev>]

On 02/01/2024 06:46, Stefan Monnier wrote:
>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>> index 264b95dc3a3..46ebadcf057 100644
>> --- a/lisp/treesit.el
>> +++ b/lisp/treesit.el
>> @@ -1150,7 +1150,7 @@ treesit--pre-syntax-ppss
>>       (if (and new-start (< new-start start))
>>           (progn
>>             (setq treesit--syntax-propertize-start nil)
>> -          (cons new-start end))
>> +          (cons (max new-start (point-min)) end))
>>         nil)))
>>
>>   ;;; Indent
> Sounds fine to me.  Often the caller of `syntax-propertize-function`
> should widen beforehand, but in cases like `mmm-mode` indeed that's not
> always desired.

Thank you, I've pushed that fix.

>> Or maybe syntax-propertize itself should have a protection against going
>> outside of bounds:
>>
>> diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el
>> index e35992298a6..61a9e79b59c 100644
>> --- a/lisp/emacs-lisp/syntax.el
>> +++ b/lisp/emacs-lisp/syntax.el
>> @@ -431,7 +431,7 @@ syntax-propertize
>>                        (if (or (null new)
>>                                (and (>= (car new) start) (<= (cdr new) end)))
>>                            nil
>> -                       (setq start (car new))
>> +                       (setq start (max (car new) (point-min)))
>>                          (setq end (cdr new))
>>                          ;; If there's been a change, we should go through
>>                            the
>>                          ;; list again since this new position may
> I think it's preferable for the expand-region function to perform this
> test.  We could `cl-assert` that it's within BOB...EOB to help catch
> such bugs (and clarify who's in charge of avoiding the problem), but
> maybe we can mention it in the docstring.  But I personally consider
> that anything that sends buffer positions is in charge to make sure that
> they're within bounds, unless specifically documented otherwise, so
> documenting it seems redundant.

Fair enough.

And I'll leave the question of adding cl-assert (or not) to you or 
others. Though might be that the signaled args-out-of-range will be a 
reliable indicator of this situation anyway.