Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 12:16:20 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 07:16:19 2025 Received: from localhost ([127.0.0.1]:55020 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tc1Yp-0004Na-AW for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 07:16:19 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:58250) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tc1Ym-0004NG-8P for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 07:16:17 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <eliz@HIDDEN>) id 1tc1Yc-00041z-N3; Sun, 26 Jan 2025 07:16:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=RA3bmHMJCqBwoJT1KGNWkD9xVrNmlr/L9hmj0vNKdwc=; b=JEMQcqoPpf4chdiqMq3R P7ssmzNDuLs+Exe71npAZni+7JZ2GUZz3vr3uXgXtDNByeeEtYQz9bzagize1eXceenMQ2OQO4sde Rpkrs++cAfMTl4l3pcK7o2HP1SKwHEaBLIi1oNMOMUriWeIvXQXEuSz4eIOygcq8pJEYPxC1wk0Am QaX9yXCUAHae2vZPX1yuvua9+yzhFmhS8POyO3uJzIc8SJ4eNtIZSD/T3EWh0LswJPdArFN0+C7a5 nb0ZE6wiFdtq8ISY4wmBd1elJBIk6onCxGNiRb7ytBgZpUhSzIa7OqYZCoheCqNdxytR67q6cDk3B GUPZdwMIda3Glg==; Date: Sun, 26 Jan 2025 14:16:03 +0200 Message-Id: <86bjvtsrcs.fsf@HIDDEN> From: Eli Zaretskii <eliz@HIDDEN> To: Pip Cet <pipcet@HIDDEN> In-Reply-To: <87y0yxn5h5.fsf@HIDDEN> (message from Pip Cet on Sun, 26 Jan 2025 12:07:50 +0000) Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN> <86jzait664.fsf@HIDDEN> <jwvmsfd6e3y.fsf-monnier+emacs@HIDDEN> <87y0yxn5h5.fsf@HIDDEN> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 75845 Cc: mattiase@HIDDEN, monnier@HIDDEN, 75845 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -3.3 (---) > Date: Sun, 26 Jan 2025 12:07:50 +0000 > From: Pip Cet <pipcet@HIDDEN> > Cc: Eli Zaretskii <eliz@HIDDEN>, Mattias Engdegård <mattiase@HIDDEN>, 75845 <at> debbugs.gnu.org > > >> string arg of mapcar or mapconcat clobbered by its mapping function arg > > TBH, "string arg of ... mapconcat" sounds like it's referring to the > separator string, to me. If that is a concern, we could say First arg of mapcar etc. clobbered ...
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 12:08:04 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 07:08:03 2025
Received: from localhost ([127.0.0.1]:54999 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tc1Qp-0003vz-Dj
for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 07:08:03 -0500
Received: from mail-10628.protonmail.ch ([79.135.106.28]:47273)
by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.84_2) (envelope-from <pipcet@HIDDEN>)
id 1tc1Qm-0003vI-Un
for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 07:08:01 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;
s=protonmail3; t=1737893274; x=1738152474;
bh=fhTeDPiiNTQLUbGi6bFCNdUodbwdgCobzZhZ8lK1k84=;
h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:
Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:
Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;
b=IrQs4xjXg1XYk9qebt3IDAdpoiMl9idoxG9dM9Chq3eaF6qaoJ1I9MnlM2FrtsB0d
hAuQj3MQ07bfy0Rkm7i8HJyeF/0GMizbhHRFOcimWT8cWhU76iw2dPSMPmJH1TcSa5
BKQBikdSEYbcDpTUyGHRu/U6JCFEg7H0ujTn6N+xutAo6N8eZuLTIqMZunBLCcTm8h
KAe0giuCOpV4RBR5JkDRk9KEjPBJCxuirkOAAZi3e4FfFW2/lh9b1Yz2kt/GHs2Q7g
QxRJF+NB4IV+RhHz6Cd9oph56EKk9pUPw1DukjoWvZXj0wB2ZhKo70RBHzyOJueCTH
B3Iy/q91UFC2w==
Date: Sun, 26 Jan 2025 12:07:50 +0000
To: Stefan Monnier <monnier@HIDDEN>
From: Pip Cet <pipcet@HIDDEN>
Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications
Message-ID: <87y0yxn5h5.fsf@HIDDEN>
In-Reply-To: <jwvmsfd6e3y.fsf-monnier+emacs@HIDDEN>
References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN>
<86jzait664.fsf@HIDDEN> <jwvmsfd6e3y.fsf-monnier+emacs@HIDDEN>
Feedback-ID: 112775352:user:proton
X-Pm-Message-ID: 1f0e4dd4556a887ae97821683db68e6b9603b1b6
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: -0.0 (/)
X-Debbugs-Envelope-To: 75845
Cc: =?utf-8?Q?Mattias_Engdeg=C3=A5rd?= <mattiase@HIDDEN>,
Eli Zaretskii <eliz@HIDDEN>, 75845 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)
"Stefan Monnier" <monnier@HIDDEN> writes:
>> Thanks, adding Stefan and Mattias to the discussion, in the hope that
>> they could review the patch and comment.
>
> The patch LGTM.
Thanks. I'll wait for a decision to be made on which precise error to
throw in this extremely rare situation.
>
>>> =09 if (! CONSP (tail))
>>> -=09 return i;
>>> +=09 error ("list modified by mapping function");
>>
>> I think this error message is not explicit enough: it doesn't tell
FWIW, I started out by copying
signal_error ("hash table test modifies table", obj);
since that's the only similar case I could think of right away, and I
thought this might not require additional thought. Upon reflection, my
current suggestion is to define an error symbol vaguely equivalent to
Java's ConcurrentModificationException, but with a less puzzling name
(also, hopefully, this would happen much less often than that common
exception).
Having a single error symbol and documenting it once, for all cases,
with general advice for what might have gone wrong and how to fix it
(rewrite callbacks to be as pure as possible) doesn't seem harder than
arguing about this specific corner case, but does provide more benefits.
>> string arg of mapcar or mapconcat clobbered by its mapping function ar=
g
TBH, "string arg of ... mapconcat" sounds like it's referring to the
separator string, to me.
> +1 for "clobbered"
+1 from me, too. "Concurrent modification exceptions" are puzzling to
programmers partly because the name doesn't make it clear who was at
fault. "Clobbered" clearly indicates that the modifying code was
careless and is the part that needs to be fixed, and that we were lucky
to detect this was attempted or happened.
PUT_ERROR (Qdata_clobbered_error, "Lisp callback unexpectedly clobbered dat=
a");
on the assumption that xsignal2 is the right thing to use here, and that
the reference to the clobbering function would be the first and the data
clobbered would be the second argument.
No objections to reversing the string and arguments if that's clearer,
or any rewording that makes sense to you.
(There's a tiny white lie here: I don't think it's worth it to
distinguish cases in which the clobberer was successful from those in
which an illicit modification was prevented. Even if it's a single
error for all cases, the combined probability isn't great enough to
warrant two error symbols).
As a tangent, I'd prefer providing format arguments in PUT_ERROR, on the
understanding that expanding them is optional: omitting the format
arguments would yield the current error message, and including them
would provide additional output in situations where the user desires
this.
Pip
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 10:55:59 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 05:55:59 2025
Received: from localhost ([127.0.0.1]:54867 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tc0J4-0000Jz-OL
for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 05:55:59 -0500
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:21954)
by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.84_2) (envelope-from <monnier@HIDDEN>)
id 1tc0J2-0000Jf-5t
for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 05:55:57 -0500
Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1])
by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 909624405F2;
Sun, 26 Jan 2025 05:55:49 -0500 (EST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
s=mail; t=1737888947;
bh=pvqo61QjedU+V54+EKc5kD+VDRbvuqTkWdDJgu9Tm2E=;
h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
b=MEQoiEtZCPn6g6ddXeQdMnA1TdiLEfHEtXqxBns+hTUQIz6geINWWhYbVsSzs1jT0
pt1q8RqmhrAdqTDRUvsWbIKYpYnMgiJjb8lMzEJAoSBOctEyE8Kg2NUxYCqXvhN1Tt
RsxbY++HQo9fN5gPFXj500fcGfu8toXsD4n17Wp/XkJqPrfzK02Y25bQGpbov/c9e1
LazboV1S58f4OPHuzPRbkITuvdxj5UlFWzWM8f70oLEdctrccV0sQliDbwBPHqogMS
n+H6FWxI4TqTLf991ktocUfrlG+rlFDk4hkTlyp6FkoYWsRw3wQps9fxlsTgnkO7aP
654vE7uSN9IoQ==
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id AB6E64408FA;
Sun, 26 Jan 2025 05:55:47 -0500 (EST)
Received: from asado (dyn.144-85-147-102.dsl.vtx.ch [144.85.147.102])
by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id B013112084B;
Sun, 26 Jan 2025 05:55:46 -0500 (EST)
From: Stefan Monnier <monnier@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications
In-Reply-To: <86jzait664.fsf@HIDDEN> (Eli Zaretskii's message of "Sun, 26 Jan
2025 08:56:03 +0200")
Message-ID: <jwvmsfd6e3y.fsf-monnier+emacs@HIDDEN>
References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN>
<86jzait664.fsf@HIDDEN>
Date: Sun, 26 Jan 2025 05:55:44 -0500
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results: 0
ALL_TRUSTED -1 Passed through trusted hosts only via SMTP
AWL 0.117 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DKIM_SIGNED 0.1 Message has a DKIM or DK signature,
not necessarily valid
DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
domain
DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from
domain T_SCC_BODY_TEXT_LINE -0.01 -
X-SPAM-LEVEL:
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 75845
Cc: Mattias =?windows-1252?Q?Engdeg=E5rd?= <mattiase@HIDDEN>,
Pip Cet <pipcet@HIDDEN>, 75845 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)
> Thanks, adding Stefan and Mattias to the discussion, in the hope that
> they could review the patch and comment.
The patch LGTM.
>> if (! CONSP (tail))
>> - return i;
>> + error ("list modified by mapping function");
>
> I think this error message is not explicit enough: it doesn't tell
> which list was modified by what function. mapcar and mapconcat are
> many times invoked by code that has little to do with the level which
> could make sense to the user, so we should try to provide as much
> contextual information as possible. Perhaps something like
>
> sequence arg to `mapcar' or `mapconcat' modified by its mapping function arg
Since this is in the list-part of the code, I'd prefer "list" rather
than "sequence", since it's both shorter and more precise. But I'd
capitalize that first word.
> string arg of mapcar or mapconcat clobbered by its mapping function arg
+1 for "clobbered"
Stefan
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 10:24:37 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 05:24:37 2025 Received: from localhost ([127.0.0.1]:54782 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tbzoj-00077F-3U for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 05:24:37 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:56636) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tbzof-00076w-D1 for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 05:24:35 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <eliz@HIDDEN>) id 1tbzoZ-0001aZ-1T; Sun, 26 Jan 2025 05:24:27 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=4xWjeXiXtvdiUJmoS+YK2BhJIryJ3wBmWMfQ0cCSHHk=; b=kYTrn4hbs1KfSZ4hyYds 4B0JdJHAHJhhtxJsRmF/dA0dqJml9uWOQgvxgeuz4jZy5xVSGe6dGBmgV5l4nimnVJmNCmSTIEZSH gT0LvBof7RevpjTEHOAvLhXpHW8kMZJ3rDGCfZt+y2egrl9TsF54UvU6HsOZ5/NMz30oIxK28D9TZ N3QZCfvQC2PCTqmN/ci8QBRA06LClrMMtAX/rAKcwrtkhAgKqtHk5HsbnBjc4hNAOmMCorCyCVGux ptr8vpgsg3A4y2Q9NDQ6a8QmtbalWiLF4i5fmz3wvmM/K6Ax1BlgKz+n1sb1p+1q4jEaOfjfp94kB 6ydA5UqvNQR9pg==; Date: Sun, 26 Jan 2025 12:24:23 +0200 Message-Id: <86ikq1swiw.fsf@HIDDEN> From: Eli Zaretskii <eliz@HIDDEN> To: Pip Cet <pipcet@HIDDEN> In-Reply-To: <87r04poqh9.fsf@HIDDEN> (message from Pip Cet on Sun, 26 Jan 2025 09:48:48 +0000) Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN> <86jzait664.fsf@HIDDEN> <87r04poqh9.fsf@HIDDEN> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 75845 Cc: mattiase@HIDDEN, monnier@HIDDEN, 75845 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -3.3 (---) > Date: Sun, 26 Jan 2025 09:48:48 +0000 > From: Pip Cet <pipcet@HIDDEN> > Cc: Stefan Monnier <monnier@HIDDEN>, Mattias Engdegård <mattiase@HIDDEN>, 75845 <at> debbugs.gnu.org > > I don't think this situation is expected to be common enough to warrant > a more specific error message, TBH, particularly not if it then fails to > apply to all possible cases. I didn't want the message to be specific. I wanted it to explain which function signaled the error, so the bug could be easier to look for and spot. > My suggestion is to merge the two error messages to save some space, I'm okay with using a single error message. But let's make its text provide more context than just saying that some list was modified by some function. > > Does it prevent us from crashing in some cases? > > Yes. Silly programmers like myself (see first suggested patch in > another bug) may attempt to use out-of-bounds values in the array, which > are uninitialized and would cause crashes. No, I meant can we crash in the following code. If the caller then uses the value somewhere else, without validating it, it's a separate problem that should be perhaps dealt with at that other place. > > sequence arg to `mapcar' or `mapconcat' modified by its mapping function arg > > I like the idea of using the same message for both, reducing the memory > requirement for the unused error string. Then how about sequence arg to `mapcar' etc. modified by its mapping function arg > (I may be unaware of a good > error function to use in this case; I ended up not providing additional > arguments because it seems unlikely to me that this problem, if really > accidental, is something the programmer can fix without a Lisp > backtrace, which indicates the situation more clearly). The above just provides more context, to make it easier to look for the likely culprits. Arguments are not provided, just mentioned. > >> - ptrdiff_t nmapped = mapcar1 (leni, args, function, sequence); > >> - eassert (nmapped == leni); > >> + leni = mapcar1 (leni, args, function, sequence); > > > > then can we have the test as eassert? > > I see no reason to set this trap for the next person who modifies > mapcar1. If they see the return value is checked by some callers, they > might miss the one caller which crashes. If they remove the return > value, they might miss that the eassert, too, needs to be removed, > making their job harder. > > If we want that eassert (why?), it belongs in mapcar1, not Fmapconcat. That's what I meant: to make the test an assertion in mapcar1.
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 09:49:11 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 04:49:11 2025
Received: from localhost ([127.0.0.1]:54683 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tbzGQ-0005H7-Dr
for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 04:49:11 -0500
Received: from mail-4322.protonmail.ch ([185.70.43.22]:55943)
by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.84_2) (envelope-from <pipcet@HIDDEN>)
id 1tbzGE-0005GC-N8
for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 04:49:00 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;
s=protonmail3; t=1737884932; x=1738144132;
bh=ubWoe4sJX6Bz7gJsQOCLWG2HCQ050iRXh7c/f16DFbg=;
h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:
Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:
Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;
b=knh003FD9OI1pm7LJn9vRn0kqAW262d+Owf5bcpEjNlPsWYq3MDo0/uSic/pSB3Jt
zLCicwDVuoVFcxNY/lUxzOyQeL2T2UNK4NszOnR066moB0Ai2O5qllb3F4yOeZoyY2
+nMD06LD/J7+iNJMocPDHBDoTa6N57vOfmqqbAHO5Oij6E/18930JUCTZJupMaPGUy
Ouw5bWTF6yh7q2nI+BpwoPyNdkSACd9gg6Lb2oBgRiVVPgmyqB8oQM/GXbQVt3zmRD
R/U1wK1Og2FV7kGuxEVZxA1EAwLWdIOnRnC9vvM7jtsdWLQbRQtMcu2qPQTJPOp749
7mG0gEDmYGSNw==
Date: Sun, 26 Jan 2025 09:48:48 +0000
To: Eli Zaretskii <eliz@HIDDEN>
From: Pip Cet <pipcet@HIDDEN>
Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications
Message-ID: <87r04poqh9.fsf@HIDDEN>
In-Reply-To: <86jzait664.fsf@HIDDEN>
References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN>
<86jzait664.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
X-Pm-Message-ID: c21864927890ec4671d3bfdd32e805b84ecd557d
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: -0.1 (/)
X-Debbugs-Envelope-To: 75845
Cc: =?utf-8?Q?Mattias_Engdeg=C3=A5rd?= <mattiase@HIDDEN>,
Stefan Monnier <monnier@HIDDEN>, 75845 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.1 (-)
R"Eli Zaretskii" <eliz@HIDDEN> writes:
>> Date: Sun, 26 Jan 2025 02:37:37 +0000
>> From: Pip Cet via "Bug reports for GNU Emacs,
>> the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
>>
>> Pip Cet <pipcet@HIDDEN> writes:
>>
>> > If the mapping function supplied to mapconcat modifies the sequence
>> > being mapped over, Emacs can crash.
>> >
>> > This seems easily fixable, but I wanted to file a bug to make sure we
>> > get it right: our current highly-optimized implementation of mapconcat
>> > will fail in many cases in which a naive implementation of mapconcat (=
in
>> > Lisp, say) would succeed. Some of those limitations are documented (t=
he
>> > argument must be a sequence when the function is called), some aren't
>> > (the sequence must not become shorter or longer).
>> >
>> > My suggestion is to make the ordinary no-modification case work as fas=
t
>> > as is possible under the constraint that extraordinary
>> > self-modifications should not result in crashes. They can and will
>> > result in unpredictable behavior, though.
>> >
>> > These crash:
>> >
>> > (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) (st=
ring x)) s))
>> >
>> > Patch will follow once this has a bug number.
>>
>> This patch does the minimum work necessary; while the previous code
^^^^^^^
I don't think this situation is expected to be common enough to warrant
a more specific error message, TBH, particularly not if it then fails to
apply to all possible cases.
My suggestion is to merge the two error messages to save some space,
apply the patch, and make a mental note to include both
mapconcat/mapcar1 and the error handling situation for further review
when we have time.
The rest of this email mostly consists of notes for that (future)
discussion, except for this, which I've reordered:
>> + if (i < leni)
>> +=09error ("string modified by mapping function");
>
> Why do we need this test?
mapconcat assumed the wrong thing, I think other future callers might,
as well. It's not obvious that mapcar1's return value can be different
from its first argument, and given the semi-optimized state of this
function I think future work is more likely.
> Does it prevent us from crashing in some cases?
Yes. Silly programmers like myself (see first suggested patch in
another bug) may attempt to use out-of-bounds values in the array, which
are uninitialized and would cause crashes.
Also, a minor point in performance-sensitive code is that conditional
branches are bad, but not if they call error(). Such branches will be
optimized by GCC to be predicted not to call error in a static way,
independently of the CPU's limited branch prediction cache.
Here's the rest:
>> attempted to return the "correct" length of a list which was modified,
>> crashing in an eassert in mapconcat, the new code errors instead. As
>> error is marked with ATTRIBUTE_COLD, this should result in better code.
>>
>> The string case is slightly trickier: we don't keep around an SDATA
>> pointer, which is good, but we do remember byte and charpos index
>> values, and those might get out of sync. So verify we're looking at a
>> CHAR_HEAD when we need to, which is for multibyte strings.
>>
>> mapconcat itself is also fixed not to assert but to use whatever mapcar1
>> returns.
>
> Thanks, adding Stefan and Mattias to the discussion, in the hope that
> they could review the patch and comment.
Thanks!
>> =09 if (! CONSP (tail))
>> -=09 return i;
>> +=09 error ("list modified by mapping function");
>
> I think this error message is not explicit enough: it doesn't tell
> which list was modified by what function. mapcar and mapconcat are
> many times invoked by code that has little to do with the level which
> could make sense to the user, so we should try to provide as much
> contextual information as possible. Perhaps something like
>
> sequence arg to `mapcar' or `mapconcat' modified by its mapping functio=
n arg
I like the idea of using the same message for both, reducing the memory
requirement for the unused error string. (I may be unaware of a good
error function to use in this case; I ended up not providing additional
arguments because it seems unlikely to me that this problem, if really
accidental, is something the programmer can fix without a Lisp
backtrace, which indicates the situation more clearly).
However, all functions using mapcar1 are affected by the fix, if not the
current bug: mapcan and mapc are as well. Making a message more
specific is bad if that makes it incorrect, and mapc is the most likely
candidate for modifying its sequence argument. (I've never seen (mapc
function "string") though, that I can recall).
>> +=09 if (STRING_MULTIBYTE (seq) && !CHAR_HEAD_P (SREF (seq, i_byte)))
>> +=09 error ("string modified by mapping function");
>
> AFAIU, this doesn't check whether the string was modified, it checks
> for modifications that invalidate the relation between i and i_byte,
> right? So I would suggest
Not even all of those, really, only if they happen to leave us looking
at the middle of a multibyte sequence. Again, minimal effort.
> Because if we want to disallow _any_ modifications of the string, we'd
> need to use a different test (e.g., unibyte ASCII strings could also
> be modified by the mapping function).
I don't think we do want that.
However, my minority opinion on the question of read-only objects is
just that, so I'm not a good person to ask if you want to reintroduce
such objects after most of them are removed when scratch/no-purespace is
merged.
> If that replaces the below:
>> @@ -3473,8 +3478,7 @@ DEFUN ("mapconcat", Fmapconcat, Smapconcat, 2, 3, =
0,
>> =09 goto concat;
>> =09}
>> }
>> - ptrdiff_t nmapped =3D mapcar1 (leni, args, function, sequence);
>> - eassert (nmapped =3D=3D leni);
>> + leni =3D mapcar1 (leni, args, function, sequence);
>
> then can we have the test as eassert?
I see no reason to set this trap for the next person who modifies
mapcar1. If they see the return value is checked by some callers, they
might miss the one caller which crashes. If they remove the return
value, they might miss that the eassert, too, needs to be removed,
making their job harder.
If we want that eassert (why?), it belongs in mapcar1, not Fmapconcat.
mapconcat in general seems to be semi-optimized to me, to be honest, and
it makes the code very long and (apparently) bug-prone. Until we have
better structures for growing strings, we could do worse than to call
Fappend on the sequence argument and handle only the (simple) list case.
If there are significant users of the vector case, we could add that,
too, but strings and bool vectors add many lines of unnecessary code.
(Bool vectors are obsolete, I'm not aware of any new users since I
suggested obsoleting and removing them long ago. IMHO, that remains the
right thing to do, but no new arguments for removing them have appeared,
either. I'd do the change locally, but can't as bool vectors complicate
alloc.c in so many places (which is why I want to remove them in the
first place) that this would introduce too many merge conflicts.)
Again, it's unclear to me which cases mapconcat has chosen to optimize,
and why. I'm not proposing to optimize it for other cases at this point
because I don't know what the criterion is, but I think we went
overboard there supporting weird sequences instead of making the common
case fast and reducing all other cases to it.
I understand we can't go with the 7-line Lisp version of it, but maybe a
compromise can be found.
Pip
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
Received: (at 75845) by debbugs.gnu.org; 26 Jan 2025 06:56:17 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Jan 26 01:56:17 2025
Received: from localhost ([127.0.0.1]:54020 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tbwZ6-0004CO-GT
for submit <at> debbugs.gnu.org; Sun, 26 Jan 2025 01:56:16 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10]:42102)
by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tbwZ3-0004C5-5n
for 75845 <at> debbugs.gnu.org; Sun, 26 Jan 2025 01:56:14 -0500
Received: from fencepost.gnu.org ([2001:470:142:3::e])
by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
id 1tbwYw-0001ym-Rv; Sun, 26 Jan 2025 01:56:06 -0500
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From:
Date; bh=7xXPlxc6bJw3BtIEB97mLUsl+Wr7LQ5KBHIXvGSxvkI=; b=Aof66tTTcyPZlb/3QwED
rRpjHhR4qE4xtOF5GKvTjKievR/Mhj5GeJO/gEB25Nkry+DpMWs2H9KO1ZLkGhXXjo2HB461tIksG
hYpsv9i+Ef4XnyR4ZS03koFoiov06CIm1rZmBUkVsDJ97+b1/qJ972wCLQaCW9wGb0BQO5jAWkZvE
9GY36qsE3t7L+479ARXGgLljnKo3VBoQcVc6aaxzG28ca9wI3OXpVl0gIwTYOTpeOQlxezFMMjWNl
2bbA9vR1ZfG4Sww5vfoedlMYrPS2OS+bjnNlonqwhF+HAM5N4E9u0QfGHkpaIDIO5ASzga4JifPsX
+1LEqZRyl3qY5g==;
Date: Sun, 26 Jan 2025 08:56:03 +0200
Message-Id: <86jzait664.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Pip Cet <pipcet@HIDDEN>, Stefan Monnier <monnier@HIDDEN>,
Mattias =?utf-8?Q?Engdeg=C3=A5rd?= <mattiase@HIDDEN>
In-Reply-To: <87ikq2pafv.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN)
Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications
References: <87r04qpc2m.fsf@HIDDEN> <87ikq2pafv.fsf@HIDDEN>
MIME-version: 1.0
Content-type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 75845
Cc: 75845 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)
> Date: Sun, 26 Jan 2025 02:37:37 +0000
> From: Pip Cet via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
>
> Pip Cet <pipcet@HIDDEN> writes:
>
> > If the mapping function supplied to mapconcat modifies the sequence
> > being mapped over, Emacs can crash.
> >
> > This seems easily fixable, but I wanted to file a bug to make sure we
> > get it right: our current highly-optimized implementation of mapconcat
> > will fail in many cases in which a naive implementation of mapconcat (in
> > Lisp, say) would succeed. Some of those limitations are documented (the
> > argument must be a sequence when the function is called), some aren't
> > (the sequence must not become shorter or longer).
> >
> > My suggestion is to make the ordinary no-modification case work as fast
> > as is possible under the constraint that extraordinary
> > self-modifications should not result in crashes. They can and will
> > result in unpredictable behavior, though.
> >
> > These crash:
> >
> > (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))
> >
> > Patch will follow once this has a bug number.
>
> This patch does the minimum work necessary; while the previous code
> attempted to return the "correct" length of a list which was modified,
> crashing in an eassert in mapconcat, the new code errors instead. As
> error is marked with ATTRIBUTE_COLD, this should result in better code.
>
> The string case is slightly trickier: we don't keep around an SDATA
> pointer, which is good, but we do remember byte and charpos index
> values, and those might get out of sync. So verify we're looking at a
> CHAR_HEAD when we need to, which is for multibyte strings.
>
> mapconcat itself is also fixed not to assert but to use whatever mapcar1
> returns.
Thanks, adding Stefan and Mattias to the discussion, in the hope that
they could review the patch and comment.
> if (! CONSP (tail))
> - return i;
> + error ("list modified by mapping function");
I think this error message is not explicit enough: it doesn't tell
which list was modified by what function. mapcar and mapconcat are
many times invoked by code that has little to do with the level which
could make sense to the user, so we should try to provide as much
contextual information as possible. Perhaps something like
sequence arg to `mapcar' or `mapconcat' modified by its mapping function arg
Better suggestions are welcome.
> + if (STRING_MULTIBYTE (seq) && !CHAR_HEAD_P (SREF (seq, i_byte)))
> + error ("string modified by mapping function");
AFAIU, this doesn't check whether the string was modified, it checks
for modifications that invalidate the relation between i and i_byte,
right? So I would suggest
string arg of mapcar or mapconcat clobbered by its mapping function arg
Because if we want to disallow _any_ modifications of the string, we'd
need to use a different test (e.g., unibyte ASCII strings could also
be modified by the mapping function).
> + if (i < leni)
> + error ("string modified by mapping function");
Why do we need this test? Does it prevent us from crashing in some
cases? If that replaces the below:
> @@ -3473,8 +3478,7 @@ DEFUN ("mapconcat", Fmapconcat, Smapconcat, 2, 3, 0,
> goto concat;
> }
> }
> - ptrdiff_t nmapped = mapcar1 (leni, args, function, sequence);
> - eassert (nmapped == leni);
> + leni = mapcar1 (leni, args, function, sequence);
then can we have the test as eassert?
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
Received: (at submit) by debbugs.gnu.org; 26 Jan 2025 02:37:59 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sat Jan 25 21:37:59 2025
Received: from localhost ([127.0.0.1]:53411 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1tbsX8-0006tS-EU
for submit <at> debbugs.gnu.org; Sat, 25 Jan 2025 21:37:58 -0500
Received: from lists.gnu.org ([2001:470:142::17]:60192)
by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.84_2) (envelope-from <pipcet@HIDDEN>)
id 1tbsX4-0006sp-L6
for submit <at> debbugs.gnu.org; Sat, 25 Jan 2025 21:37:55 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10])
by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <pipcet@HIDDEN>)
id 1tbsWx-0001v7-Dv
for bug-gnu-emacs@HIDDEN; Sat, 25 Jan 2025 21:37:47 -0500
Received: from mail-4322.protonmail.ch ([185.70.43.22])
by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <pipcet@HIDDEN>)
id 1tbsWu-0001gJ-AB
for bug-gnu-emacs@HIDDEN; Sat, 25 Jan 2025 21:37:46 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;
s=protonmail3; t=1737859061; x=1738118261;
bh=f7yvlsW1EIyrggiJD3TBcjAWsgwzrj72ED/f5BbYOrg=;
h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date:
Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector:
List-Unsubscribe:List-Unsubscribe-Post;
b=kOoedIEhhn76tMmG5mKtH3VpXJt2zG637qw/qmlm7jPIfY7HdOU2f0riUmuaczjMi
MuOeVIbPW1VL+eL7QksMF+9As7MANvKisLLU//HVIcZg6OgXaO85bSNU6Osdekg5x3
Iosdo39+PaCkzQPBxtMCpYa2K03J2X8EZOYsdRU0AWONhK5CzY0wcFJXqARs3Ri+RD
o+96DSqTWChLWjE52y3LbEVy4epniVVYh4wJKa5klTDAaFwdGoq7h4HaKXpPf2tsn+
QAopnoGCNi81He3QJt9IEaNwwDc/E3UF6dbUSL20S4ZgHO6RpWJCdRXsVnENnQjHee
plIE+ZCTTJelw==
Date: Sun, 26 Jan 2025 02:37:37 +0000
To: bug-gnu-emacs@HIDDEN
From: Pip Cet <pipcet@HIDDEN>
Subject: Re: bug#75845: mapconcat crashes for unreasonable self-modifications
Message-ID: <87ikq2pafv.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
X-Pm-Message-ID: 1498d1f964813c7cc3e7d9314bbf51e3cb7865db
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Received-SPF: pass client-ip=185.70.43.22; envelope-from=pipcet@HIDDEN;
helo=mail-4322.protonmail.ch
X-Spam_score_int: -21
X-Spam_score: -2.2
X-Spam_bar: --
X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.049,
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,
SPF_HELO_PASS=-0.001, SPF_PASS=-0.001,
T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 1.0 (+)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.0 (/)
Pip Cet <pipcet@HIDDEN> writes:
> If the mapping function supplied to mapconcat modifies the sequence
> being mapped over, Emacs can crash.
>
> This seems easily fixable, but I wanted to file a bug to make sure we
> get it right: our current highly-optimized implementation of mapconcat
> will fail in many cases in which a naive implementation of mapconcat (in
> Lisp, say) would succeed. Some of those limitations are documented (the
> argument must be a sequence when the function is called), some aren't
> (the sequence must not become shorter or longer).
>
> My suggestion is to make the ordinary no-modification case work as fast
> as is possible under the constraint that extraordinary
> self-modifications should not result in crashes. They can and will
> result in unpredictable behavior, though.
>
> These crash:
>
> (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) (strin=
g x)) s))
>
> Patch will follow once this has a bug number.
This patch does the minimum work necessary; while the previous code
attempted to return the "correct" length of a list which was modified,
crashing in an eassert in mapconcat, the new code errors instead. As
error is marked with ATTRIBUTE_COLD, this should result in better code.
The string case is slightly trickier: we don't keep around an SDATA
pointer, which is good, but we do remember byte and charpos index
values, and those might get out of sync. So verify we're looking at a
CHAR_HEAD when we need to, which is for multibyte strings.
mapconcat itself is also fixed not to assert but to use whatever mapcar1
returns.
From cf861bc042650c968f2702d842ffa4a36f0105e9 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH] Fix crashes in mapconcat etc. (bug#75845)
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 modifications by the mapping
function.
(mapconcat): Don't eassert that 'mapcar1' never returns a
partially-filled vector, even though that is currently correct.
---
src/fns.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/fns.c b/src/fns.c
index 081ed2b9f51..8e8b9584509 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -3385,7 +3385,7 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Obje=
ct fn, Lisp_Object seq)
for (ptrdiff_t i =3D 0; i < leni; i++)
=09{
=09 if (! CONSP (tail))
-=09 return i;
+=09 error ("list modified by mapping function");
=09 Lisp_Object dummy =3D calln (fn, XCAR (tail));
=09 if (vals)
=09 vals[i] =3D dummy;
@@ -3403,16 +3403,21 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Ob=
ject fn, Lisp_Object seq)
}
else if (STRINGP (seq))
{
+ ptrdiff_t i =3D 0;
ptrdiff_t i_byte =3D 0;
=20
- for (ptrdiff_t i =3D 0; i < leni;)
+ while (i_byte < SBYTES (seq))
=09{
=09 ptrdiff_t i_before =3D i;
+=09 if (STRING_MULTIBYTE (seq) && !CHAR_HEAD_P (SREF (seq, i_byte)))
+=09 error ("string modified by mapping function");
=09 int c =3D fetch_string_char_advance (seq, &i, &i_byte);
=09 Lisp_Object dummy =3D calln (fn, make_fixnum (c));
=09 if (vals)
=09 vals[i_before] =3D dummy;
=09}
+ if (i < leni)
+=09error ("string modified by mapping function");
}
else
{
@@ -3473,8 +3478,7 @@ DEFUN ("mapconcat", Fmapconcat, Smapconcat, 2, 3, 0,
=09 goto concat;
=09}
}
- ptrdiff_t nmapped =3D mapcar1 (leni, args, function, sequence);
- eassert (nmapped =3D=3D leni);
+ leni =3D mapcar1 (leni, args, function, sequence);
=20
concat: ;
ptrdiff_t nargs =3D args_alloc;
--=20
2.47.1
bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.Received: (at submit) by debbugs.gnu.org; 26 Jan 2025 02:02:40 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sat Jan 25 21:02:40 2025 Received: from localhost ([127.0.0.1]:53354 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1tbryy-00052u-7q for submit <at> debbugs.gnu.org; Sat, 25 Jan 2025 21:02:40 -0500 Received: from lists.gnu.org ([2001:470:142::17]:38902) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from <pipcet@HIDDEN>) id 1tbryu-00052b-9k for submit <at> debbugs.gnu.org; Sat, 25 Jan 2025 21:02:38 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <pipcet@HIDDEN>) id 1tbryo-0005qZ-Ey for bug-gnu-emacs@HIDDEN; Sat, 25 Jan 2025 21:02:30 -0500 Received: from mail-40133.protonmail.ch ([185.70.40.133]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <pipcet@HIDDEN>) id 1tbrym-0006sz-8a for bug-gnu-emacs@HIDDEN; Sat, 25 Jan 2025 21:02:30 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1737856945; x=1738116145; bh=OZEA48SIi+ThEJXi+I2APTHlkWBcDtoF+wTHicINGT0=; h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector: List-Unsubscribe:List-Unsubscribe-Post; b=RAlWlRIWzQMxZ8Bjuu1D35svX9Tybt6b9D7JG9NTzsdyDnTau09+5iy7oHWUJcScs V2gGrSpdO7aGh1w7QAAoeCpIHn78Ayrwy3FVWXjmFFhiQqxk/cWg5AxurnSKALsg+I SNIpz//rfryXJhcAKPo69LJIeRfSmDJWDalu3g/c/eRDmXF+mshSnsXLF6ahqA+FwH dlPNpmpMTLplhO/wRuyXX9/f05Lc7/PtZhmlVU6/vB93clgnnTv1LPpsGCWAD2drkP h/q1jvC79pyYMqNDJeMJocFjFw5zTDhzyXRAW3AIfEO6q5d8NTSTzFChkpW5h1qi+E mvfJ26MrZry8g== Date: Sun, 26 Jan 2025 02:02:22 +0000 To: bug-gnu-emacs@HIDDEN From: Pip Cet <pipcet@HIDDEN> Subject: mapconcat crashes for unreasonable self-modifications Message-ID: <87r04qpc2m.fsf@HIDDEN> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 46221e7f570bfeb04b88ac3947e001b5c00dc602 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=185.70.40.133; envelope-from=pipcet@HIDDEN; helo=mail-40133.protonmail.ch X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 1.0 (+) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -0.0 (/) If the mapping function supplied to mapconcat modifies the sequence being mapped over, Emacs can crash. This seems easily fixable, but I wanted to file a bug to make sure we get it right: our current highly-optimized implementation of mapconcat will fail in many cases in which a naive implementation of mapconcat (in Lisp, say) would succeed. Some of those limitations are documented (the argument must be a sequence when the function is called), some aren't (the sequence must not become shorter or longer). My suggestion is to make the ordinary no-modification case work as fast as is possible under the constraint that extraordinary self-modifications should not result in crashes. They can and will result in unpredictable behavior, though. These crash: (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)) Patch will follow once this has a bug number.
Pip Cet <pipcet@HIDDEN>:bug-gnu-emacs@HIDDEN.
Full text available.bug-gnu-emacs@HIDDEN:bug#75845; Package emacs.
Full text available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997 nCipher Corporation Ltd,
1994-97 Ian Jackson.