unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70122: 29.3.50; transpose-regions can crash Emacs
@ 2024-04-01 10:02 Braun Gábor
  2024-04-01 11:55 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Braun Gábor @ 2024-04-01 10:02 UTC (permalink / raw)
  To: 70122

[-- Attachment #1: Type: text/plain, Size: 30919 bytes --]

Hi,

I have the following file:

File ~/.emacs.d/trans-regions-bug.el:

(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))

End of file ~/.emacs.d/trans-regions-bug.el:


After building Emacs as described in the INSTALL file, the following
command crashes Emacs:

  src/emacs -Q --batch -l ~/.emacs.d/trans-regions-bug.el

The following appears:

Fatal error 11: Segmentation fault


See below for debugging information.

Please find attached a patch to transpose-regions
with added tests.  (The patch intends to fix typos:
makes sure lengths in bytes and characters are not confused.
One case (likely for optimization only) has been removed,
seemed too much trouble to get it right.)


gdb session with non-session specific material (help, copyright...) removed:

Current directory is /home/gabor/src/build/emacs-29.3/src/
GNU gdb (Debian 13.1-3) 13.1
Reading symbols from /home/gabor/src/build/emacs-29.3/src/emacs...
SIGINT is used by the debugger.
Are you sure you want to change it? (y or n) [answered Y; input not from terminal]
DISPLAY = :0
TERM = dumb
Breakpoint 2 at 0x13a480: file ../../../emacs/src/xterm.c, line 26136.
.gdbinit:1: Error in sourced command file:
/home/gabor/src/build/emacs-29.3/../../emacs/src/.gdbinit:1375: Error in sourced command file:
Scripting in the "Python" language is not supported in this copy of GDB.
Downloading source file /home/gabor/src/build/emacs-29.3/src/<built-in>...
(gdb) run
Starting program: /home/gabor/src/build/emacs-29.3/src/emacs -Q --batch -l \~/.emacs.d/trans-regions-bug.el
Downloading separate debug info for /lib64/ld-linux-x86-64.so.2...
Downloading separate debug info for system-supplied DSO at 0x7ffff7fc9000...
Downloading separate debug info for /lib/x86_64-linux-gnu/libdbus-1.so.3...
Downloading separate debug info for /lib/x86_64-linux-gnu/libc.so.6...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Downloading separate debug info for /usr/lib/x86_64-linux-gnu/libfribidi.so.0...

Program received signal SIGSEGV, Segmentation fault.
0x00005555557c47b3 in graft_intervals_into_buffer (source=source@entry=0x555555e2b568, position=position@entry=12, length=length@entry=13, buffer=0x555555ed1170, inherit=inherit@entry=false) at ../../../emacs/src/intervals.c:1669
1669	      if (LENGTH (over) - over_used < LENGTH (under))
(gdb) bt full
#0  0x00005555557c47b3 in graft_intervals_into_buffer (source=source@entry=0x555555e2b568, position=position@entry=12, length=length@entry=13, buffer=0x555555ed1170, inherit=inherit@entry=false) at ../../../emacs/src/intervals.c:1669
        tree = <optimized out>
        under = 0x0
        over = 0x555555e2b5a0
        this = <optimized out>
        over_used = 0
#1  0x000055555575194b in Ftranspose_regions (startr1=<optimized out>, endr1=<optimized out>, startr2=<optimized out>, endr2=<optimized out>, leave_markers=0x0) at ../../../emacs/src/editfns.c:4751
        sa_avail = <optimized out>
        sa_count = <optimized out>
        start1 = 1
        end1 = 1
        start2 = 10
        end2 = 21
        start1_byte = <optimized out>
        start2_byte = 14
        len1_byte = 0
        len2_byte = 11
        end2_byte = 25
        gap = <optimized out>
        len1 = 0
        len_mid = 13
        len2 = 11
        start1_addr = <optimized out>
        start2_addr = <optimized out>
        temp = 0x7fffffffd830 "v: scaling\nUUU"
        cur_intv = <optimized out>
        tmp_interval1 = 0x0
        tmp_interval_mid = 0x555555e2b568
        tmp_interval2 = 0x555555e2b648
        tmp_interval3 = <optimized out>
        buf = <optimized out>
#2  0x0000555555760147 in eval_sub (form=<optimized out>) at ../../../emacs/src/eval.c:2516
        i = <optimized out>
        maxargs = 5
        args_left = 0x0
        numargs = 4
        original_fun = <optimized out>
        original_args = 0x555555eb48f3
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x6, 0x6, 0x2a, 0x56, 0x0, 0x1000000010, 0x1000000003, 0xe00000010}
        retry = <optimized out>
#3  0x000055555576043d in Fprogn (body=0x0) at ../../../emacs/src/eval.c:436
        form = <optimized out>
        val = 0x0
#4  0x000055555575fef7 in eval_sub (form=<optimized out>) at ../../../emacs/src/eval.c:2453
        args_left = 0x555555eb49b3
        numargs = 7
        original_fun = 0xd650
        original_args = 0x555555eb49b3
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x555555eae440, 0x7ffff1a7e708, 0x7ffff1a7e708, 0x555555d7e780 <lispsym>, 0x1, 0x555555748739 <find_symbol_value+185>, 0x2aaa9bdc2b50, 0x555555748779 <Fsymbol_value+9>}
        retry = <optimized out>
#5  0x0000555555762167 in Funwind_protect (args=0x555555eb4a73) at ../../../emacs/src/lisp.h:1523
        val = <optimized out>
        count = <optimized out>
#6  0x000055555575fef7 in eval_sub (form=<optimized out>) at ../../../emacs/src/eval.c:2453
        args_left = 0x555555eb4a73
        numargs = 2
        original_fun = 0x2aaa9bd25680
        original_args = 0x555555eb4a73
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x555555ed1175, 0x0, 0x7fffffffdbc0, 0x30, 0x2, 0x7fffffffdb50, 0x460, 0x55555576164c <apply_lambda+204>}
        retry = <optimized out>
#7  0x000055555576043d in Fprogn (body=0x0, body@entry=0x555555eb4bd3) at ../../../emacs/src/eval.c:436
        form = <optimized out>
        val = 0x555555ed1175
#8  0x000055555574cae7 in Fsave_current_buffer (args=0x555555eb4bd3) at ../../../emacs/src/editfns.c:875
        count = <optimized out>
#9  0x000055555575fef7 in eval_sub (form=<optimized out>) at ../../../emacs/src/eval.c:2453
        args_left = 0x555555eb4bd3
        numargs = 2
        original_fun = 0x2aaa9bd255c0
        original_args = 0x555555eb4bd3
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x7ffff1a7e70d, 0x55555575c1f2 <unbind_to+594>, 0x555555eb4000, 0x55555576a3a4 <Fnconc+244>, 0xb, 0xd500, 0x0, 0x7ffff1a7e70d}
        retry = <optimized out>
#10 0x0000555555761be5 in Fprogn (body=0x0) at ../../../emacs/src/eval.c:436
        form = <optimized out>
        val = 0x0
        val = <optimized out>
        form = <optimized out>
#11 Flet (args=0x555555eb40a3) at ../../../emacs/src/eval.c:1026
        temps = 0x7fffffffdc80
        tem = <optimized out>
        lexenv = 0x0
        elt = <optimized out>
        count = {
          bytes = 1120
        }
        argnum = <optimized out>
        sa_avail = <optimized out>
        sa_count = {
          bytes = 1120
        }
        varlist = <optimized out>
        varlist_len = <optimized out>
        nvars = <optimized out>
#12 0x000055555575fef7 in eval_sub (form=<optimized out>) at ../../../emacs/src/eval.c:2453
        args_left = 0x555555eb40a3
        numargs = 2
        original_fun = 0xa560
        original_args = 0x555555eb40a3
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x0, 0x555555786954 <readevalloop_eager_expand_eval+340>, 0x29, 0x1, 0x2aaa9c0d7690, 0x555555eb4ae3, 0x30, 0x7fffffffde00}
        retry = <optimized out>
