From 8af3916962093b1ec49d46c366bca94ff6f9c60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Thu, 13 Aug 2020 18:51:50 +0200 Subject: [PATCH 1/8] crypt-port: Add the bits for compiling with link-time optimization. GCC 10 and LLVM/Clang 10 offer initial support for building libraries, that are using symbol versioning features, with LTO. To make use of this with GCC 10, the exported versioned symbols need to be declared explicitly with __attribute__((symver (...))). LLVM/Clang 10 supports symbol versioning with LTO out of the box without any changes needed. Fixes #24. --- lib/crypt-port.h | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/crypt-port.h b/lib/crypt-port.h index bec36aca..9b804577 100644 --- a/lib/crypt-port.h +++ b/lib/crypt-port.h @@ -179,11 +179,29 @@ _crypt_strcpy_or_abort (void *, const size_t, const void *); # define _strong_alias(name, aliasname) \ extern __typeof (name) aliasname __THROW __attribute__ ((alias (#name))) +/* Starting with GCC 10, we can use the symver attribute, which also works + with link-time optimization enabled. */ +# if __GNUC__ >= 10 + +/* Set the symbol version for EXTNAME, which uses INTNAME as its + implementation. */ +# define symver_set(extstr, intname, version, mode) \ + extern __typeof (intname) intname __THROW \ + __attribute__((symver (extstr mode #version))) + +/* Referencing specific _compatibility_ symbols still needs inline asm. */ +# define _symver_ref(extstr, intname, version) \ + __asm__ (".symver " #intname "," extstr "@" #version) + +# else + /* Set the symbol version for EXTNAME, which uses INTNAME as its implementation. */ # define symver_set(extstr, intname, version, mode) \ __asm__ (".symver " #intname "," extstr mode #version) +# endif + #else # error "Don't know how to do symbol versioning with this compiler" #endif @@ -239,9 +257,14 @@ _crypt_strcpy_or_abort (void *, const size_t, const void *); /* Tests may need to _refer_ to compatibility symbols, but should never need to _define_ them. */ - #define symver_ref(extstr, intname, version) \ + _symver_ref(extstr, intname, version) + +/* Generic way for referencing specific _compatibility_ symbols. */ +#ifndef _symver_ref +#define _symver_ref(extstr, intname, version) \ symver_set(extstr, intname, version, "@") +#endif /* Define configuration macros used during compile-time by the GOST R 34.11-2012 "Streebog" hash function. */ From 21a74c6936f51f87092879d7c57016ead823e183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Thu, 13 Aug 2020 18:55:07 +0200 Subject: [PATCH 2/8] README: Add instructions about building with link-time optimization. --- README.md | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index d0197a01..c0a40a63 100644 --- a/README.md +++ b/README.md @@ -93,13 +93,29 @@ not always be able to retrieve cryptographically-sound random numbers from the operating system; if you call these functions with a null pointer for the “rbytes” argument, be prepared for them to fail. -As of mid-2018, GCC and LLVM don’t support link-time optimization of -libraries that use symbol versioning. If you build libxcrypt with -either of these compilers, do not use `-flto`. See [GCC bug 48200][1] -for specifics; the problem is very similar for LLVM. Because this is, -at its root, a set of missing compiler features, we expect link-time -optimization won’t work in other C compilers either, but we haven’t -tested it ourselves. +As of August 2020, link-time optimization (`-flto`) of libraries that use +symbol versioning is initially supported by GCC 10 and LLVM/Clang 10, if +their compiler specific toolchain is used during compilation. + +For GCC the needed tools are called with: +``` +./configure CC="gcc" AR="gcc-ar" NM="gcc-nm" RANLIB="gcc-ranlib" \ +CFLAGS="-O2 -flto -g" ... +``` +and for LLVM/Clang those tools are called with: +``` +./configure CC="clang" AR="llvm-ar" NM="llvm-nm" RANLIB="llvm-ranlib" \ +CFLAGS="-O2 -flto -g" ... +``` + +To build static libraries, that can be linked against non-LTO objects +one may also want to add `-ffat-lto-objects` to the CFLAGS. + +If you build libxcrypt with earlier versions of these compilers, do not +use `-flto`. See [GCC bug 48200][1] for specifics; the problem is very +similar for LLVM. Because this is, at its root, a set of missing compiler +features, we expect link-time optimization won’t work in other C compilers +either, but we haven’t tested it ourselves. [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48200 From 88f9f5e78e81fe2786e9b524819b501daa23befe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Thu, 13 Aug 2020 18:56:27 +0200 Subject: [PATCH 3/8] Update NEWS. --- NEWS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS b/NEWS index 67b1aa2b..34839efe 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,11 @@ Version 4.4.17 newline '\n' character, since all parameters of the user data must be on the same line within the Unix shadow file. +* Building with link-time optimization (-flto) enabled is now + possible with either GCC 10 or LLVM/Clang 10 (issue #24). + See the 'Portability Notes' section of the README.md file for + detailed instructions and possible caveats. + Version 4.4.16 * Add support for the e2k architecture. From 4b68c68afc7f930879f65092e6659063bc6d476a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Tue, 18 Aug 2020 15:33:23 +0200 Subject: [PATCH 4/8] rpkg: Re-enable LTO. This reverts commit 328563be4eca503565de5fcb23dac656e2317ad6. --- libxcrypt.spec.rpkg | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libxcrypt.spec.rpkg b/libxcrypt.spec.rpkg index 12e24426..e8ecf883 100644 --- a/libxcrypt.spec.rpkg +++ b/libxcrypt.spec.rpkg @@ -151,11 +151,6 @@ fi \ %global _ld_strict_symbol_defs 1 -# The library uses symbol versioning in way -# that is not compatible with LTO currently. -%undefine _lto_cflags - - Name: {{{ git_name }}} Version: {{{ git_real_version }}} Release: 0.{{{ git_real_release }}}%{?dist} From 01e438bff2f293dca9fc26854357c5115733b7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Tue, 18 Aug 2020 17:59:47 +0200 Subject: [PATCH 5/8] TravisCI: Add LTO test build with LLVM/Clang and ASan+UBSan. --- .travis.yml | 10 +++++++++- .travis_script.sh | 13 +++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5dcd81a2..ae8676dc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,10 +5,11 @@ addons: packages: - autoconf - automake + - clang - gawk - lcov - libtool - - llvm + - llvm-dev - pkg-config - python3-venv - valgrind @@ -84,6 +85,13 @@ matrix: env: - CONF="--enable-obsolete-api --enable-hashes=all" - SANITIZER=1 + - name: "Linux, Clang, all hashes, obsolete API, LTO, ASan+UBSan" + compiler: clang + os: linux + env: + - CONF="--enable-obsolete-api --enable-hashes=all" + - LTO=1 + - SANITIZER=1 - name: "Linux, GCC, all hashes, static lib" compiler: gcc os: linux diff --git a/.travis_script.sh b/.travis_script.sh index d3af788c..8615de0e 100755 --- a/.travis_script.sh +++ b/.travis_script.sh @@ -133,6 +133,19 @@ if [[ "$SANITIZER" == "1" ]]; then export CXXFLAGS="$CXXFLAGS -fsanitize=undefined,address" fi +if [[ "$LTO" == "1" ]]; then + if [[ "$CC" == "clang" ]]; then + export CC="/usr/bin/clang" + export CPP="/usr/bin/clang-cpp-10" + export CXX="/usr/bin/clang++" + export AR="/usr/bin/llvm-ar" + export NM="/usr/bin/llvm-nm" + export RANLIB="/usr/bin/llvm-ranlib" + export CFLAGS="$CFLAGS -flto" + export CXXFLAGS="$CXXFLAGS -flto" + fi +fi + pushd build log_time preparation From 6a2a31407028065a7e1634294c08a4f15a85434a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Tue, 18 Aug 2020 18:18:37 +0200 Subject: [PATCH 6/8] TravisCI: Add LTO test build with GCC-10 and ASan+UBSan. --- .travis.yml | 8 ++++++++ .travis_script.sh | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/.travis.yml b/.travis.yml index ae8676dc..41a52d19 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ addons: - automake - clang - gawk + - gcc-10 - lcov - libtool - llvm-dev @@ -85,6 +86,13 @@ matrix: env: - CONF="--enable-obsolete-api --enable-hashes=all" - SANITIZER=1 + - name: "Linux, GCC, all hashes, obsolete API, LTO, ASan+UBSan" + compiler: gcc + os: linux + env: + - CONF="--enable-obsolete-api --enable-hashes=all" + - LTO=1 + - SANITIZER=1 - name: "Linux, Clang, all hashes, obsolete API, LTO, ASan+UBSan" compiler: clang os: linux diff --git a/.travis_script.sh b/.travis_script.sh index 8615de0e..146a8045 100755 --- a/.travis_script.sh +++ b/.travis_script.sh @@ -134,6 +134,16 @@ if [[ "$SANITIZER" == "1" ]]; then fi if [[ "$LTO" == "1" ]]; then + if [[ "$CC" == "gcc" ]]; then + export CC="gcc-10" + export CPP="cpp-10" + export CXX="g++-10" + export AR="gcc-ar-10" + export NM="gcc-nm-10" + export RANLIB="gcc-ranlib-10" + export CFLAGS="$CFLAGS -flto=auto -ffat-lto-objects" + export CXXFLAGS="$CXXFLAGS -flto=auto -ffat-lto-objects" + fi if [[ "$CC" == "clang" ]]; then export CC="/usr/bin/clang" export CPP="/usr/bin/clang-cpp-10" From cc5c4572dcfb24e840e640aa9c39c38f9742fd90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Tue, 18 Aug 2020 21:15:52 +0200 Subject: [PATCH 7/8] some trickery about unistd_h --- lib/crypt-port.h | 14 ++++++++++++++ lib/randombytes.c | 3 --- test/badsalt.c | 1 - test/crypt-badargs.c | 1 - test/getrandom-fallbacks.c | 3 --- test/getrandom-interface.c | 1 - 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/crypt-port.h b/lib/crypt-port.h index 9b804577..bbbe92dd 100644 --- a/lib/crypt-port.h +++ b/lib/crypt-port.h @@ -50,6 +50,20 @@ #include #endif +/* The prototype of the crypt() function possibly defined in + needs to be renamed as it might be incompatible with our declaration. + Defining this macro *AFTER* the crypt-symbol-vers header, which also + defines the same macro for symbol-versioning purposes, was included, + would lead to a clashing redefinition of this macro, and thus cause + our symbol-versioning would not work properly. */ +#ifdef HAVE_UNISTD_H +#define crypt unistd_crypt_is_incompatible +#define crypt_r unistd_crypt_r_is_incompatible +#include +#undef crypt +#undef crypt_r +#endif + #ifndef HAVE_SYS_CDEFS_THROW #define __THROW /* nothing */ #endif diff --git a/lib/randombytes.c b/lib/randombytes.c index 2c80bc7d..f7af93d4 100644 --- a/lib/randombytes.c +++ b/lib/randombytes.c @@ -31,9 +31,6 @@ #ifdef HAVE_SYS_STAT_H #include #endif -#ifdef HAVE_UNISTD_H -#include -#endif /* If we have O_CLOEXEC, we use it, but if we don't, we don't worry about it. */ diff --git a/test/badsalt.c b/test/badsalt.c index 803b5758..2e28623c 100644 --- a/test/badsalt.c +++ b/test/badsalt.c @@ -22,7 +22,6 @@ #include #include #include -#include static const char phrase[] = "values of β will give rise to dom!"; diff --git a/test/crypt-badargs.c b/test/crypt-badargs.c index 59c6690a..ae690320 100644 --- a/test/crypt-badargs.c +++ b/test/crypt-badargs.c @@ -15,7 +15,6 @@ #include #include #include -#include /* The behavior tested below should be consistent for all hashing methods. */ diff --git a/test/getrandom-fallbacks.c b/test/getrandom-fallbacks.c index 9f93cbad..8469a2ec 100644 --- a/test/getrandom-fallbacks.c +++ b/test/getrandom-fallbacks.c @@ -23,9 +23,6 @@ #ifdef HAVE_SYS_STAT_H #include #endif -#ifdef HAVE_UNISTD_H -#include -#endif /* If arc4random_buf is available, all of the fallback logic is compiled out and this test is unnecessary. If ld --wrap is not available this diff --git a/test/getrandom-interface.c b/test/getrandom-interface.c index 82be78af..e0336872 100644 --- a/test/getrandom-interface.c +++ b/test/getrandom-interface.c @@ -15,7 +15,6 @@ #include #include #include -#include static bool error_occurred; From 5d8b07f758b31463e16e563be98c1e71aa632e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Tue, 18 Aug 2020 21:29:22 +0200 Subject: [PATCH 8/8] do not include crypt_h directly --- test/badsalt.c | 1 - test/short-outbuf.c | 1 - 2 files changed, 2 deletions(-) diff --git a/test/badsalt.c b/test/badsalt.c index 2e28623c..2eecc19c 100644 --- a/test/badsalt.c +++ b/test/badsalt.c @@ -17,7 +17,6 @@ . */ #include "crypt-port.h" -#include #include #include #include diff --git a/test/short-outbuf.c b/test/short-outbuf.c index 9418c2a7..be274311 100644 --- a/test/short-outbuf.c +++ b/test/short-outbuf.c @@ -17,7 +17,6 @@ */ #include "crypt-port.h" -#include #include #include #include