sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

Polybori segfaults when built with clang on OS X #22677

Closed kiwifb closed 7 years ago

kiwifb commented 7 years ago

Split from #12426. Doctest involving polybori segfault when everything is built with clang

sage -t --long --warn-long 62.3 src/sage/rings/polynomial/multi_polynomial_sequence.py
**********************************************************************
File "src/sage/rings/polynomial/multi_polynomial_sequence.py", line 106, in sage.rings.polynomial.multi_polynomial_sequence
Failed example:
    C[0].groebner_basis()
Exception raised:
    Traceback (most recent call last):
      File "/Users/fbissey/build/sage-clang/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/fbissey/build/sage-clang/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.polynomial.multi_polynomial_sequence[7]>", line 1, in <module>
        C[Integer(0)].groebner_basis()
      File "/Users/fbissey/build/sage-clang/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_sequence.py", line 518, in groebner_basis
        return self.ideal().groebner_basis(*args, **kwargs)
      File "sage/rings/polynomial/pbori.pyx", line 5080, in sage.rings.polynomial.pbori.BooleanPolynomialIdeal.groebner_basis (/Users/fbissey/build/sage-clang/src/build/cythonized/sage/rings/polynomial/pbori.cpp:42498)
        sig_on()
      File "src/cysignals/signals.pyx", line 109, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1695)
    SignalError: Segmentation fault
    Killed due to segmentation fault

Backtrace

sage: F.eliminate_linear_variables() # everything vanishes ## line 1147 ##
------------------------------------------------------------------------
0   signals.so                          0x000000010589c5d1 print_backtrace + 65
1   signals.so                          0x000000010589fc0c sigdie + 60
2   signals.so                          0x000000010589fb7f cysigs_signal_handler + 351
3   libsystem_platform.dylib            0x00007fffd0769bba _sigtramp + 26
4   ???                                 0x00000001052a9a00 0x0 + 4381645312
5   pbori.so                            0x000000010f0257ef _ZN8polybori8groebner11linalg_stepERNSt3__16vectorINS_15BoolePolynomialENS1_9allocatorIS3_EEEENS_8BooleSetES8_bbPKc + 479
6   pbori.so                            0x000000010f0252d3 _ZN8polybori8groebner14gauss_on_polysERKNSt3__16vectorINS_15BoolePolynomialENS1_9allocatorIS3_EEEE + 275
7   pbori.so                            0x000000010f024fe0 _ZL58__pyx_pw_4sage_5rings_10polynomial_5pbori_39gauss_on_polysP7_objectS0_ + 192
8   libpython2.7.dylib                  0x000000010514cfdc PyEval_EvalFrameEx + 29500
9   libpython2.7.dylib                  0x0000000105151e02 fast_function + 338

Similarly sage/rings/polynomial/pbori.pyx