#13 0x000055555578e869 in readevalloop (readcharfun=readcharfun@entry=0x555555ec6f8d, infile0=infile0@entry=0x0, sourcename=sourcename@entry=0x555555ecfd14, printflag=printflag@entry=false, unibyte=unibyte@entry=0x0, readfun=readfun@entry=0x0, start=0x0, end=<optimized out>) at ../../../emacs/src/lread.c:2348
        count1 = <optimized out>
        c = <optimized out>
        val = 0x555555eb4e13
        count = <optimized out>
        b = <optimized out>
        continue_reading_p = true
        lex_bound = <optimized out>
        whole_buffer = true
        first_sexp = <optimized out>
        macroexpand = 0x2aaa9c0d7690
#14 0x000055555578f967 in Feval_buffer (buffer=<optimized out>, printflag=0x0, filename=0x555555ecfd14, unibyte=0x0, do_allow_print=<optimized out>) at ../../../emacs/src/lread.c:2421
        count = <optimized out>
        tem = <optimized out>
        buf = 0x555555ec6f8d
#15 0x00005555557a4e92 in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at ../../../emacs/src/bytecode.c:809
        call_nargs = 5
        call_fun = <optimized out>
        count1 = <optimized out>
        template = <optimized out>
        val = <optimized out>
        call_args = 0x7ffff11ff290
        original_fun = 0x2aaa9bfe9d40
        bytecode = <optimized out>
        op = 5
        type = <optimized out>
        targets = {0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a527d <exec_byte_code+1981>, 0x5555557a5278 <exec_byte_code+1976>, 0x5555557a5273 <exec_byte_code+1971>, 0x5555557a4c8e <exec_byte_code+462>, 0x5555557a4c8e <exec_byte_code+462>, 0x5555557a523f <exec_byte_code+1919>, 0x5555557a520b <exec_byte_code+1867>, 0x5555557a59e1 <exec_byte_code+3873>, 0x5555557a59dc <exec_byte_code+3868>, 0x5555557a59d7 <exec_byte_code+3863>, 0x5555557a59d2 <exec_byte_code+3858>, 0x5555557a4cbd <exec_byte_code+509>, 0x5555557a4cc0 <exec_byte_code+512>, 0x5555557a59c4 <exec_byte_code+3844>, 0x5555557a59e6 <exec_byte_code+3878>, 0x5555557a5a71 <exec_byte_code+4017>, 0x5555557a5a6c <exec_byte_code+4012>, 0x5555557a5a67 <exec_byte_code+4007>, 0x5555557a5a62 <exec_byte_code+4002>, 0x5555557a4c1a <exec_byte_code+346>, 0x5555557a4c20 <exec_byte_code+352>, 0x5555557a5a46 <exec_byte_code+3974>, 0x5555557a5a54 <exec_byte_code+3988>, 0x5555557a59f9 <exec_byte_code+3897>, 0x5555557a59f4 <exec_byte_code+3892>, 0x5555557a5fb5 <exec_byte_code+5365>, 0x5555557a5fb0 <exec_byte_code+5360>, 0x5555557a4f1a <exec_byte_code+1114>, 0x5555557a4f20 <exec_byte_code+1120>, 0x5555557a5a0c <exec_byte_code+3916>, 0x5555557a59fe <exec_byte_code+3902>, 0x5555557a5f8f <exec_byte_code+5327>, 0x5555557a5f8a <exec_byte_code+5322>, 0x5555557a5f85 <exec_byte_code+5317>, 0x5555557a5f80 <exec_byte_code+5312>, 0x5555557a4d2b <exec_byte_code+619>, 0x5555557a4d30 <exec_byte_code+624>, 0x5555557a5fa2 <exec_byte_code+5346>, 0x5555557a5f94 <exec_byte_code+5332>, 0x5555557a5f5f <exec_byte_code+5279>, 0x5555557a5f5a <exec_byte_code+5274>, 0x5555557a5f55 <exec_byte_code+5269>, 0x5555557a5f50 <exec_byte_code+5264>, 0x5555557a4f63 <exec_byte_code+1187>, 0x5555557a4f68 <exec_byte_code+1192>, 0x5555557a5f72 <exec_byte_code+5298>, 0x5555557a5f64 <exec_byte_code+5284>, 0x5555557a5bc9 <exec_byte_code+4361>, 0x5555557a5bf6 <exec_byte_code+4406>, 0x5555557a5c60 <exec_byte_code+4512>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a6ddf <exec_byte_code+8991>, 0x5555557a6d73 <exec_byte_code+8883>, 0x5555557a6d37 <exec_byte_code+8823>, 0x5555557a6cfb <exec_byte_code+8763>, 0x5555557a6cbd <exec_byte_code+8701>, 0x5555557a5ae3 <exec_byte_code+4131>, 0x5555557a5aaa <exec_byte_code+4074>, 0x5555557a6c92 <exec_byte_code+8658>, 0x5555557a5b95 <exec_byte_code+4309>, 0x5555557a5a76 <exec_byte_code+4022>, 0x5555557a6c59 <exec_byte_code+8601>, 0x5555557a6c30 <exec_byte_code+8560>, 0x5555557a6bf7 <exec_byte_code+8503>, 0x5555557a6bc1 <exec_byte_code+8449>, 0x5555557a6b87 <exec_byte_code+8391>, 0x5555557a6b22 <exec_byte_code+8290>, 0x5555557a6ab4 <exec_byte_code+8180>, 0x5555557a6a3f <exec_byte_code+8063>, 0x5555557a6a16 <exec_byte_code+8022>, 0x5555557a69ed <exec_byte_code+7981>, 0x5555557a69b4 <exec_byte_code+7924>, 0x5555557a697b <exec_byte_code+7867>, 0x5555557a6942 <exec_byte_code+7810>, 0x5555557a6905 <exec_byte_code+7749>, 0x5555557a68d2 <exec_byte_code+7698>, 0x5555557a689f <exec_byte_code+7647>, 0x5555557a686c <exec_byte_code+7596>, 0x5555557a67db <exec_byte_code+7451>, 0x5555557a6786 <exec_byte_code+7366>, 0x5555557a673c <exec_byte_code+7292>, 0x5555557a66ef <exec_byte_code+7215>, 0x5555557a66a2 <exec_byte_code+7138>, 0x5555557a6655 <exec_byte_code+7061>, 0x5555557a6608 <exec_byte_code+6984>, 0x5555557a65b7 <exec_byte_code+6903>, 0x5555557a6561 <exec_byte_code+6817>, 0x5555557a6510 <exec_byte_code+6736>, 0x5555557a64bf <exec_byte_code+6655>, 0x5555557a646e <exec_byte_code+6574>, 0x5555557a641c <exec_byte_code+6492>, 0x5555557a6340 <exec_byte_code+6272>, 0x5555557a4fa9 <exec_byte_code+1257>, 0x5555557a6317 <exec_byte_code+6231>, 0x5555557a62e9 <exec_byte_code+6185>, 0x5555557a626a <exec_byte_code+6058>, 0x5555557a6227 <exec_byte_code+5991>, 0x5555557a61fe <exec_byte_code+5950>, 0x5555557a61d3 <exec_byte_code+5907>, 0x5555557a61a8 <exec_byte_code+5864>, 0x5555557a6175 <exec_byte_code+5813>, 0x5555557a614a <exec_byte_code+5770>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a611f <exec_byte_code+5727>, 0x5555557a60f4 <exec_byte_code+5684>, 0x5555557a60c9 <exec_byte_code+5641>, 0x5555557a609e <exec_byte_code+5598>, 0x5555557a6073 <exec_byte_code+5555>, 0x5555557a604a <exec_byte_code+5514>, 0x5555557a4fa9 <exec_byte_code+1257>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a600c <exec_byte_code+5452>, 0x5555557a5fe3 <exec_byte_code+5411>, 0x5555557a5fba <exec_byte_code+5370>, 0x5555557a58cb <exec_byte_code+3595>, 0x5555557a5892 <exec_byte_code+3538>, 0x5555557a5869 <exec_byte_code+3497>, 0x5555557a5840 <exec_byte_code+3456>, 0x5555557a5807 <exec_byte_code+3399>, 0x5555557a57ce <exec_byte_code+3342>, 0x5555557a5795 <exec_byte_code+3285>, 0x5555557a576a <exec_byte_code+3242>, 0x5555557a5741 <exec_byte_code+3201>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5d50 <exec_byte_code+4752>, 0x5555557a5ee1 <exec_byte_code+5153>, 0x5555557a598a <exec_byte_code+3786>, 0x5555557a5ea7 <exec_byte_code+5095>, 0x5555557a5e70 <exec_byte_code+5040>, 0x5555557a5e39 <exec_byte_code+4985>, 0x5555557a5da2 <exec_byte_code+4834>, 0x5555557a5d84 <exec_byte_code+4804>, 0x5555557a5a1a <exec_byte_code+3930>, 0x5555557a5d32 <exec_byte_code+4722>, 0x5555557a5cd6 <exec_byte_code+4630>, 0x5555557a5ca8 <exec_byte_code+4584>, 0x5555557a5c68 <exec_byte_code+4520>, 0x5555557a6efb <exec_byte_code+9275>, 0x5555557a6ebe <exec_byte_code+9214>, 0x5555557a6e7b <exec_byte_code+9147>, 0x5555557a6e25 <exec_byte_code+9061>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5704 <exec_byte_code+3140>, 0x5555557a56db <exec_byte_code+3099>, 0x5555557a56b2 <exec_byte_code+3058>, 0x5555557a5689 <exec_byte_code+3017>, 0x5555557a5660 <exec_byte_code+2976>, 0x5555557a5627 <exec_byte_code+2919>, 0x5555557a55ee <exec_byte_code+2862>, 0x5555557a55b5 <exec_byte_code+2805>, 0x5555557a557c <exec_byte_code+2748>, 0x5555557a552f <exec_byte_code+2671>, 0x5555557a54f6 <exec_byte_code+2614>, 0x5555557a54bd <exec_byte_code+2557>, 0x5555557a5497 <exec_byte_code+2519>, 0x5555557a543c <exec_byte_code+2428>, 0x5555557a53e1 <exec_byte_code+2337>, 0x5555557a53ae <exec_byte_code+2286>, 0x5555557a537b <exec_byte_code+2235>, 0x5555557a534b <exec_byte_code+2187>, 0x5555557a63cb <exec_byte_code+6411>, 0x5555557a6383 <exec_byte_code+6339>, 0x5555557a52e5 <exec_byte_code+2085>, 0x5555557a5282 <exec_byte_code+1986>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a6b4b <exec_byte_code+8331>, 0x5555557a6830 <exec_byte_code+7536>, 0x5555557a62ad <exec_byte_code+6125>, 0x5555557a51cf <exec_byte_code+1807>, 0x5555557a5193 <exec_byte_code+1747>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5164 <exec_byte_code+1700>, 0x5555557a5934 <exec_byte_code+3700>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5904 <exec_byte_code+3652> <repeats 64 times>}
        quitcounter = 1 '\001'
        bc = 0x555555cf3e70 <main_thread+496>
        top = <optimized out>
        pc = <optimized out>
        bytestr = <optimized out>
        vector = <optimized out>
        maxdepth = <optimized out>
        const_length = <optimized out>
        bytestr_length = <optimized out>
        vectorp = 0x7ffff1d685c8
        max_stack = <optimized out>
        frame_base = <optimized out>
        fp = <optimized out>
        bytestr_data = <optimized out>
        rest = <optimized out>
        mandatory = <optimized out>
        nonrest = <optimized out>
        pushedargs = <optimized out>
        result = <optimized out>
