Emacs build fails [MSYS2/MINGW64]

Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

Starting with this commit:

Paul Eggert :
Mark _Noreturn error functions as cold
2019-04-18 00:35:18

the build of Emacs up to master (the last I tried is 75ee20364) fails (bootstrap-emacs.pdmp SEGMENT FAULT). 

The last commit which builds is

YAMAMOTO Mitsuharu :
src/ftcrfont.c (ftcrfont_glyph_extents): Fix last change
2019-04-18 11:30:17

I have found this with a binary search using the tarballs from http://git.savannah.gnu.org/cgit/emacs.git/snapshot.

This seems to affect only Windows build (MSYS2/MINGW64) because master builds on GNU/Linux Mint.

To reproduce, update your MSYS2 installation (pacman -Syu) and if you have ImageMagick installed, add '--without-imagemagick' to 'configure' otherwise it fails at compile time. 

In a MINGW64 shell:

  wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-master.tar.gz
  tar -xof emacs-master.tar.gz
  cd emacs-master
  ./autogen.sh
  ./configure [--without-imagemagick]
  make
  [...]
  Loading c:/msys64/tmp/emacs-master/lisp/emacs-lisp/lisp-mode.el (source)...
  Loading c:/msys64/tmp/emacs-master/lisp/progmodes/elisp-mode.el (source)...
  make[1]: *** [Makefile:808: bootstrap-emacs.pdmp] Segmentation fault
  make[1]: uscita dalla directory "/tmp/emacs-master/src"
  make: *** [Makefile:423: src] Error 2


Ciao,
 Angelo.

Re: Emacs build fails [MSYS2/MINGW64]

[Paul Eggert]

Angelo Graziosi wrote:
> Starting with this commit:
> 
> Paul Eggert :
> Mark _Noreturn error functions as cold
> 2019-04-18 00:35:18
> 
> the build of Emacs up to master (the last I tried is 75ee20364) fails (bootstrap-emacs.pdmp SEGMENT FAULT).

That's odd, since adding __attribute__ ((cold)) should affect only performance. 
If it causes a core dump, that suggests that there is a bug either somewhere 
else in Emacs (which __attribute__ ((cold)) happens to tickle) or in the 
compiler. Can you reproduce the problem when compiling with '-g3 -O0 -gdwarf-4' 
and get a backtrace, as suggested in etc/DEBUG?

Re: Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

> Il 28 aprile 2019 alle 18.10 Paul Eggert <eggert@cs.ucla.edu> ha scritto:
> 
> 
> Angelo Graziosi wrote:
> > Starting with this commit:
> > 
> > Paul Eggert :
> > Mark _Noreturn error functions as cold
> > 2019-04-18 00:35:18
> > 
> > the build of Emacs up to master (the last I tried is 75ee20364) fails (bootstrap-emacs.pdmp SEGMENT FAULT).
> 
> That's odd, since adding __attribute__ ((cold)) should affect only performance. 
> If it causes a core dump, that suggests that there is a bug either somewhere 
> else in Emacs (which __attribute__ ((cold)) happens to tickle) or in the 
> compiler. Can you reproduce the problem when compiling with '-g3 -O0 -gdwarf-4' 
> and get a backtrace, as suggested in etc/DEBUG?

No, see my replay to Eli...

Re: Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

> Il 28 aprile 2019 alle 18.10 Paul Eggert <eggert@cs.ucla.edu> ha scritto:
> 
> 
> Angelo Graziosi wrote:
> > Starting with this commit:
> > 
> > Paul Eggert :
> > Mark _Noreturn error functions as cold
> > 2019-04-18 00:35:18
> > 
> > the build of Emacs up to master (the last I tried is 75ee20364) fails (bootstrap-emacs.pdmp SEGMENT FAULT).
> 
> That's odd, since adding __attribute__ ((cold)) should affect only performance. 
> If it causes a core dump, that suggests that there is a bug either somewhere 
> else in Emacs (which __attribute__ ((cold)) happens to tickle) or in the 
> compiler. Can you reproduce the problem when compiling with '-g3 -O0 -gdwarf-4' 
> and get a backtrace, as suggested in etc/DEBUG?

Just for completeness...

It seems that it is the optimization flag which causes the issue. Using "-O0" on place of "-O2" or "-O3" in my script fixes the issue...

