Bug #100410 cmake code to disable LTO is too simple
Submitted: 2 Aug 2020 11:17 Modified: 11 Aug 2020 14:38
Reporter: Terje Røsten Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Compiling Severity:S3 (Non-critical)
Version:8.0.21 OS:Any
Assigned to: Tor Didriksen CPU Architecture:Any

[2 Aug 2020 11:17] Terje Røsten
Description:
InnoDB Memcached plugin don't build with lto and cmake code does:

./plugin/innodb_memcached/CMakeLists.txt:  STRING(REPLACE "-flto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
./plugin/innodb_memcached/CMakeLists.txt:  STRING(REPLACE "-flto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")

to fix this, however GCC supports (and this is used by Linux distros): 

  -flto=auto

hence option might be converted like this:

 -flto=auto  -> =auto

This is  of course invalid and causes build break.

How to repeat:
Use:

 -DCMAKE_C_FLAGS='-flto=auto'
 -DCMAKE_CXX_FLAGS='-flto=auto'
 -DWITH_INNODB_MEMCACHED=ON

with e.g. GCC 9 and try to build.

Suggested fix:
Something like this:

diff --git a/plugin/innodb_memcached/CMakeLists.txt b/plugin/innodb_memcached/CMakeLists.txt
index b9d93da..c471057 100644
--- a/plugin/innodb_memcached/CMakeLists.txt
+++ b/plugin/innodb_memcached/CMakeLists.txt
@@ -76,8 +76,11 @@ IF(WITH_INNODB_MEMCACHED AND UNIX)
   ENDIF()
 
   # -Werror=lto-type-mismatch for misc functions.
-  STRING(REPLACE "-flto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
-  STRING(REPLACE "-flto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  STRING(REPLACE "-flto " "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+  STRING(REPLACE "-flto " "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+  STRING(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+  STRING(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 
   ADD_SUBDIRECTORY(daemon_memcached)
   ADD_SUBDIRECTORY(innodb_memcache)

or make InnoDB memcached plugin build with LTO enabled.
[2 Aug 2020 12:05] Terje Røsten
There is additional problem for mysql_upgrade, while linking it uses ./archive_output_directory/libmysqlclient.a  *twice*:

cd /w/t/mysql/b/client && /usr/bin/cmake -E cmake_link_script CMakeFiles/mysql_upgrade.dir/link.txt --verbose=1
/usr/bin/c++ -std=c++14 -fno-omit-frame-pointer -ftls-model=initial-exec -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wall -Wextra -Wformat-security -Wvla -Wundef -Wmissing-format-attribute -Woverloaded-virtual -Wcast-qual -Wimplicit-fallthrough=2 -Wstringop-truncation -Wsuggest-override -Wlogical-op -DDBUG_OFF -ffunction-sections -fdata-sections -O2 -g -DNDEBUG  -fuse-ld=gold -Wl,--gc-sections CMakeFiles/mysql_upgrade.dir/upgrade/program.cc.o -o ../runtime_output_directory/mysql_upgrade  -lpthread 

../archive_output_directory/libmysqlclient.a 

base/libclient_base.a 

../archive_output_directory/libmysqlclient.a 

-lpthread -lm -lrt /usr/lib64/libssl.so /usr/lib64/libcrypto.so -ldl /usr/lib64/libssl.so /usr/lib64/libcrypto.so -ldl /usr/lib64/libresolv.so

causing build break:

/usr/bin/ld.gold: error: ../archive_output_directory/libmysqlclient.a(libmysql.cc.o): multiple definition of 'handle_local_infile(MYSQL*, char const*)'
/usr/bin/ld.gold: /tmp/mysql_upgrade.lrZXoM.ltrans1.ltrans.o: previous definition here

To reproduce (build on rawhide system):

$ mkdir build && cd build

$ cmake .. -DCMAKE_C_FLAGS="$(rpm --eval %optflags)" -DCMAKE_CXX_FLAGS="$(rpm --eval %optflags)"

$ cd client

$ make -j$(nproc) VERBOSE=1 mysql_upgrade
[2 Aug 2020 16:56] Terje Røsten
After fixing client problems, even more problems shows up.

During linking of pfs_connect_attr-t:

cd /builddir/build/BUILD/mysql-8.0.21/build/storage/perfschema/unittest
&& /usr/bin/cmake -E cmake_link_script
CMakeFiles/pfs_connect_attr-t.dir/link.txt --verbose=1
/usr/bin/g++ -std=c++14 -fno-omit-frame-pointer
-ftls-model=initial-exec -O2 -flto=auto -ffat-lto-objects
-fexceptions -g -grecord-gcc-switches -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
-Wp,-D_GLIBCXX_ASSERTIONS
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-fstack-protector-strong
-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
-fasynchronous-unwind-tables -fstack-clash-protection
-fcf-protection -Wall -Wextra -Wformat-security -Wvla -Wundef
-Wmissing-format-attribute -Woverloaded-virtual -Wcast-qual
-Wimplicit-fallthrough=2 -Wstringop-truncation -Wlogical-op
-DDBUG_OFF -ffunction-sections -fdata-sections -O2 -g -DNDEBUG
-Wl,-z,relro -Wl,--as-needed -Wl,-z,now
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fuse-ld=gold
-Wl,--gc-sections -pie -Wl,-z,relro -Wl,--as-needed -Wl,-z,now
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld
CMakeFiles/pfs_connect_attr-t.dir/pfs_connect_attr-t.cc.o
CMakeFiles/pfs_connect_attr-t.dir/__/__/__/sql/sql_builtin.cc.o
-o ../../../runtime_output_directory/pfs_connect_attr-t -lpthread
../../../unittest/mytap/libmytap.a
../../../libserver_unittest_library.a
../../archive/libarchive.a
/usr/lib64/libz.so
../../blackhole/libblackhole.a
../../csv/libcsv.a
../../federated/libfederated.a
../../heap/libheap.a
../../heap/libheap_library.a
../../innobase/libinnobase.a
../../../sql/libsql_dd.a
../../../sql/libsql_gis.a
/usr/lib64/libz.so /usr/lib64/liblz4.so
-lnuma
../../myisam/libmyisam.a
../../myisam/libmyisam_library.a
../../myisammrg/libmyisammrg.a
../libperfschema.a
../../../sql/libsql_main.a
../../../sql/librpl.a
../../../sql/libmaster.a
../../../sql/libslave.a
../../../sql/libbinlog.a
../../innobase/libinnobase.a
../../../sql/libsql_dd.a
../../../sql/libsql_gis.a
../../../sql/libsql_main.a
../../../sql/librpl.a
../../../sql/libmaster.a
../../../sql/libslave.a
../../../sql/libbinlog.a
../../innobase/libinnobase.a
../../../sql/libsql_dd.a
../../../sql/libsql_gis.a
../../../sql/libsql_main.a
../../../sql/librpl.a
../../../sql/libmaster.a
../../../sql/libslave.a
../../../sql/libbinlog.a -laio
../../archive/libarchive.a
../../blackhole/libblackhole.a
../../csv/libcsv.a
../../federated/libfederated.a
../../heap/libheap.a
../../heap/libheap_library.a
-lnuma
../../myisam/libmyisam.a
../../myisam/libmyisam_library.a
../../myisammrg/libmyisammrg.a
../libperfschema.a
../../temptable/libtemptable.a
../../../plugin/fulltext/libngram_parser.a
../../../plugin/x/libmysqlx.a
/usr/lib64/libevent_core.so
/usr/lib64/libevent_extra.so
/usr/lib64/libevent_openssl.so
/usr/lib64/liblz4.so
../../../plugin/x/protocol/protobuf/libmysqlxmessages_lite.a
/usr/lib64/libprotobuf-lite.so
../../../sql/server_component/libmysql_server_component_services.a
../../../archive_output_directory/libvio.a
-lcrypt
../../../libbinlogevents/lib/libbinlogevents.a
../../../archive_output_directory/libmysys.a
../../../archive_output_directory/libstrings.a
../../../archive_output_directory/libmysys.a
../../../archive_output_directory/libstrings.a /usr/lib64/libz.so
../../../archive_output_directory/libmytime.a
-lm
-lrt
/usr/lib64/libssl.so /usr/lib64/libcrypto.so
-ldl
/usr/lib64/libzstd.so
../../../components/libminchassis/libminchassis.a
-lpthread
/usr/lib64/libicuuc.so /usr/lib64/libicuio.so
/usr/lib64/libicudata.so /usr/lib64/libicui18n.so

Some sort and uniq processing on this shows lots od dups (or even triplets):

 ../../myisam/libmyisam.a
 ../../myisammrg/libmyisammrg.a
 ../../../sql/libbinlog.a
 ../../../sql/libmaster.a
 ../../../sql/librpl.a
 ../../../sql/libsql_dd.a
 ../../../sql/libsql_gis.a
 ../../../sql/libsql_main.a

Some clean up seems to be needed.
[3 Aug 2020 9:13] Tor Didriksen
We have libraries with mutual dependencis, so the repeated inclusion of some libraries is intentional, see cmake source code:
SET_TARGET_PROPERTIES(sql_gis PROPERTIES LINK_INTERFACE_MULTIPLICITY 3)
[3 Aug 2020 11:52] Tor Didriksen
This IMHO is a bug in the gold linker.
Note that we already switch LLD off by default for LTO builds.
# LTO build fails with lld, so turn it off by default.
IF(LINUX AND NOT WITH_LTO)
  OPTION(USE_LD_LLD "Use llvm lld linker" ON)

Seems we should do the same thing for USE_LD_GOLD

Linking the same static library multiple times is perfectly legal,
and in our case, necessary, since we have libraries which are mutually dependent.
[11 Aug 2020 14:38] Paul DuBois
Posted by developer:
 
Fixed in 8.0.22.

For some third-party libraries, enabling link-time optimization
caused build failures.