#16 0x000055555575c626 in Ffuncall (nargs=nargs@entry=5, args=args@entry=0x7fffffffe040) at ../../../emacs/src/eval.c:2999
        count = <optimized out>
        val = <optimized out>
#17 0x000055555578f5ed in call4 (arg4=0x30, arg3=<optimized out>, arg2=0x555555ecfd14, arg1=<optimized out>, fn=<optimized out>) at ../../../emacs/src/lisp.h:3270
No locals.
#18 Fload (file=0x555555ecfa34, noerror=<optimized out>, nomessage=<optimized out>, nosuffix=<optimized out>, must_suffix=<optimized out>) at ../../../emacs/src/lread.c:1484
        val = <optimized out>
        stream = 0x0
        fd = 4
        fd_index = <optimized out>
        count = <optimized out>
        found = 0x555555ecfd14
        efound = <optimized out>
        hist_file_name = 0x555555ecfd14
        newer = false
        compiled = false
        handler = <optimized out>
        fmode = 0x555555834a9b "r"
        version = <optimized out>
        no_native = <optimized out>
        is_module = false
        is_native_elisp = false
        found_eff = <optimized out>
        is_elc = false
        input = {
          stream = 0x555555ecfa34,
          lookahead = 96 '`',
          buf = "\001\000\000"
        }
