GNU bug report logs - #61504
29.0.60; executing byte-code from previous build causes SIGSEGV crash

Previous Next

Package: emacs;

Reported by: Istvan Marko <mi-ebugs <at> kismala.com>

Date: Tue, 14 Feb 2023 06:35:02 UTC

Severity: normal

Found in version 29.0.60

To reply to this bug, email your comments to 61504 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 06:35:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Istvan Marko <mi-ebugs <at> kismala.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 14 Feb 2023 06:35:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Istvan Marko <mi-ebugs <at> kismala.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.60; executing byte-code from previous build causes SIGSEGV crash
Date: Mon, 13 Feb 2023 22:33:54 -0800
There seems to be a byte-code change between versions
0ec0a610ed226419269f519021cbe8fb2dde2ed5 (old) and
a4aa32bdfff7aaf54efbacbb04b7f2b52fef92a7 (new) which causes the new
version to crash with SIGSEGV when executing certain code from an .elc
built using the old version. Recompiling the file with the new version
causes the new .elc to work correctly.

I am able to reproduce this by calling (pdf-tools-install-noverify) from
the pdf-tools.elc (compiled with the older emacs version) from the
pdf-tools package which is available at
https://github.com/politza/pdf-tools

This snippet in particular triggers the crash:

  (dolist (buf (buffer-list))
    ;; This when check should not be necessary, but somehow dead
    ;; buffers are showing up here. See
    ;; https://github.com/vedang/pdf-tools/pull/93
    (when (buffer-live-p buf)
      (with-current-buffer buf
        (when (and (not (derived-mode-p 'pdf-view-mode))
                   (pdf-tools-pdf-buffer-p)
                   (buffer-file-name))
          (pdf-view-mode)))))

The crash happens in the pdf-tools-pdf-buffer-p function:

(defun pdf-tools-pdf-buffer-p (&optional buffer)
  "Check if the current buffer is a PDF document.

Optionally, take BUFFER as an argument and check if it is a PDF document."
  (save-current-buffer
    (when buffer (set-buffer buffer))
    (save-excursion
      (save-restriction
        (widen)
        (goto-char 1)
        (looking-at "%PDF")))))

I can try to create a smaller standalone reproducer if needed.

backtrace:

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
0x00001555529d17a7 in free () from /lib64/libc.so.6

(gdb) bt
#0  0x00001555529d17a7 in free () from /lib64/libc.so.6
#1  0x00000000001e66bc in ?? ()
#2  0x00005555557693d4 in safe_free (sa_count=...) at /mnt/sdc1/tmp/emacs/src/lisp.h:5385
#3  apply_lambda (fun=0x555557654ccd, args=<optimized out>, count=...) at eval.c:3109
#4  0x00005555557679e6 in eval_sub (form=<optimized out>) at eval.c:2588
#5  0x000055555576800d in Fprogn (body=0x555557678033) at eval.c:436
#6  0x0000555555767cf2 in eval_sub (form=<optimized out>) at eval.c:2449
#7  0x000055555576a2b9 in internal_lisp_condition_case (var=0x927910, bodyform=0x555557677753, handlers=<optimized out>) at eval.c:1428
#8  0x0000555555767cf2 in eval_sub (form=<optimized out>) at eval.c:2449
#9  0x00005555557688bd in Fprogn (body=0x0) at eval.c:436
#10 Fif (args=<optimized out>) at eval.c:392
#11 Fif (args=<optimized out>) at eval.c:378
#12 0x0000555555767cf2 in eval_sub (form=<optimized out>) at eval.c:2449
#13 0x000055555576800d in Fprogn (body=0x5555576b4ee3) at eval.c:436
#14 0x0000555555767cf2 in eval_sub (form=<optimized out>) at eval.c:2449
#15 0x000055555576a2b9 in internal_lisp_condition_case (var=0x927910, bodyform=0x5555576ad993, handlers=<optimized out>) at eval.c:1428
#16 0x0000555555767cf2 in eval_sub (form=<optimized out>) at eval.c:2449
#17 0x000055555578f800 in readevalloop_eager_expand_eval (val=0x0, macroexpand=0xffffbffff97a1430) at /mnt/sdc1/tmp/emacs/src/lisp.h:1516
#18 0x0000555555797a7b in readevalloop (readcharfun=0x555555f7ecad, infile0=0x0, sourcename=0x55555618c784, printflag=false, unibyte=<optimized out>, readfun=0x0, start=0x0,
    end=0x0) at lread.c:2347
#19 0x0000555555798cfc in Feval_buffer (buffer=<optimized out>, printflag=0x0, filename=0x55555618c784, unibyte=0x0, do_allow_print=0x30) at lread.c:2420
#20 0x00005555557ab3e7 in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:809
#21 0x0000555555763f63 in Ffuncall (nargs=nargs <at> entry=5, args=args <at> entry=0x7fffffffdae0) at eval.c:2995
#22 0x0000555555798a9d in call4 (arg4=0x30, arg3=0x30, arg2=0x55555618c784, arg1=0x55555618c784, fn=<optimized out>) at /mnt/sdc1/tmp/emacs/src/lisp.h:3269
#23 Fload (file=<optimized out>, noerror=0xffffbffff9503bb0, nomessage=0xffffbffff9503ab0, nosuffix=<optimized out>, must_suffix=<optimized out>) at lread.c:1484
#24 0x00005555557ab3e7 in exec_byte_code (fun=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:809
#25 0x0000555555769387 in apply_lambda (fun=0x15554f4bac2d, args=<optimized out>, count=...) at eval.c:3103
#26 0x00005555557679e6 in eval_sub (form=<optimized out>) at eval.c:2588
#27 0x000055555576a4b7 in Feval (form=0x15554f8f188b, lexical=<optimized out>) at eval.c:2361
#28 0x0000555555762627 in internal_condition_case (bfun=bfun <at> entry=0x5555556d5510 <top_level_2>, handlers=handlers <at> entry=0x90, hfun=hfun <at> entry=0x5555556dcd10 <cmd_error>)
    at eval.c:1474
#29 0x00005555556d5ee6 in top_level_1 (ignore=ignore <at> entry=0x0) at keyboard.c:1141
#30 0x0000555555762581 in internal_catch (tag=tag <at> entry=0xffc0, func=func <at> entry=0x5555556d5ec0 <top_level_1>, arg=arg <at> entry=0x0) at eval.c:1197
#31 0x00005555556d548f in command_loop () at keyboard.c:1101
#32 0x00005555556dc894 in recursive_edit_1 () at keyboard.c:711
#33 0x00005555556dcc1c in Frecursive_edit () at keyboard.c:794
#34 0x00005555555aa4bd in main (argc=<optimized out>, argv=<optimized out>) at emacs.c:2529

(gdb) xbacktrace
"pdf-tools-install-noverify" (0x4ed262b8)
"pdf-tools-install" (0xffffd120)
"progn" (0xffffd2a0)
"condition-case" (0xffffd400)
"if" (0xffffd4e0)
"progn" (0xffffd5c0)
"condition-case" (0xffffd720)
"eval-buffer" (0x4ed26248)
"load-with-code-conversion" (0xffffdae8)
"load" (0x4ed26168)
"startup--load-user-init-file" (0x4ed260c0)
"command-line" (0x4ed26040)
"normal-top-level" (0xffffdc50)



In GNU Emacs 29.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.16.0) of 2023-02-13 built on foo.bar.com
Repository revision: a4aa32bdfff7aaf54efbacbb04b7f2b52fef92a7
Repository branch: HEAD
System Description: Gentoo/Linux

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


