From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Pip Cet via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#72802: 31.0.50; Crash in (equal sub-char-table-a sub-char-table-b) Date: Sun, 25 Aug 2024 15:15:14 +0000 Message-ID: <87le0k3a81.fsf@protonmail.com> References: <871q2c4uf6.fsf@protonmail.com> <87v7zo3d6m.fsf@protonmail.com> <86a5h0lkrg.fsf@gnu.org> Reply-To: Pip Cet Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="10489"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 72802@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Aug 25 17:16:34 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1siEyn-0002XP-MF for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 25 Aug 2024 17:16:33 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1siEyc-0000Q9-TH; Sun, 25 Aug 2024 11:16:24 -0400 Original-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 ) id 1siEyS-0000Pf-D9 for bug-gnu-emacs@gnu.org; Sun, 25 Aug 2024 11:16:13 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1siEyR-0001Y2-Ve for bug-gnu-emacs@gnu.org; Sun, 25 Aug 2024 11:16:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:References:In-Reply-To:From:Date:To:Subject; bh=cuJNGoyY2tWMbR6b5DyARVsEgUq9Pa+ABbE2GkaVvqU=; b=kULPvWskCNI2UuKdpEQkowR5ei+6ne1ALWuwLkZfz5uIKzbHPw/C3+b1qyszuqteAsXIQGkuKMqMPwKcrvuLnLC+O46btpGPx0FrRnS6SwV8sPTm7B768YCOuW9z+VP4F+ouzlw4ZiT1CkY9LCLA0bnmBJ03mx8v0gcMxBraYXWWPZ8L3RZmbFrMU8fvxv+LkrF1J1reuNmgA4ydqBIHhFVir7RqXJaZd1YnvcXdi40sbonIl7IGlR/vu7UJzYmow9FERSy8IqvDA9WYfIYs9AEo2OrwG3wPfEkHP3JKyxpG/Lp9vNoXaI+hz5Iy1MlcfAZM3+f+Mjx9hn/9UIOkwA==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1siEzF-0000wp-V0 for bug-gnu-emacs@gnu.org; Sun, 25 Aug 2024 11:17:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Pip Cet Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 25 Aug 2024 15:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 72802 X-GNU-PR-Package: emacs Original-Received: via spool by 72802-submit@debbugs.gnu.org id=B72802.17245989753575 (code B ref 72802); Sun, 25 Aug 2024 15:17:01 +0000 Original-Received: (at 72802) by debbugs.gnu.org; 25 Aug 2024 15:16:15 +0000 Original-Received: from localhost ([127.0.0.1]:43160 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1siEyU-0000vb-Ip for submit@debbugs.gnu.org; Sun, 25 Aug 2024 11:16:15 -0400 Original-Received: from mail-40131.protonmail.ch ([185.70.40.131]:48073) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1siEyS-0000vL-Io for 72802@debbugs.gnu.org; Sun, 25 Aug 2024 11:16:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1724598916; x=1724858116; bh=cuJNGoyY2tWMbR6b5DyARVsEgUq9Pa+ABbE2GkaVvqU=; 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; b=xp1gXYXSkY6teerBy5cYJ/F5G76Svm0eDP2+FLscd+CcPkRLt+NNLoY1YIuecie5+ GJmOTl54//MXKBW69pTiVzzIDbaziRIrLGqkm6ymHo2xnRCrft0pYpv/3YsBwaixWi k9u8xndks2SCRbzsArBExcVEMT+HU5meeCWaX7HIywU4SUTOh/Jvd6CzkYie+OEHsJ wUgZ1nYRReLrcgZYOiyot/hsxkXW475+e9NjDtaI04pZxACUvbiiik4IHkmfDLTT/a ceDpdsME6vXCBd/Wk/P1aj92JcD8+W/rUj8SwQlfDlAvzPTrj9PwPNTh4pSwH8p+aX C+yrnXSkHfRDg== In-Reply-To: <86a5h0lkrg.fsf@gnu.org> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 4edca4eff24f15817fbca113d31403b2f17e6236 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:290742 Archived-At: "Eli Zaretskii" writes: >> Date: Sun, 25 Aug 2024 14:11:17 +0000 >> From: Pip Cet via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" >> >> Pip Cet writes: >> > The code in internal_equal compares sub-char-tables incorrectly >> >> This patch should fix things for the release branch: > > I don't see a reason to install this on the release branch now. Your call. > Not even on master, I think, unless we see a bug related to it that is no= t > caused by a specially-concocted Lisp program or GDB command. It's a clear bug, whether or not the Lisp programs that cause it are "specially-concocted" (what's that supposed to mean, anyway? We can't just delay fixing what are clearly bugs until they pop up on random users' machines!) So I think it's important to fix this soon (not "now") on the master branch. > If this is required for the no-pure-space branch, I think you should > install this on that branch. Then it will be merged together with the > branch. It's not, it just popped up during work on that branch. > If you see some urgent need to fix this ASAP on master, please tell > why you think so. Not "ASAP", and not "urgent", no. We certainly can take the time to address your comments and fix up things. >> +=09 /* Bug#72802. See 'mark_char_table' in alloc.c. */ >> +=09 if (SUB_CHAR_TABLE_P (o1)) >> +=09 { >> +=09=09i =3D SUB_CHAR_TABLE_OFFSET; >> +=09=09if (XSUB_CHAR_TABLE (o1)->depth !=3D >> +=09=09 XSUB_CHAR_TABLE (o2)->depth) >> +=09=09 return false; >> +=09=09if (XSUB_CHAR_TABLE (o1)->min_char !=3D >> +=09=09 XSUB_CHAR_TABLE (o2)->min_char) >> +=09=09 return false; >> +=09 } > > I looked at mark_char_table, trying to understand what the above > comments wants to say, and couldn't. So I think the comment should > be improved, so that it would be more clear what it wants to say. As you will have seen, then, we compare the same "vector" elements in internal_equal as are marked in mark_char_table, but I agree the comment (and the code) can be improved, and will try to do that. > Also, please move the assignment of SUB_CHAR_TABLE_OFFSET to i to > after the two early 'return's. Gladly. >> For the master branch, I think the right thing to do is to turn the >> first two, non-Lisp members of Lisp_Sub_Char_Table ('depth' and >> 'min_char') into 'Lisp_Object's. Then we can simplify the code and >> compare sub char tables as we do ordinary vectors, at the cost of eight >> bytes of extra storage per sub char table on machines with 64-bit >> EMACS_INTs. > > I'm not sure we want to pay this cost. What bothers me is mainly the > run-time cost of extracting integers from Lisp objects. That might be noticeable on 32-bit machines with EMACS_WIDE_INT, I suppose, or on very old machines where memory isn't so much slower than register manipulation. > char-table is > supposed to be very efficient, both memory-wise and CPU-wise, and I > think the performance here trumps simplicity. How about using an "ordinary" pseudovector with its non-Lisp elements at the end? >> BTW, I'm surprised this code returns nil; I think that should be >> documented. >> >> (setq a (make-char-table nil)) >> (setq b (make-char-table nil)) >> (aset a 1 nil) >> (dotimes (i (max-char)) >> (unless (equal (aref a i) (aref b i)) >> (error "i =3D %S" i))) >> (equal a b) > > Why are you surprised? Setting a single cell of a char-table changes > its structure, usually in quite a radical way. 'aref' does some very > special things for char-tables; the semantics of accessing an element > of a vector is only correct superficially, not in the details. Indeed, and I expected (and still expect) 'equal' to ignore such details. > The internals of a char-table are not really documented in the ELisp > manual; the description there is mostly phenomenological, without any > details. I don't think 'equal' behavior is part of those "internals", and it certainly isn't a detail. Given the great trouble we're going to to make 'equal' work on char tables at all, I'm still surprised we didn't actually make it work the way it does on vectors. > If you want to document the internals, I think the proper Not the internals, just how 'equal' works. Pip