#19 0x00005555557a4e92 in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at ../../../emacs/src/bytecode.c:809
        call_nargs = 3
        call_fun = <optimized out>
        count1 = <optimized out>
        template = <optimized out>
        val = <optimized out>
        call_args = 0x7ffff11ff1b0
        original_fun = 0xa9b0
        bytecode = <optimized out>
        op = 3
        type = <optimized out>
        targets = {0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a527d <exec_byte_code+1981>, 0x5555557a5278 <exec_byte_code+1976>, 0x5555557a5273 <exec_byte_code+1971>, 0x5555557a4c8e <exec_byte_code+462>, 0x5555557a4c8e <exec_byte_code+462>, 0x5555557a523f <exec_byte_code+1919>, 0x5555557a520b <exec_byte_code+1867>, 0x5555557a59e1 <exec_byte_code+3873>, 0x5555557a59dc <exec_byte_code+3868>, 0x5555557a59d7 <exec_byte_code+3863>, 0x5555557a59d2 <exec_byte_code+3858>, 0x5555557a4cbd <exec_byte_code+509>, 0x5555557a4cc0 <exec_byte_code+512>, 0x5555557a59c4 <exec_byte_code+3844>, 0x5555557a59e6 <exec_byte_code+3878>, 0x5555557a5a71 <exec_byte_code+4017>, 0x5555557a5a6c <exec_byte_code+4012>, 0x5555557a5a67 <exec_byte_code+4007>, 0x5555557a5a62 <exec_byte_code+4002>, 0x5555557a4c1a <exec_byte_code+346>, 0x5555557a4c20 <exec_byte_code+352>, 0x5555557a5a46 <exec_byte_code+3974>, 0x5555557a5a54 <exec_byte_code+3988>, 0x5555557a59f9 <exec_byte_code+3897>, 0x5555557a59f4 <exec_byte_code+3892>, 0x5555557a5fb5 <exec_byte_code+5365>, 0x5555557a5fb0 <exec_byte_code+5360>, 0x5555557a4f1a <exec_byte_code+1114>, 0x5555557a4f20 <exec_byte_code+1120>, 0x5555557a5a0c <exec_byte_code+3916>, 0x5555557a59fe <exec_byte_code+3902>, 0x5555557a5f8f <exec_byte_code+5327>, 0x5555557a5f8a <exec_byte_code+5322>, 0x5555557a5f85 <exec_byte_code+5317>, 0x5555557a5f80 <exec_byte_code+5312>, 0x5555557a4d2b <exec_byte_code+619>, 0x5555557a4d30 <exec_byte_code+624>, 0x5555557a5fa2 <exec_byte_code+5346>, 0x5555557a5f94 <exec_byte_code+5332>, 0x5555557a5f5f <exec_byte_code+5279>, 0x5555557a5f5a <exec_byte_code+5274>, 0x5555557a5f55 <exec_byte_code+5269>, 0x5555557a5f50 <exec_byte_code+5264>, 0x5555557a4f63 <exec_byte_code+1187>, 0x5555557a4f68 <exec_byte_code+1192>, 0x5555557a5f72 <exec_byte_code+5298>, 0x5555557a5f64 <exec_byte_code+5284>, 0x5555557a5bc9 <exec_byte_code+4361>, 0x5555557a5bf6 <exec_byte_code+4406>, 0x5555557a5c60 <exec_byte_code+4512>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a6ddf <exec_byte_code+8991>, 0x5555557a6d73 <exec_byte_code+8883>, 0x5555557a6d37 <exec_byte_code+8823>, 0x5555557a6cfb <exec_byte_code+8763>, 0x5555557a6cbd <exec_byte_code+8701>, 0x5555557a5ae3 <exec_byte_code+4131>, 0x5555557a5aaa <exec_byte_code+4074>, 0x5555557a6c92 <exec_byte_code+8658>, 0x5555557a5b95 <exec_byte_code+4309>, 0x5555557a5a76 <exec_byte_code+4022>, 0x5555557a6c59 <exec_byte_code+8601>, 0x5555557a6c30 <exec_byte_code+8560>, 0x5555557a6bf7 <exec_byte_code+8503>, 0x5555557a6bc1 <exec_byte_code+8449>, 0x5555557a6b87 <exec_byte_code+8391>, 0x5555557a6b22 <exec_byte_code+8290>, 0x5555557a6ab4 <exec_byte_code+8180>, 0x5555557a6a3f <exec_byte_code+8063>, 0x5555557a6a16 <exec_byte_code+8022>, 0x5555557a69ed <exec_byte_code+7981>, 0x5555557a69b4 <exec_byte_code+7924>, 0x5555557a697b <exec_byte_code+7867>, 0x5555557a6942 <exec_byte_code+7810>, 0x5555557a6905 <exec_byte_code+7749>, 0x5555557a68d2 <exec_byte_code+7698>, 0x5555557a689f <exec_byte_code+7647>, 0x5555557a686c <exec_byte_code+7596>, 0x5555557a67db <exec_byte_code+7451>, 0x5555557a6786 <exec_byte_code+7366>, 0x5555557a673c <exec_byte_code+7292>, 0x5555557a66ef <exec_byte_code+7215>, 0x5555557a66a2 <exec_byte_code+7138>, 0x5555557a6655 <exec_byte_code+7061>, 0x5555557a6608 <exec_byte_code+6984>, 0x5555557a65b7 <exec_byte_code+6903>, 0x5555557a6561 <exec_byte_code+6817>, 0x5555557a6510 <exec_byte_code+6736>, 0x5555557a64bf <exec_byte_code+6655>, 0x5555557a646e <exec_byte_code+6574>, 0x5555557a641c <exec_byte_code+6492>, 0x5555557a6340 <exec_byte_code+6272>, 0x5555557a4fa9 <exec_byte_code+1257>, 0x5555557a6317 <exec_byte_code+6231>, 0x5555557a62e9 <exec_byte_code+6185>, 0x5555557a626a <exec_byte_code+6058>, 0x5555557a6227 <exec_byte_code+5991>, 0x5555557a61fe <exec_byte_code+5950>, 0x5555557a61d3 <exec_byte_code+5907>, 0x5555557a61a8 <exec_byte_code+5864>, 0x5555557a6175 <exec_byte_code+5813>, 0x5555557a614a <exec_byte_code+5770>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a611f <exec_byte_code+5727>, 0x5555557a60f4 <exec_byte_code+5684>, 0x5555557a60c9 <exec_byte_code+5641>, 0x5555557a609e <exec_byte_code+5598>, 0x5555557a6073 <exec_byte_code+5555>, 0x5555557a604a <exec_byte_code+5514>, 0x5555557a4fa9 <exec_byte_code+1257>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a600c <exec_byte_code+5452>, 0x5555557a5fe3 <exec_byte_code+5411>, 0x5555557a5fba <exec_byte_code+5370>, 0x5555557a58cb <exec_byte_code+3595>, 0x5555557a5892 <exec_byte_code+3538>, 0x5555557a5869 <exec_byte_code+3497>, 0x5555557a5840 <exec_byte_code+3456>, 0x5555557a5807 <exec_byte_code+3399>, 0x5555557a57ce <exec_byte_code+3342>, 0x5555557a5795 <exec_byte_code+3285>, 0x5555557a576a <exec_byte_code+3242>, 0x5555557a5741 <exec_byte_code+3201>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5d50 <exec_byte_code+4752>, 0x5555557a5ee1 <exec_byte_code+5153>, 0x5555557a598a <exec_byte_code+3786>, 0x5555557a5ea7 <exec_byte_code+5095>, 0x5555557a5e70 <exec_byte_code+5040>, 0x5555557a5e39 <exec_byte_code+4985>, 0x5555557a5da2 <exec_byte_code+4834>, 0x5555557a5d84 <exec_byte_code+4804>, 0x5555557a5a1a <exec_byte_code+3930>, 0x5555557a5d32 <exec_byte_code+4722>, 0x5555557a5cd6 <exec_byte_code+4630>, 0x5555557a5ca8 <exec_byte_code+4584>, 0x5555557a5c68 <exec_byte_code+4520>, 0x5555557a6efb <exec_byte_code+9275>, 0x5555557a6ebe <exec_byte_code+9214>, 0x5555557a6e7b <exec_byte_code+9147>, 0x5555557a6e25 <exec_byte_code+9061>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5704 <exec_byte_code+3140>, 0x5555557a56db <exec_byte_code+3099>, 0x5555557a56b2 <exec_byte_code+3058>, 0x5555557a5689 <exec_byte_code+3017>, 0x5555557a5660 <exec_byte_code+2976>, 0x5555557a5627 <exec_byte_code+2919>, 0x5555557a55ee <exec_byte_code+2862>, 0x5555557a55b5 <exec_byte_code+2805>, 0x5555557a557c <exec_byte_code+2748>, 0x5555557a552f <exec_byte_code+2671>, 0x5555557a54f6 <exec_byte_code+2614>, 0x5555557a54bd <exec_byte_code+2557>, 0x5555557a5497 <exec_byte_code+2519>, 0x5555557a543c <exec_byte_code+2428>, 0x5555557a53e1 <exec_byte_code+2337>, 0x5555557a53ae <exec_byte_code+2286>, 0x5555557a537b <exec_byte_code+2235>, 0x5555557a534b <exec_byte_code+2187>, 0x5555557a63cb <exec_byte_code+6411>, 0x5555557a6383 <exec_byte_code+6339>, 0x5555557a52e5 <exec_byte_code+2085>, 0x5555557a5282 <exec_byte_code+1986>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a6b4b <exec_byte_code+8331>, 0x5555557a6830 <exec_byte_code+7536>, 0x5555557a62ad <exec_byte_code+6125>, 0x5555557a51cf <exec_byte_code+1807>, 0x5555557a5193 <exec_byte_code+1747>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5164 <exec_byte_code+1700>, 0x5555557a5934 <exec_byte_code+3700>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555555a4e6a <exec_byte_code-2096214>, 0x5555557a5904 <exec_byte_code+3652> <repeats 64 times>}
        quitcounter = 38 '&'
        bc = 0x555555cf3e70 <main_thread+496>
        top = <optimized out>
        pc = <optimized out>
        bytestr = <optimized out>
        vector = <optimized out>
        maxdepth = <optimized out>
        const_length = <optimized out>
        bytestr_length = <optimized out>
        vectorp = 0x7ffff1d5bac0
        max_stack = <optimized out>
        frame_base = <optimized out>
        fp = <optimized out>
        bytestr_data = <optimized out>
        rest = <optimized out>
        mandatory = <optimized out>
        nonrest = <optimized out>
        pushedargs = <optimized out>
        result = <optimized out>
