Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#20926 closed defect (fixed)

C++11 workarounds

Reported by: vbraun Owned by:
Priority: blocker Milestone: sage-7.3
Component: packages: standard Keywords: gcc6 c++11 c++14
Cc: Merged in:
Authors: Volker Braun Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 86e6a4b (Commits) Commit:
Dependencies: #20927 #20741 #20738 Stopgaps:

Description (last modified by vbraun)

The purpose of this ticket is to get Sage to build with gcc6 asap; This requires some workarounds since gcc6 defaults to std=gnu++14 and some of our packages aren't ready.


  • BRiAl -- configure issue, see #20741
  • eclib -- builds with -std=c++98
  • LinBox -- builds with -std=c++98, see #20929
  • PPL -- builds with -std=c++98, but can be updated, see #20927
  • Singular -- see #20738
  • Sage library (because of inclusion of e.g LinBox headers)

Change History (62)

comment:1 Changed 4 years ago by vbraun

  • Authors set to Volker Braun
  • Component changed from PLEASE CHANGE to packages: standard
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 4 years ago by vbraun

  • Branch set to u/vbraun/c__11_workarounds

comment:3 Changed 4 years ago by vbraun

  • Commit set to 1ff411e1e19641262c9b4c9542bb49c21efd47b7

Proper fix for gfan shall go to https://trac.sagemath.org/ticket/20925


New commits:

1ff411eBuild gfan with gnu++98

comment:4 Changed 4 years ago by vbraun

  • Keywords gcc6 c++11 added

comment:5 Changed 4 years ago by vbraun

  • Dependencies set to #20927

comment:6 Changed 4 years ago by git

  • Commit changed from 1ff411e1e19641262c9b4c9542bb49c21efd47b7 to 097b1acd2f8e008acd45f1760e83778b6bbd4246

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b0b13d7Update to ppl-1.2
097b1acBuild gfan with gnu++98

comment:7 Changed 4 years ago by vbraun

  • Description modified (diff)

comment:8 Changed 4 years ago by vbraun

  • Dependencies changed from #20927 to #20927 #20741

comment:9 Changed 4 years ago by vbraun

Proper fix for eclib shall go to https://trac.sagemath.org/ticket/20928

comment:10 Changed 4 years ago by git

  • Commit changed from 097b1acd2f8e008acd45f1760e83778b6bbd4246 to dd8be5f08e02a91bd8d3e98fa2c6ef5a393e08b4

Branch pushed to git repo; I updated commit sha1. New commits:

da85dbdUpdate brial to 0.8.5
386e1d3Merge #20741 into #20926
dd8be5fBuild eclib with gnu++98

comment:11 Changed 4 years ago by vbraun

  • Dependencies changed from #20927 #20741 to #20927 #20741 #20738

comment:12 Changed 4 years ago by leif

  • Description modified (diff)

comment:13 follow-up: Changed 4 years ago by leif

I've posted a patch for the Sage library on sage-release.

