Fix build when SIGSTKSZ is not a constant. #151

kliem · 2021-09-28T09:48:40Z

This is intended to fix #150 as done in rr-debugger/rr#2916.

src/cysignals/implementation.c

…ise in case of OSX bug

enriqueartal · 2021-11-06T18:15:42Z

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

kliem · 2021-11-06T19:36:42Z

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

I requested the fixes to cysignals-CSI already in #149.

enriqueartal · 2021-11-06T21:39:58Z

A test of a giac file did not pass, with a complain to cysignals but I do not know if it is related with this fix.

./sage -t --long --warn-long 131.6 --random-seed=0 src/sage/libs/giac/init.py
Running doctests with ID 2021-11-06-22-37-54-f7ebb02c.
Git branch: u/mkoeppe/test_ticket__python_3_10_development_releases
Using --optional=build,dochtml,fedora,pip,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg
Doctesting 1 file.
sage -t --long --warn-long 131.6 --random-seed=0 src/sage/libs/giac/init.py

File "src/sage/libs/giac/init.py", line 18, in sage.libs.giac
Failed example:
B = gb_giac(I.gens()) # random
Exception raised:
Traceback (most recent call last):
File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.libs.giac[3]>", line 1, in
B = gb_giac(I.gens()) # random
File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/libs/giac/init.py", line 118, in wrapper
return func(*args, **kwds)
File "/usr/local/sage-310/local/var/lib/sage/venv-python3.10/lib64/python3.10/site-packages/sage/libs/giac/init.py", line 353, in groebner_basis
gb_giac = F.gbasis(list(var_names), giac_order)
File "sage/libs/giac/auto-methods.pxi", line 6442, in sage.libs.giac.giac.GiacMethods_base.gbasis (build/cythonized/sage/libs/giac/giac.cpp:56012)
return GiacMethods'gbasis'
File "sage/libs/giac/giac.pyx", line 2102, in sage.libs.giac.giac.GiacFunction.call (build/cythonized/sage/libs/giac/giac.cpp:153070)
sig_on()
RuntimeError: Aborted

kliem · 2021-11-08T07:38:39Z

I think the giac failure is not related. It's just wrapped in sig_on ... sig_off, so it jumps back to sig_on and raises an exception then, which is exactly what should happen. (At least from cysignals point of view, giac shouldn't fail of course).

kliem · 2021-11-08T07:46:16Z

I used kliem:fix_non_constant_MINSIGSTKSZ to creat a cysignals package to be able to compile Sagemath in Fedora 35. The test of src/sage/rings/polynomial/polynomial_zmod_flint.pyx complains about a ) in line 68 of cysignals-CSI; with the correction the test is successful

I requested the fixes to cysignals-CSI already in #149.

However, it also makes sense to have the fixes here. #149 is not as urgent as this issue.

dimpase · 2021-11-22T07:58:14Z

should we merge this?

kliem · 2021-11-22T08:11:25Z

should we merge this?

I think so and make a release pretty soon. I just closed #139, so that we can run some tests.

kliem · 2021-11-22T12:48:16Z

The testsuite fails apparently. Have to wait until the artifacts are uploaded.

More importantly my test that I added to configure does not work at all. Any help is welcome.

dimpase · 2021-11-22T12:50:42Z

What's the branch you're testing?

dimpase · 2021-11-22T12:51:11Z

Some of the runs seem to work, no?

kliem · 2021-11-22T13:07:34Z

Yes, but configure.ac isn't doing what it is supposed to do. Fedora 35 fails with exactly the problem we are trying to fix.

enriqueartal · 2021-11-22T13:08:15Z

Some of the runs seem to work, no?

I can confirm that it is possible to build a running copy of sage (from 9.4 with python 3.9 and from develop with python 3.10). There are more tickets involved (to be able to build in F35) and some tests fail, but I can use it in the daily work.

kliem · 2021-11-22T13:11:58Z

Some of the runs seem to work, no?

I can confirm that it is possible to build a running copy of sage (from 9.4 with python 3.9 and from develop with python 3.10). There are more tickets involved (to be able to build in F35) and some tests fail, but I can use it in the daily work.