#20 0x000055555576164c in apply_lambda (fun=fun@entry=0x7ffff1d4384d, args=<optimized out>, count=count@entry=...) at ../../../emacs/src/eval.c:3107
        arg_vector = 0x7fffffffe1a0
        tem = <optimized out>
        sa_avail = <optimized out>
        sa_count = {
          bytes = 160
        }
        numargs = 0
        args_left = 0x0
#21 0x000055555575fc03 in eval_sub (form=form@entry=0x7ffff2206efb) at ../../../emacs/src/eval.c:2592
        original_fun = 0x2aaa9bfc5098
        original_args = 0x0
        count = <optimized out>
        fun = <optimized out>
        val = <optimized out>
        funcar = <optimized out>
        argvals = {0x12, 0xfffffffffffffb90, 0x90, 0x5, 0x7fffffffe960, 0x7ffff57589fa <malloc+410>, 0x60, 0x555555e8e623}
        retry = <optimized out>
#22 0x0000555555762706 in Feval (form=0x7ffff2206efb, lexical=<optimized out>) at ../../../emacs/src/eval.c:2365
        count = <optimized out>
#23 0x000055555575ad37 in internal_condition_case (bfun=bfun@entry=0x5555556ced40 <top_level_2>, handlers=handlers@entry=0x90, hfun=hfun@entry=0x5555556d5f90 <cmd_error>) at ../../../emacs/src/eval.c:1474
        val = <optimized out>
        c = 0x555555e931d0
#24 0x00005555556cf686 in top_level_1 (ignore=ignore@entry=0x0) at ../../../emacs/src/keyboard.c:1150
No locals.
#25 0x000055555575ac91 in internal_catch (tag=tag@entry=0x10080, func=func@entry=0x5555556cf660 <top_level_1>, arg=arg@entry=0x0) at ../../../emacs/src/eval.c:1197
        val = <optimized out>
        c = 0x555555e74440
#26 0x00005555556cecbf in command_loop () at ../../../emacs/src/keyboard.c:1110
No locals.
#27 0x00005555556d5b41 in recursive_edit_1 () at ../../../emacs/src/keyboard.c:720
        count = <optimized out>
        val = <optimized out>
#28 0x00005555556d5ec0 in Frecursive_edit () at ../../../emacs/src/keyboard.c:803
        count = <optimized out>
        buffer = <optimized out>
#29 0x00005555555a8977 in main (argc=5, argv=0x7fffffffe628) at ../../../emacs/src/emacs.c:2521
        stack_bottom_variable = 0x0
        no_loadup = false
        junk = 0x0
        dname_arg = 0x0
        ch_to_dir = 0x0
        original_pwd = <optimized out>
        dump_mode = <optimized out>
        skip_args = 1
        temacs = 0x0
        attempt_load_pdump = <optimized out>
        only_version = false
        rlim = {
          rlim_cur = 10022912,
          rlim_max = 18446744073709551615
        }
        lc_all = <optimized out>
        sockfd = -1
        module_assertions = <optimized out>

Lisp Backtrace:
"transpose-regions" (0xffffd970)
"progn" (0xffffda30)
"unwind-protect" (0xffffdb00)
"save-current-buffer" (0xffffdbf0)
"let" (0xffffdd20)
"eval-buffer" (0xf11ff290)
"load-with-code-conversion" (0xffffe048)
"load" (0xf11ff1b0)
"command-line-1" (0xf11ff0b8)
"command-line" (0xf11ff040)
"normal-top-level" (0xffffe1a0)
(gdb) quit
Debugger finished

End of gdb session.

In GNU Emacs 29.3.50 (build 5, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.16.0) of 2024-04-01 built on gabor
Repository revision: 946d4aad1dfb244352dfd0845a8bc3078fe9bca4
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Devuan GNU/Linux 5 (daedalus)

Configured using:
 'configure --without-libsystemd --without-pop --without-sound
 --without-gconf --without-mailutils --without-native-compilation
 --with-cairo --with-x=yes --with-x-toolkit=gtk3
 --with-toolkit-scroll-bars'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM
XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: hu_HU.UTF-8
  locale-coding-system: utf-8-unix

Best wishes,

     Gábor

[-- Attachment #2: 0001-transpose-regions-fix-crash-add-tests-for-text-prope.patch --]
[-- Type: text/x-patch, Size: 7711 bytes --]

From 4f27eb41c638270a2c421126382cbb5073986288 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A1bor=20Braun?= <gabor.braun@uni-duisburg-essen.de>
Date: Sun, 31 Mar 2024 12:54:33 +0200
Subject: [PATCH] transpose-regions: fix crash, add tests for text properties.

The tests contain non-ASCII characters and text properties,
and crash previous Emacs versions.

No longer confuse lengths in bytes and characters:
1. For consistent naming split len_mid into len_mid and len_mid_byte.
2. Remove the case len1_byte == len2_byte:
   only one memmove() could be optimized out compared
   to the other cases.
   Even tmp_interval_mid is necessary, as the intervals need position
   adjustments.
   Don't bother with different undo records in this case.
---
 src/editfns.c             | 52 +++++++++-----------------------------
 test/src/editfns-tests.el | 53 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/src/editfns.c b/src/editfns.c
index 85f7739df07..ba589ee497e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4495,6 +4495,7 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
 {
   register ptrdiff_t start1, end1, start2, end2;
   ptrdiff_t start1_byte, start2_byte, len1_byte, len2_byte, end2_byte;
+  ptrdiff_t len_mid_byte;
   ptrdiff_t gap, len1, len_mid, len2;
   unsigned char *start1_addr, *start2_addr, *temp;
 
@@ -4653,42 +4654,10 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
   /* Non-adjacent regions, because end1 != start2, bleagh...  */
   else
     {
-      len_mid = start2_byte - (start1_byte + len1_byte);
+      len_mid_byte = start2_byte - (start1_byte + len1_byte);
+      len_mid = start2 - end1;
 
-      if (len1_byte == len2_byte)
-	/* Regions are same size, though, how nice.  */
-        {
-	  USE_SAFE_ALLOCA;
-
-          modify_text (start1, end2);
-          record_change (start1, len1);
-          record_change (start2, len2);
-          tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval2 = copy_intervals (cur_intv, start2, len2);
-
-	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr1, 0);
-	  if (tmp_interval3)
-	    set_text_properties_1 (startr1, endr1, Qnil, buf, tmp_interval3);
-
-	  tmp_interval3 = validate_interval_range (buf, &startr2, &endr2, 0);
-	  if (tmp_interval3)
-	    set_text_properties_1 (startr2, endr2, Qnil, buf, tmp_interval3);
-
-	  temp = SAFE_ALLOCA (len1_byte);
-	  start1_addr = BYTE_POS_ADDR (start1_byte);
-	  start2_addr = BYTE_POS_ADDR (start2_byte);
-          memcpy (temp, start1_addr, len1_byte);
-          memcpy (start1_addr, start2_addr, len2_byte);
-          memcpy (start2_addr, temp, len1_byte);
-	  SAFE_FREE ();
-
-          graft_intervals_into_buffer (tmp_interval1, start2,
-                                       len1, current_buffer, 0);
-          graft_intervals_into_buffer (tmp_interval2, start1,
-                                       len2, current_buffer, 0);
-        }
-
-      else if (len1_byte < len2_byte)	/* Second region larger than first */
+      if (len1_byte < len2_byte)	/* Second region larger than first */
         /* Non-adjacent & unequal size, area between must also be shifted.  */
         {
 	  USE_SAFE_ALLOCA;
@@ -4708,8 +4677,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
 	  start1_addr = BYTE_POS_ADDR (start1_byte);
 	  start2_addr = BYTE_POS_ADDR (start2_byte);
           memcpy (temp, start2_addr, len2_byte);
-          memcpy (start1_addr + len_mid + len2_byte, start1_addr, len1_byte);
-          memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid);
+	  memcpy (start1_addr + len_mid_byte + len2_byte, start1_addr, len1_byte);
+	  memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid_byte);
           memcpy (start1_addr, temp, len2_byte);
 	  SAFE_FREE ();
 
@@ -4721,7 +4690,10 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
                                        len2, current_buffer, 0);
         }
       else