While Singular builds, the executable and libsingular (hence Sage) crash for me and others (cf. #20738 and sage-release).

comment:14 in reply to: ↑ 13 Changed 4 years ago by leif

Replying to leif:

I've posted a patch for the Sage library on sage-release.

  • src/sage/libs/linbox/linbox.pxd

    diff --git a/src/sage/libs/linbox/linbox.pxd b/src/sage/libs/linbox/linbox.pxd
    index 0afd7bf..87a5e79 100644
    a b  
     1# distutils: language = c++
     2# distutils: extra_compile_args = -std=c++98
     3
    14from sage.libs.gmp.types cimport mpz_t
    25
    36include 'sage/modules/vector_modn_sparse_h.pxi'
  • src/sage/matrix/matrix_modn_sparse.pxd

    diff --git a/src/sage/matrix/matrix_modn_sparse.pxd b/src/sage/matrix/matrix_modn_sparse.pxd
    index 5273ec3..14a98e7 100644
    a b  
     1# distutils: language = c
     2
    13cimport matrix_sparse
    24
    35include 'sage/modules/vector_modn_sparse_h.pxi'

comment:15 Changed 4 years ago by leif

Forgot the hunk for module_list:

  • src/module_list.py

    diff --git a/src/module_list.py b/src/module_list.py
    index a49ed36..8441106 100644
    a b ext_modules = [ 
    926926              libraries = ['ntl', 'linbox', 'givaro', 'mpfr', 'gmpxx', 'gmp'] + cblas_libs,
    927927              library_dirs = cblas_library_dirs,
    928928              include_dirs = cblas_include_dirs,
    929               extra_compile_args = ['-DDISABLE_COMMENTATOR'] + givaro_extra_compile_args),
     929              extra_compile_args = ['-std=c++98', '-DDISABLE_COMMENTATOR'] + givaro_extra_compile_args),
    930930
    931931    Extension('sage.matrix.matrix_modn_dense_double',
    932932              sources = ['sage/matrix/matrix_modn_dense_double.pyx'],
    ext_modules = [ 
    934934              libraries = ['ntl', 'linbox', 'givaro', 'mpfr', 'gmpxx', 'gmp'] + cblas_libs,
    935935              library_dirs = cblas_library_dirs,
    936936              include_dirs = cblas_include_dirs,
    937               extra_compile_args = ["-D_XPG6", "-DDISABLE_COMMENTATOR"]
     937              extra_compile_args = ['-std=c++98', "-D_XPG6", "-DDISABLE_COMMENTATOR"]
    938938                    + m4ri_extra_compile_args + givaro_extra_compile_args),
    939939
    940940    Extension('sage.matrix.matrix_modn_sparse',

comment:16 Changed 4 years ago by leif

  • Description modified (diff)
  • Keywords c++14 added

comment:17 Changed 4 years ago by git

  • Commit changed from dd8be5f08e02a91bd8d3e98fa2c6ef5a393e08b4 to a277a5770f343af475c31e0ce79c718e04dfdaac

Branch pushed to git repo; I updated commit sha1. New commits:

d0da72bsingular: fix build with GCC6
04c0af9replace &mach_o[0] by mach_o
2e32c59Merge #20738 into #20926
a3352caBuild linbox with gnu++98
a277a57Use c++98 in the Sage library when compiling with linbox headers

comment:18 Changed 4 years ago by leif

I don't like adding -std=gnu++98 when -std=c++98 is sufficient.

comment:19 Changed 4 years ago by vbraun

  • Description modified (diff)

comment:20 Changed 4 years ago by git

  • Commit changed from a277a5770f343af475c31e0ce79c718e04dfdaac to a9f1b9a7174b40a83ac998659799aa1538fac127

Branch pushed to git repo; I updated commit sha1. New commits:

b089732Workaround Singular segfault with gcc6
a9f1b9aUse c++98 instead of gnu++98

comment:21 Changed 4 years ago by leif

I'd prefer if we only add -fno-delete-null-pointer-checks when necessary, as neither GCC 4.9.x nor 5.x are broken w.r.t. this:

case `$CXX -dumpversion` in
    6|6.*)
        CXXFLAGS="$CXXFLAGS -fno-delete-null-pointer-checks"
        ;;
esac

(The 6 is necessary for OpenSuSE's GCC IIRC. CXXFLAGS already get exported earlier in Singular's spkg-install.)

comment:22 follow-up: Changed 4 years ago by vbraun

For singular, -fno-strict-overflow seems to be an even better flag. This does not surprise too much as there are numerous warnings about conversions between pointers and signed integers.

comment:23 in reply to: ↑ 22 ; follow-up: Changed 4 years ago by leif

Replying to vbraun:

For singular, -fno-strict-overflow seems to be an even better flag.

You mean instead of -fno-delete-null-pointer-checks?

(In contrast to the latter, it's disabled with -O1, which worked for me, but only in conjunction with the latter, i.e. -O1 -fno-delete-null-pointer-checks, IIRC, cf. #20738 comment 6.)


This does not surprise too much as there are numerous warnings about conversions between pointers and signed integers.

Besides a lot of other suspicious warnings...

comment:24 Changed 4 years ago by leif

Or did you add some extra_compile_args in src/sage/libs/singular/*.pxd as well?

comment:25 Changed 4 years ago by git

  • Commit changed from a9f1b9a7174b40a83ac998659799aa1538fac127 to 7406306748d076dd8278491ee612bb6130d509a1

Branch pushed to git repo; I updated commit sha1. New commits:

7406306Use -fno-strict-overflow when building Singular

comment:26 in reply to: ↑ 23 Changed 4 years ago by leif

Replying to leif:

Replying to vbraun:

For singular, -fno-strict-overflow seems to be an even better flag.

You mean instead of -fno-delete-null-pointer-checks?

(In contrast to the latter, it's disabled with -O1, which worked for me, but only in conjunction with the latter, i.e. -O1 -fno-delete-null-pointer-checks, IIRC, cf. #20738 comment 6.)

With just -fno-strict-overflow added to (just) CXXFLAGS, ./sage --singular immediately segfaults for me.

Last edited 4 years ago by leif (previous) (diff)

comment:27 Changed 4 years ago by vbraun

The remaining three failures seem to be due to brial/polybori

sage -t --long src/sage/rings/polynomial/multi_polynomial_sequence.py  # Killed due to segmentation fault
sage -t --long src/sage/crypto/mq/sr.py  # Killed due to segmentation fault
sage -t --long src/sage/rings/polynomial/pbori.pyx  # Killed due to abort

comment:28 Changed 4 years ago by git

  • Commit changed from 7406306748d076dd8278491ee612bb6130d509a1 to 04eac6fa4e8b0329bc24d363c64765997f1329e9

Branch pushed to git repo; I updated commit sha1. New commits:

04eac6fUse -fno-delete-null-pointer-checks -fno-strict-overflow when building Singular

comment:29 follow-up: Changed 4 years ago by git

  • Commit changed from 04eac6fa4e8b0329bc24d363c64765997f1329e9 to 86e6a4b21cb4a51c2aa9ed8d01b62239b8fdbd13

Branch pushed to git repo; I updated commit sha1. New commits:

86e6a4bBuild brial with c++98

comment:30 in reply to: ↑ 29 ; follow-up: Changed 4 years ago by fbissey

Replying to git:

Branch pushed to git repo; I updated commit sha1. New commits:

86e6a4bBuild brial with c++98

As I remember I advocated for something like that initially. Actually I said I should do something about the configure script and I forgot.

comment:31 in reply to: ↑ 30 ; follow-up: Changed 4 years ago by vbraun

Replying to fbissey:

As I remember I advocated for something like that initially. Actually I said I should do something about the configure script and I forgot.

There is probably a bug in the Brial code path, this should be fixed on the C++ level. But in another ticket.

comment:32 Changed 4 years ago by vbraun

  • Priority changed from major to blocker
  • Status changed from new to needs_review

All tests pass for me, we should merge this asap...

comment:33 Changed 4 years ago by leif

I see "trac's automerging failed" (branch is colored red)...


Going to test on top of beta7...

comment:34 in reply to: ↑ 31 Changed 4 years ago by leif

Replying to vbraun:

There is probably a bug in the Brial code path, this should be fixed on the C++ level. But in another ticket.

Does this refer to the three segfaults / aborts in ptestlong you reported above? How did you get these?


With Sage 7.3.beta7 (plus the branch here) and vanilla FSF GCC 6.1.0, GCC configured to produce native code by default, but otherwise no *FLAGS set, ptestlong passes for me (modulo the usual failure in one test due to numeric noise and another GAP-related which nearly always fails in the first place, but passes when rerun) on Linux x86_64 (Haswell-E).

comment:35 follow-up: Changed 4 years ago by leif

As mentioned, I'd prefer if we only mess with optimization flags when necessary (i.e., only disable some in Singular's spkg-install if $CXX is really GCC 6.x).

Otherwise I'm ok with your changes, and mine as well... ;-)

comment:36 in reply to: ↑ 35 Changed 4 years ago by leif

Replying to leif:

As mentioned, I'd prefer if we only mess with optimization flags when necessary (i.e., only disable some in Singular's spkg-install if $CXX is really GCC 6.x).

P.S.: There should be a 2>/dev/null in `$CXX -dumpversion`, and while I don't mind re-exporting CXXFLAGS, I'd move their modification a bit up (to not mix it with changes toMAKE):

  • build/pkgs/singular/spkg-install

    diff --git a/build/pkgs/singular/spkg-install b/build/pkgs/singular/spkg-install
    index 0b21be7..024b1e1 100755
    a b export CPPFLAGS="-I$SAGE_LOCAL/include $CPPFLAGS" 
    7373export CXXFLAGS="$CXXFLAGS -fPIC"
    7474export CFLAGS="$CFLAGS -fPIC"
    7575
     76# Workaround for GCC6: https://trac.sagemath.org/ticket/20926
     77case `$CXX -dumpversion 2>/dev/null` in
     78    6|6.*)
     79        export CXXFLAGS="$CXXFLAGS -fno-delete-null-pointer-checks -fno-strict-overflow"
     80        ;;
     81esac
     82
    7683# Singular does not respect LDFLAGS; Ugly hack:
    7784# Used on Linux and Darwin (see singular-3.1.7-use_cxx_for_linking.patch)
    7885export CXX="$CXX $LDFLAGS"
    7986
     87
    8088# The Sun assembler has problems with -pipe, so disable it.
    8189# This only affects compile time, not the compiled programs/libraries.
    8290if [ "$UNAME" = "SunOS" ]; then
    fi 
    8896# parallel sometimes fails (Trac #17774)
    8997export MAKE="$MAKE -j1"
    9098
    91 # Workaround for GCC6: https://trac.sagemath.org/ticket/20926
    92 export CXXFLAGS="$CXXFLAGS -fno-delete-null-pointer-checks -fno-strict-overflow"
    9399
    94100choose_patches()
    95101{

comment:37 follow-up: Changed 4 years ago by vbraun

Imho it depends on whether the compiler or the source is at fault. In the case of Singular I don't think its a compiler bug. So it most likely won't go away with gcc 7...

comment:38 in reply to: ↑ 37 Changed 4 years ago by leif

Replying to vbraun:

Imho it depends on whether the compiler or the source is at fault. In the case of Singular I don't think its a compiler bug. So it most likely won't go away with gcc 7...

Well, it goes away with GCC < 6.x, and we shouldn't throw arbitrary options at $CXX, which might -- for example -- be clang. (Most parts of Sage did build with the latter, at least a while ago. clang accepts a couple of GCC options, but spits out warnings for others it doesn't, and I wouldn't rely on the latter, i.e. wouldn't risk breaking something which previously worked, for no reason.)

comment:39 Changed 4 years ago by vbraun

Presumably, the fact that Singular ever built with gcc < 6 just was a bug in gcc (insufficient checking). This is a fugly workaround that hopefully can be removed with the Singular update. I'm against putting too much lipstick on a pig; If anything work on the Singular update.

comment:40 follow-up: Changed 4 years ago by jsrn

I'll just confirm that this patch makes Sage compile on my machine (Arch up-to-date, gcc 6.1.1, 64 bits).

comment:41 in reply to: ↑ 40 Changed 4 years ago by leif

Replying to jsrn:

I'll just confirm that this patch makes Sage compile on my machine (Arch up-to-date, gcc 6.1.1, 64 bits).

Thanks for testing. Did you also run make [p]testlong?

comment:42 follow-up: Changed 4 years ago by jsrn

I'm still having another issue -- many default packages do not get installed during make, after make distclaen -- so there's tons of doctests failing. It seems to me that this issue isn't related to the new gcc, but I don't know where to start looking for a solution.

comment:43 in reply to: ↑ 42 ; follow-up: Changed 4 years ago by leif

Replying to jsrn:

I'm still having another issue -- many default packages do not get installed during make, after make distclaen -- so there's tons of doctests failing. It seems to me that this issue isn't related to the new gcc, but I don't know where to start looking for a solution.

OT (I think), but did you try downgrading make (to 4.1, say)?

(Or debug the Sage build process with your version as suggested on sage-devel, e.g. by starting with make -n pkg_which_didnt_get_built, then perhaps successively adding some debug options until you see why the package doesn't get built.)

comment:44 in reply to: ↑ 43 Changed 4 years ago by jsrn

OT (I think), but did you try downgrading make (to 4.1, say)?

(Or debug the Sage build process with your version as suggested on sage-devel, e.g. by starting with make -n pkg_which_didnt_get_built, then perhaps successively adding some debug options until you see why the package doesn't get built.)

Sorry, I missed your reply on sage-devel! I've downgraded make and tried another run, this time with --trace -bvjm. I'll report the results on sage-devel.

comment:45 follow-up: Changed 4 years ago by dimpase

People report errors with gcc 6.1.1, but it's not even an official release, but some RH-only hack. How does one deal with this?

comment:46 in reply to: ↑ 45 Changed 4 years ago by vbraun

Replying to dimpase:

How does one deal with this?

I'd say by merging this ticket asap; I checked that it allows Sage to build on Fedora 24.

comment:47 Changed 4 years ago by dimpase

  • Status changed from needs_review to positive_review

here you go

comment:48 follow-up: Changed 4 years ago by dimpase

well, except that there is something funny with the branch...

comment:49 Changed 4 years ago by vbraun

  • Reviewers set to Dima Pasechnik

comment:50 Changed 4 years ago by vbraun

Its just the libgit (in the trac plugin) not managing the merge, but the command-line git does it just fine.

comment:51 in reply to: ↑ 48 ; follow-up: Changed 4 years ago by leif

Replying to dimpase:

well, except that there is something funny with the branch...

Pull the trigger, ask later... ;-)

Nasty people might think you didn't take a look at the patch, nor read the comments.

comment:52 in reply to: ↑ 51 Changed 4 years ago by dimpase

Replying to leif:

Replying to dimpase:

well, except that there is something funny with the branch...

Pull the trigger, ask later... ;-) Nasty people might think you didn't take a look at the patch, nor read the comments.

I read individual commits, and did not find any surprises there...

comment:53 Changed 4 years ago by leif

I would have been more happy if someone confirmed also the documentation now builds (and tests pass) on Arch (where the first reports originated from), since segfaults do not happen during the build, but when running Sage (or e.g. Singular).

comment:54 follow-up: Changed 4 years ago by vbraun

I'll also be more than happy to have this tested on Arch, but since nobody did it in a timely manner our chances are better by releasing it first. Open a followup if there is still an issue.

comment:55 in reply to: ↑ 54 Changed 4 years ago by leif

Replying to vbraun:

I'll also be more than happy to have this tested on Arch, but since nobody did it in a timely manner our chances are better by releasing it first.

Well, you asked for further reviewers, but didn't rebase the branch here, nor anser my question regarding the segfaults in BRiAl you reported yourself, within a week.

(I don't see a related commit here or a follow-up ticket "fixing a bug in the BRiAl code path on the C++ level" either.)

Last edited 4 years ago by leif (previous) (diff)

comment:56 Changed 4 years ago by jsrn

After running ./sage -i jmol and ./sage pip install sympy, I can build the documentation. It's running now, so far no problems. Assuming that the package problem is orthogonal to the gcc-issue, then it's looking good.

comment:58 Changed 4 years ago by vbraun

  • Branch changed from u/vbraun/c__11_workarounds to 86e6a4b21cb4a51c2aa9ed8d01b62239b8fdbd13
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:59 in reply to: ↑ 57 Changed 4 years ago by leif

  • Commit 86e6a4b21cb4a51c2aa9ed8d01b62239b8fdbd13 deleted

Replying to vbraun:

Followups to https://github.com/BRiAl/BRiAl/issues/11

To not lose this on trac, I've opened #21083 which refers to the issue on github ("Fix compilation with c++11 (default for gcc>=6)").

comment:60 follow-up: Changed 4 years ago by leif

For the record: Givaro's testsuite also fails to build; builds with -std=c++98.

comment:61 in reply to: ↑ 60 Changed 4 years ago by leif

Replying to leif:

For the record: Givaro's testsuite also fails to build; builds with -std=c++98.

This is now #21169.

comment:62 Changed 4 years ago by jdemeyer

Why was the line

# distutils: language = c

added to src/sage/matrix/matrix_modn_sparse.pxd?

Does that even do something, given that c is the default?

Note: See TracTickets for help on using tickets.