* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 12:57 bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE Cyril Arnould
@ 2023-05-27 13:42 ` Eli Zaretskii
2023-05-27 14:32 ` bug#63752: AW: " Cyril Arnould
2023-06-30 22:41 ` Cyril Arnould
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-05-27 13:42 UTC (permalink / raw)
To: Cyril Arnould; +Cc: 63752
> From: Cyril Arnould <cyril.arnould@outlook.com>
> Date: Sat, 27 May 2023 12:57:44 +0000
>
> First of all, I'm on Windows 10, using the MINGW64 MSYS2 environment, so
> this issue might be specific to MinGW. Note: While I'm using a UCRT
> build of Emacs to generate this report, the problem is present in the
> MinGW GCC build as well. Emacs has some weird behaviour regarding
> subprocess handling (?) when compiled with
> "-D_FORTIFY_SOURCE=1". Setting the value to 2 causes the same issue, I
> haven't tested setting it to 3. I believe this is the why the MSYS2
> Emacs builds have been broken for a while now:
Thank you for your report.
What happens if you don't use -D_FORTIFY_SOURCE= at all?
I don't know what this option odes for MSYS2 MinGW GCC compiler and
its header files. Up until now I knew that this option is only used
on GNU/Linux. In any case, I see no reason for end-user builds to use
it, as it is (AFAIU) intended for developers, to facilitate detection
of problematic code.
> https://github.com/msys2/MINGW-packages/issues/16190
> https://github.com/msys2/MINGW-packages/issues/16375
> https://github.com/msys2/MINGW-packages/issues/16413
I see nothing in these issues that would point out some upstream
problem. The issues are all related to the Emacs builds produced by
the MSYS2 folks. So I think the initial investigation should be
theirs, until they identify the code in Emacs which misbehaves.
> In the example above, I built Emacs from the emacs-28.2 tag, but I also
> managed to reproduce the issue in the emacs-28, emacs-29 and master
> branches.
FWIW, Emacs 28 and 29 (and also Emacs 30) builds fine for me with
MinGW, both with and without native-compilation, but I don't use
_FORTIFY_SOURCE (and don't plan on using it any time soon).
Which version of GCC is this, btw? If it's the latest GCC 13, did you
try downgrading to GCC 12?
Bottom line, I see no evidence yet that this is an upstream problem of
Emacs, and my MinGW builds are all fine.
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#63752: AW: bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 13:42 ` Eli Zaretskii
@ 2023-05-27 14:32 ` Cyril Arnould
2023-06-01 7:31 ` András Svraka
0 siblings, 1 reply; 9+ messages in thread
From: Cyril Arnould @ 2023-05-27 14:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 63752@debbugs.gnu.org
[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]
> What happens if you don't use -D_FORTIFY_SOURCE= at all?
I have no problems when not using -D_FORTIFY_SOURCE. It's set to
2 by default in the MSYS2 builds, I figured lowering it to 1
might work better but neither seem to work.
> I see nothing in these issues that would point out some upstream
> problem. The issues are all related to the Emacs builds produced by
> the MSYS2 folks. So I think the initial investigation should be
> theirs, until they identify the code in Emacs which misbehaves.
I understand. I'll reach out to see how they want to proceed on
their side.
I've also just noticed that the -D_FORTIFY_SOURCE option seems to
have been a rather recent addition to the MSYS2 build options and
it seems like this was only recently even made possible. And the
last successful MSYS2 build of Emacs was before that change.
> Which version of GCC is this, btw? If it's the latest GCC 13, did you
> try downgrading to GCC 12?
It's GCC 13.1, yes, I haven't tested GCC 12. I don't know how
easy it is to downgrade GCC on MSYS2 though, I assume there's a
lot of dependencies that can go wrong.
> FWIW, Emacs 28 and 29 (and also Emacs 30) builds fine for me with
> MinGW, both with and without native-compilation, but I don't use
> _FORTIFY_SOURCE (and don't plan on using it any time soon).
This makes me wonder if GCC 13.1 Emacs builds with
_FORTIFY_SOURCE are working fine in other operating systems? It's
not something I can easily test, however.
[-- Attachment #2: Type: text/html, Size: 3672 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 14:32 ` bug#63752: AW: " Cyril Arnould
@ 2023-06-01 7:31 ` András Svraka
0 siblings, 0 replies; 9+ messages in thread
From: András Svraka @ 2023-06-01 7:31 UTC (permalink / raw)
To: 63752@debbugs.gnu.org; +Cc: Eli Zaretskii, Cyril Arnould
Dear all,
> On 2023. May 27., at 16:32, Cyril Arnould <cyril.arnould@outlook.com> wrote:
> > Which version of GCC is this, btw? If it's the latest GCC 13, did you
> > try downgrading to GCC 12?
> It's GCC 13.1, yes, I haven't tested GCC 12. I don't know how
> easy it is to downgrade GCC on MSYS2 though, I assume there's a
> lot of dependencies that can go wrong.
I tried downgrading to GCC 12.2 in MSY2 and the problem exist there as well.
> > FWIW, Emacs 28 and 29 (and also Emacs 30) builds fine for me with
> > MinGW, both with and without native-compilation, but I don't use
> > _FORTIFY_SOURCE (and don't plan on using it any time soon).
> This makes me wonder if GCC 13.1 Emacs builds with
> _FORTIFY_SOURCE are working fine in other operating systems? It's
> not something I can easily test, however.
Arch Linux also uses _FORTIFY_SOURCE and AFAIU MSYS2’s motivation was to follow them [1]. I did not test it though but couldn’t find related bug reports.
[1]: https://github.com/msys2/MSYS2-packages/pull/3222
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 12:57 bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE Cyril Arnould
2023-05-27 13:42 ` Eli Zaretskii
@ 2023-06-30 22:41 ` Cyril Arnould
2023-07-01 6:40 ` Eli Zaretskii
2023-07-05 20:23 ` Cyril Arnould
2023-07-06 19:28 ` Cyril Arnould
3 siblings, 1 reply; 9+ messages in thread
From: Cyril Arnould @ 2023-06-30 22:41 UTC (permalink / raw)
To: eliz@gnu.org; +Cc: András Svraka, 63752@debbugs.gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 572 bytes --]
I've found that I can narrow it down similar to bug #63365: As long as I
don't compile src/sysdep.c with -D_FORTIFY_SOURCE, the resulting
binaries seem to behave normally. Does that maybe suggest an upstream
problem? I compiled it as follows:
git clean -xdf
git checkout emacs-28.2
./autogen.sh
./configure
cd src
make sysdep.o CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
make sysdep.o -W sysdep.c
cd ..
make CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
I've attached the objects as well as their dumps in case anyone wants to
take a look.
[-- Attachment #1.2: Type: text/html, Size: 2162 bytes --]
[-- Attachment #2: sysdep-diff.tar.gz --]
[-- Type: application/x-gzip, Size: 866575 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-06-30 22:41 ` Cyril Arnould
@ 2023-07-01 6:40 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2023-07-01 6:40 UTC (permalink / raw)
To: Cyril Arnould; +Cc: svraka.andras, 63752
> From: Cyril Arnould <cyril.arnould@outlook.com>
> CC: "63752@debbugs.gnu.org" <63752@debbugs.gnu.org>,
> András Svraka <svraka.andras@gmail.com>
> Date: Fri, 30 Jun 2023 22:41:26 +0000
>
> I've found that I can narrow it down similar to bug #63365: As long as I
> don't compile src/sysdep.c with -D_FORTIFY_SOURCE, the resulting
> binaries seem to behave normally. Does that maybe suggest an upstream
> problem? I compiled it as follows:
>
> git clean -xdf
> git checkout emacs-28.2
> ./autogen.sh
> ./configure
> cd src
> make sysdep.o CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
> make sysdep.o -W sysdep.c
> cd ..
> make CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
>
> I've attached the objects as well as their dumps in case anyone wants to
> take a look.
Thanks, but the differences are too large to figure out what causes
this. It sounds like -D_FORTIFY_SOURCE=2 wraps and/or replaces many
library functions with special variants, and effect of that is
unclear, because the implementation of those wrappers is elsewhere
(probably in some library linked into the executable under that
option?). E.g., by just looking at the diffs, it sounds like the
-D_FORTIFY_SOURCE=2 code doesn't call 'open' to open the null device??
Since you've already established sysdep.c alone is the culprit, I
suggest to narrow this. Create a separate file sysdep1.c, move to it
the first portion of sysdep.c which includes everything before
serial_open, and build Emacs with these two parts (you'd need to add
sysdep1.o to base_obj in src/Makefile). Once you have such an Emacs
successfully built and reproduce the problem, rebuild by compiling
sysdep1.c without -D_FORTIFY_SOURCE=2, and see if the problem goes
away. If it does, bisect sysdep1.c by moving parts of it back to
sysdep.c, until you identify the smallest part of sysdep.c that needs
to be compiled with -D_FORTIFY_SOURCE=2 to reproduce the problem.
Then we will have to examine the effect of -D_FORTIFY_SOURCE=2 on that
smallest part, and see if we can figure this out.
This is a lot of work, so kudos if you are motivated to go ahead and
do it. From my POV, the -D_FORTIFY_SOURCE=2 build is currently
problematic on Windows and therefore not supported by the upstream
project. (IMNSHO, it is also wrong to use this in production builds
of programs such as Emacs, but that's me, and I know others disagree.)
My advice to MSYS2 folks at this time is to stop building Emacs that
way until the problem is resolved.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 12:57 bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE Cyril Arnould
2023-05-27 13:42 ` Eli Zaretskii
2023-06-30 22:41 ` Cyril Arnould
@ 2023-07-05 20:23 ` Cyril Arnould
2023-07-06 5:28 ` Eli Zaretskii
2023-07-06 19:28 ` Cyril Arnould
3 siblings, 1 reply; 9+ messages in thread
From: Cyril Arnould @ 2023-07-05 20:23 UTC (permalink / raw)
To: eliz@gnu.org; +Cc: svraka.andras@gmail.com, 63752@debbugs.gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 2894 bytes --]
> Since you've already established sysdep.c alone is the culprit, I
> suggest to narrow this. Create a separate file sysdep1.c, move to it
> the first portion of sysdep.c which includes everything before
> serial_open, and build Emacs with these two parts (you'd need to add
> sysdep1.o to base_obj in src/Makefile). Once you have such an Emacs
> successfully built and reproduce the problem, rebuild by compiling
> sysdep1.c without -D_FORTIFY_SOURCE=2, and see if the problem goes
> away. If it does, bisect sysdep1.c by moving parts of it back to
> sysdep.c, until you identify the smallest part of sysdep.c that needs
> to be compiled with -D_FORTIFY_SOURCE=2 to reproduce the problem.
> Then we will have to examine the effect of -D_FORTIFY_SOURCE=2 on that
> smallest part, and see if we can figure this out.
Thanks for the pointers! Following your advice, the first sysdep1.c with
code up until serial_open was working quite quickly. On a hunch, I've
singled out the emacs_intr_read function and got lucky: If I only put
the emacs_intr_read, emacs_read and emacs_read_quit functions into
sysdep1.c, the problems go away if sysdep1.o is compiled without
-D_FORTIFY_SOURCE=2.
Aside from those functions, sysdep1.c also contains all of the includes
of the original file; I figured it doesn't make much sense to narrow
those down further. I also had to add the MAX_RW_COUNT
definition. There's a patch attached; it can be tested as follows:
git clean -xdf
git checkout emacs-28.2
patch -p1 -i 0001-Single-out-emacs_read-as-culprit.patch
./autogen.sh
./configure
make CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
# The following command will result in an Emacs window where the
# asynchronous process doesn't terminate:
./src/emacs -Q --eval '(async-shell-command "dir")'
cd src
make sysdep1.o -W sysdep1.c
cd ..
make CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2'
# This time the process will terminate:
./src/emacs -Q --eval '(async-shell-command "dir")'
I've also attached another archive with the objects, dumps and
diff. This time it seems like the entire file is different. Is it
possible that GCC messes up with the read function that gets defined to
sys_read at the top?
> This is a lot of work, so kudos if you are motivated to go ahead and
> do it. From my POV, the -D_FORTIFY_SOURCE=2 build is currently
> problematic on Windows and therefore not supported by the upstream
> project. (IMNSHO, it is also wrong to use this in production builds
> of programs such as Emacs, but that's me, and I know others disagree.)
Fully understand, I really appreciate you helping despite the problem
being of low priority.
> My advice to MSYS2 folks at this time is to stop building Emacs that
> way until the problem is resolved.
The current release of Emacs on MSYS2 is indeed built without
-D_FORTIFY_SOURCE.
[-- Attachment #1.2: Type: text/html, Size: 5761 bytes --]
[-- Attachment #2: 0001-Single-out-emacs_read-as-culprit.patch --]
[-- Type: application/octet-stream, Size: 6977 bytes --]
From 67522e3f7728b7d6c833f813f6d8c8390e3f0e3e Mon Sep 17 00:00:00 2001
From: Cyril Arnould <cyril.arnould@outlook.com>
Date: Wed, 5 Jul 2023 19:08:09 +0200
Subject: [PATCH] Single out emacs_read as culprit
Compiling everything but sysdep1.c with -D_FORTIFY_SOURCE=2 results in
a working binary
---
src/Makefile.in | 2 +-
src/sysdep.c | 41 ------------
src/sysdep1.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 167 insertions(+), 42 deletions(-)
create mode 100644 src/sysdep1.c
diff --git a/src/Makefile.in b/src/Makefile.in
index 29e1513ab5f..3bd17b8f6b5 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -394,7 +394,7 @@ .m.o:
base_obj = dispnew.o frame.o scroll.o xdisp.o menu.o $(XMENU_OBJ) window.o \
charset.o coding.o category.o ccl.o character.o chartab.o bidi.o \
$(CM_OBJ) term.o terminal.o xfaces.o $(XOBJ) $(GTK_OBJ) $(DBUS_OBJ) \
- emacs.o keyboard.o macros.o keymap.o sysdep.o \
+ emacs.o keyboard.o macros.o keymap.o sysdep.o sysdep1.o \
bignum.o buffer.o filelock.o insdel.o marker.o \
minibuf.o fileio.o dired.o \
cmds.o casetab.o casefiddle.o indent.o search.o regex-emacs.o undo.o \
diff --git a/src/sysdep.c b/src/sysdep.c
index 72be25f6610..ad344ef44f0 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -2474,47 +2474,6 @@ verify (MAX_RW_COUNT <= INT_MAX);
verify (MAX_RW_COUNT <= UINT_MAX);
#endif
-/* Read from FD to a buffer BUF with size NBYTE.
- If interrupted, process any quits and pending signals immediately
- if INTERRUPTIBLE, and then retry the read unless quitting.
- Return the number of bytes read, which might be less than NBYTE.
- On error, set errno to a value other than EINTR, and return -1. */
-static ptrdiff_t
-emacs_intr_read (int fd, void *buf, ptrdiff_t nbyte, bool interruptible)
-{
- /* No caller should ever pass a too-large size to emacs_read. */
- eassert (nbyte <= MAX_RW_COUNT);
-
- ssize_t result;
-
- do
- {
- if (interruptible)
- maybe_quit ();
- result = read (fd, buf, nbyte);
- }
- while (result < 0 && errno == EINTR);
-
- return result;
-}
-
-/* Read from FD to a buffer BUF with size NBYTE.
- If interrupted, retry the read. Return the number of bytes read,
- which might be less than NBYTE. On error, set errno to a value
- other than EINTR, and return -1. */
-ptrdiff_t
-emacs_read (int fd, void *buf, ptrdiff_t nbyte)
-{
- return emacs_intr_read (fd, buf, nbyte, false);
-}
-
-/* Like emacs_read, but also process quits and pending signals. */
-ptrdiff_t
-emacs_read_quit (int fd, void *buf, ptrdiff_t nbyte)
-{
- return emacs_intr_read (fd, buf, nbyte, true);
-}
-
/* Write to FILEDES from a buffer BUF with size NBYTE, retrying if
interrupted or if a partial write occurs. Process any quits
immediately if INTERRUPTIBLE is positive, and process any pending
diff --git a/src/sysdep1.c b/src/sysdep1.c
new file mode 100644
index 00000000000..d39692fd577
--- /dev/null
+++ b/src/sysdep1.c
@@ -0,0 +1,166 @@
+/* Interfaces to system-dependent kernel and library entries.
+ Copyright (C) 1985-1988, 1993-1995, 1999-2022 Free Software
+ Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or (at
+your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */
+
+#include <config.h>
+
+#include <execinfo.h>
+#include "sysstdio.h"
+#ifdef HAVE_PWD_H
+#include <pwd.h>
+#include <grp.h>
+#endif /* HAVE_PWD_H */
+#include <limits.h>
+#include <stdlib.h>
+#include <sys/random.h>
+#include <unistd.h>
+
+#include <c-ctype.h>
+#include <close-stream.h>
+#include <pathmax.h>
+#include <utimens.h>
+
+#include "lisp.h"
+#include "sheap.h"
+#include "sysselect.h"
+#include "blockinput.h"
+
+#ifdef HAVE_LINUX_FS_H
+# include <linux/fs.h>
+# include <sys/syscall.h>
+#endif
+
+#ifdef CYGWIN
+# include <cygwin/fs.h>
+#endif
+
+#if defined DARWIN_OS || defined __FreeBSD__ || defined __OpenBSD__
+# include <sys/sysctl.h>
+#endif
+
+#if defined __OpenBSD__
+# include <sys/proc.h>
+#endif
+
+#ifdef DARWIN_OS
+# include <libproc.h>
+#endif
+
+#ifdef __FreeBSD__
+/* Sparc/ARM machine/frame.h has 'struct frame' which conflicts with Emacs's
+ 'struct frame', so rename it. */
+# define frame freebsd_frame
+# include <sys/user.h>
+# undef frame
+
+# include <math.h>
+#endif
+
+#ifdef HAVE_SOCKETS
+#include <sys/socket.h>
+#include <netdb.h>
+#endif /* HAVE_SOCKETS */
+
+#ifdef WINDOWSNT
+#define read sys_read
+#define write sys_write
+#ifndef STDERR_FILENO
+#define STDERR_FILENO fileno(GetStdHandle(STD_ERROR_HANDLE))
+#endif
+#include "w32.h"
+#endif /* WINDOWSNT */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+
+/* Get SI_SRPC_DOMAIN, if it is available. */
+#ifdef HAVE_SYS_SYSTEMINFO_H
+#include <sys/systeminfo.h>
+#endif
+
+#ifdef MSDOS /* Demacs 1.1.2 91/10/20 Manabu Higashida, MW Aug 1993 */
+#include "msdos.h"
+#endif
+
+#include <sys/param.h>
+#include <sys/file.h>
+#include <fcntl.h>
+
+#include "syssignal.h"
+#include "systime.h"
+#include "systty.h"
+#include "syswait.h"
+
+#ifdef HAVE_SYS_RESOURCE_H
+# include <sys/resource.h>
+#endif
+
+#ifdef HAVE_SYS_UTSNAME_H
+# include <sys/utsname.h>
+# include <memory.h>
+#endif
+
+#include "keyboard.h"
+#include "frame.h"
+#include "termhooks.h"
+#include "termchar.h"
+#include "termopts.h"
+#include "process.h"
+#include "cm.h"
+
+#ifndef MAX_RW_COUNT
+#define MAX_RW_COUNT (INT_MAX >> 18 << 18)
+#endif
+
+/* Read from FD to a buffer BUF with size NBYTE.
+ If interrupted, process any quits and pending signals immediately
+ if INTERRUPTIBLE, and then retry the read unless quitting.
+ Return the number of bytes read, which might be less than NBYTE.
+ On error, set errno to a value other than EINTR, and return -1. */
+static ptrdiff_t
+emacs_intr_read (int fd, void *buf, ptrdiff_t nbyte, bool interruptible)
+{
+ /* No caller should ever pass a too-large size to emacs_read. */
+ eassert (nbyte <= MAX_RW_COUNT);
+
+ ssize_t result;
+
+ do
+ {
+ if (interruptible)
+ maybe_quit ();
+ result = read (fd, buf, nbyte);
+ }
+ while (result < 0 && errno == EINTR);
+
+ return result;
+}
+
+ptrdiff_t
+emacs_read (int fd, void *buf, ptrdiff_t nbyte)
+{
+ return emacs_intr_read (fd, buf, nbyte, false);
+}
+
+/* Like emacs_read, but also process quits and pending signals. */
+ptrdiff_t
+emacs_read_quit (int fd, void *buf, ptrdiff_t nbyte)
+{
+ return emacs_intr_read (fd, buf, nbyte, true);
+}
--
2.36.0.windows.1
[-- Attachment #3: sysdep1-diff.tar.gz --]
[-- Type: application/x-gzip, Size: 609882 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-07-05 20:23 ` Cyril Arnould
@ 2023-07-06 5:28 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2023-07-06 5:28 UTC (permalink / raw)
To: Cyril Arnould, Andrea Corallo; +Cc: svraka.andras, 63752
> From: Cyril Arnould <cyril.arnould@outlook.com>
> CC: "63752@debbugs.gnu.org" <63752@debbugs.gnu.org>, "svraka.andras@gmail.com"
> <svraka.andras@gmail.com>
> Date: Wed, 5 Jul 2023 20:23:33 +0000
>
> Thanks for the pointers! Following your advice, the first sysdep1.c with
> code up until serial_open was working quite quickly. On a hunch, I've
> singled out the emacs_intr_read function and got lucky: If I only put
> the emacs_intr_read, emacs_read and emacs_read_quit functions into
> sysdep1.c, the problems go away if sysdep1.o is compiled without
> -D_FORTIFY_SOURCE=2.
Thanks, I hope Andrea will have ideas after looking at the
differences.
There are several symbols in the diffs which are used only in the
-D_FORTIFY_SOURCE=2 code, whose precise meanings are unclear to me.
Those are the declaration qualifier __mingw_bos_extern_ovr and the
functions __mingw_bos_ptr_chk_warn and __mingw_call__read.
Under __MINGW_FORTIFY_LEVEL > 0 __mingw_bos_extern_ovr is defined like
this:
# define __mingw_bos_extern_ovr extern __inline__ __cdecl \
__attribute__((__always_inline__, __gnu_inline__)) \
__mingw_attribute_artificial
Otherwise, it's defined to nothing. The above just controls the
inlining, but that the 'artificial' attribute is described like this
in the GCC manual:
'artificial'
This attribute is useful for small inline wrappers that if possible
should appear during debugging as a unit. Depending on the debug
info format it either means marking the function as artificial or
using the caller location for all instructions within the inlined
body.
I'm not sure I understand what this means in the context of the issue
we are discussing here.
The other 2 symbols seem to be about checking a pointer and calling
'_read' while checking the pointer to the buffer passed to it.
__mingw_bos_ptr_chk_warn is defined like this under
__MINGW_FORTIFY_LEVEL > 0:
# define __mingw_bos_ptr_chk_warn(p, n, maxtype) \
((__mingw_bos_known(p) \
&& __builtin_constant_p(__mingw_bos(p, maxtype) < (size_t)(n)) \
&& __mingw_bos(p, maxtype) < (size_t)(n)) \
? __mingw_chk_fail_warn() : __mingw_bos_ptr_chk(p, n, maxtype))
And here's __mingw_call__read:
#define __MINGW_ASM_CRT_CALL(func) __asm__(__STRINGIFY(func))
_CRTIMP int __cdecl __mingw_call__read(int, void *, unsigned int) __MINGW_ASM_CRT_CALL(_read);
AFAICT, __mingw_chk_fail_warn eventually does this:
void __cdecl __attribute__((__noreturn__)) __chk_fail(void) {
static const char msg[] = "*** buffer overflow detected ***: terminated\n";
write(STDERR_FILENO, msg, strlen(msg));
if (IsProcessorFeaturePresent(PF_FASTFAIL_AVAILABLE)) {
__fastfail(FAST_FAIL_RANGE_CHECK_FAILURE);
} else {
TerminateProcess(GetCurrentProcess(), STATUS_STACK_BUFFER_OVERRUN);
__builtin_unreachable();
}
}
Again, not sure what this means for the issue at hand. In particular,
__fastfail emits "int 0x29", see
https://learn.microsoft.com/en-us/cpp/intrinsics/fastfail?view=msvc-170
> Aside from those functions, sysdep1.c also contains all of the includes
> of the original file; I figured it doesn't make much sense to narrow
> those down further. I also had to add the MAX_RW_COUNT
> definition. There's a patch attached; it can be tested as follows:
Thanks, I hope Andrea will have some ideas.
It's too bad the problem goes away under GDB, because we don't even
know whether the crash is due to some fatal error in the Emacs code,
or due to those extra-checking routines inserted into the code by the
FORTIFY option. IOW, it could be that the problem happens because the
FORTIFY routines don't understand something Emacs does and consider it
a bug worthy of aborting the program.
Another interesting question is why the problems happen only in Emacs
built with native compilation. Do we use emacs_read or its callers in
some special ways when they are called from comp.c or comp.el?
Maybe we should ask the MinGW64 folks who implemented the FORTIFY
support for MinGW64 GCC to join this discussion and help us figure out
which part(s) of this puzzle are relevant and why? Cyril, can you
reach out to them and ask them help us out here? There are too many
unknowns here, and having someone on board who understands what the
various FORTIFY additions mean and do, and perhaps also how to
selectively disable them (to find the one that really matters) will
definitely help.
Thanks.
P.S. Please keep ANdrea CC'd on this discussion.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE
2023-05-27 12:57 bug#63752: 28.2; GCC 13.1 breaks Emacs subprocess handling when built with -D_FORTIFY_SOURCE Cyril Arnould
` (2 preceding siblings ...)
2023-07-05 20:23 ` Cyril Arnould
@ 2023-07-06 19:28 ` Cyril Arnould
3 siblings, 0 replies; 9+ messages in thread
From: Cyril Arnould @ 2023-07-06 19:28 UTC (permalink / raw)
To: eliz@gnu.org; +Cc: acorallo@gnu.org, András Svraka, 63752@debbugs.gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 4255 bytes --]
> It's too bad the problem goes away under GDB, because we don't even
> know whether the crash is due to some fatal error in the Emacs code,
> or due to those extra-checking routines inserted into the code by the
> FORTIFY option. IOW, it could be that the problem happens because the
> FORTIFY routines don't understand something Emacs does and consider it
> a bug worthy of aborting the program.
Oh, I think there might be some confusion with #63365, sorry if I
haven't been precise enough. This fortify bug does *not* go away under
GDB (unless I'm misunderstanding). I get the following output when
running the binary compiled with -D_FORTIFY_SOURCE:
$ gdb -q --args ./src/emacs -Q --eval '(async-shell-command "dir")'
Reading symbols from ./src/emacs...
(gdb) run
Starting program: I:\Git\emacs\src\emacs.exe -Q --eval "(async-shell-command \"dir\")"
[New Thread 15740.0x22e0]
[New Thread 15740.0x2b50]
[New Thread 15740.0x10cc]
[New Thread 15740.0x4f4]
[New Thread 15740.0x3eac]
[New Thread 15740.0x3c80]
The output then continues once I close the window:
[Thread 15740.0x4f4 exited with code 0]
[New Thread 15740.0x3b00]
[New Thread 15740.0x9ac]
[Thread 15740.0x3c80 exited with code 0]
[Thread 15740.0x10cc exited with code 0]
[Thread 15740.0x2b50 exited with code 0]
[Thread 15740.0x22e0 exited with code 0]
[Thread 15740.0x3b00 exited with code 0]
[Thread 15740.0x3eac exited with code 0]
[Thread 15740.0x9ac exited with code 0]
[Inferior 1 (process 15740) exited normally]
(gdb) quit
When running the binary where sysdep.o is compiled without
-D_FORTIFY_SOURCE, I get the following instead:
$ gdb -q --args ./src/emacs -Q --eval '(async-shell-command "dir")'
Reading symbols from ./src/emacs...
(gdb) run
Starting program: I:\Git\emacs\src\emacs.exe -Q --eval "(async-shell-command \"dir\")"
[New Thread 4364.0x36d4]
[New Thread 4364.0x3f38]
[New Thread 4364.0x3eec]
[New Thread 4364.0x924]
[New Thread 4364.0x38bc]
[New Thread 4364.0x2f8c]
[Thread 4364.0x2f8c exited with code 0]
And once I close the window:
[Thread 4364.0x924 exited with code 0]
[Thread 4364.0x36d4 exited with code 0]
[Thread 4364.0x3eec exited with code 0]
[Thread 4364.0x3f38 exited with code 0]
[Thread 4364.0x38bc exited with code 0]
[Inferior 1 (process 4364) exited normally]
(gdb) quit
The big difference being that in the latter case, the last thread exits
immediately (which I assume is the "dir" command), while with
-D_FORTIFY_SOURCE the thread only exits once I close the window. I can
perform further tests with GDB of course.
> Another interesting question is why the problems happen only in Emacs
> built with native compilation. Do we use emacs_read or its callers in
> some special ways when they are called from comp.c or comp.el?
Again, there's some confusion with #63365. AFAICT, the fortify bug
occurs regardless of whether Emacs is built --with-native-compilation or
not. The third emacs-28.2 release on MSYS2 had the fortify bug and was
compiled --with-native-compilation. So far in *this* bug report I have
been using the default ./configure options, so native compilation should
be off in my objdumps.
In fact, I think the two bugs are not related at all. #63365 occurs
regardless of whether Emacs is built with or without -D_FORTIFY_SOURCE;
see the fourth and fifth release of Emacs on MSYS2.
Now, to make sure the two are not related, I've now repeated the process
with the following flags:
CFLAGS='-g3 -O2 -gdwarf-2 -Wp,-D_FORTIFY_SOURCE=2 -fno-optimize-sibling-calls'
The fortify bug was still there, so it doesn't seem like
-foptimize-sibling-calls is the root cause for #63752. The objdump of
sysdep1.o is slightly different from the one compiled with
-foptimize-sibling-calls, I've attached it in case someone wants to take
a look but I don't think it's relevant.
> Maybe we should ask the MinGW64 folks who implemented the FORTIFY
> support for MinGW64 GCC to join this discussion and help us figure out
> which part(s) of this puzzle are relevant and why? Cyril, can you
> reach out to them and ask them help us out here?
Sure, I've opened a support request:
https://sourceforge.net/p/mingw-w64/support-requests/193/
[-- Attachment #1.2: Type: text/html, Size: 8367 bytes --]
[-- Attachment #2: sysdep1-fortify-nosiblings.txt --]
[-- Type: text/plain, Size: 4368 bytes --]
src/sysdep1.o: file format pe-x86-64
Disassembly of section .text:
0000000000000000 <emacs_intr_read>:
if INTERRUPTIBLE, and then retry the read unless quitting.
Return the number of bytes read, which might be less than NBYTE.
On error, set errno to a value other than EINTR, and return -1. */
static ptrdiff_t
emacs_intr_read (int fd, void *buf, ptrdiff_t nbyte, bool interruptible)
{
0: 41 56 push %r14
2: 41 55 push %r13
4: 41 54 push %r12
6: 55 push %rbp
7: 57 push %rdi
8: 56 push %rsi
9: 53 push %rbx
a: 48 83 ec 20 sub $0x20,%rsp
e: 4c 8b 2d 00 00 00 00 mov 0x0(%rip),%r13 # 15 <emacs_intr_read+0x15>
{
if (interruptible)
maybe_quit ();
result = read (fd, buf, nbyte);
}
while (result < 0 && errno == EINTR);
15: 4c 8b 35 00 00 00 00 mov 0x0(%rip),%r14 # 1c <emacs_intr_read+0x1c>
{
1c: 41 89 cc mov %ecx,%r12d
1f: 48 89 d5 mov %rdx,%rbp
22: 44 89 cf mov %r9d,%edi
result = read (fd, buf, nbyte);
25: 44 89 c6 mov %r8d,%esi
28: eb 22 jmp 4c <emacs_intr_read+0x4c>
2a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
__mingw_bos_extern_ovr
int _read(int __fh, void * __dst, unsigned int __n)
{
__mingw_bos_ptr_chk_warn(__dst, __n, 0);
return __mingw_call__read(__fh, __dst, __n);
30: 41 89 f0 mov %esi,%r8d
33: 48 89 ea mov %rbp,%rdx
36: 44 89 e1 mov %r12d,%ecx
39: 41 ff d5 call *%r13
3c: 48 63 d8 movslq %eax,%rbx
while (result < 0 && errno == EINTR);
3f: 48 85 db test %rbx,%rbx
42: 79 1c jns 60 <emacs_intr_read+0x60>
44: 41 ff d6 call *%r14
47: 83 38 04 cmpl $0x4,(%rax)
4a: 75 14 jne 60 <emacs_intr_read+0x60>
if (interruptible)
4c: 40 84 ff test %dil,%dil
4f: 74 df je 30 <emacs_intr_read+0x30>
maybe_quit ();
51: e8 00 00 00 00 call 56 <emacs_intr_read+0x56>
56: eb d8 jmp 30 <emacs_intr_read+0x30>
58: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
5f: 00
return result;
}
60: 48 89 d8 mov %rbx,%rax
63: 48 83 c4 20 add $0x20,%rsp
67: 5b pop %rbx
68: 5e pop %rsi
69: 5f pop %rdi
6a: 5d pop %rbp
6b: 41 5c pop %r12
6d: 41 5d pop %r13
6f: 41 5e pop %r14
71: c3 ret
72: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
79: 00 00 00 00
7d: 0f 1f 00 nopl (%rax)
0000000000000080 <emacs_read>:
ptrdiff_t
emacs_read (int fd, void *buf, ptrdiff_t nbyte)
{
80: 48 83 ec 28 sub $0x28,%rsp
return emacs_intr_read (fd, buf, nbyte, false);
84: 45 31 c9 xor %r9d,%r9d
87: e8 74 ff ff ff call 0 <emacs_intr_read>
}
8c: 48 83 c4 28 add $0x28,%rsp
90: c3 ret
91: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
98: 00 00 00 00
9c: 0f 1f 40 00 nopl 0x0(%rax)
00000000000000a0 <emacs_read_quit>:
/* Like emacs_read, but also process quits and pending signals. */
ptrdiff_t
emacs_read_quit (int fd, void *buf, ptrdiff_t nbyte)
{
a0: 48 83 ec 28 sub $0x28,%rsp
return emacs_intr_read (fd, buf, nbyte, true);
a4: 41 b9 01 00 00 00 mov $0x1,%r9d
aa: e8 51 ff ff ff call 0 <emacs_intr_read>
}
af: 48 83 c4 28 add $0x28,%rsp
b3: c3 ret
b4: 90 nop
b5: 90 nop
b6: 90 nop
b7: 90 nop
b8: 90 nop
b9: 90 nop
ba: 90 nop
bb: 90 nop
bc: 90 nop
bd: 90 nop
be: 90 nop
bf: 90 nop
^ permalink raw reply [flat|nested] 9+ messages in thread