-- 
	Istvan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 09:30:02 GMT) Full text and rfc822 format available.

Message #8 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Istvan Marko <mi-ebugs <at> kismala.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 09:29:32 +0000
[Message part 1 (text/plain, inline)]
>
> There seems to be a byte-code change between versions 
> 0ec0a610ed226419269f519021cbe8fb2dde2ed5 (old) and 
> a4aa32bdfff7aaf54efbacbb04b7f2b52fef92a7 (new) which causes the new 
> version to crash with SIGSEGV when executing certain code from an .elc 
> built using the old version. Recompiling the file with the new version 
> causes the new .elc to work correctly.
>

The bytecode did not change, but the byte codes generated by the 
save-restriction form did change.

Now that I think of it again, it is possible make that change while 
preserving backward compatibility.

Eli, what do you think of the attached patch?  It restores the 'unbind 1' 
at the end of save-restriction, and puts the two data elements into a cons 
instead of pushing them separately.  (Of course this passes make and make 
check, with and without native compilation.)
[Improve-backward-compatibility-of-save-restriction.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 13:31:02 GMT) Full text and rfc822 format available.

Message #11 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>,
 Mattias Engdegård <mattiase <at> acm.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: mi-ebugs <at> kismala.com, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 15:29:50 +0200
> Date: Tue, 14 Feb 2023 09:29:32 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: Eli Zaretskii <eliz <at> gnu.org>, 61504 <at> debbugs.gnu.org
> 
> > There seems to be a byte-code change between versions 
> > 0ec0a610ed226419269f519021cbe8fb2dde2ed5 (old) and 
> > a4aa32bdfff7aaf54efbacbb04b7f2b52fef92a7 (new) which causes the new 
> > version to crash with SIGSEGV when executing certain code from an .elc 
> > built using the old version. Recompiling the file with the new version 
> > causes the new .elc to work correctly.
> >
> 
> The bytecode did not change, but the byte codes generated by the 
> save-restriction form did change.
> 
> Now that I think of it again, it is possible make that change while 
> preserving backward compatibility.
> 
> Eli, what do you think of the attached patch?  It restores the 'unbind 1' 
> at the end of save-restriction, and puts the two data elements into a cons 
> instead of pushing them separately.  (Of course this passes make and make 
> check, with and without native compilation.)