I believe that the current state of the branch is not working, because configure.ac is not working.

You can try this with cloning this rep and just run make configure and ./configure. I'm guessing that the last test outputs:

checking whether MINSIGSTKSZ is constant... yes

Not what I was aiming for, because on Fedora 35 it is not constant. On the other hand, if I do slight modifications, it fails for me, e.g.

diff --git a/configure.ac b/configure.ac
index 003a5bc..552b71a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -229,7 +229,7 @@ AC_MSG_CHECKING([whether MINSIGSTKSZ is constant])
 AC_COMPILE_IFELSE([AC_LANG_SOURCE(
     [
     #include <signal.h>
-    static char alt_stack[MINSIGSTKSZ];
+    static char alt_stack[MINSIGSTKSZ + 5120];
     ])],
     dnl YES
     [AC_MSG_RESULT([yes])]

So I'm gathering that it is not testing, what it is supposed to test.

enriqueartal · 2021-11-22T13:13:03Z

Yes, but configure.ac isn't doing what it is supposed to do. Fedora 35 fails with exactly the problem we are trying to fix.

I just bundled your branch and pass it to the build of sage in F35 (I got errors if I build it directly, there are some hacks in spkg-install.in

enriqueartal · 2021-11-22T13:23:25Z

I do not understand what happens, I have errors when building outside sage; I attach the log of the sage installation
cysignals_inside_sage.log

kliem · 2021-11-22T14:07:24Z

I do not understand what happens, I have errors when building outside sage; I attach the log of the sage installation cysignals_inside_sage.log

Does the latest push work for you. As you probably guessed, your config.log should say:
checking whether MINSIGSTKSZ is constant... no

enriqueartal · 2021-11-22T16:08:41Z

Have you checked the patch in the src.rpm package for Fedora 35. There are several ones, maybe this one is relevant. It builds, though using your configure.ac, it still says it is a constant.

--- src/cysignals/implementation.c.orig 2021-05-27 14:03:35.653774857 -0600
+++ src/cysignals/implementation.c 2021-05-27 14:19:42.225312505 -0600
@@ -445,7 +445,8 @@ static void setup_alt_stack(void)
/* Static space for the alternate signal stack. The size should be
* of the form MINSIGSTKSZ + constant. The constant is chosen rather
* ad hoc but sufficiently large. */

static char alt_stack[MINSIGSTKSZ + 5120 + BACKTRACELEN * sizeof(void*)];

/* MINSIGSTKSIZE is now nonconstant. */
static char alt_stack[4096 + 5120 + BACKTRACELEN * sizeof(void*)];

stack_t ss;
ss.ss_sp = alt_stack;

dimpase

it seems some pairs of [ ] are missing in the test program. Moreover iirc include statements come apart from the rest of the source, like in a different parameter?

kliem · 2021-11-22T19:33:36Z

it seems some pairs of [ ] are missing in the test program. Moreover iirc include statements come apart from the rest of the source, like in a different parameter?

Thank you. This seems to be exactly the problem:
https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Generating-Sources.html

Edit: Should have checked with make configure. What it compiled was

static char alt_stackMINSIGSTKSZ;

Of course this is going to work independent of whether MINSIGSTKSZ is a constant.

This reverts commit 1187e24.

enriqueartal · 2021-11-22T23:23:39Z

Now it builds for me in Fedora 35. I will try now inside sage. Thanks!

kliem · 2021-11-24T08:53:32Z

As can be observed by the checks of #152, the changes here did not break anything.

But we do fix Fedora 35 and other new systems.

dimpase · 2021-11-24T09:05:21Z

A new release then?

kliem · 2021-11-24T09:17:04Z

Yes, I'm just trying to figure out a two bug fixes yet. The macos test suite doesn't work at all and ubuntu-trusty fails. If there are no easy solutions to them, I'll just leave it at that.

I'm running the doctests verbose to see if I can figure out the ubuntu-trusty issue:

https://github.com/kliem/cysignals/actions/runs/1498523090

The original logs were not useful at all:

ulimit 2>/dev/null -s 1024; python3 -B rundoctests.py src/cysignals/*.pyx
cd example && python3 setup.py clean build
Doctesting 5 files.
src/cysignals/alarm.pyx
running clean
running build
Compiling cysignals_example.pyx because it changed.
[1/1] Cythonizing cysignals_example.pyx
running build_ext
building 'cysignals_example' extension
creating build
creating build/temp.linux-x86_64-3.9
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -g -O2 -g -O2 -Wp,-U_FORTIFY_SOURCE -fPIC -I/sage/local/var/lib/sage/venv-python3.9.7/lib/python+++3.9/site-packages/cysignals -I/sage/local/var/lib/sage/venv-python3.9.7/include/python3.9 -c cysignals_example.cpp -o build/temp.linux-x86_64-3.9/cysignals_example.o
creating build/lib.linux-x86_64-3.9
g++ -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath+++-link,/sage/local/var/lib/sage/venv-python3.9.7/lib -Wl,-rpath,/sage/local/var/lib/sage/venv-python3.9.7/lib -L. -L/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/+++lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/lo+++cal/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -g -O2 -Wp,-U_FORTIFY_SOURCE build/temp.linux-x86_64-3.9/cysignals_example.o -L/sage/local/v+++ar/lib/sage/venv-python3.9.7/lib -o build/lib.linux-x86_64-3.9/cysignals_example.cpython-39-x86_64-linux-gnu.so -lpari
src/cysignals/pselect.pyx
src/cysignals/pysignals.pyx
src/cysignals/signals.pyx
src/cysignals/tests.pyx
make[3]: *** [check-doctest] Error 1
make[3]: Target `check-install' not remade because of errors.

#154 attempts to fix the mac test suite problem. If it doesn't work, it doesn't work.

dimpase · 2021-11-24T09:59:58Z

Does this happen locally on macOS too, or it's GitHub actions-only problem?

…

On Wed, 24 Nov 2021, 09:17 Jonathan Kliem, ***@***.***> wrote: Yes, I'm just trying to figure out a two bug fixes yet. The macos test suite doesn't work at all and ubuntu-trusty fails. If there are no easy solutions to them, I'll just leave it at that. I'm running the doctests verbose to see if I can figure out the ubuntu-trusty issue: https://github.com/kliem/cysignals/actions/runs/1498523090 The original logs were not useful at all: ulimit 2>/dev/null -s 1024; python3 -B rundoctests.py src/cysignals/*.pyx cd example && python3 setup.py clean build Doctesting 5 files. src/cysignals/alarm.pyx running clean running build Compiling cysignals_example.pyx because it changed. [1/1] Cythonizing cysignals_example.pyx running build_ext building 'cysignals_example' extension creating build creating build/temp.linux-x86_64-3.9 gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -g -O2 -g -O2 -Wp,-U_FORTIFY_SOURCE -fPIC -I/sage/local/var/lib/sage/venv-python3.9.7/lib/python+++3.9/site-packages/cysignals -I/sage/local/var/lib/sage/venv-python3.9.7/include/python3.9 -c cysignals_example.cpp -o build/temp.linux-x86_64-3.9/cysignals_example.o creating build/lib.linux-x86_64-3.9 g++ -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath+++-link,/sage/local/var/lib/sage/venv-python3.9.7/lib -Wl,-rpath,/sage/local/var/lib/sage/venv-python3.9.7/lib -L. -L/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/+++lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/lo+++cal/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -g -O2 -Wp,-U_FORTIFY_SOURCE build/temp.linux-x86_64-3.9/cysignals_example.o -L/sage/local/v+++ar/lib/sage/venv-python3.9.7/lib -o build/lib.linux-x86_64-3.9/cysignals_example.cpython-39-x86_64-linux-gnu.so -lpari src/cysignals/pselect.pyx src/cysignals/pysignals.pyx src/cysignals/signals.pyx src/cysignals/tests.pyx make[3]: *** [check-doctest] Error 1 make[3]: Target `check-install' not remade because of errors. #154 <#154> attempts to fix the mac test suite problem. If it doesn't work, it doesn't work. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#151 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJXYHFI3GTTXQLXJE2OO33UNSURXANCNFSM5E47TOBQ> .

kliem · 2021-11-24T10:10:10Z

I don't know. I don't have a macOS.

Fix build when SIGSTKSZ is not a constant.

d7b65f9

embray requested changes Oct 1, 2021

View reviewed changes

src/cysignals/implementation.c Outdated Show resolved Hide resolved

src/cysignals/implementation.c Show resolved Hide resolved

Jonathan Kliem added 2 commits October 1, 2021 15:07

typo

5fee9ed

fix non-mmap case

d589e5e

This was referenced Oct 17, 2021

MINSIGSTKSZ is no longer a constant #150

Closed

pipenv install errors on unused cysignals package commaai/openpilot#22629

Closed

resolve non-constant MINSIGSTKSZ with malloc; keep old version otherw…

2e6c3e2

…ise in case of OSX bug

kliem requested a review from embray November 6, 2021 19:36

two fixes for cysignals-CSI

c1bbf34

update release notes

b50932a

try for a simpler solution

1187e24

dimpase reviewed Nov 22, 2021

View reviewed changes

Jonathan Kliem added 2 commits November 22, 2021 20:34

Revert "try for a simpler solution"

92a510e

This reverts commit 1187e24.

fix configure syntax

7b653d4

kliem mentioned this pull request Nov 22, 2021

Add GH Actions workflow for integration testing with Sage #139

Merged

kliem merged commit d351f04 into sagemath:master Nov 24, 2021

kliem deleted the fix_non_constant_MINSIGSTKSZ branch November 24, 2021 21:41

kliem added this to the 1.11.0 milestone Nov 25, 2021

kliem mentioned this pull request Dec 23, 2021

Cysignals not compatible with glibc >= 2.34 (fedora-35, ubuntu-impish) sagemath/sage#32576

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build when SIGSTKSZ is not a constant. #151

Fix build when SIGSTKSZ is not a constant. #151

kliem commented Sep 28, 2021

enriqueartal commented Nov 6, 2021

kliem commented Nov 6, 2021

enriqueartal commented Nov 6, 2021

kliem commented Nov 8, 2021

kliem commented Nov 8, 2021

dimpase commented Nov 22, 2021

kliem commented Nov 22, 2021

kliem commented Nov 22, 2021

dimpase commented Nov 22, 2021

dimpase commented Nov 22, 2021

kliem commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

kliem commented Nov 22, 2021 •

edited

Loading

enriqueartal commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

kliem commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

dimpase left a comment

kliem commented Nov 22, 2021 •

edited

Loading

enriqueartal commented Nov 22, 2021

kliem commented Nov 24, 2021

dimpase commented Nov 24, 2021

kliem commented Nov 24, 2021

dimpase commented Nov 24, 2021 via email

kliem commented Nov 24, 2021

Fix build when SIGSTKSZ is not a constant. #151

Fix build when SIGSTKSZ is not a constant. #151

Conversation

kliem commented Sep 28, 2021

enriqueartal commented Nov 6, 2021

kliem commented Nov 6, 2021

enriqueartal commented Nov 6, 2021

kliem commented Nov 8, 2021

kliem commented Nov 8, 2021

dimpase commented Nov 22, 2021

kliem commented Nov 22, 2021

kliem commented Nov 22, 2021

dimpase commented Nov 22, 2021

dimpase commented Nov 22, 2021

kliem commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

kliem commented Nov 22, 2021 • edited Loading

enriqueartal commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

kliem commented Nov 22, 2021

enriqueartal commented Nov 22, 2021

dimpase left a comment

Choose a reason for hiding this comment

kliem commented Nov 22, 2021 • edited Loading

enriqueartal commented Nov 22, 2021

kliem commented Nov 24, 2021

dimpase commented Nov 24, 2021

kliem commented Nov 24, 2021

dimpase commented Nov 24, 2021 via email

kliem commented Nov 24, 2021

kliem commented Nov 22, 2021 •

edited

Loading

kliem commented Nov 22, 2021 •

edited

Loading