LilyIssues / Issues / #4814 grob.cc segfaults on Fedora 24 with gcc6

Valentin Villenave - 2016-04-02

The exact same LilyPond snapshot, built with gcc 5.3.1 (and the corresponding libstd version) instead of gcc 6.0.0, doesn’t segfault.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Federico Bruni - 2016-04-02

I confirm no segfault on Fedora 23 and gcc at version 5.3.1.
I'm going to upgrade to Fedora 24 around May/June and I hope that this will be fixed.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Federico Bruni - 2016-06-23

I've just switched to Fedora 24 (and GCC6) and I get segfault on some files.
Let me know if you need other information

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

I cannot 'make doc' anymore because of the regtests, as reported by Valentin.
Here's the backtrace:

(gdb) run input/regression/dynamics-context-textspan.ly
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /home/fede/src/lilypond-git/out/bin/lilypond input/regression/dynamics-context-textspan.ly
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
GNU LilyPond 2.19.45
Processing `input/regression/dynamics-context-textspan.ly'
Parsing...
Interpreting music...
Preprocessing graphical objects...
Program received signal SIGSEGV, Segmentation fault.
0x000000000044042f in Grob::get_offset (this=this@entry=0x0, a=a@entry=X_AXIS) at grob.cc:400
400   if (dim_cache_[a].offset_)
(gdb) 
(gdb) 
(gdb) 
(gdb) 
(gdb) 
(gdb) 
(gdb) bt
#0  0x000000000044042f in Grob::get_offset(Axis) const (this=this@entry=0x0, a=a@entry=X_AXIS)
    at grob.cc:400
#1  0x0000000000440528 in Grob::relative_coordinate(Grob const*, Axis) const (this=0x0, refp=0xecf920, a=X_AXIS) at grob.cc:341
#2  0x000000000044054e in Grob::relative_coordinate(Grob const*, Axis) const (this=0xea5000, refp=0xecf920, a=X_AXIS) at grob.cc:345
#3  0x000000000044054e in Grob::relative_coordinate(Grob const*, Axis) const (this=0xea59e0, refp=refp@entry=0xecf920, a=a@entry=X_AXIS) at grob.cc:345
#4  0x00000000005e5aae in Side_position_interface::aligned_side(Grob*, Axis, bool, int, int, double*) (me=me@entry=0xecf920, a=a@entry=Y_AXIS, pure=true, start=start@entry=0, end=end@entry=2, current_off=current_off@entry=0x0) at side-position-interface.cc:223
#5  0x00000000005e6448 in axis_aligned_side_helper(scm_unused_struct*, Axis, bool, int, int, scm_unused_struct*) (smob=<optimized out>, a=Y_AXIS, pure=<optimized out>, start=0, end=2, current_off_scm=0x204)
    at side-position-interface.cc:105
#6  0x00007ffff791fedf in scm_dapply (proc=0x7ffff30ed4f0, arg1=0x7ffff3aa6380, args=0x7ffff0141060)
    at eval.c:4930
#7  0x0000000000441257 in Grob::pure_relative_y_coordinate(Grob const*, int, int) (this=this@entry=0xecf920, refp=refp@entry=0xecfc00, start=start@entry=0, end=end@entry=2) at grob.cc:371
#8  0x00000000004413e1 in Grob::pure_y_extent(Grob*, int, int) (this=0xecf920, refp=0xecfc00, start=0, end=2) at grob.cc:497
#9  0x000000000046df17 in Axis_group_interface::adjacent_pure_heights(scm_unused_struct*) (smob=<optimized out>) at axis-group-interface.cc:287
#10 0x00007ffff791fcf5 in scm_dapply (proc=0x7ffff39780b0, arg1=0x7ffff0392e20, args=0x404)
    at eval.c:4895
#11 0x0000000000673311 in Grob::try_callback_on_alist(scm_unused_struct**, scm_unused_struct*, scm_unused_struct*) (this=this@entry=0xecfc00, alist=alist@entry=0xecfc60, sym=0x7ffff30b0360, proc=0x7ffff39780b0) at grob-property.cc:232
#12 0x000000000067345d in Grob::internal_get_property(scm_unused_struct*) const (this=this@entry=0xecfc00, sym=<optimized out>) at grob-property.cc:184
#13 0x000000000046a918 in Axis_group_interface::part_of_line_pure_height(Grob*, bool, int, int) (me=me@entry=0xecfc00, begin=begin@entry=true, start=start@entry=0, end=end@entry=1)
    at axis-group-interface.cc:162
#14 0x000000000046aa53 in Axis_group_interface::begin_of_line_pure_height(Grob*, int) (me=me@entry=0xecfc00, start=start@entry=0) at axis-group-interface.cc:186
#15 0x000000000046aad5 in Axis_group_interface::sum_partial_pure_heights(Grob*, int, int) (me=me@entry=0xecfc00, start=start@entry=0, end=end@entry=2147483647) at axis-group-interface.cc:143
#16 0x000000000046acf4 in Axis_group_interface::relative_pure_height(Grob*, int, int) (me=me@entry=0xecfc00, start=start@entry=0, end=end@entry=2147483647) at axis-group-interface.cc:334
#17 0x000000000046b5b0 in Axis_group_interface::pure_group_height(Grob*, int, int) (me=me@entry=0xecfc00, start=start@entry=0, end=end@entry=2147483647) at axis-group-interface.cc:592
#18 0x0000000000437cee in Hara_kiri_group_spanner::pure_height(scm_unused_struct*, scm_unused_struct*, scm_unused_struct*) (smob=<optimized out>, start_scm=<optimized out>, end_scm=<optimized out>)
    at hara-kiri-group-spanner.cc:58
#19 0x00007ffff791ff5d in scm_dapply (proc=0x7ffff3964450, arg1=0x7ffff0392e20, args=0x7ffff01426c0)
    at eval.c:4927
#20 0x00000000004413a4 in Grob::pure_y_extent(Grob*, int, int) (this=0xecfc00, refp=0xecfc00, start=0, end=2147483647) at grob.cc:495
#21 0x000000000056edbd in Align_interface::internal_get_minimum_translations(Grob*, std::vector<Grob*, std::allocator<Grob*> > const&, Axis, bool, bool, int, int) (end=2147483647, start=0, pure=true, other_common=0xea58c0, a=Y_AXIS, g=0xecfc00, this=<optimized out>) at align-interface.cc:96
#22 0x000000000056edbd in Align_interface::internal_get_minimum_translations(Grob*, std::vector<Grob*, std::allocator<Grob*> > const&, Axis, bool, bool, int, int) (me=me@entry=0xed0700, elems=std::vector of length 3, capacity 4 = {...}, a=a@entry=Y_AXIS, include_fixed_spacing=<optimized out>, 
    include_fixed_spacing@entry=true, pure=pure@entry=true, start=<optimized out>, end=<optimized out>)
    at align-interface.cc:212
#23 0x000000000056f6dd in Align_interface::get_pure_minimum_translations(Grob*, std::vector<Grob*, std::allocator<Grob*> > const&, Axis, int, int) (me=me@entry=0xed0700, all_grobs=std::vector of length 3, capacity 4 = {...}, a=a@entry=Y_AXIS, start=start@entry=0, end=end@entry=2147483647)

David Kastrup - 2016-07-05

Ok, bad news. I have managed to find a g++-6 package for Ubuntu and managed to compile 64-bit executables with them.

The version is
g++-6 (Ubuntu 6.1.1-2ubuntu12~16.04) 6.1.1 20160510
Copyright (C) 2016 Free Software Foundation, Inc.

and using either --disable-optimising, --enable-checking, or no option at all I have been able to make check (the stats are nonsensical without one of the options enabling checking but other than that the results are identical).

This can have a number of reasons:

g++ 6.1.1 might generate better code than g++ 6.0 used in Fedora
The Fedora g++ might have additional patches that are a bad idea
The Ubuntu g++ might have additional patches that are a good idea
The problem might occur when compiling one of the libraries (mine are not compiled with g++-6).
The difference might be caused by one of the options used for compiling, likely on behalf on some libraries xxx-config program stating so (for example, my compilations are made with -fwrapv, probably on demand by python-config: -I/usr/include/python2.7 -I/usr/include/i386-linux-gnu/python2.7 -fno-strict-aliasing -g -fstack-protector-strong -g -fwrapv).

So I think the next sane step would be for Fedora users to see whether they can get a hold of some g++-6.1 version and look whether they fare better with that (of course using libguile 1.8.8 as we don't support anything else).

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

On Fedora 24 I currently have:

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/6.1.1/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --disable-libgcj --with-isl --enable-libmpx --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 6.1.1 20160621 (Red Hat 6.1.1-3) (GCC)

If I run configure without options, 'make check' stops almost immediately.
If I run configure with --disable-optimising, 'make check' stops after a while with the following error:

make[2]: Leaving directory '/home/fede/src/lilypond-git/input/regression/lilypond-book'
make[1]: Leaving directory '/home/fede/src/lilypond-git/input/regression/lilypond-book'
rm -rf /home/fede/src/lilypond-git/out/test-results
mkdir -p /home/fede/src/lilypond-git/out/test-results
/home/fede/src/lilypond-git/scripts/build/out/output-distance --local-datadir --create-images --output-dir /home/fede/src/lilypond-git/out/test-results \
    input/regression/out-test-baseline input/regression/out-test \
    input/regression/midi/out-test-baseline input/regression/midi/out-test 
Failed to walk through input/regression/out-test-baseline. This can be caused by forgetting to run make test-baseline.
GNUmakefile:331: recipe for target 'local-check' failed
make: *** [local-check] Error 1

David Kastrup - 2016-07-05

That's mostly irrelevant. make check indeed requires a successful run of make test-baseline for comparison. It is only make test which does not rely on a baseline, and it will still require a successful run of make before that in order to have fonts and executables to work with.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Federico Bruni - 2016-07-05

You are right.

Good news is that I'm able to run 'make check' and 'make doc', thanks to --disable-optimising.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- David Kastrup - 2016-07-05
  
  But not otherwise? That would be disconcerting. It would mean that the problem is attributable to a difference in code generation, even though our current GCC6 compiler versions are pretty similar.
  
  That would either point to different options (like the -fwrapv I mentioned) or different compiler patches/defaults.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Federico Bruni - 2016-07-07

Correct, it fails unless I use --disable-optimising.

I searched some information about GCC6 in Fedora:

https://www.phoronix.com/scan.php?page=news_item&px=Fedora-GCC6-Rebuild-Results

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/DH7M2ADHM6XCRFTRRSKZD6MWFUJKHBZK/

https://apps.fedoraproject.org/packages/gcc/sources/patches/

Last edit: Federico Bruni 2016-07-07
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Guido Aulisi - 2016-07-22

I had the same problem, I think it's due to this change in gcc 6:

Optimizations remove null pointer checks for this

When optimizing, GCC now assumes the this pointer can never be null, which is guaranteed by the language rules. Invalid programs which assume it is OK to invoke a member function through a null pointer (possibly relying on checks like this != NULL) may crash or otherwise fail at run time if null pointer checks are optimized away.

I also faced a FTBFS problem with const initialization:
'constexpr' needed for in-class initialization of static data member

I made 2 patches that seem to solve the problems and I emailed them to lilypond-devel, I'll attach them here,too.

0001-Avoid-segfault-in-grob.cc-with-gcc-6-see-issue-4814.txt

0002-Fix-FTBFS-with-GCC-6.txt

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- David Kastrup - 2016-07-22
  
  When optimizing, GCC now assumes the this pointer can never be null, which is guaranteed by the language rules. Invalid programs which assume it is OK to invoke a member function through a null pointer (possibly relying on checks like this != NULL) may crash or otherwise fail at run time if null pointer checks are optimized away.
  
  Ugh. Another one of those "undefined behavior means that we can screw the user by throwing out code written intentionally without warning" things. So something like assert(this); will become a nop, causing much harder to debug crashes later on. I thought that one developer particularly known for this hobby had moved to new pastures mostly.
  
  At any rate, this might explain why I still fail to see the crash with GCC6 on Ubuntu; it's an option likely to get overruled with local patches from people trying to keep a distribution running.
  
  Hm. Does the unmodified source work after
  
  ./configure CXX="g++ -fno-delete-null-pointer-checks" make clean make
  
  ? The description is not completely suggestive of that optimization:
  
  '-fdelete-null-pointer-checks'
  Assume that programs cannot safely dereference null pointers, and
  that no code or data element resides at address zero. This option
  enables simple constant folding optimizations at all optimization
  levels. In addition, other optimization passes in GCC use this
  flag to control global dataflow analyses that eliminate useless
  checks for null pointers; these assume that a memory access to
  address zero always results in a trap, so that if a pointer is
  checked after it has already been dereferenced, it cannot be null.
  
  Note however that in some environments this assumption is not true.
  Use '-fno-delete-null-pointer-checks' to disable this optimization
  for programs that depend on that behavior.
  
  This option is enabled by default on most targets. On Nios II ELF,
  it defaults to off. On AVR and CR16, this option is completely
  disabled.
  
  Passes that use the dataflow information are enabled independently
  at different optimization levels.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Guido Aulisi - 2016-07-22

I compiled the master branch (commit 445bf3bb2fbd1) adding "-fno-delete-null-pointer-checks -std=gnu++98" to the normal Fedora 24 CXXFLAGS and it works, I no longer get the crash and my score is correctly rendered.

But if I remember well Fedora policy is to compile all c++ program at least with -std=c++11, and I don't know what the policy is about pointer check optimization.
g++ manual reports that other optimizations are disabled with this option.

With c++11 this is not permitted:
struct X {const static double i = 10;};
but this is permitted:
struct X {constexpr static double i = 10;};

So if we use c++11, we have problems with:
const Real Audio_span_dynamic::MINIMUM_VOLUME;
const Real Audio_span_dynamic::MAXIMUM_VOLUME;
const Real Audio_span_dynamic::DEFAULT_VOLUME;

and this is the FTBFS I tried to fix with my second patch.

I think assuming "this" is never NULL makes sense, "this" is used inside instances when they exist.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- H T LilyPond - 2016-07-23
  
  With c++11 this is not permitted:
  struct X {const static double i = 10;};
  but this is permitted:
  struct X {constexpr static double i = 10;};
  
  So if we use c++11, we have problems with:
  const Real Audio_span_dynamic::MINIMUM_VOLUME;
  const Real Audio_span_dynamic::MAXIMUM_VOLUME;
  const Real Audio_span_dynamic::DEFAULT_VOLUME;
  
  'constexpr' is not available in the older C++ standard which is still the default used in previous versions of GCC such as GCC 4.9 (which is still in use, for example, in GUB, if I'm not mistaken). Here's a sample of the output that I get locally with the second patch (and intentionally using GCC 4.9):
  
  $ g++-4.9 --version
  g++-4.9 (Debian 4.9.3-14) 4.9.3
  Copyright (C) 2015 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions. There is NO
  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
  
  $ CC=gcc-4.9 CXX=g++-4.9 ../configure
  ...
  $ make all
  ...
  [...]/lily/include/audio-item.hh:51:3: warning: identifier 'constexpr' is a keyword in C++11 [-Wc++0x-compat]
  static constexpr Real MINIMUM_VOLUME = 0.0;
  ^
  [...]/lily/include/audio-item.hh:51:10: error: 'constexpr' does not name a type
  static constexpr Real MINIMUM_VOLUME = 0.0;
  ^
  [...]/lily/include/audio-item.hh:51:10: note: C++11 'constexpr' only available with -std=c++11 or -std=gnu++11
  ...
  $
  
  An alternative fix that would likely be better compatible across C++ standard versions would be to just move the initialization of the static constants away from the Audio_span_dynamic class declaration (to audio-item.cc).
  
  Is it OK if I prepare a fix for this as a separate patch issue (since the FTBFS problem doesn't concern the segfault)?
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - David Kastrup - 2016-07-23
    
    Definitely a separate patch. Should I try rolling the segfault issue into Rietveld then? I've checked for other obvious comparisons of this with NULL without finding any. It's sort-of squishy that the small-smobs.hh classes use the this-pointer as a fake SCM value though. I don't see comparisons with 0 here, but then who knows what other deductions G++ is going to make eventually.
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
    - H T LilyPond - 2016-07-23
      
      Definitely a separate patch.
      
      The fix for the FTBFS is now Issue 4944.
      
      Should I try rolling the segfault issue into Rietveld then? I've checked for other obvious comparisons of this with NULL without finding any. It's sort-of squishy that the small-smobs.hh classes use the this-pointer as a fake SCM value though. I don't see comparisons with 0 here, but then who knows what other deductions G++ is going to make eventually.
      
      Please do, I haven't looked at the crash (I only noticed the report about the build problem, which happened to be caused by code that I reviewed a while ago).
      
      Last edit: H T LilyPond 2016-07-23
      
      If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Guido Aulisi - 2016-07-23
    
    An alternative fix that would likely be better compatible across C++ standard versions would be to just move the initialization of the static constants away from the Audio_span_dynamic class declaration (to audio-item.cc).
    
    I agree, this would be ok with all c++ dialects.
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2016-07-23

On 22/07/16 14:41, Guido Aulisi wrote:

Hello,
I made 2 patches for issue #4814 and for a FTBFS witch gcc 6 on Fedora 24.
These 2 patches seem to solve that problem for me.

Best regards
Guido Aulisi

0001-Avoid-segfault-in-grob.cc-with-gcc-6-see-issue-4814.txt

0002-Fix-FTBFS-with-GCC-6.txt

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

David Kastrup - 2016-07-23

Issue 4814: grob.cc segfaults with gcc6

From the release notes of GCC 6:

Optimizations remove null pointer checks for this When optimizing, GCC now assumes the this pointer can never be null, which is guaranteed by the language rules. Invalid programs which assume it is OK to invoke a member function through a null pointer (possibly relying on checks like this != NULL) may crash or otherwise fail at run time if null pointer checks are optimized away. With the -Wnull-dereference option the compiler tries to warn when it detects such invalid code. If the program cannot be fixed to remove the undefined behavior then the option -fno-delete-null-pointer-checks can be used to disable this optimization. That option also disables other optimizations involving pointers, not only those involving this.

As a consequence, we cannot call a member function on a prospective null
pointer (which actually is a bad idea for a number of other reasons,
like when anything tries accessing the vtable) and then try sorting out
the condition in the routine itself.

This problem was first observed with Fedora 24. The Ubuntu GCC6
prerelease does not show this problem; presumably the respective
optimization has been disabled in the Ubuntu/Debian packaging because of
affecting other programs.

Commit-message-by: David Kastrup dak@gnu.org
Signed-off-by: David Kastrup dak@gnu.org

http://codereview.appspot.com/309750043
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Anonymous - 2016-07-23
  
  With just this patch I cannot do any 'make' operations it fails
  
  --snip--
  
  make[1]: [out/dynamic-performer.o] Error 1
  make[1]: Waiting for unfinished jobs....
  /home/james/lilypond-git/lily/lily-lexer.cc: In member function 'int Lily_lexer::lookup_keyword(const string&)':
  /home/james/lilypond-git/lily/lily-lexer.cc:185:28: warning: conversion to 'int' from 'vsize {aka long unsigned int}' may alter its value [-Wconversion]
  return keytable_->lookup (s.c_str ());
  ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
  /home/james/lilypond-git/lily/score.cc: In member function 'scm_unused_struct Score::book_rendering(Output_def, Output_def*)':
  /home/james/lilypond-git/lily/score.cc:125:33: warning: conversion to 'int' from 'std::vector<Output_def*>::size_type {aka long unsigned int}' may alter its value [-Wconversion]
  int outdef_count = defs_.size ();
  ~~~~~~~~~~~^~
  make[1]: Leaving directory '/home/james/lilypond-git/build/lily'
  /home/james/lilypond-git/stepmake/stepmake/generic-targets.make:6: recipe for target 'all' failed
  
  --snip--
  
  With the patch above (which I think includes the edits to grob.cc in your Rietveld issue David, I can do all the make tests.
  
  At the moment (at home anyway) I cannot do any patch testing while I am on Fedora 24 with GCC 6.1.1. Withour, it seems, these fixes from yourself/Guido.
  
  Patchy staging is run from Work and is using Ubuntu 16.04 which is on an older version of GCC.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2016-07-23

I applied both patches provided by Guido in comment https://sourceforge.net/p/testlilyissues/issues/4814/#94d9 (above) and I was able to make, make test-baseline (I cannot do a make check because I cannot compile at all without the patch while using gcc (GCC) 6.1.1 20160621 (Red Hat 6.1.1-3) which is what current Fedora uses).

It seems that the issue above (David K's comment) includes the fixes to 'grob.cc', but not the other parts of the patches provided by Guido.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2016-07-23

Oh and a full make doc too (sorry, I forgot to mention that).

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- David Kastrup - 2016-07-23
  
  Ok, this seems like a (pretty trivial) showstopper so I'll unceremoniously push it after proofreading it once again. I'll leave the decision about the other patch to Heikki.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

David Kastrup - 2016-07-23

assigned_to: David Kastrup

Needs: -->

Patch: new --> review

Type: --> Crash
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

David Kastrup - 2016-07-23

labels: --> Fixed_2_19_46

status: Started --> Fixed

Patch: review -->
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

grob.cc segfaults on Fedora 24 with gcc6

Issue Tracker for LilyPond

Searches

Help

#4814 grob.cc segfaults on Fedora 24 with gcc6

Discussion