Re: Emacs build fails [MSYS2/MINGW64]

[martin rudalics]

 > It seems that it is the optimization flag which causes the issue. Using "-O0" on place of "-O2" or "-O3" in my script fixes the issue...

Same here.  See also Bug#35410 for different behaviors of O3 and O0
builds with gcc 7.3.0 32-bit builds using MSYS2.

martin

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 29 Apr 2019 14:49:42 +0200
> Cc: emacs-devel@gnu.org
> 
>  > It seems that it is the optimization flag which causes the issue. Using "-O0" on place of "-O2" or "-O3" in my script fixes the issue...
> 
> Same here.  See also Bug#35410 for different behaviors of O3 and O0
> builds with gcc 7.3.0 32-bit builds using MSYS2.

So GCC 7.x might also have problems with 'cold'...

I can confirm that building with MinGW GCC 8.2.0 and -O2 causes
crashes in several places during the bootstrap.  The crashes that I
saw are different from what Angelo reported, and are also different
from the report in bug#35409, but I guess this is a kind of Heisenbug,
so I'm not surprised to see it pop in different places on different
systems.  In one case the cause of the crash was a wrong-type-argument
error where the offending object was a pseudo-vector of the type
PVEC_FREE, i.e. it was already put on the free list; this caused a
print.c function to barf trying to display the object.  Clearly, some
bad code was involved, and I suspect that bad code came from GCC, not
from us.

Just applying the simple patch below allows the build to run
flawlessly to completion:

--- src/conf_post.h~0	2019-04-28 09:39:12.000000000 +0300
+++ src/conf_post.h	2019-04-29 08:08:30.619991000 +0300
@@ -225,7 +225,7 @@
 extern char *emacs_getenv_TZ (void);
 extern int emacs_setenv_TZ (char const *);
 
-#if __has_attribute (cold)
+#if __has_attribute_cold && !defined(__MINGW32__)
 # define ATTRIBUTE_COLD __attribute__ ((cold))
 #else
 # define ATTRIBUTE_COLD

This could be a MinGW-specific bug, but then again, it could be that
other platforms are just lucky, or maybe not enough people tried to
build the master branch with optimizations lately.

Stepping back a little, I'm not sure we want this ATTRIBUTE_COLD
business in Emacs, on any platform.  At the very least, it causes
unintended consequences, see, for example, the comments by Florian
Weimer in this GCC bug report:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88793

Also, the GCC manual indicates that marking a function as 'cold'
causes that function to be optimized for size, and we already know
that GCC 8.x has at least one bug with -Os used together with -O2,
which seems to be exactly what happens when 'cold' is being used in an
otherwise -O2 build.

Moreover, I think we have way too many functions marked with this
attribute.  Functions like wrong_type_argument or Fthrow or
Fabort_recursive_edit or Fsignal or error or Ftop_level -- do they
really fit the below description?

       The 'cold' attribute on functions is used to inform the compiler
       that the function is unlikely to be executed.  The function is
       optimized for size rather than speed and on many targets it is
       placed into a special subsection of the text section so all cold
       functions appear close together, improving code locality of
       non-cold parts of program.  The paths leading to calls of cold
       functions within code are marked as unlikely by the branch
       prediction mechanism.  It is thus useful to mark functions used to
       handle unlikely conditions, such as 'perror', as cold to improve
       optimization of hot functions that do call marked functions in rare
       occasions.

I don't think the above functions are called "rarely" enough in Emacs
to justify marking them as 'cold'.  Stuff like emacs_abort or
memory_full, maybe; but functions that jump to top-level are not in
that class, IMO, not in Emacs Lisp.

The problem I see with marking too many functions 'cold' is that then
any path leading to those functions is also considered "unlikely", and
who knows what that does to our code?  "nm -A" on an Emacs executable
compiled with ATTRIBUTE_COLD shows no less than 2400(!) local symbols
with the ".cold" suffix, whereas a "normal" executable has just 7 of
them.  And what are the benefits we get for all this risky business?

IMO, we should simply avoid this feature.  But if we leave it in the
sources, we should disable it for MinGW, it seems.

Re: Emacs build fails [MSYS2/MINGW64]

[Paul Eggert]

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

On 4/29/19 7:53 AM, Eli Zaretskii wrote:
> see, for example, the comments by Florian
> Weimer in this GCC bug report:
>
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88793

