Opened 18 months ago

Closed 17 months ago

Last modified 16 months ago

#25391 closed defect (duplicate)

SageMath fails to build on on Fedora 28 with gcc 8.1

Reported by: Vaush Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: gcc8.1 python3 fedora28 compilation build
Cc: dimpase Merged in:
Authors: Dario Asprone Reviewers: Dima Pasechnik
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: u/Vaush/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1 (Commits) Commit: 35c3c33f42df5da1b00351205c1d984c06019eba
Dependencies: Stopgaps:

Description (last modified by Vaush)

SageMath 8.3.beta1 building produced 3 different errors when run on Fedora 28 64bit with gcc 8.1, specifically when building the packages:

  • python 3.6.1 (fixed in #25771)
  • python 2.7.14 (fixed in #25204)
  • linbox 1.5.2 (fixed in #25353)

The 1st error manifests itself as a failure to import the crypt module during the build of Python 3. It is due to an unspecified issue with Fedora 28 implementation of crypt(), which as reported in https://fedoraproject.org/wiki/Changes/Replace_glibc_libcrypt_with_libxcrypt is not the standard glibc implementation.

This was fixed in https://bugs.python.org/issue32635

Attachments (7)

multiple_templates.patch (570 bytes) - added by Vaush 18 months ago.
Linbox patch
packing.patch (1.2 KB) - added by Vaush 18 months ago.
Python2 patch
crypt.patch (2.1 KB) - added by Vaush 18 months ago.
Python3 patch - 1 of 2
zzz_conf.patch (7.4 KB) - added by Vaush 18 months ago.
Python3 patch - 2 of 2
python3-3.6.1.p1.log (3.6 MB) - added by Vaush 17 months ago.
Log of the failed python3 build
crypt_debug.patch (636 bytes) - added by jdemeyer 17 months ago.
gdb_screenshot.png (589.1 KB) - added by Vaush 17 months ago.
Screenshot of gdb's output

Change History (63)

Changed 18 months ago by Vaush

Linbox patch

Changed 18 months ago by Vaush

Python2 patch

Changed 18 months ago by Vaush

Python3 patch - 1 of 2

Changed 18 months ago by Vaush

Python3 patch - 2 of 2

comment:1 Changed 18 months ago by Vaush

  • Dependencies set to #25204,#25353

comment:2 Changed 18 months ago by Vaush

I added dependencies to #25204,#25353 which respectively fix the python 2 issue and the linbox (and fflas, which is not described here) issue. The commit to be made will be based on them, and will only modify the python 3 package

comment:3 Changed 18 months ago by Vaush

  • Branch set to u/Vaush/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:4 Changed 18 months ago by Vaush

  • Commit set to 473f2149a035a87b0941ecab426f3b32bcd20472
  • Status changed from new to needs_review

New commits:

4c1223eFixed python 3.6.1 crypt issue on Fedora 28
cb566eafixing gcc-8.1 compilation errors.
b20fd48Merge remote-tracking branch 'trac/u/cpernet/fflas_and_linbox_broken_with_gcc_8_1_0' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1
d14acaeUpgrade to Python 2.7.15
473f214Merge remote-tracking branch 'trac/u/jdemeyer/upgrade_to_python_2_7_15' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:5 Changed 18 months ago by Vaush

  • Cc dimpase added

comment:6 Changed 18 months ago by dimpase

I'll be checking that this does not break on other systems.

comment:7 Changed 18 months ago by dimpase

Have you run tests on your branch?: make ptest (or make ptestlong for more tests)

This is something the ticket's author should do in such cases. (It would not be surprising if something unrelated to your changes fails, as gcc 8 is pretty much untested, but good to know anyway).

comment:8 Changed 18 months ago by Vaush

I haven't, but will do

comment:9 Changed 18 months ago by git

  • Commit changed from 473f2149a035a87b0941ecab426f3b32bcd20472 to 5be7adc8c90d54d53c77fa8a20a8df1a223ea577

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

5be7adcMerge branch 'master' into t/25391/sagemath_fails_to_build_on_on_fedora_28_with_gcc_8_1

comment:10 Changed 17 months ago by dimpase

What is happening with this on the current 8.3.beta6? Note that python2 has been updated.

comment:11 Changed 17 months ago by dimpase

For Python3, it's apparently https://github.com/python/cpython/pull/4691 which is still under review.

comment:12 Changed 17 months ago by jdemeyer

  • Status changed from needs_review to needs_info

I am missing some context and documentation. The current branch only fixes Python 3. That is not consistent with the ticket description which mentions several packages and doesn't even mention the actual failure that the Python 3 patch is trying to fix.

comment:13 Changed 17 months ago by jdemeyer

The patch files should also contain some description, at a minimum a link to the upstream issue.

And if you add a patch to a package, you should bump the package version.

comment:14 follow-up: Changed 17 months ago by Vaush

Ok, I'm sorry if the description or the execution aren't up to par, this is my first ticket, so I'm getting used to it.
The ticket was originally opened to describe the bug. After that, I succeeded in manually fixing all the bugs listed in the description, but it was brought to my attention that two of my bugs were solved in separate tickets at the same time (see the dependencies field), so I merged those tickets and added my own fix for python3, thus fixing all the issues and solving the problem stated in the ticket title, that is SageMath not compiling on Fedora 28 with gcc 8.1.
Since the commits include the fixes for the other 2 bugs, even if by merging, I thought this counted as having a fix for all 3 of them in the branch, is this wrong?
There is a description for the python 3 bug, since the bug is literally "Python tries to call crypt instead of crypt_r on Fedora, and this for some reason breaks things, so let's allow it to call crypt_r", and the reason why it breaks things is, as pointed out by dimpase, apparently that fedora has a completely different crypt implementation, as can be seen here https://fedoraproject.org/wiki/Changes/Replace_glibc_libcrypt_with_libxcrypt.
I'll expand on the description a bit, anyway, to make this clearer, and also to reflect the fact that this ticket only directly fixes the python3 bug, since it's only reported in the comments. I'll also quickly add a description and a link to the patch

comment:15 Changed 17 months ago by Vaush

  • Description modified (diff)

comment:16 Changed 17 months ago by git

  • Commit changed from 5be7adc8c90d54d53c77fa8a20a8df1a223ea577 to 35c3c33f42df5da1b00351205c1d984c06019eba

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

35c3c33Added comments to the patches, and modified the package version

comment:17 follow-up: Changed 17 months ago by dimpase

  • Description modified (diff)
  • Keywords python2 linbox removed
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_info to needs_review

Jeroen, how do you feel about merging Python 3 patch which is still being out for reviewing by Python people?

comment:18 Changed 17 months ago by dimpase

  • Dependencies #25204,#25353 deleted

comment:19 in reply to: ↑ 14 Changed 17 months ago by jdemeyer

Replying to Vaush:

Ok, I'm sorry if the description or the execution aren't up to par, this is my first ticket, so I'm getting used to it.

As a general tip: when you fix a bug, it's always important to mention what the bug is. You're certainly not the first one to post a fix for something without even mentioning what that fix is supposed to fix.

comment:20 in reply to: ↑ description Changed 17 months ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. Developers acknowledge bug.
  • Status changed from needs_review to needs_info

Do you have a build log of the failed Python 3 build?

comment:21 in reply to: ↑ 17 Changed 17 months ago by jdemeyer

Replying to dimpase:

Jeroen, how do you feel about merging Python 3 patch which is still being out for reviewing by Python people?

I don't mind in principle. However, I'd very much like to know what the actual bug is that this ticket is supposed to fix.

comment:22 Changed 17 months ago by Vaush

I'm attaching the log file of the failed build for python 3, when the patches are not applied.

Changed 17 months ago by Vaush

Log of the failed python3 build

comment:23 follow-up: Changed 17 months ago by dimpase

The log has a number of tries to build python3, only the one at the bottom of the log has this error, see line 32452:

Testing importing of various modules...
ctypes module imported OK
math module imported OK
hashlib module imported OK
./spkg-build: line 157: 21845 Segmentation fault      (core dumped) $PYTHON -c "import $module"
crypt module failed to import
readline module imported OK
socket module imported OK
Error: One or more modules failed to import.

and this is due to

sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c: In function 'crypt_crypt_impl':
sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c:39:31: warning: implicit declaration of function 'crypt' [-Wimplicit-function-declaration]
     return Py_BuildValue("s", crypt(word, salt));
                               ^~~~~

Basically, there is no crypt implementation available in the libraries used, one needs to use another library, and this is done in the branch of this ticket.

Last edited 17 months ago by dimpase (previous) (diff)

comment:24 Changed 17 months ago by dimpase

see also https://bugzilla.redhat.com/show_bug.cgi?id=1576671 (Redhat/Fedora bug tracker)

Last edited 17 months ago by dimpase (previous) (diff)

comment:25 Changed 17 months ago by dimpase

  • Status changed from needs_info to needs_review

comment:26 Changed 17 months ago by jdemeyer

  • Status changed from needs_review to needs_info

Thanks for the info. It's clear to me now that the bug that we're fixing is a different one from the upstream pull request https://github.com/python/cpython/pull/4691

Can you try if Python 3.7.0 (currently in rc: https://www.python.org/downloads/release/python-370rc1/) by chance fixes it?

Last edited 17 months ago by jdemeyer (previous) (diff)

comment:27 Changed 17 months ago by jdemeyer

Something else to try is compiling Python 3 without optimization (-O0). Can you do that for Python 3.7.0 and post the log file?

comment:28 Changed 17 months ago by dimpase

To clarify: the upstream's ​https://github.com/python/cpython/pull/4691 does not fix a bug, it provides for using crypt_r if available.

It also does a proper handling (introduces HAVE_CRYPT_H) of crypt and crypt_r at the configuration stage---without it there is a reliance on glibc providing crypt and a header different from crypt.h providing a prototype.

Changed 17 months ago by jdemeyer

comment:29 Changed 17 months ago by jdemeyer

One final thing to try: put the file crypt_debug.patch in build/pkgs/python3/patches, rebuild Python 3.6.1 with it and report back what happens.

comment:30 in reply to: ↑ 23 ; follow-up: Changed 17 months ago by jdemeyer

Replying to dimpase:

and this is due to

sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c: In function 'crypt_crypt_impl':
sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c:39:31: warning: implicit declaration of function 'crypt' [-Wimplicit-function-declaration]
     return Py_BuildValue("s", crypt(word, salt));
                               ^~~~~

Basically, there is no crypt implementation available in the libraries used, one needs to use another library, and this is done in the branch of this ticket.

I'm not convinced by this argument. If there would no crypt implementation, then we would get a linking error (either at compile time or runtime). But in the log file, there is a segmentation fault. So my impression is that crypt() is called but that it's causing a crash somehow.

comment:31 in reply to: ↑ 30 Changed 17 months ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

and this is due to

sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c: In function 'crypt_crypt_impl':
sage/build/python3-3.6.1.p1/src/Modules/_cryptmodule.c:39:31: warning: implicit declaration of function 'crypt' [-Wimplicit-function-declaration]
     return Py_BuildValue("s", crypt(word, salt));
                               ^~~~~

Basically, there is no crypt implementation available in the libraries used, one needs to use another library, and this is done in the branch of this ticket.

I'm not convinced by this argument. If there would no crypt implementation, then we would get a linking error (either at compile time or runtime). But in the log file, there is a segmentation fault. So my impression is that crypt() is called but that it's causing a crash somehow.

you are right, my bad here. Anyhow, crypt is called without a prototype. One can in fact make sure that it is called with a prototype, by removing the chunk about using crypt_r from the branch on this ticket, i.e. the lines

+@@ -36,7 +41,13 @@
+ {
+     /* On some platforms (AtheOS) crypt returns NULL for an invalid
+        salt. Return None in that case. XXX Maybe raise an exception?  */
++#ifdef HAVE_CRYPT_R
++    struct crypt_data data;
++    data.initialized = 0;
++    return Py_BuildValue("s", crypt_r(word, salt, &data));
++#else
+     return Py_BuildValue("s", crypt(word, salt));
++#endif
+ }
+ 
+ 

perhaps there would be more messages from the compiler then...

comment:32 follow-ups: Changed 17 months ago by Vaush

I'm sorry for the confusion, I think I can explain what happens (not why, but at least what).
The crypt() call goes through perfectly, without issues, and returns a seemingly correct value. This value is then passed to Py_BuildValue and retrieved through varargs, but if one checks the two char* in question, that is the return value of crypt and the one Py_BuildValue retrieves, the latter is almost identical, but with the first 3-4 digits zeroed out. As an example, if crypt returns a pointer to 0xfe5ab234 (I completely made up this address), Py_BuildValue will get a pointer to 0x000ab234, and get illegal memory access when it tries to use it.
The missing prototype is not really a problem, at least it wasn't when I tried, because I tried adding the needed header without success.
I want to try python 3.7.0, but are there any major differences one would have to account for, or can I just put the source code in the upstream folder and modify checksum.ini?

comment:33 in reply to: ↑ 32 Changed 17 months ago by jdemeyer

Replying to Vaush:

I'm sorry for the confusion, I think I can explain what happens (not why, but at least what).
The crypt() call goes through perfectly, without issues, and returns a seemingly correct value. This value is then passed to Py_BuildValue and retrieved through varargs, but if one checks the two char* in question, that is the return value of crypt and the one Py_BuildValue retrieves, the latter is almost identical, but with the first 3-4 digits zeroed out. As an example, if crypt returns a pointer to 0xfe5ab234 (I completely made up this address), Py_BuildValue will get a pointer to 0x000ab234, and get illegal memory access when it tries to use it.

If that's true, then the error has nothing to with crypt() at all and I would expect much more breakage. Can you post the code that you used to come to the conclusion above and the exact results (no "completely made up" address!).

comment:34 in reply to: ↑ 32 Changed 17 months ago by jdemeyer

Replying to Vaush:

I want to try python 3.7.0, but are there any major differences one would have to account for, or can I just put the source code in the upstream folder and modify checksum.ini?

I don't think anybody has tried. I guess what you said should just work.

comment:35 follow-ups: Changed 17 months ago by Vaush

I can't post the code because I didn't use any, I just stepped through the code with gdb. I checked the stack trace for the segfault, set a breakpoint to the line that called the function which calls crypt, and then stepped through manually to understand what was happening.
I can try and get some proof from gdb later in the day, but there's not much more I can do.

comment:36 in reply to: ↑ 35 Changed 17 months ago by dimpase

Replying to Vaush:

I can't post the code because I didn't use any, I just stepped through the code with gdb. I checked the stack trace for the segfault, set a breakpoint to the line that called the function which calls crypt, and then stepped through manually to understand what was happening.
I can try and get some proof from gdb later in the day, but there's not much more I can do.

My understanding is that crypt returns a pointer to a statically allocated location. Indeed, its man page says: return value points to static data whose content is overwritten by each call. Whereas crypt_r does something else, as such design as for crypt is not thread-safe.

Perhaps this unusual design of crypt is the reason the breakage is not bigger?

comment:37 follow-up: Changed 17 months ago by dimpase

In fact, the call to Py_BuildValue is

return Py_BuildValue("s", crypt(word, salt));

(where we see a warning in the logs).

Here "s" stands for string It would be very interesting to see what values of word and salt are used. Otherwise what length it assumes by default is anyone's guess (perhaps that's why you see some bits chopped off). Indeed, it does

Convert a null-terminated C string to a Python str object using 'utf-8' encoding. 

Note that Python 2 does not do utf-8 here, so this might be a problem. How about you try modifying the patch as I suggest in comment 31 (removing crypt_r-related chunk)? This would call crypt, but supply a correct prototype.

comment:38 Changed 17 months ago by Vaush

I can do that later if you want, but it's not the string that gets chopped, it's the string's address.

comment:39 Changed 17 months ago by dimpase

What I don't know is what is supposed to happen on the import attempt. Does crypt get called at all? This does not seem to be necessary, as the import is about setting up a structure to call crypt from Python, not about calling it with any values.

Did you see crypt actually being called? And with what values of word and salt? This is something that is totally unclear. If they are set to bad values, or just left uninitialised, stuff can happen...

comment:40 Changed 17 months ago by Vaush

I remember them being set up correctly, and the call worked. Still, I am recompiling the whole of sage with debug symbols, I'll step through it again and report when I have news

comment:41 in reply to: ↑ 35 ; follow-up: Changed 17 months ago by jdemeyer

Replying to Vaush:

and then stepped through manually to understand what was happening.

Posting the actual output from gdb would have been useful here...

comment:42 in reply to: ↑ 37 Changed 17 months ago by jdemeyer

Replying to dimpase:

Here "s" stands for string It would be very interesting to see what values of word and salt are used.

That's what my debugging patch crypt_debug.patch does.

comment:43 in reply to: ↑ 41 Changed 17 months ago by Vaush

Replying to jdemeyer:

Replying to Vaush:

and then stepped through manually to understand what was happening.

Posting the actual output from gdb would have been useful here...

Yes, the point is that I don't have it anymore, since I did that 2 or 3 weeks ago. As I said, I am now recompiling sage with debug symbols to redo it and post the output.

Changed 17 months ago by Vaush

Screenshot of gdb's output

comment:44 Changed 17 months ago by Vaush

Ok, for some reason I can't seem to step into crypt anymore, while earlier I could see the fact that crypt on my system was a wrap around crypt_r.
Anyway, I used jdemeyer's patch, and I'm attaching gdb's output.
What happens is that Python now segfaults on the printing attempt. I don't have any way of confirming what I said earlier about the value being returned by crypt being fine, but causing segfaults when accessed outside of it.

comment:45 Changed 17 months ago by dimpase

Could you post the output of ldd called on the corresponding *.so file?

Did you use a custom-built crypt library at your 1st attempt?

And, finally, could you add a print to show the value of the pointer res before the print that causes the crash? (E.g. it could return NULL, or some obviously invalid value...)

Last edited 17 months ago by dimpase (previous) (diff)

comment:46 follow-up: Changed 17 months ago by Vaush

I don't remember using any custom-built library, so the answer would be no.
This is the output of ldd on the built python executable and on libcrypt:

[vaush@jesu-c290 src]$ ldd /lib64/libcrypt.so.1
	linux-vdso.so.1 (0x00007ffe913d6000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f32ada5e000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f32ae046000)
[vaush@jesu-c290 src]$ ldd ./python
	linux-vdso.so.1 (0x00007ffd247ec000)
	libpython3.6dm.so.1.0 => not found
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd8b8512000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007fd8b830e000)
	libutil.so.1 => /lib64/libutil.so.1 (0x00007fd8b810b000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fd8b7d77000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fd8b79b8000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd8b8731000)

Finally, no, I can't print anything about it, unless you mean before the return value of crypt is assigned to it, since we introduced it with jdemeyer's patch.

comment:47 in reply to: ↑ 46 Changed 17 months ago by dimpase

Replying to Vaush:

I don't remember using any custom-built library, so the answer would be no.
This is the output of ldd on the built python executable and on libcrypt:

no, I mean the crypt.*.so or _crypt.*.so or whatever it is called; on one system I have it's here:

$ ldd local/lib/python3.6/lib-dynload/_crypt.cpython-36m-x86_64-linux-gnu.so
        linux-vdso.so.1 =>  (0x00007fffdff5e000)
        libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x00007da738262000)
        libpython3.6m.so.1.0 => /home/dima/sagetrac-mirror/local/lib/libpython3.6m.so.1.0 (0x00007da737d32000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007da737b15000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007da73774b000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007da737547000)
        libutil.so.1 => /lib/x86_64-linux-gnu/libutil.so.1 (0x00007da737344000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007da73703b000)
        /lib64/ld-linux-x86-64.so.2 (0x00007da73869c000)

Finally, no, I can't print anything about it, unless you mean before the return value of crypt is assigned to it, since we introduced it with jdemeyer's patch.

I meant, certainly, that you modify Jeroen's patch so that you print the value of the pointer res (i.e. the address) after the call to crypt(), before printing the string it points to (the latter print crashes).

comment:48 follow-up: Changed 17 months ago by Vaush

Ok, sorry for the misunderstandings.
Here is the output of ldd:

vaush@jesu-c290:src$ ldd build/lib.linux-x86_64-3.6-pydebug/_crypt.cpython-36dm-x86_64-linux-gnu.so
	linux-vdso.so.1 (0x00007ffeca7f6000)
	libcrypt.so.1 => /lib64/libcrypt.so.1 (0x00007fe30d625000)
	libpython3.6dm.so.1.0 => /run/media/vaush/SageMath/SageMath/local/var/tmp/sage/build/python3-3.6.1.p1/src/libpython3.6dm.so.1.0 (0x00007fe30d0c4000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fe30cea5000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fe30cae6000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007fe30c8e2000)
	libutil.so.1 => /lib64/libutil.so.1 (0x00007fe30c6df000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fe30c34b000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fe30da50000)

While now python outputs this during compilation:

[python3-3.6.1.p1] crypt('', '$6$Ukf2OXfG3jHYFFCX')
[python3-3.6.1.p1] Address = '0xc4c020'

As you can see, the address is compatible with the theory I presented, that in some way the value of the pointer gets chopped when coming out of crypt. Still no success in stepping into crypt, but I don't see why, it was so easy the first time around...

comment:49 in reply to: ↑ 48 Changed 17 months ago by dimpase

Replying to Vaush:

Still no success in stepping into crypt, but I don't see why, it was so easy the first time around...

I don't see how you'd step in a system library in a meaningful way - they usually are stripped, you won't see any symbols.

Version 0, edited 17 months ago by dimpase (next)

comment:50 Changed 17 months ago by Vaush

I installed the debug infos already, to no avail. Anyway, I know that it shouldn't be possible, but the first time around I just naively did it, and stepped into the function in some way

comment:51 follow-up: Changed 17 months ago by jdemeyer

I have a theory. The implicit declaration of function 'crypt' warning causes GCC to guess the return type as int (which is 32 bits). Now in reality the return type is char * (64 bits). So that might explain the zeroed bits.

If this theory is correct, then simply adding

#include <crypt.h>

should fix the problem.

Could you try just adding that line to _cryptmodule.c?

comment:52 in reply to: ↑ 51 ; follow-up: Changed 17 months ago by dimpase

Replying to jdemeyer:

I have a theory. The implicit declaration of function 'crypt' warning causes GCC to guess the return type as int (which is 32 bits). Now in reality the return type is char * (64 bits). So that might explain the zeroed bits.

If this theory is correct, then simply adding

#include <crypt.h>

should fix the problem.

Could you try just adding that line to _cryptmodule.c?

Or, as I have been saying already, just change the patch you are adding so that it drops crypt_r chunk, as it would be precisely the same thing... Anyhow, this theory is easy to confirm:

/* cry.c */
#include <stdio.h>
#ifdef CRYP
#include <crypt.h>
#endif
int main()
{
char *word="blah";
char *salt="foo";
char *res;
res = crypt(word, salt);
printf("%s\n",res);
return 0;
}

Now trying this out, with the right include:

$ gcc -DCRYP cry.c -lcrypt
$ ./a.out 
fo9NXBQQtNBSA

and without include

$ gcc cry.c -lcrypt
cry.c: In function ‘main’:
cry.c:10:7: warning: implicit declaration of function ‘crypt’ [-Wimplicit-function-declaration]
 res = crypt(word, salt);
       ^~~~~
cry.c:10:5: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
 res = crypt(word, salt);
     ^
$ ./a.out 
Segmentation fault

Morale: pay attention to compiler warnings :-)

comment:53 in reply to: ↑ 52 Changed 17 months ago by jdemeyer

Replying to dimpase:

Morale: pay attention to compiler warnings :-)

Absolutely. I was totally confused earlier because the upstream PR incorrectly states "Because crypt() only depends on primitive C types, we currently get away with calling it without it being declared."

comment:54 Changed 17 months ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-8.3 to sage-duplicate/invalid/wontfix
  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.
  • Resolution set to duplicate
  • Status changed from needs_info to closed

This should be fixed in 3.7.0, so we should just upgrade when that version comes out.

comment:55 Changed 17 months ago by vbraun

I made #25771 to update to Python 3.6.6 which contains the backported crypt fix

comment:56 Changed 16 months ago by Vaush

  • Description modified (diff)
Note: See TracTickets for help on using tickets.