-	/* Second region smaller than first.  */
+	/* Second region not larger than first.  */
+	/* The equal-length case is not simpler,
+	   the position stored in intervals for the text inbetween
+	   still needs adjustment. */
         {
 	  USE_SAFE_ALLOCA;
 
@@ -4742,8 +4714,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
 	  start2_addr = BYTE_POS_ADDR (start2_byte);
           memcpy (temp, start1_addr, len1_byte);
           memcpy (start1_addr, start2_addr, len2_byte);
-          memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid);
-          memcpy (start1_addr + len2_byte + len_mid, temp, len1_byte);
+	  memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid_byte);
+	  memcpy (start1_addr + len2_byte + len_mid_byte, temp, len1_byte);
 	  SAFE_FREE ();
 
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index b3b7da65ad3..82f0bcd1e71 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -132,6 +132,59 @@ propertize/error-even-number-of-args
   "Number of args for `propertize' must be odd."
   (should-error (propertize "foo" 'bar) :type 'wrong-number-of-arguments))
 
+;; Tests for `transpose-region'
+
+(ert-deftest transpose-regions-text-properties2 ()
+  "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")))))
+
 ;; Tests for bug#5131.
 (defun transpose-test-reverse-word (start end)
   "Reverse characters in a word by transposing pairs of characters."
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-01 10:02 bug#70122: 29.3.50; transpose-regions can crash Emacs Braun Gábor
@ 2024-04-01 11:55 ` Eli Zaretskii
  2024-04-01 13:17   ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-04-01 11:55 UTC (permalink / raw)
  To: Braun Gábor; +Cc: 70122

> From: Braun Gábor <braungb88@gmail.com>
> Date: Mon, 01 Apr 2024 12:02:35 +0200
> 
> I have the following file:
> 
> File ~/.emacs.d/trans-regions-bug.el:
> 
> (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))
> 
> End of file ~/.emacs.d/trans-regions-bug.el:
> 
> 
> After building Emacs as described in the INSTALL file, the following
> command crashes Emacs:
> 
>   src/emacs -Q --batch -l ~/.emacs.d/trans-regions-bug.el
> 
> The following appears:
> 
> Fatal error 11: Segmentation fault
> 
> 
> See below for debugging information.
> 
> Please find attached a patch to transpose-regions
> with added tests.  (The patch intends to fix typos:
> makes sure lengths in bytes and characters are not confused.

Thanks, but could you please show the minimal change required to fix
just the particular problem with this scenario (and perhaps explain
the reason for the crash in words), without any cleanup and
typo/confusion fixes?  That would make it easier to review the patch,
whereas with what you sent, it is hard to understand what exactly is
being fixed.

> One case (likely for optimization only) has been removed,
> seemed too much trouble to get it right.)

If you explain the reason for the crash, perhaps we could leave the
optimization alone.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-01 11:55 ` Eli Zaretskii
@ 2024-04-01 13:17   ` Eli Zaretskii
  2024-04-03 18:52     ` Braun Gábor
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-04-01 13:17 UTC (permalink / raw)
  To: braungb88; +Cc: 70122

> Cc: 70122@debbugs.gnu.org
> Date: Mon, 01 Apr 2024 14:55:21 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Please find attached a patch to transpose-regions
> > with added tests.  (The patch intends to fix typos:
> > makes sure lengths in bytes and characters are not confused.
> 
> Thanks, but could you please show the minimal change required to fix
> just the particular problem with this scenario (and perhaps explain
> the reason for the crash in words), without any cleanup and
> typo/confusion fixes?  That would make it easier to review the patch,
> whereas with what you sent, it is hard to understand what exactly is
> being fixed.

I came up with the patch below.  The problem is that len_mid, which is
a difference between byte positions, was sometimes used in calls that
expect differences between character positions instead.  The patch
below passes both your test and the already-existing tests in
test/src/editfns-tests.el.  WDYT?

> > One case (likely for optimization only) has been removed,
> > seemed too much trouble to get it right.)
> 
> If you explain the reason for the crash, perhaps we could leave the
> optimization alone.

I found that len_mid was not used in that branch, so we can leave it
alone.

Here's the patch:

diff --git a/src/editfns.c b/src/editfns.c
index 4ccf765..124fd47 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4677,7 +4677,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
           record_change (start1, (end2 - start1));
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+          tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - (start1 + len1));
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4697,7 +4698,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       len_mid, current_buffer, 0);
+                                       start2 - (start1 + len1),
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }
@@ -4710,7 +4712,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
 
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+          tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - (start1 + len1));
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4730,7 +4733,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       len_mid, current_buffer, 0);
+                                       start2 - (start1 + len1),
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-01 13:17   ` Eli Zaretskii
@ 2024-04-03 18:52     ` Braun Gábor
  2024-04-04  4:48       ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Braun Gábor @ 2024-04-03 18:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70122

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]

Hi Eli,

> Thanks, but could you please show the minimal change required to 
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 == len2_byte
assumes len1 == len2 in several places:
undo records, positions after the transposition,
and the least obvious one is that even intervals between the 
region seem to need adjustment of positions.
(I don't know enough of intervals to understand it, but had failed 
tests with text properties at wrong places.)

Anyway, I've just added len1 == 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.

For 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 
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ábor

[-- Attachment #2: 0001-transpose-regions-fix-wrong-lengths-add-missing-cond.patch --]
[-- Type: text/x-patch, Size: 8365 bytes --]

From 5a0cf2542388104cde444b2130d994ee36ce5392 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A1bor=20Braun?= <gabor.braun@uni-duisburg-essen.de>
Date: Wed, 3 Apr 2024 20:03:40 +0200
Subject: [PATCH] transpose-regions: fix wrong lengths; add missing condition.

Case len1_byte == len2_byte:
  The code assumes len1 == 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.
---
 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
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4655,12 +4655,13 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
     {
       len_mid = start2_byte - (start1_byte + len1_byte);
 
-      if (len1_byte == len2_byte)
+      if (len1_byte == len2_byte && len1 == len2)
 	/* Regions are same size, though, how nice.  */
         {
 	  USE_SAFE_ALLOCA;
 
           modify_text (start1, end2);
+	  /* The undo records are only correct for len1 == len2. */
           record_change (start1, len1);
           record_change (start2, len2);
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
@@ -4682,10 +4683,14 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           memcpy (start2_addr, temp, len1_byte);
 	  SAFE_FREE ();
 
+	  /* In the following line start2 is only correct for
+	     len1 == 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 == len2. */
         }
 
       else if (len1_byte < len2_byte)	/* Second region larger than first */
@@ -4696,7 +4701,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
           record_change (start1, (end2 - start1));
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+	  tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - end1);
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4716,7 +4722,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       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, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
 
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+	  tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - end1);
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4749,7 +4757,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       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
--- 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))
 
+;; 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ŐT" :test 0))
+         (string2 (propertize "f\x2013nd" :test 2)))
+    (cl-assert (= (length string1) (length string2)))
+    (cl-assert (= (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."
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-03 18:52     ` Braun Gábor
@ 2024-04-04  4:48       ` Eli Zaretskii
  2024-04-12  9:39         ` Braun Gábor
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-04-04  4:48 UTC (permalink / raw)
  To: Braun Gábor; +Cc: 70122

