GNU bug report logs -
#61504
29.0.60; executing byte-code from previous build causes SIGSEGV crash
Previous Next
To reply to this bug, email your comments to 61504 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
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):
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):
[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):
> 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):
> 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):
[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):
> 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):
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):
>> 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):
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):
> 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):
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):
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):
>> 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):
>> 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):
>>> 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):
>>> 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 322 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.