I guess we have no choice.

Mattias, Stefan: any better alternatives, or comments/objections?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 15:23:02 GMT) Full text and rfc822 format available.

Message #14 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: Istvan Marko <mi-ebugs <at> kismala.com>, Eli Zaretskii <eliz <at> gnu.org>,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 10:22:05 -0500
> Eli, what do you think of the attached patch?  It restores the 'unbind 1' at
> the end of save-restriction, and puts the two data elements into a cons
> instead of pushing them separately.  (Of course this passes make and make
> check, with and without native compilation.)

Looks right to me.
Some comments below.


        Stefan


> @@ -940,10 +940,9 @@ #define DEFINE(name, value) [name] = &&insn_ ## name,
>  	  }
>  
>  	CASE (Bsave_restriction):
> -	  record_unwind_protect (save_restriction_restore,
> -				 save_restriction_save ());
> -	  record_unwind_protect (narrowing_locks_restore,
> -				 narrowing_locks_save ());
> +	  record_unwind_protect (save_restriction_and_narrowing_locks_restore,
> +				 Fcons (save_restriction_save (),
> +					narrowing_locks_save ()));
>  	  NEXT;

Shouldn't the value returned by `save_restriction_save` include the
narrowing locks already?

IOW rather than changing this `bytecode.c` code, the locks handling
should be "hidden" inside `save_restriction_restore` and
`save_restriction_save`, don't you think?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 16:01:02 GMT) Full text and rfc822 format available.

Message #17 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Istvan Marko <mi-ebugs <at> kismala.com>, Eli Zaretskii <eliz <at> gnu.org>,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 16:00:35 +0000
[Message part 1 (text/plain, inline)]
>
> Looks right to me.
>

Many thanks for your review!

>> @@ -940,10 +940,9 @@ #define DEFINE(name, value) [name] = &&insn_ ## name,
>>  	  }
>>
>>  	CASE (Bsave_restriction):
>> -	  record_unwind_protect (save_restriction_restore,
>> -				 save_restriction_save ());
>> -	  record_unwind_protect (narrowing_locks_restore,
>> -				 narrowing_locks_save ());
>> +	  record_unwind_protect (save_restriction_and_narrowing_locks_restore,
>> +				 Fcons (save_restriction_save (),
>> +					narrowing_locks_save ()));
>>  	  NEXT;
>
> Shouldn't the value returned by `save_restriction_save` include the 
> narrowing locks already?
>
> IOW rather than changing this `bytecode.c` code, the locks handling 
> should be "hidden" inside `save_restriction_restore` and 
> `save_restriction_save`, don't you think?
>

