sagemath / sage

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

Fix crash when specify invalid base for RR and RIF construction #39001

Open user202729 opened 3 days ago

user202729 commented 3 days ago

As in the title.

Note that currently ZZ(str, base=int) only support 2 to 36. Should this be changed for consistency?

:memo: Checklist

:hourglass: Dependencies


The test on Conda (Mac) fails with

2024-11-19T04:02:36.0696760Z ./bootstrap: line 142: aclocal: command not found
2024-11-19T04:02:36.0697900Z Bootstrap failed. Either install autotools; or run bootstrap with
2024-11-19T04:02:36.0698610Z the -d option to download the auto-generated files instead.

Looks unrelated.

Another one (Ubuntu 3.9) is

2024-11-19T04:26:04.0471925Z **********************************************************************
2024-11-19T04:26:04.0591101Z File "src/sage/plot/plot.py", line 1857, in sage.plot.plot.plot
2024-11-19T04:26:04.0591946Z Failed example:
2024-11-19T04:26:04.0592761Z     plot(f, (x, -3.5, 3.5), detect_poles='show', exclude=[-3..3],
2024-11-19T04:26:04.0593588Z          ymin=-5, ymax=5)
2024-11-19T04:26:04.0594061Z Expected:
2024-11-19T04:26:04.0649808Z     Graphics object consisting of 12 graphics primitives
2024-11-19T04:26:04.0650471Z Got:
2024-11-19T04:26:04.1009273Z     Graphics object consisting of 13 graphics primitives
2024-11-19T04:26:26.6704957Z **********************************************************************

Also looks unrelated. Reported in #39002.

github-actions[bot] commented 3 days ago

Documentation preview for this PR (built with commit 91063db0f305693218aad0cf310df2c98b6f7264; changes) is ready! :tada: This preview will update shortly after each push to this PR.

JosePisco commented 2 days ago

Is it a test that fails or sagemath that crashes? I can't reproduce or a weird behavior on my side. I have read your commit but I'm not sure to catch what's the problem, a check that was not done properly?

user202729 commented 2 days ago

Before the patch when you run the test with invalid base it will crash SageMath. @JosePisco

e.g. RR("1", base=63)

strtofr.c:951: MPFR assertion failed: base == 0 || (base >= 2 && base <= 62)
------------------------------------------------------------------------
/usr/lib/python3.12/site-packages/cysignals/signals.cpython-312-x86_64-linux-gnu.so(+0x81c5) [0x7aa2fc6f91c5]
/usr/lib/python3.12/site-packages/cysignals/signals.cpython-312-x86_64-linux-gnu.so(+0x8319) [0x7aa2fc6f9319]
/usr/lib/python3.12/site-packages/cysignals/signals.cpython-312-x86_64-linux-gnu.so(+0xc64a) [0x7aa2fc6fd64a]
/usr/lib/libc.so.6(+0x3d1d0) [0x7aa2fc84c1d0]
/usr/lib/libc.so.6(+0x963f4) [0x7aa2fc8a53f4]
/usr/lib/libc.so.6(gsignal+0x20) [0x7aa2fc84c120]
/usr/lib/libc.so.6(abort+0xdf) [0x7aa2fc8334c3]
/usr/lib/libmpfr.so.6(+0xf843) [0x7aa2fa9be843]
/usr/lib/libmpfr.so.6(mpfr_strtofr+0x579) [0x7aa2faa024c9]
/usr/lib/libmpfr.so.6(mpfr_set_str+0x32) [0x7aa2fa9df7b2]
...

Maybe your libmpfr build has assertion disabled by default (?) not sure.

Maybe the first 5 tests are not directly related to the patch, but might as well be careful meanwhile.