Assertion failed: (result != map.end()), function get, file ../../groebner/include/polybori/groebner/PolyEntryIndices.h, line 115.
------------------------------------------------------------------------
0   signals.so                          0x000000010bc6a5d1 print_backtrace + 65
1   signals.so                          0x000000010bc6dc0c sigdie + 60
2   signals.so                          0x000000010bc6db5d cysigs_signal_handler + 317
3   libsystem_platform.dylib            0x00007fffd0769bba _sigtramp + 26
4   ???                                 0x00007fd399cbd6a0 0x0 + 140546795099808
5   libsystem_c.dylib                   0x00007fffd05f0420 abort + 129
6   libsystem_c.dylib                   0x00007fffd05b7893 basename_r + 0
7   libbrial_groebner.0.dylib           0x0000000115658737 _ZNK8polybori8groebner16PolyEntryIndices3getINSt3__13mapINS_13BooleExponentEiNS_13MapComparatorIS5_EENS3_9allocatorINS3_4pairIKS5_iEEEEEES5_EEiRKT_RKT0_NS1_7uncheckE + 551
8   libbrial_groebner.0.dylib           0x00000001156584ff _ZNK8polybori8groebner16PolyEntryIndices3getINS1_7uncheckEEEiRKNS_13BooleExponentET_ + 47
9   libbrial_groebner.0.dylib           0x00000001156584bd _ZNK8polybori8groebner16PolyEntryIndicesclINS_13BooleExponentEEEiRKT_ + 29
10  libbrial_groebner.0.dylib           0x000000011561105c _ZNK8polybori8groebner15PolyEntryVector5indexINS_13BooleExponentEEEmRKT_ + 44
11  libbrial_groebner.0.dylib           0x00000001156609ad _ZN8polybori8groebner15PolyEntryVectorclINS_13BooleExponentEEENS0_18PolyEntryReferenceERKT_ + 61
12  libbrial_groebner.0.dylib           0x0000000115692ea7 _ZNK8polybori8groebner20SetAssociatedMinimalINS_13BooleExponentELb0EEclERKS2_ + 39
13  libbrial_groebner.0.dylib           0x000000011569108a _ZN8polybori8groebner17ReductionStrategy28unmarkNonMinimalLeadingTermsENS_8BooleSetE + 186
14  libbrial_groebner.0.dylib           0x0000000115690e16 _ZN8polybori8groebner17ReductionStrategy19setupSetsForElementERKNS0_9PolyEntryE + 262
15  pbori.so                            0x00000001153af381 _ZL76__pyx_pw_4sage_5rings_10polynomial_5pbori_17ReductionStrategy_5add_generatorP7_objectS0_ + 177
16  pbori.so                            0x000000011533967a _ZL25__Pyx_PyObject_CallOneArgP7_objectS0_ + 154
17  pbori.so                            0x00000001153974b1 _ZL71__pyx_pw_4sage_5rings_10polynomial_5pbori_17BooleanPolynomial_129reduceP7_objectS0_ + 2289
18  libpython2.7.dylib                  0x000000010b51bfdc PyEval_EvalFrameEx + 29500
19  libpython2.7.dylib                  0x000000010b514927 PyEval_EvalCodeEx + 2119

finally the failures in

sage -t --long src/sage/crypto/mq/sr.py  # 7 doctests failed

are consequences of segfaults above.

tarball https://github.com/BRiAl/BRiAl/releases/download/0.8.7/brial-0.8.7.tar.bz2

Upstream: Fixed upstream, in a later stable release.

Component: porting

Author: François Bissey, Dima Pasechnik

Branch/Commit: 382b25c

Reviewer: François Bissey, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/22677

kiwifb commented 7 years ago

Upstream: Fixed upstream, but not in a stable release.

kiwifb commented 7 years ago
comment:1

All the above are solved by upgrading BRiAl to git master. I'll prepare a release of BRiAl.

dimpase commented 7 years ago

Commit: a3e4c6b

dimpase commented 7 years ago

Author: Dima Pasechnik

dimpase commented 7 years ago
comment:2

I need this for #22679 :-)


New commits:

a3e4c6bbrial updated to commit 63f747626822f5e0fa2bf975f7801fcc988eb530
dimpase commented 7 years ago

Branch: u/dimpase/brial086

dimpase commented 7 years ago

Description changed:

--- 
+++ 
@@ -71,3 +71,5 @@

are consequences of segfaults above.

+tarball http://users.ox.ac.uk/~coml0531/sage/brial-0.8.6.tar.bz2 +

kiwifb commented 7 years ago
comment:4

I know, sorry I cannot seem to move my ass to make a proper release. On it now.

kiwifb commented 7 years ago

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

kiwifb commented 7 years ago
comment:5

Need to fix the checksum to the properly released tarball. I'll put it back in review once it is done.

kiwifb commented 7 years ago

Description changed:

--- 
+++ 
@@ -71,5 +71,5 @@

are consequences of segfaults above.

-tarball http://users.ox.ac.uk/~coml0531/sage/brial-0.8.6.tar.bz2 +tarball https://github.com/BRiAl/BRiAl/releases/download/0.8.6/brial-0.8.6.tar.bz2

kiwifb commented 7 years ago

Changed commit from a3e4c6b to 0943ae5

kiwifb commented 7 years ago

Reviewer: François Bissey

kiwifb commented 7 years ago
comment:6

I'll put it in positive review but can you double check that the new tarball works for you.


New commits:

0943ae5updating checksum for official release
kiwifb commented 7 years ago

Changed branch from u/dimpase/brial086 to u/fbissey/brial-0.8.6