You mean, the attached patch?  That's probably even better, indeed. 
(Again it passes make and make check, with and without native 
compilation.)
[Improve-backward-compatibility-of-save-restriction.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 16:48:01 GMT) Full text and rfc822 format available.

Message #20 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: Istvan Marko <mi-ebugs <at> kismala.com>, Eli Zaretskii <eliz <at> gnu.org>,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 11:47:36 -0500
> You mean, the attached patch?

Yes.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 16:51:01 GMT) Full text and rfc822 format available.

Message #23 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mi-ebugs <at> kismala.com, Gregory Heytings <gregory <at> heytings.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 17:50:38 +0100
The patch is all right, I suppose, but it would be nice to do without the extra cons. Maybe a new specpdl case is warranted? `save-excursion` has one.

By the way, doesn't the patch switch the restoration order of narrowing and restriction, respectively? Maybe it doesn't matter?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 16:52:02 GMT) Full text and rfc822 format available.

Message #26 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Istvan Marko <mi-ebugs <at> kismala.com>, Eli Zaretskii <eliz <at> gnu.org>,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 16:51:27 +0000
>> You mean, the attached patch?
>
> Yes.
>

Great, thanks again!

Eli, OK to install?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 17:01:02 GMT) Full text and rfc822 format available.

Message #29 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: mi-ebugs <at> kismala.com, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 17:00:55 +0000
Thanks for the review!

>
> The patch is all right, I suppose, but it would be nice to do without 
> the extra cons. Maybe a new specpdl case is warranted? `save-excursion` 
> has one.
>

That's a possible improvement, but I think it's not safe enough for Emacs 
29.

>
> By the way, doesn't the patch switch the restoration order of narrowing 
> and restriction, respectively? Maybe it doesn't matter?
>

Hmmm, that's a good question!  The evaluation order of parameters is 
unspecified in C, so actually the order could be switched or not, 
depending on what the compiler chooses to do.  That being said, AFAICS it 
doesn't matter in this case, indeed.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 17:17:01 GMT) Full text and rfc822 format available.

Message #32 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: mi-ebugs <at> kismala.com, mattiase <at> acm.org, monnier <at> iro.umontreal.ca,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 19:16:21 +0200
> Date: Tue, 14 Feb 2023 17:00:55 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: Eli Zaretskii <eliz <at> gnu.org>, mi-ebugs <at> kismala.com, 
>     Stefan Monnier <monnier <at> iro.umontreal.ca>, 61504 <at> debbugs.gnu.org
> 
> > By the way, doesn't the patch switch the restoration order of narrowing 
> > and restriction, respectively? Maybe it doesn't matter?
> 
> Hmmm, that's a good question!  The evaluation order of parameters is 
> unspecified in C, so actually the order could be switched or not, 
> depending on what the compiler chooses to do.

But you could rewrite the code so that the parameters are evaluated
one after the other, and only after that call Fcons.  The compiler
could still change the order, but that would be less probable.

> That being said, AFAICS it doesn't matter in this case, indeed.

It is IME better to write code that doesn't trigger such questions to
begin with.

> Eli, OK to install?

With the change of order per the above, yes.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 17:22:01 GMT) Full text and rfc822 format available.

Message #35 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: mi-ebugs <at> kismala.com, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 18:21:28 +0100
14 feb. 2023 kl. 18.00 skrev Gregory Heytings <gregory <at> heytings.org>:

>> By the way, doesn't the patch switch the restoration order of narrowing and restriction, respectively? Maybe it doesn't matter?
>> 
> 
> Hmmm, that's a good question!  The evaluation order of parameters is unspecified in C, so actually the order could be switched or not, depending on what the compiler chooses to do.