Those comments don't invalidate all uses of __attribute__ ((cold)) in
Emacs. Florian says that in cold functions on some platforms, strlen on
large (5000-byte) strings can be as much as 60 times slower due to
questionable optimizations in GCC that are currently being rethought
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88809#c5>. However, if the
functions currently marked 'cold' in Emacs are called so rarely (at
least on such large strings) that the performance cost (factor of 60 for
rarely-used code) is outweighed by the performance benefit (small factor
of improvement in commonly-used code), then from a performance viewpoint
it's still preferable to mark these functions 'cold'.


> the GCC manual indicates that marking a function as 'cold'
> causes that function to be optimized for size, and we already know
> that GCC 8.x has at least one bug with -Os used together with -O2,

The GCC bug report <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90020>
says that the bug can also occur with -O2, and can occur without
__attribute__ ((cold)). So it's not as simple as saying "__attribute__
((cold)) is buggy".


> I think we have way too many functions marked with this
> attribute.  Functions like wrong_type_argument or Fthrow or
> Fabort_recursive_edit or Fsignal or error or Ftop_level -- do they
> really fit the below description?

It should be purely a performance issue.

For example, on my platform (x86-64, GCC 8.3.1, -O2, Fedora 29) after
inlining, Emacs has 1066 static calls to wrong_type_argument, and with
__attribute__ ((cold)) all these calling sites can be optimized. In
bytecode.c this causes GCC to move wrong_type_argument calls to a cold
section of the text, which means the bytecode interpreter should run
faster for all the usual branch-prediction and icache reasons.