dimpase commented 7 years ago
comment:7

ironically, it does not work on gentoo Lunix with gcc 5.4.0.

sage -t --long --warn-long 118.6 src/sage/rings/polynomial/multi_polynomial_sequence.py  # Killed due to segmentation fault

and two more brial-related errors. While numerous

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

are probably OK,

[sagelib-7.6] In file included from /home/dima/Sage/sage-dev/src/sage/libs/polybori/pb_wrap.h:22:0,
 from /home/dima/Sage/sage-dev/src/build/cythonized/sage/rings/polynomial/pbori.cpp:517:
/home/dima/Sage/sage-dev/src/sage/ext/ccobject.h: In instantiation of ‘void Delete(T*) [with T = polybori::groebner::ReductionStrategy]’:
 /home/dima/Sage/sage-dev/src/build/cythonized/sage/rings/polynomial/pbori.cpp:50398:51:   required from here
 /home/dima/Sage/sage-dev/src/sage/ext/ccobject.h:77:3: warning: 
deleting object of polymorphic class type ‘polybori::groebner::ReductionStrategy’ 
which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor]
   delete mem;
   ^

might spell trouble. It could also be that we must switch to gcc 6. Can you check in what Linux configuration this works for you?

(meahwhile I'll do distclean and a full rebuild, just to see if this still persists)

kiwifb commented 7 years ago
comment:8

I confirm, it breaks things here too with gcc-5.4.0. I think it may be down to https://github.com/BRiAl/BRiAl/issues/11#issuecomment-269957098 I'll have to check on that.

dimpase commented 7 years ago
comment:9

I'll have a look at how it plays together with #22554 (which isn't in this branch yet).

dimpase commented 7 years ago
comment:10

Replying to @dimpase:

I'll have a look at how it plays together with #22554 (which isn't in this branch yet).

no, it does not help.

kiwifb commented 7 years ago
comment:11

OK I have a fix. I cut a bit too much stuff. Actually adding one character would have been sufficient to fix gcc-6 compilation. It is still worth cleaning some stuff that wouldn't work anyway. I am not sure when I will have time to push the fix, I am rather busy this week end.

kiwifb commented 7 years ago
comment:12

brial-0.8.7 (now linked to in the ticket description) should work for all reasonably recent compilers. I am not sure about keeping -std=c++98 in CXXFLAGS as it could cause problem in the near future I think. I haven't updated the branch yet, feel free to beat me to it.

kiwifb commented 7 years ago

Description changed:

--- 
+++ 
@@ -71,5 +71,5 @@

are consequences of segfaults above.

-tarball https://github.com/BRiAl/BRiAl/releases/download/0.8.6/brial-0.8.6.tar.bz2 +tarball https://github.com/BRiAl/BRiAl/releases/download/0.8.7/brial-0.8.7.tar.bz2

dimpase commented 7 years ago
comment:13

0.8.7 works well on FreeBSD+clang. I'm happy to give it positive review once we have a branch...

kiwifb commented 7 years ago
comment:14

Did you try on gentoo as well? Since 0.8.6 was working on FreeBSD+clang but not on Gentoo+gcc?

dimpase commented 7 years ago
comment:15

checking now on Linux. Will take a while---doing a clean rebuild of sage 8.0.beta...

dimpase commented 7 years ago

Changed branch from u/fbissey/brial-0.8.6 to u/dimpase/brial-0.8.6

dimpase commented 7 years ago

Changed commit from 0943ae5 to none

dimpase commented 7 years ago
comment:16

OK, it works on Linux too. No idea why the branch is red though.

kiwifb commented 7 years ago
comment:17

Interesting message: "branch doesn't exist".

dimpase commented 7 years ago
comment:18

oops...

dimpase commented 7 years ago

Changed branch from u/dimpase/brial-0.8.6 to u/dimpase/brial086

dimpase commented 7 years ago

Commit: 382b25c

dimpase commented 7 years ago

Changed reviewer from François Bissey to François Bissey, Dima Pasechnik

dimpase commented 7 years ago

Changed author from Dima Pasechnik to François Bissey, Dima Pasechnik

vbraun commented 7 years ago

Changed branch from u/dimpase/brial086 to 382b25c