> From: Braun Gábor <braungb88@gmail.com>
> Cc: 70122@debbugs.gnu.org
> Date: Wed, 03 Apr 2024 20:52:47 +0200
> 
> 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 == len2_byte
> assumes len1 == len2 in several places:
> undo records, positions after the transposition,
> and the least obvious one is that even intervals between the 
> region seem to need adjustment of positions.

Can we lift that restriction by augmenting the len1_byte == len2_byte
branch so that the len1 == len2 condition is not needed?

> (I don't know enough of intervals to understand it, but had failed 
> tests with text properties at wrong places.)

Intervals work with character positions, that's all.  Any call to
functions that examine or modify intervals should use character
positions, not byte positions, and accordingly length in terms of
characters, not bytes.  Is this sufficient for you to come up with
changes to remove the len1 == len2 restriction from that branch?

> Anyway, I've just added len1 == len2 as a condition to that branch
> with comments where I think the assumption is used.

If we cannot easily lift that restriction, that is the right fallback,
yes.

> I've also added a new test for this case.

Thanks.

> > The patch
> > below passes both your test and the already-existing tests in
> > test/src/editfns-tests.el.
> 
> For 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 
> fault
> make[1]: Leaving directory "/home/gabor/src/build/emacs-29.3/test"
> make: *** [Makefile:247: src/editfns-tests] Error 2

This says the test that crashed was the one of the two new ones you
added, and it is different from the one you presented at the beginning
of this bug report.  So it is a small wonder I didn't see any crash in
my testing.

> 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.

Can we also have a test where the byte lengths are equal, but the
character lengths aren't?

> I hoped that byte length is not system dependent,

It isn't, not if you consider the internal representation of
characters in Emacs buffers and strings.

> So what differences exist or byte length?

I don't think there are differences, but if you show an example where
you see such a difference, we can discuss the example and see what
happens there.

Last, but not least: with the added tests your patch becomes larger
than what we can accept without your assigning the copyright to the
FSF.  Would you like to start the legal paperwork of copyright
assignment at this time, so that we could accept all of your
contributions without limitations, including any future contributions?
If yes, I will send you the form to fill and instructions to email the
filled form, to start the process.

Thanks.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-04  4:48       ` Eli Zaretskii
@ 2024-04-12  9:39         ` Braun Gábor
  2024-04-12  9:42           ` Braun Gábor
  2024-04-13 10:34           ` Eli Zaretskii
  0 siblings, 2 replies; 11+ messages in thread
From: Braun Gábor @ 2024-04-12  9:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70122

Hi Eli,

Sorry for answering late.

> Can we lift that restriction by augmenting the len1_byte == 
len2_byte
> branch so that the len1 == len2 condition is not needed?

I could only come up with one that has minimal difference to
to the other branches.  I've attached it.

I've tried to preserve the undo entries as changes in the two 
regions, but I couldn't make it (one of my tests failed),
so now it is a change in the large region as in the other 
branches, and the tests pass.

The issue I was unable to solve is that the functions
set_text_properties_1 and graft_intervals_into_buffer
record text property changes in undo history, but this is unwanted 
here as transpose-regions handles undo history itself.
These entries don't cause trouble because they happen to be 
followed by a deletion of the text where properties change,
and this applies to all branches of transpose-regions.

I'd really like to use a version of these functions with: "change 
text properties, but leave it to us to record it in undo history".

 
> This says the test that crashed was the one of the two new ones 
you
> added, and it is different from the one you presented at the 
beginning
> of this bug report.

OK, sorry for that.

> > 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.
> 
> Can we also have a test where the byte lengths are equal, but 
the
> character lengths aren't?

There is already one there:
the test which loops over examples and adds characters
is meant to cover various cases for the byte lengths,
including when they are equal.

As mentioned above, 
I had this test failing in this case during patch development.

> > I hoped that byte length is not system dependent,
> 
> It isn't, not if you consider the internal representation of
> characters in Emacs buffers and strings.

OK, maybe the issue was that I've sent a different test,
as you mentioned.

> Last, but not least: with the added tests your patch becomes 
larger
> than what we can accept without your assigning the copyright to 
the
> FSF.  Would you like to start the legal paperwork of copyright
> assignment at this time

Let's start the paperwork.

Best wishes,

	Gábor Braun