Yes, the saving order is undefined but the restoring order seems well-defined. It currently restores narrowing locks first, then the restriction, but your patch flips the order.

Please at least make the saving order well-defined, preferably in the reverse order of restoration for symmetry.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 17:37:01 GMT) Full text and rfc822 format available.

Message #38 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Istvan Marko <mi-ebugs <at> kismala.com>
To: Gregory Heytings <gregory <at> heytings.org>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>
Cc: Istvan Marko <mi-ebugs <at> kismala.com>, Eli Zaretskii <eliz <at> gnu.org>,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 09:36:12 -0800
I've tested with Gregory's second patch on top of current emacs-29 and
it avoids the crash with my original test case.

-- 
	Istvan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 20:34:02 GMT) Full text and rfc822 format available.

Message #41 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: mi-ebugs <at> kismala.com,
 Mattias Engdegård <mattiase <at> acm.org>,
 Eli Zaretskii <eliz <at> gnu.org>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 15:32:54 -0500
>> By the way, doesn't the patch switch the restoration order of narrowing
>> and restriction, respectively? Maybe it doesn't matter?
> Hmmm, that's a good question!  The evaluation order of parameters is
> unspecified in C,

The problem is not in the evaluation order of params in
`save_restriction_save` (this order doesn't matter because the code is
pure), it's in `+save_restriction_restore`.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 20:45:01 GMT) Full text and rfc822 format available.

Message #44 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mi-ebugs <at> kismala.com, mattiase <at> acm.org, monnier <at> iro.umontreal.ca,
 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 20:44:46 +0000
>> Hmmm, that's a good question!  The evaluation order of parameters is 
>> unspecified in C, so actually the order could be switched or not, 
>> depending on what the compiler chooses to do.
>
> But you could rewrite the code so that the parameters are evaluated one 
> after the other, and only after that call Fcons.  The compiler could 
> still change the order, but that would be less probable.
>

Agreed.

>> That being said, AFAICS it doesn't matter in this case, indeed.
>
> It is IME better to write code that doesn't trigger such questions to 
> begin with.
>

Agreed again.

>> Eli, OK to install?
>
> With the change of order per the above, yes.
>

Now done.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 20:47:02 GMT) Full text and rfc822 format available.

Message #47 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: mi-ebugs <at> kismala.com, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 20:46:10 +0000
>>> By the way, doesn't the patch switch the restoration order of 
>>> narrowing and restriction, respectively? Maybe it doesn't matter?
>>
>> Hmmm, that's a good question!  The evaluation order of parameters is 
>> unspecified in C, so actually the order could be switched or not, 
>> depending on what the compiler chooses to do.
>
> Yes, the saving order is undefined but the restoring order seems 
> well-defined. It currently restores narrowing locks first, then the 
> restriction, but your patch flips the order.
>

Indeed, I misunderstood what you said above, now I got it!

>
> Please at least make the saving order well-defined, preferably in the 
> reverse order of restoration for symmetry.
>

I did that.

Thanks again for your review/feedback.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61504; Package emacs. (Tue, 14 Feb 2023 20:48:01 GMT) Full text and rfc822 format available.

Message #50 received at 61504 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: mi-ebugs <at> kismala.com, Mattias Engdegård <mattiase <at> acm.org>,
 Eli Zaretskii <eliz <at> gnu.org>, 61504 <at> debbugs.gnu.org
Subject: Re: bug#61504: 29.0.60; executing byte-code from previous build
 causes SIGSEGV crash
Date: Tue, 14 Feb 2023 20:47:28 +0000
>>> By the way, doesn't the patch switch the restoration order of 
>>> narrowing and restriction, respectively? Maybe it doesn't matter?
>>
>> Hmmm, that's a good question!  The evaluation order of parameters is 
>> unspecified in C,
>
> The problem is not in the evaluation order of params in 
> `save_restriction_save` (this order doesn't matter because the code is 
> pure), it's in `save_restriction_restore`.
>

Indeed, I initially misunderstood what Mattias said, but after reading it 
again I got it.  This is now fixed.

Thanks!





This bug report was last modified 1 year and 79 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.