Subject: bug#75784: Typo "unreadeable" in print.c; was exposed to Lisp

From: Pip Cet <[email-masked]>
Date: Fri, 24 Jan 2025 23:38:55 +0000

"Stefan Kangas" <[email-masked]> writes:

> Pip Cet <[email-masked]> writes:
>
>> Sorry. I meant to mention that I ran out of time for adding comments,
>> then I ran out of time for mentioning I'd run out of time.
>
> :-)
>
>>>> +		  add_to_log ("Obsolete symbol `unreadeable-function' used.  Use `unreadable-function' instead.");
>>
>> BTW: add-to-log here, or message?  While my reasons for using the former
>> were that I was confused, I think this is most likely going to happen in
>> a noninteractive program, where messages might be used to generate its
>> output.
>
> While add_to_log is probably enough to notify users, maybe message is
> indeed better because it's harder to miss.  So I'd probably lean towards
> the latter, but I have no strong opinion.
>
>>>> +      CHECK_LIST_END (overrides, original_overrides);
>>
>> This should signal if 'overrides' isn't a proper, finite, non-dotted list.
>>
>> (Lisp doesn't have "infinite" (non-circular) lists, but I tend to use
>> that term for a list that is modified from a Lisp callback while we walk
>> it, and keeps adding new cons cells so we never see the end of it.
>>
>> I've yet to find a way this causes a bug, but it's a scenario that would
>> give me great personal satisfaction to fix it.  However, it'd be a
>> strange bug, because calling Lisp means we should have a way to quit.)
>
> No objection to fixing this, assuming that we can do it cleanly.

Oh, sorry, I wasn't clear: usually, growing a list from Lisp while it's
being scanned doesn't cause bugs: an infloop, maybe, but a quittable one
because it's Lisp.  A bug would be something like C code checking a
list's length, then calling a predicate (such as a sort predicate) which
then extends the list, and assuming that the list length was limited so
overrunning its buffer.

Found one!  Well, two:

(let ((l (make-list 1000 nil))) (mapconcat (lambda (x) (ntake 1 l)) l))

(let ((s (string ?a 1000))) (mapconcat (lambda (x) (aset s 0 1000) (string x)) s))

My first impulse is to change as little as possible to prevent crashes.
A trivial mapconcat can handle many cases in which our mapconcat fails
or produces unexpected results.  That's okay.  Don't modify the sequence
and you'll be fine.

I still have to check that the old, more forgiving behavior for lists
isn't relied upon anywhere.

>From 1eb35efa41da5dd00f1c824c6b396c89bf4f0ca8 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH] Fix crashes in mapconcat etc. (bug#TBD)

This doesn't attempt to catch all cases in which the mapping function
modifies the sequence, only those which would cause crashes.

* src/fns.c (mapcar1): Error if we detect seq to have been modified by
fn.
---
 src/fns.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/fns.c b/src/fns.c
index 081ed2b9f51..ce09e8c086c 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -3385,7 +3385,7 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Object fn, Lisp_Object seq)
       for (ptrdiff_t i = 0; i < leni; i++)
 	{
 	  if (! CONSP (tail))
-	    return i;
+	    error ("list modified by mapping function");
 	  Lisp_Object dummy = calln (fn, XCAR (tail));
 	  if (vals)
 	    vals[i] = dummy;
@@ -3403,11 +3403,14 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Object fn, Lisp_Object seq)
     }
   else if (STRINGP (seq))
     {
+      ptrdiff_t i = 0;
       ptrdiff_t i_byte = 0;
 
-      for (ptrdiff_t i = 0; i < leni;)
+      while (i_byte < SBYTES (seq))
 	{
 	  ptrdiff_t i_before = i;
+	  if (! CHAR_HEAD_P (SREF (seq, i_byte)))
+	    error ("string modified by mapping function");
 	  int c = fetch_string_char_advance (seq, &i, &i_byte);
 	  Lisp_Object dummy = calln (fn, make_fixnum (c));
 	  if (vals)
-- 
2.47.1