^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-12  9:39         ` Braun Gábor
@ 2024-04-12  9:42           ` Braun Gábor
  2024-04-13 10:34           ` Eli Zaretskii
  1 sibling, 0 replies; 11+ messages in thread
From: Braun Gábor @ 2024-04-12  9:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70122

[-- Attachment #1: Type: text/plain, Size: 120 bytes --]

Hi,

Sorry, I've forgotten to attach the new version of the patch
in the previous email.

Best wishes,

	Gábor

[-- Attachment #2: 0001-transpose-regions-fix-wrong-lengths-and-case-of-equa.patch --]
[-- Type: text/x-patch, Size: 9185 bytes --]

From fd7f6b9c63f3989863dc789e76fc1dd85f0ab630 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A1bor=20Braun?= <gabor.braun@uni-duisburg-essen.de>
Date: Wed, 3 Apr 2024 20:03:40 +0200
Subject: [PATCH] transpose-regions: fix wrong lengths and case of equal
 length.

Case len1_byte == len2_byte:
  Adjust everything assuming len1 == len2:
  1. Adjust intervals even between the two regions,
     as positions may change.
  2. undo entries: make a simple replacement change as in the other
     branches instead of separately on the two regions.
     This is to work around
     set_text_properties_1 and
     graft_intervals_into_buffer
     recording text property changes in undo history
     (which is unwanted here).
  3. Adjust positions and position differences which rely
     on len1 == len2.

Replace len_mid with end1 - start2 where number of characters
is expected instead of number of bytes.
---
 src/editfns.c             | 35 ++++++++++-------
 test/src/editfns-tests.el | 79 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 14 deletions(-)

diff --git a/src/editfns.c b/src/editfns.c
index 85f7739df07..dd174e8dfcd 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4661,18 +4661,18 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
 	  USE_SAFE_ALLOCA;
 
           modify_text (start1, end2);
-          record_change (start1, len1);
-          record_change (start2, len2);
+	  record_change (start1, (end2 - start1));
+
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval2 = copy_intervals (cur_intv, start2, len2);
+	  /* Intervals in-between need adjustment unless len1 == len2.
+	   */
+	  tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - end1);
+	  tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
-	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr1, 0);
+	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
 	  if (tmp_interval3)
-	    set_text_properties_1 (startr1, endr1, Qnil, buf, tmp_interval3);
-
-	  tmp_interval3 = validate_interval_range (buf, &startr2, &endr2, 0);
-	  if (tmp_interval3)
-	    set_text_properties_1 (startr2, endr2, Qnil, buf, tmp_interval3);
+	    set_text_properties_1 (startr1, endr2, Qnil, buf, tmp_interval3);
 
 	  temp = SAFE_ALLOCA (len1_byte);
 	  start1_addr = BYTE_POS_ADDR (start1_byte);
@@ -4682,8 +4682,11 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           memcpy (start2_addr, temp, len1_byte);
 	  SAFE_FREE ();
 
-          graft_intervals_into_buffer (tmp_interval1, start2,
+	  graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
+	  graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
+				       start2 - end1,
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }
@@ -4696,7 +4699,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
           record_change (start1, (end2 - start1));
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+	  tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - end1);
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4716,7 +4720,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       len_mid, current_buffer, 0);
+				       start2 - end1,
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }
@@ -4729,7 +4734,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
 
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+	  tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - end1);
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4749,7 +4755,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       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
--- 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))
 
+;; 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ŐT" :test 0))
+         (string2 (propertize "f\x2013nd" :test 2)))
+    (cl-assert (= (length string1) (length string2)))
+    (cl-assert (= (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."
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-12  9:39         ` Braun Gábor
  2024-04-12  9:42           ` Braun Gábor
@ 2024-04-13 10:34           ` Eli Zaretskii
  2024-04-16 14:26             ` Braun Gábor
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-04-13 10:34 UTC (permalink / raw)
  To: Braun Gábor; +Cc: 70122

> From: Braun Gábor <braungb88@gmail.com>
> Cc: 70122@debbugs.gnu.org
> Date: Fri, 12 Apr 2024 11:39:34 +0200
> 
> Sorry for answering late.

No sweat.

> > Can we lift that restriction by augmenting the len1_byte ==
> > len2_byte branch so that the len1 == len2 condition is not needed?
> 
> I could only come up with one that has minimal difference to
> to the other branches.  I've attached it.
> 
> I've tried to preserve the undo entries as changes in the two 
> regions, but I couldn't make it (one of my tests failed),
> so now it is a change in the large region as in the other 
> branches, and the tests pass.

That doesn't sound like a serious problem, since the other branches
also do it.

> The issue I was unable to solve is that the functions
> set_text_properties_1 and graft_intervals_into_buffer
> record text property changes in undo history, but this is unwanted 
> here as transpose-regions handles undo history itself.
> These entries don't cause trouble because they happen to be 
> followed by a deletion of the text where properties change,
> and this applies to all branches of transpose-regions.
> 
> I'd really like to use a version of these functions with: "change 
> text properties, but leave it to us to record it in undo history".

But this is not a new problem, right?  The code called
set_text_properties_1 and graft_intervals_into_buffer before the
changes as well, and had the same effect on undo history.  Right?

If this is something caused by these changes, could you please explain
the issue in more detail, with references to the relevant parts of the
code?

> > Last, but not least: with the added tests your patch becomes
> > larger than what we can accept without your assigning the
> > copyright to the FSF.  Would you like to start the legal paperwork
> > of copyright assignment at this time
> 
> Let's start the paperwork.

Will send the form off-list shortly.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-13 10:34           ` Eli Zaretskii
@ 2024-04-16 14:26             ` Braun Gábor
  2024-04-20  7:50               ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Braun Gábor @ 2024-04-16 14:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70122

Hi Eli,

> > The issue I was unable to solve is that the functions
> > set_text_properties_1 and graft_intervals_into_buffer
> > record text property changes in undo history, but this is 
unwanted
> > here as transpose-regions handles undo history itself.
...
> 
> But this is not a new problem, right?  The code called
> set_text_properties_1 and graft_intervals_into_buffer before the
> changes as well, and had the same effect on undo history.  
Right?
> 
> If this is something caused by these changes, could you please 
explain
> the issue in more detail, with references to the relevant parts 
of the
> code?

I don't think it is a new problem, and the other branches have the 
same problem.

In detail, in all branches transpose-regions adds the undo entries 
early for its changes (calls to record_change),
then during making the changes already recorded in undo history
calls functions adding additional undo entries 
(set_text_properties_1, graft_intervals_into_buffer),
i.e. at a time when the buffer state and undo history does not 
match.  Luckily these entries are text property changes to a text 
deleted immediately by the following entries, so their effects are 
not visible, but this is due to the initial undo entries
recording change to a larger span of text than just the two 
swapped regions.

When not just the byte length but the character length of the 
swapped regions are the same, the original code didn't change text 
properties between the two regions, i.e., the text property 
changes were restricted to the swapped regions, and hence
restricting the initial undo entries to these region worked.

The new changes do make text property changes to the text between 
the swapped regions additionally (as interval positions need 
adjustment), which makes it harder for the undo entries,
and that's why I did what the other branches do.


All in all, the issue is not about correctness of code, but rather 
cleanness of code: separate responsibilities: which part of the 
code is responsible for (which) undo entries.

Best wishes,

	Gábor








^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-16 14:26             ` Braun Gábor
@ 2024-04-20  7:50               ` Eli Zaretskii
  2024-04-24 12:35                 ` Braun Gábor
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-04-20  7:50 UTC (permalink / raw)
  To: Braun Gábor; +Cc: 70122

> From: Braun Gábor <braungb88@gmail.com>
> Cc: 70122@debbugs.gnu.org
> Date: Tue, 16 Apr 2024 16:26:46 +0200
> 
> Hi Eli,
> 
> > > The issue I was unable to solve is that the functions
> > > set_text_properties_1 and graft_intervals_into_buffer
> > > record text property changes in undo history, but this is 
> unwanted
> > > here as transpose-regions handles undo history itself.
> ...
> > 
> > But this is not a new problem, right?  The code called
> > set_text_properties_1 and graft_intervals_into_buffer before the
> > changes as well, and had the same effect on undo history.  
> Right?
> > 
> > If this is something caused by these changes, could you please 
> explain
> > the issue in more detail, with references to the relevant parts 
> of the
> > code?
> 
> I don't think it is a new problem, and the other branches have the 
> same problem.
> 
> In detail, in all branches transpose-regions adds the undo entries 
> early for its changes (calls to record_change),
> then during making the changes already recorded in undo history
> calls functions adding additional undo entries 
> (set_text_properties_1, graft_intervals_into_buffer),
> i.e. at a time when the buffer state and undo history does not 
> match.  Luckily these entries are text property changes to a text 
> deleted immediately by the following entries, so their effects are 
> not visible, but this is due to the initial undo entries
> recording change to a larger span of text than just the two 
> swapped regions.
> 
> When not just the byte length but the character length of the 
> swapped regions are the same, the original code didn't change text 
> properties between the two regions, i.e., the text property 
> changes were restricted to the swapped regions, and hence
> restricting the initial undo entries to these region worked.
> 
> The new changes do make text property changes to the text between 
> the swapped regions additionally (as interval positions need 
> adjustment), which makes it harder for the undo entries,
> and that's why I did what the other branches do.
> 
> 
> All in all, the issue is not about correctness of code, but rather 
> cleanness of code: separate responsibilities: which part of the 
> code is responsible for (which) undo entries.

Did you receive the copyright assignment form, and if so, did you fill
and email it according to instructions?  I'm waiting for the
assignment paperwork to come to completion before installing these
changes.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#70122: 29.3.50; transpose-regions can crash Emacs
  2024-04-20  7:50               ` Eli Zaretskii
@ 2024-04-24 12:35                 ` Braun Gábor
  0 siblings, 0 replies; 11+ messages in thread
From: Braun Gábor @ 2024-04-24 12:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70122

Hi Eli,

> Did you receive the copyright assignment form, and if so, did you 
fill
> and email it according to instructions?  I'm waiting for the
> assignment paperwork to come to completion before installing these
> changes.

I received an initial email, and answered it, but after that I 
haven't received anything further.
I haven't received any form to fill out or sign.

Best wishes,

	Gábor










^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-04-24 12:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-01 10:02 bug#70122: 29.3.50; transpose-regions can crash Emacs Braun Gábor
2024-04-01 11:55 ` Eli Zaretskii
2024-04-01 13:17   ` Eli Zaretskii
2024-04-03 18:52     ` Braun Gábor
2024-04-04  4:48       ` Eli Zaretskii
2024-04-12  9:39         ` Braun Gábor
2024-04-12  9:42           ` Braun Gábor
2024-04-13 10:34           ` Eli Zaretskii
2024-04-16 14:26             ` Braun Gábor
2024-04-20  7:50               ` Eli Zaretskii
2024-04-24 12:35                 ` Braun Gábor

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).