From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Braun =?UTF-8?Q?G=C3=A1bor?= Newsgroups: gmane.emacs.bugs Subject: bug#70122: 29.3.50; transpose-regions can crash Emacs Date: Wed, 03 Apr 2024 20:52:47 +0200 Message-ID: <3216728.5fSG56mABF@gabor> References: <2318820.ElGaqSPkdT@gabor> <86wmph9u1i.fsf@gnu.org> <86ttkl9q9b.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart5928865.MhkbZ0Pkbq" Content-Transfer-Encoding: 7Bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40199"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 70122@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Apr 03 20:54:26 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 1rs5ke-000AIA-Pz for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 03 Apr 2024 20:54:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rs5kG-0002Y5-0d; Wed, 03 Apr 2024 14:54:00 -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 1rs5kE-0002X2-Cw for bug-gnu-emacs@gnu.org; Wed, 03 Apr 2024 14:53:58 -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 1rs5kE-0007KW-3s for bug-gnu-emacs@gnu.org; Wed, 03 Apr 2024 14:53:58 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rs5kI-00066E-JP for bug-gnu-emacs@gnu.org; Wed, 03 Apr 2024 14:54:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Braun =?UTF-8?Q?G=C3=A1bor?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 03 Apr 2024 18:54:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70122 X-GNU-PR-Package: emacs Original-Received: via spool by 70122-submit@debbugs.gnu.org id=B70122.171217041323232 (code B ref 70122); Wed, 03 Apr 2024 18:54:02 +0000 Original-Received: (at 70122) by debbugs.gnu.org; 3 Apr 2024 18:53:33 +0000 Original-Received: from localhost ([127.0.0.1]:59607 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rs5jn-00062V-Mp for submit@debbugs.gnu.org; Wed, 03 Apr 2024 14:53:32 -0400 Original-Received: from mail-lf1-x132.google.com ([2a00:1450:4864:20::132]:59434) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rs5jk-00061g-J9 for 70122@debbugs.gnu.org; Wed, 03 Apr 2024 14:53:30 -0400 Original-Received: by mail-lf1-x132.google.com with SMTP id 2adb3069b0e04-513e134f73aso228627e87.2 for <70122@debbugs.gnu.org>; Wed, 03 Apr 2024 11:53:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712170398; x=1712775198; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KjZxb+0xElzjjLkd5UZMZvv8cqW/f/uhi8hcNqudcnA=; b=R2CS3dXFrMQ/9+pjmR7Z0WfvxmLN3EtR6MiQFwauTDsLmhnALz9rJLQ1GVFL6H/TJf GP5+FECBC6QhO7sO/b4huaHdTAldTcdpojLU5nxnFYiNjrBMlDoA3aLKgmjsj1HCyMgc f/xoVmkj8t/K/v8BEYvQY9fENBTGhdFHpqIHcmzvHtjPMZU2C7gh3cLN1QMjJcpjpcHU 8mwOZtDNMl4P9CtXyyNNBS5OVeXg+xisPhGnhYGLgr3WsBgkME1CJ1L0EZQnjJ6RCpc6 bxBG4w+BajehbfWXJniK/Yiy1biTUzFBaxlvYmXjKUXDzTdhFgI3uzsM4oRuSuRtPEdu SBYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712170398; x=1712775198; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KjZxb+0xElzjjLkd5UZMZvv8cqW/f/uhi8hcNqudcnA=; b=TLIsw0EJienlfSja8+2HG0RQIkpiWqz6e2ezqEdi2tNOBuhd3h12Ieut6tnHhfFuwK iWIX3ndMINymhSkVYRKT5X3fGScsvTlRT2AKyGuIbt+nVuMOp/sJUu9ZH6AJC4V9P9/u Ih2eyASqW3O5zH9mj6FaA1H7QengNxxMfrTFmRdaolQKXWSxcD5CoWUHum1loUVNvjkB Af2dz6zlXLFquJj4NTg3eD09yqI7QbdkQ9I9ppoXE0TvHT8Equb5mQiok7OoZghEtpYN QsWYmXDzLIXwM/7LY13DVU2glGaX6ajYVn6pbZ4QjXcHJLcPMbgzD60XznEY12DeZGyU bbcA== X-Gm-Message-State: AOJu0YwbcL4ysJlFpVhG1OavxyVPtT6i5AzuCBqKnXhrX0uJvL86k2cZ 0V9L+ObZqoUQkis4a5SbVKAwV67ipwUgG08v+L3Rsi4726HAnsyt X-Google-Smtp-Source: AGHT+IH3866bkJ6Vl2SUGBXGCbUC3iCYVuRqQSMafI1hC1bcl7jh2C0oJPxpWx7yKNACSWVwyjeiLw== X-Received: by 2002:ac2:5b5d:0:b0:513:c2e4:9daf with SMTP id i29-20020ac25b5d000000b00513c2e49dafmr284288lfp.52.1712170397706; Wed, 03 Apr 2024 11:53:17 -0700 (PDT) Original-Received: from gabor.localnet ([2001:9e8:1548:b200:c5ff:2148:8b70:b48f]) by smtp.gmail.com with ESMTPSA id u17-20020a17090626d100b00a4e1aa345f6sm8010610ejc.115.2024.04.03.11.53.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Apr 2024 11:53:17 -0700 (PDT) In-Reply-To: <86ttkl9q9b.fsf@gnu.org> 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:282590 Archived-At: This is a multi-part message in MIME format. --nextPart5928865.MhkbZ0Pkbq Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Hi Eli, > Thanks, but could you please show the minimal change required to=20 fix > just the particular problem with this scenario I've attached a new self-contained patch based on yours, thank you for coming up with it. In my opinion, one of the problems is really as you said that len_mid (length in bytes) is uses where length of characters is expected. The patch contains an additional such case, and replaces (start1 + len1) in your patch with the equivalent but shorter end1. The other problem is that the branch len1_byte =3D=3D len2_byte assumes len1 =3D=3D len2 in several places: undo records, positions after the transposition, and the least obvious one is that even intervals between the=20 region seem to need adjustment of positions. (I don't know enough of intervals to understand it, but had failed=20 tests with text properties at wrong places.) Anyway, I've just added len1 =3D=3D len2 as a condition to that branch with comments where I think the assumption is used. I've also added a new test for this case. > The patch > below passes both your test and the already-existing tests in > test/src/editfns-tests.el. =46or me after applying your patch, the tests crashed. The crash message was hidden in the end of the output: passed 21/24 transpose-nonascii-regions-test-1 (0.000067 sec) passed 22/24 transpose-nonascii-regions-test-2 (0.000068 sec) passed 23/24 transpose-regions-text-properties (0.000074 sec) Undo Undo make[1]: *** [Makefile:181: src/editfns-tests.log] segmentation=20 fault make[1]: Leaving directory "/home/gabor/src/build/emacs-29.3/test" make: *** [Makefile:247: src/editfns-tests] Error 2 With the tests I intended to test all the branches in the code where the regions don't touch each other, catching mistakes where the wrong length is used. I hoped that byte length is not system dependent, it seems I have been mistaken, and the tests still need improvement to be thorough on all systems. So what differences exist or byte length? Best wishes, G=C3=A1bor --nextPart5928865.MhkbZ0Pkbq Content-Disposition: attachment; filename="0001-transpose-regions-fix-wrong-lengths-add-missing-cond.patch" Content-Transfer-Encoding: quoted-printable Content-Type: text/x-patch; charset="utf-8"; name="0001-transpose-regions-fix-wrong-lengths-add-missing-cond.patch" =46rom 5a0cf2542388104cde444b2130d994ee36ce5392 Mon Sep 17 00:00:00 2001 =46rom: =3D?UTF-8?q?G=3DC3=3DA1bor=3D20Braun?=3D Date: Wed, 3 Apr 2024 20:03:40 +0200 Subject: [PATCH] transpose-regions: fix wrong lengths; add missing conditio= n. Case len1_byte =3D=3D len2_byte: The code assumes len1 =3D=3D len2 at several places, see comments. As a fix, add that condition to the case. (This case is not strictly necessary, as the other cases cover it, so restricting the case should be no problem.) Replace len_mid with end1 - start2 where number of characters is expected instead of number of bytes. =2D-- src/editfns.c | 19 +++++++--- test/src/editfns-tests.el | 79 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/src/editfns.c b/src/editfns.c index 85f7739df07..65570a84472 100644 =2D-- a/src/editfns.c +++ b/src/editfns.c @@ -4655,12 +4655,13 @@ DEFUN ("transpose-regions", Ftranspose_regions, Str= anspose_regions, 4, 5, { len_mid =3D start2_byte - (start1_byte + len1_byte); =20 =2D if (len1_byte =3D=3D len2_byte) + if (len1_byte =3D=3D len2_byte && len1 =3D=3D len2) /* Regions are same size, though, how nice. */ { USE_SAFE_ALLOCA; =20 modify_text (start1, end2); + /* The undo records are only correct for len1 =3D=3D len2. */ record_change (start1, len1); record_change (start2, len2); tmp_interval1 =3D copy_intervals (cur_intv, start1, len1); @@ -4682,10 +4683,14 @@ DEFUN ("transpose-regions", Ftranspose_regions, Str= anspose_regions, 4, 5, memcpy (start2_addr, temp, len1_byte); SAFE_FREE (); =20 + /* In the following line start2 is only correct for + len1 =3D=3D len2. Otherwise it should be end2 - len1. */ graft_intervals_into_buffer (tmp_interval1, start2, len1, current_buffer, 0); graft_intervals_into_buffer (tmp_interval2, start1, len2, current_buffer, 0); + /* The intervals between end1 and start2 need no adjustment + only because len1 =3D=3D len2. */ } =20 else if (len1_byte < len2_byte) /* Second region larger than first */ @@ -4696,7 +4701,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stran= spose_regions, 4, 5, modify_text (start1, end2); record_change (start1, (end2 - start1)); tmp_interval1 =3D copy_intervals (cur_intv, start1, len1); =2D tmp_interval_mid =3D copy_intervals (cur_intv, end1, len_mid); + tmp_interval_mid =3D copy_intervals (cur_intv, end1, + start2 - end1); tmp_interval2 =3D copy_intervals (cur_intv, start2, len2); =20 tmp_interval3 =3D validate_interval_range (buf, &startr1, &endr2, 0); @@ -4716,7 +4722,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stran= spose_regions, 4, 5, graft_intervals_into_buffer (tmp_interval1, end2 - len1, len1, current_buffer, 0); graft_intervals_into_buffer (tmp_interval_mid, start1 + len2, =2D len_mid, current_buffer, 0); + start2 - end1, + current_buffer, 0); graft_intervals_into_buffer (tmp_interval2, start1, len2, current_buffer, 0); } @@ -4729,7 +4736,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stran= spose_regions, 4, 5, modify_text (start1, end2); =20 tmp_interval1 =3D copy_intervals (cur_intv, start1, len1); =2D tmp_interval_mid =3D copy_intervals (cur_intv, end1, len_mid); + tmp_interval_mid =3D copy_intervals (cur_intv, end1, + start2 - end1); tmp_interval2 =3D copy_intervals (cur_intv, start2, len2); =20 tmp_interval3 =3D validate_interval_range (buf, &startr1, &endr2, 0); @@ -4749,7 +4757,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stran= spose_regions, 4, 5, graft_intervals_into_buffer (tmp_interval1, end2 - len1, len1, current_buffer, 0); graft_intervals_into_buffer (tmp_interval_mid, start1 + len2, =2D len_mid, current_buffer, 0); + start2 - end1, + current_buffer, 0); graft_intervals_into_buffer (tmp_interval2, start1, len2, current_buffer, 0); } diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el index b3b7da65ad3..7c3e3837c8c 100644 =2D-- a/test/src/editfns-tests.el +++ b/test/src/editfns-tests.el @@ -132,6 +132,85 @@ propertize/error-even-number-of-args "Number of args for `propertize' must be odd." (should-error (propertize "foo" 'bar) :type 'wrong-number-of-arguments)) =20 +;; Tests for `transpose-region' + +(ert-deftest transpose-regions-text-properties-2 () + "Test `transpose-regions' thoroughly with text properties." + (let* ((string1pre (propertize "a\x2013" :test 1)) + (middle (propertize "c\x00e9h" :test 0)) + (string2 (propertize "\x25cf\x25cb" :test 2)) + (bytes1 (string-bytes string1pre)) + (bytes2 (string-bytes string2))) + ;; (cl-assert (< bytes1 bytes2)) + (dotimes (i (+ 3 (- bytes2 bytes1))) + (let ((string1 (concat string1pre + (propertize (make-string i ?X) + :test t)))) + (with-temp-buffer + (insert string1 middle string2) + (buffer-enable-undo) + (transpose-regions + 1 (1+ (length string1)) + (- (point) (length string2)) (point)) + (should (equal-including-properties + (buffer-string) + (concat string2 middle string1))) + (undo-boundary) + (let ((this-command #'undo) + (last-command #'ert)) ; anything but undo + (undo)) + (should (equal-including-properties + (buffer-string) + (concat string1 middle string2)))))))) + +(ert-deftest transpose-regions-text-properties () + "Test `transpose-regions' with text properties. +This test is known to crash Emacs 28.2, 29.2, 29.3." + (with-temp-buffer + (insert (propertize "a" 'face 'font-lock-variable-name-face)) + (insert ":\n") + (insert (propertize "b" 'face 'font-lock-variable-name-face)) + (insert ": \x2113\x2080\n") + (insert (propertize "v" 'face 'font-lock-variable-name-face)) + (insert ": scaling\n") + ;; Move last line to the beginning + (transpose-regions 1 1 10 21) + (should (equal-including-properties + (buffer-string) + (concat + (propertize "v" 'face 'font-lock-variable-name-face) + ": scaling\n" + (propertize "a" 'face 'font-lock-variable-name-face) + ":\n" + (propertize "b" 'face 'font-lock-variable-name-face) + ": \x2113\x2080\n"))))) + +(ert-deftest transpose-regions-equal-size () + "Test `transpose-regions' on regions equal-size regions. +Both the number of characters and bytes are equal of the transposed +regions." + (let* ((string1 (propertize "a\x2013\ bc" :test 1)) + (middle (propertize "R=C5=90T" :test 0)) + (string2 (propertize "f\x2013nd" :test 2))) + (cl-assert (=3D (length string1) (length string2))) + (cl-assert (=3D (string-bytes string1) (string-bytes string2))) + (with-temp-buffer + (insert string1 middle string2) + (buffer-enable-undo) + (transpose-regions + 1 (1+ (length string1)) + (- (point) (length string2)) (point)) + (should (equal-including-properties + (buffer-string) + (concat string2 middle string1))) + (undo-boundary) + (let ((this-command #'undo) + (last-command #'ert)) ; anything but undo + (undo)) + (should (equal-including-properties + (buffer-string) + (concat string1 middle string2)))))) + ;; Tests for bug#5131. (defun transpose-test-reverse-word (start end) "Reverse characters in a word by transposing pairs of characters." =2D-=20 2.39.2 --nextPart5928865.MhkbZ0Pkbq--