besser82 / libxcrypt

Extended crypt library for descrypt, md5crypt, bcrypt, and others
GNU Lesser General Public License v2.1
178 stars 49 forks source link

Compilation fails with -O3 and GCC >= 11 #179

Open oynqr opened 6 months ago

oynqr commented 6 months ago

Compiling with -O3 on GCC tries to inline these function calls:

https://github.com/besser82/libxcrypt/blob/72f75aa370ae96ccd2cc44ea3cf4182d8679ffbe/lib/alg-sha1.c#L265 https://github.com/besser82/libxcrypt/blob/72f75aa370ae96ccd2cc44ea3cf4182d8679ffbe/lib/alg-sha1.c#L267

This triggers an overflow warning on this line, which fails the build due to -Werror: https://github.com/besser82/libxcrypt/blob/72f75aa370ae96ccd2cc44ea3cf4182d8679ffbe/lib/alg-sha1.c#L244

Looking at the code a bit, I don't think this line is actually reachable in this case, so it's probably a false positive.

zackw commented 3 months ago

At present this is a "don't do that then" situation. There are several places in libxcrypt where the code technically has undefined behavior but the compiler will generate correct machine code as long as the inlining level is not too high. You have found one of them. This is one of the reasons why the documentation says not to use -flto, and I guess we should add -O3 to that as well.

It is a bug and we will fix it eventually, but we have very limited time to do any work on libxcrypt presently, and this class of issues is considered low priority, especially the subset that affects obsolete hashing methods only.

I would be happy to review a patch that restructured alg-sha1.c so that it compiled cleanly with -O3, especially if you also adjust things so that when compiled with -O2 -S -fsanitize=undefined -fno-sanitize-recover=all the generated assembly code contains no calls to any function whose name begins with __ubsan (this is an incomplete but practically useful approximation to "this source code is well-defined according to the C standard").