I measured the effect of __attribute__ ((cold)) (including both costs
and benefits) as improving overall performance of a big benchmark ('make
compile-always') by 1.3% on my platform. This test included all the
roughly 50 functions currently marked as cold in Emacs. Although I can
imagine that removing __attribute__ ((cold)) from some of these
functions would not hurt overall performance significantly, I didn't
think it worth the time to investigate which ones. That is, I didn't
investigate marking a smaller set of functions. If someone wants to do
that work then I expect that we should be able to remove markings from
some of these functions without affecting performance significantly.


> if we leave it in the
> sources, we should disable it for MinGW, it seems.

Yes, that sounds like a good idea. I installed the attached patch. It
would be good to know what the actual bug is (it's possible that it's
not a compiler bug at all, but is a portability bug in Emacs), but
that's less urgent.


[-- Attachment #2: 0001-Disable-__attribute__-cold-on-MinGW.patch --]
[-- Type: text/x-patch, Size: 1374 bytes --]

From aaf33358e79e0210f2e8dab17fd8a33ac97383b9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 29 Apr 2019 12:27:04 -0700
Subject: [PATCH] Disable __attribute__ ((cold)) on MinGW

* src/conf_post.h (ATTRIBUTE_COLD) [__MINGW32__]:
Define to empty on this platform.
---
 src/conf_post.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/conf_post.h b/src/conf_post.h
index 7699d2c95b..4af1ba9331 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -59,7 +59,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
    into the same 1-, 2-, or 4-byte allocation unit in the MinGW
    builds.  It was also needed to port to pre-C99 compilers, although
    we don't care about that any more.  */
-#if NS_IMPL_GNUSTEP || defined(__MINGW32__)
+#if NS_IMPL_GNUSTEP || defined __MINGW32__
 typedef unsigned int bool_bf;
 #else
 typedef bool bool_bf;
@@ -225,7 +225,9 @@ extern void _DebPrint (const char *fmt, ...);
 extern char *emacs_getenv_TZ (void);
 extern int emacs_setenv_TZ (char const *);
 
-#if __has_attribute (cold)
+/* Avoid __attribute__ ((cold)) on MinGW; see thread starting at
+   <https://lists.gnu.org/r/emacs-devel/2019-04/msg01152.html>. */
+#if __has_attribute (cold) && !defined __MINGW32__
 # define ATTRIBUTE_COLD __attribute__ ((cold))
 #else
 # define ATTRIBUTE_COLD
-- 
2.20.1

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 29 Apr 2019 12:38:02 -0700
> Cc: angelo.g0@libero.it, emacs-devel@gnu.org
> 
> -#if __has_attribute (cold)
> +/* Avoid __attribute__ ((cold)) on MinGW; see thread starting at
> +   <https://lists.gnu.org/r/emacs-devel/2019-04/msg01152.html>. */
> +#if __has_attribute (cold) && !defined __MINGW32__

We defined __has_attribute_cold, so no need to use __has_attribute
here, right?

Re: Emacs build fails [MSYS2/MINGW64]

[Paul Eggert]

On 4/30/19 8:45 AM, Eli Zaretskii wrote:
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Mon, 29 Apr 2019 12:38:02 -0700
>> Cc: angelo.g0@libero.it, emacs-devel@gnu.org
>>
>> -#if __has_attribute (cold)
>> +/* Avoid __attribute__ ((cold)) on MinGW; see thread starting at
>> +   <https://lists.gnu.org/r/emacs-devel/2019-04/msg01152.html>. */
>> +#if __has_attribute (cold) && !defined __MINGW32__
> We defined __has_attribute_cold, so no need to use __has_attribute
> here, right?

conf_post.h defines __has_attribute_cold only on older compilers that
lack support for __has_attribute. So we still need to use
__has_attribute here, for newer compilers (GCC 5 and later, for example).

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 30 Apr 2019 09:49:29 -0700
> 
> conf_post.h defines __has_attribute_cold only on older compilers that
> lack support for __has_attribute. So we still need to use
> __has_attribute here, for newer compilers (GCC 5 and later, for example).

Oh, it's the other way around...  Right, sorry for the noise.

Re: Emacs build fails [MSYS2/MINGW64]

[martin rudalics]

 > I installed the attached patch. It
 > would be good to know what the actual bug is (it's possible that it's
 > not a compiler bug at all, but is a portability bug in Emacs), but
 > that's less urgent.

Thanks.  Builds now without problems.

martin

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> Date: Sun, 28 Apr 2019 17:37:44 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> 
> To reproduce, update your MSYS2 installation (pacman -Syu) and if you have ImageMagick installed, add '--without-imagemagick' to 'configure' otherwise it fails at compile time. 

Please also show how it fails at compile time, we would want to fix
that part as well.

> In a MINGW64 shell:
> 
>   wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-master.tar.gz
>   tar -xof emacs-master.tar.gz
>   cd emacs-master
>   ./autogen.sh
>   ./configure [--without-imagemagick]
>   make
>   [...]
>   Loading c:/msys64/tmp/emacs-master/lisp/emacs-lisp/lisp-mode.el (source)...
>   Loading c:/msys64/tmp/emacs-master/lisp/progmodes/elisp-mode.el (source)...
>   make[1]: *** [Makefile:808: bootstrap-emacs.pdmp] Segmentation fault
>   make[1]: uscita dalla directory "/tmp/emacs-master/src"
>   make: *** [Makefile:423: src] Error 2

Thanks.  But please in the future report bugs to the bug tracker using
"M-x report-emacs-bug RET".

Re: Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

> Il 28 aprile 2019 alle 18.21 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sun, 28 Apr 2019 17:37:44 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > 
> > To reproduce, update your MSYS2 installation (pacman -Syu) and if you have ImageMagick installed, add '--without-imagemagick' to 'configure' otherwise it fails at compile time. 
> 
> Please also show how it fails at compile time, we would want to fix
> that part as well.

[...]
CCLD     temacs.exe
image.o:image.c:(.data+0x60): undefined reference to `init_imagemagick_functions'
collect2.exe: error: ld returned 1 exit status
make[1]: *** [Makefile:643: temacs.exe] Error 1


> 
> > In a MINGW64 shell:
> > 
> >   wget http://git.savannah.gnu.org/cgit/emacs.git/snapshot/emacs-master.tar.gz
> >   tar -xof emacs-master.tar.gz
> >   cd emacs-master
> >   ./autogen.sh
> >   ./configure [--without-imagemagick]
> >   make
> >   [...]
> >   Loading c:/msys64/tmp/emacs-master/lisp/emacs-lisp/lisp-mode.el (source)...
> >   Loading c:/msys64/tmp/emacs-master/lisp/progmodes/elisp-mode.el (source)...
> >   make[1]: *** [Makefile:808: bootstrap-emacs.pdmp] Segmentation fault
> >   make[1]: uscita dalla directory "/tmp/emacs-master/src"
> >   make: *** [Makefile:423: src] Error 2
> 
> Thanks.  But please in the future report bugs to the bug tracker using
> "M-x report-emacs-bug RET".

I don't see how.. I am non inside Emacs when building a new Emacs... I don't think MinTTY understands that...

Any way, with

  CFLAGS="-g3 -O0 -gdwarf-4" ./configure ...

as suggested by Paul seems to work: it does not fails...

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> Date: Sun, 28 Apr 2019 18:41:24 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org
> 
> > Thanks.  But please in the future report bugs to the bug tracker using
> > "M-x report-emacs-bug RET".
> 
> I don't see how.. I am non inside Emacs when building a new Emacs... I don't think MinTTY understands that...

It doesn't have to be from the same Emacs which crashes, it can be a
different version, if the problematic version cannot be built.

Re: Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

Stephen Leake wrote:
> I have not updated mingw64 gcc in a while, so it seems a recent update
> to that causes this problem. I have gcc 7.3.0

Indeed I wrote to update MSYS2 to reproduce the issue... mingw-w64-gcc is now 8.3..

Re: Emacs build fails [MSYS2/MINGW64]

[Angelo Graziosi]

> Il 28 aprile 2019 alle 19.30 Eli Zaretskii <eliz@gnu.org> ha scritto:
> 
> 
> > Date: Sun, 28 Apr 2019 18:41:24 +0200 (CEST)
> > From: Angelo Graziosi <angelo.g0@libero.it>
> > Cc: emacs-devel@gnu.org
> > 
> > > Thanks.  But please in the future report bugs to the bug tracker using
> > > "M-x report-emacs-bug RET".
> > 
> > I don't see how.. I am non inside Emacs when building a new Emacs... I don't think MinTTY understands that...
> 
> It doesn't have to be from the same Emacs which crashes, it can be a
> different version, if the problematic version cannot be built.

but the working copy here (built on March 30) has a different configuration.. I wonted to reproduce the issu in the simplest manner: ./configure && make..

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> Date: Sun, 28 Apr 2019 23:24:55 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org
> 
> > It doesn't have to be from the same Emacs which crashes, it can be a
> > different version, if the problematic version cannot be built.
> 
> but the working copy here (built on March 30) has a different configuration.. I wonted to reproduce the issu in the simplest manner: ./configure && make..

That's okay: it's the best you can do when the problematic
configuration doesn't build.

The main reason for my request is to have the bugs recorded and
discussed on the bug forum, where they belong.

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> Date: Sun, 28 Apr 2019 18:41:24 +0200 (CEST)
> From: Angelo Graziosi <angelo.g0@libero.it>
> Cc: emacs-devel@gnu.org
> 
> CCLD     temacs.exe
> image.o:image.c:(.data+0x60): undefined reference to `init_imagemagick_functions'
> collect2.exe: error: ld returned 1 exit status
> make[1]: *** [Makefile:643: temacs.exe] Error 1

This isn't a new problem, is it?  AFAICT, it exists for the past 7
years.

Re: Emacs build fails [MSYS2/MINGW64]

[Stephen Leake]

Angelo Graziosi <angelo.g0@libero.it> writes:

> Starting with this commit:
>
> Paul Eggert :
> Mark _Noreturn error functions as cold
> 2019-04-18 00:35:18
>
> the build of Emacs up to master (the last I tried is 75ee20364) fails (bootstrap-emacs.pdmp SEGMENT FAULT). 
>
> To reproduce, update your MSYS2 installation (pacman -Syu) and if you
> have ImageMagick installed, add '--without-imagemagick' to 'configure'
> otherwise it fails at compile time. 

I'm also using MSYS2/MINGW64, and building master works for me.

I have not updated mingw64 gcc in a while, so it seems a recent update
to that causes this problem. I have gcc 7.3.0; what do you have?

-- 
-- Stephe

Re: Emacs build fails [MSYS2/MINGW64]

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Sun, 28 Apr 2019 09:46:32 -0800
> 
> I'm also using MSYS2/MINGW64, and building master works for me.
> 
> I have not updated mingw64 gcc in a while, so it seems a recent update
> to that causes this problem. I have gcc 7.3.0; what do you have?

Try building a fresh checkout (so the build performs a full
bootstrap), and use the default compiler switches, which should
include -O2.  I'm interested to know whether the problem is limited to
GCC 8.x.

TIA