This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 638d95d295da0bddf3fa157792ec7de8889ac919 Author: Todd Lipcon <[email protected]> AuthorDate: Thu Jan 16 22:29:29 2020 -0800 build: restrict clang version, prefer lld, enable thinlto * Restricts clang to at least version 6.0, which gets rid of a couple older special cases. * We now loop through linkers trying to prefer lld where available, either using the version provided by the compiler with -fuse-lld or explicitly passing the path to the toolchain lld. lld should be faster than gold and GNU ld, and also has the advantage of being a known quantity within our toolchain. This fixes ASAN builds on my Ubuntu 18 machine, where the version of gold that ships with the system can't seem to link ASAN executables. * Switches the LTO build to use ThinLTO, which provides most of the performance benefits but at much lower cost. Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Reviewed-on: http://gerrit.cloudera.org:8080/15058 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo <[email protected]> --- CMakeLists.txt | 157 ++++++++++------------------------------- cmake_modules/KuduLinker.cmake | 154 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 120 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0b355c1..8df04a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,6 +82,7 @@ include(CMakeParseArguments) set(BUILD_SUPPORT_DIR ${CMAKE_CURRENT_SOURCE_DIR}/build-support) set(THIRDPARTY_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty) +set(THIRDPARTY_TOOLCHAIN_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/clang-toolchain) set(THIRDPARTY_INSTALL_DIR ${THIRDPARTY_DIR}/installed) set(THIRDPARTY_INSTALL_COMMON_DIR ${THIRDPARTY_INSTALL_DIR}/common) set(THIRDPARTY_INSTALL_UNINSTRUMENTED_DIR ${THIRDPARTY_INSTALL_DIR}/uninstrumented) @@ -122,6 +123,10 @@ endif() # Compiler flags ############################################################ +# Determine compiler version +include(CompilerInfo) + + # compiler flags that are common across debug/release builds # -msse4.2: Enable sse4.2 compiler intrinsics. execute_process(COMMAND uname -p OUTPUT_VARIABLE ARCH_NAME) @@ -185,12 +190,6 @@ set(CXX_FLAGS_DEBUG "-ggdb") set(CXX_FLAGS_FASTDEBUG "-ggdb -O1 -fno-omit-frame-pointer") set(CXX_FLAGS_RELEASE "-O3 -g -DNDEBUG -fno-omit-frame-pointer") -if (NOT "${KUDU_USE_LTO}" STREQUAL "") - set(CXX_FLAGS_RELEASE "${CXX_FLAGS_RELEASE} -flto -fuse-ld=lld") - set(CMAKE_AR "llvm-ar") - set(CMAKE_RANLIB "llvm-ranlib") -endif() - set(CXX_FLAGS_PROFILE_GEN "${CXX_FLAGS_RELEASE} -fprofile-generate") set(CXX_FLAGS_PROFILE_BUILD "${CXX_FLAGS_RELEASE} -fprofile-use") @@ -227,10 +226,17 @@ endif () # Add common flags set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}") -# Determine compiler version -include(CompilerInfo) - if ("${COMPILER_FAMILY}" STREQUAL "clang") + # Ensure that we don't use old versions of clang. There isn't anything + # particularly wrong with those, but no reason to support going back + # this far. + if ("${COMPILER_VERSION}" VERSION_LESS "6.0") + message(FATAL_ERROR "Clang version ${COMPILER_VERSION} too old. " + "Consider using the version of clang in ${THIRDPARTY_TOOLCHAIN_DIR}:\n" + "" + " CC=${BUILD_SUPPORT_DIR}/ccache-clang/clang CXX=$CC++ cmake <args>") + endif() + # Using Clang with ccache causes a bunch of spurious warnings that are # purportedly fixed in the next version of ccache. See the following for details: # @@ -320,18 +326,6 @@ if (${KUDU_USE_ASAN}) message(SEND_ERROR "Cannot use ASAN without clang or gcc >= 4.8") endif() - # If UBSAN is also enabled, and we're on clang < 3.5, ensure static linking is - # enabled. Otherwise, we run into https://llvm.org/bugs/show_bug.cgi?id=18211 - if("${KUDU_USE_UBSAN}" AND - "${COMPILER_FAMILY}" STREQUAL "clang" AND - "${COMPILER_VERSION}" VERSION_LESS "3.5") - if("${KUDU_LINK}" STREQUAL "a") - message("Using static linking for ASAN+UBSAN build") - set(KUDU_LINK "s") - elseif("${KUDU_LINK}" STREQUAL "d") - message(SEND_ERROR "Cannot use dynamic linking when ASAN and UBSAN are both enabled") - endif() - endif() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -DADDRESS_SANITIZER") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DLEAK_SANITIZER") endif() @@ -400,23 +394,36 @@ if (${KUDU_USE_TSAN}) endif() endif() +include("KuduLinker") +APPEND_LINKER_FLAGS() if ("${KUDU_USE_ASAN}" OR "${KUDU_USE_TSAN}" OR "${KUDU_USE_UBSAN}") # GCC 4.8 and 4.9 (latest as of this writing) don't allow you to specify a # sanitizer blacklist. if("${COMPILER_FAMILY}" STREQUAL "clang") - # Require clang 3.4 or newer; clang 3.3 has issues with TSAN and pthread - # symbol interception. - if("${COMPILER_VERSION}" VERSION_LESS "3.4") - message(SEND_ERROR "Must use clang 3.4 or newer to run a sanitizer build." - " Try using clang from thirdparty/") - endif() add_definitions("-fsanitize-blacklist=${BUILD_SUPPORT_DIR}/sanitize-blacklist.txt") else() message(WARNING "GCC does not support specifying a sanitizer blacklist. Known sanitizer check failures will not be suppressed.") endif() endif() +if (KUDU_USE_LTO) + if(NOT "${CMAKE_EXE_LINKER_FLAGS}" MATCHES "-fuse-ld=lld" OR + NOT "${COMPILER_FAMILY}" STREQUAL "clang") + message(FATAL_ERROR "must use clang and lld for LTO build") + endif() + if (NOT "${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE") + # Doesn't make much sense to do LTO on a non-release build -- it's slow + # and the only reason you'd want to do it is high performance. + message(FATAL_ERROR "LTO only supported for release builds") + endif() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto=thin") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--thinlto-cache-dir=${CMAKE_CURRENT_BINARY_DIR}/lto.cache") + set(CMAKE_AR "${THIRDPARTY_TOOLCHAIN_DIR}/bin/llvm-ar") + set(CMAKE_RANLIB "${THIRDPARTY_TOOLCHAIN_DIR}/bin/llvm-ranlib") +endif() + + # Code coverage if ("${KUDU_GENERATE_COVERAGE}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage -DCOVERAGE_BUILD") @@ -442,93 +449,6 @@ if ("${KUDU_LINK}" STREQUAL "a") endif() endif() -# Interrogates the linker version via the C++ compiler to determine whether -# we're using the gold linker, and if so, extracts its version. -# -# If the gold linker is being used, sets GOLD_VERSION in the parent scope with -# the extracted version. -# -# Any additional arguments are passed verbatim into the C++ compiler invocation. -function(GET_GOLD_VERSION) - # The gold linker is only for ELF binaries, which macOS doesn't use. - if (APPLE) - return() - endif() - - execute_process(COMMAND ${CMAKE_CXX_COMPILER} "-Wl,--version" ${ARGN} - ERROR_QUIET - OUTPUT_VARIABLE LINKER_OUTPUT) - # We're expecting LINKER_OUTPUT to look like one of these: - # GNU gold (version 2.24) 1.11 - # GNU gold (GNU Binutils for Ubuntu 2.30) 1.15 - if (LINKER_OUTPUT MATCHES "GNU gold") - string(REGEX MATCH "GNU gold \\([^\\)]*\\) (([0-9]+\\.?)+)" _ "${LINKER_OUTPUT}") - if (NOT CMAKE_MATCH_1) - message(SEND_ERROR "Could not extract GNU gold version. " - "Linker version output: ${LINKER_OUTPUT}") - endif() - set(GOLD_VERSION "${CMAKE_MATCH_1}" PARENT_SCOPE) - endif() -endfunction() - -# Is the compiler hard-wired to use the gold linker? -GET_GOLD_VERSION() -if (GOLD_VERSION) - set(MUST_USE_GOLD 1) -else() - # Can the compiler optionally enable the gold linker? - GET_GOLD_VERSION("-fuse-ld=gold") - - # We can't use the gold linker if it's inside devtoolset because the compiler - # won't find it when invoked directly from make/ninja (which is typically - # done outside devtoolset). - execute_process(COMMAND which ld.gold - OUTPUT_VARIABLE GOLD_LOCATION - OUTPUT_STRIP_TRAILING_WHITESPACE - ERROR_QUIET) - if ("${GOLD_LOCATION}" MATCHES "^/opt/rh/devtoolset") - message("Skipping optional gold linker (version ${GOLD_VERSION}) because " - "it's in devtoolset") - set(GOLD_VERSION) - endif() -endif() -if (GOLD_VERSION) - # Older versions of the gold linker are vulnerable to a bug [1] which - # prevents weak symbols from being overridden properly. This leads to - # omitting of Kudu's tcmalloc dependency if using dynamic linking. - # - # How we handle this situation depends on other factors: - # - If gold is optional, we won't use it. - # - If gold is required and we're using dynamic linking, we'll either: - # - Raise an error in RELEASE builds (we shouldn't release such a product), or - # - Drop tcmalloc in all other builds. - # - # 1. https://sourceware.org/bugzilla/show_bug.cgi?id=16979. - if ("${GOLD_VERSION}" VERSION_LESS "1.12") - set(KUDU_BUGGY_GOLD 1) - endif() - if (MUST_USE_GOLD) - message("Using hard-wired gold linker (version ${GOLD_VERSION})") - if (KUDU_BUGGY_GOLD AND "${KUDU_LINK}" STREQUAL "d") - if ("${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE") - message(SEND_ERROR "Configured to use buggy gold with dynamic linking " - "in a RELEASE build; this would cause tcmalloc symbols to get dropped") - endif() - message("Hard-wired gold linker is buggy, dropping tcmalloc dependency") - set(GOLD_INCOMPATIBLE_WITH_TCMALLOC 1) - endif() - elseif (NOT KUDU_BUGGY_GOLD) - # The Gold linker must be manually enabled. - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fuse-ld=gold") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fuse-ld=gold") - message("Using optional gold linker (version ${GOLD_VERSION})") - else() - message("Optional gold linker is buggy, using ld linker instead") - endif() -else() - message("Using ld linker") -endif() - # Having set KUDU_LINK due to build type and/or sanitizer, it's now safe to # act on its value. if ("${KUDU_LINK}" STREQUAL "d") @@ -1130,11 +1050,9 @@ endif() ## Google PerfTools ## -## Disabled with TSAN/ASAN as well as with gold+dynamic linking (see -## earlier comment). +## Disabled with TSAN/ASAN. if (NOT "${KUDU_USE_ASAN}" AND - NOT "${KUDU_USE_TSAN}" AND - NOT "${GOLD_INCOMPATIBLE_WITH_TCMALLOC}") + NOT "${KUDU_USE_TSAN}") find_package(GPerf REQUIRED) ADD_THIRDPARTY_LIB(tcmalloc STATIC_LIB "${TCMALLOC_STATIC_LIB}" @@ -1267,8 +1185,7 @@ if (APPLE) endif() else() if (KUDU_TCMALLOC_AVAILABLE AND - (("${COMPILER_FAMILY}" STREQUAL "clang" AND - "${COMPILER_VERSION}" VERSION_GREATER "3.7") OR + (("${COMPILER_FAMILY}" STREQUAL "clang") OR ("${COMPILER_FAMILY}" STREQUAL "gcc" AND "${COMPILER_VERSION}" VERSION_GREATER "5.0"))) set(COMPILER_SUPPORTS_SIZED_DEALLOCATION TRUE) diff --git a/cmake_modules/KuduLinker.cmake b/cmake_modules/KuduLinker.cmake new file mode 100644 index 0000000..f2b2ac4 --- /dev/null +++ b/cmake_modules/KuduLinker.cmake @@ -0,0 +1,154 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# The Gold linker must be manually enabled. +function(APPEND_LINKER_FLAGS) + # Search candidate linkers in the order of decreasing speed and functionality. + # In particular, lld is the best choice since it's quite fast and supports + # ThinLTO, etc. + foreach(candidate_linker lld "${THIRDPARTY_TOOLCHAIN_DIR}/bin/ld.lld" gold default) + if(candidate_linker STREQUAL "default") + set(candidate_flag "") + else() + set(candidate_flag "-fuse-ld=${candidate_linker}") + endif() + GET_LINKER_VERSION("${candidate_flag}") + if(NOT LINKER_FOUND) + continue() + endif() + + # Older versions of the gold linker are vulnerable to a bug [1] which + # prevents weak symbols from being overridden properly. This leads to + # omitting of Kudu's tcmalloc dependency if using dynamic linking. + # + # We'll skip gold in our list of candidate linkers if this bug is relevant. + # + # 1. https://sourceware.org/bugzilla/show_bug.cgi?id=16979. + if ("${LINKER_FAMILY}" STREQUAL "gold") + if("${LINKER_VERSION}" VERSION_LESS "1.12" AND + "${KUDU_LINK}" STREQUAL "d") + message(WARNING "Skipping gold <1.12 with dynamic linking.") + continue() + endif() + + # We can't use the gold linker if it's inside devtoolset because the compiler + # won't find it when invoked directly from make/ninja (which is typically + # done outside devtoolset). This seems to be fixed in devtoolset-6 or later + # by having the gcc/clang binary search the devtoolset bin directory even + # if it's not on $PATH. + execute_process(COMMAND which ld.gold + OUTPUT_VARIABLE GOLD_LOCATION + OUTPUT_STRIP_TRAILING_WHITESPACE + ERROR_QUIET) + + if ("${GOLD_LOCATION}" MATCHES "^/opt/rh/devtoolset-[3-5]/") + message(WARNING "Cannot use gold with devtoolset < 6, skipping...") + continue() + endif() + endif() + + # LLD < 10.0.0 has a bug[1] which causes the __tsan_default_options (and similar) + # symbols to be defined as WEAK instead of DEFINED in the resulting binary. This + # causes our suppressions to not work properly, resulting in failed tests. + # + # [1] https://reviews.llvm.org/D63974 + if ("${LINKER_FAMILY}" STREQUAL "lld" AND + "${LINKER_VERSION}" VERSION_LESS "10.0.0" AND + ("${KUDU_USE_TSAN}" OR "${KUDU_USE_ASAN}" OR "${KUDU_USE_UBSAN}")) + message(WARNING "Cannot use lld < 10.0.0 with TSAN/ASAN/UBSAN. Skipping...") + continue() + endif() + set(linker_flags "${candidate_flag}") + break() + endforeach() + + if(NOT DEFINED linker_flags) + message(SEND_ERROR "Could not find a suitable linker") + endif() + message(STATUS "Using linker flags: ${linker_flags}") + foreach(var CMAKE_SHARED_LINKER_FLAGS CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS) + set(${var} "${${var}} ${linker_flags}" PARENT_SCOPE) + endforeach() +endfunction() + +# Interrogates the linker version via the C++ compiler to determine which linker +# we're using, along with its version. +# +# Sets the following variables: +# LINKER_FOUND: true/false +# LINKER_FAMILY: lld/ld/gold +# LINKER_VERSION: version number of the linker +# Any additional arguments are passed verbatim into the C++ compiler invocation. +function(GET_LINKER_VERSION) + if(ARGN) + message(STATUS "Checking linker version with cflags: ${ARGN}") + else() + message(STATUS "Checking linker version when not specifying any flags") + endif() + execute_process(COMMAND ${CMAKE_CXX_COMPILER} "-Wl,--version" ${ARGN} + ERROR_QUIET + OUTPUT_VARIABLE LINKER_OUTPUT) + if (LINKER_OUTPUT MATCHES "GNU gold") + # We're expecting LINKER_OUTPUT to look like one of these: + # GNU gold (version 2.24) 1.11 + # GNU gold (GNU Binutils for Ubuntu 2.30) 1.15 + if (NOT "${LINKER_OUTPUT}" MATCHES "GNU gold \\([^\\)]*\\) (([0-9]+\\.?)+)") + message(SEND_ERROR "Could not extract GNU gold version. " + "Linker version output: ${LINKER_OUTPUT}") + endif() + set(LINKER_FAMILY "gold") + set(LINKER_VERSION "${CMAKE_MATCH_1}") + set(LINKER_FOUND TRUE) + elseif (LINKER_OUTPUT MATCHES "GNU ld") + # We're expecting LINKER_OUTPUT to look like one of these: + # GNU ld (GNU Binutils for Ubuntu) 2.30 + # GNU ld version 2.20.51.0.2-5.42.el6 20100205 + # GNU ld version 2.25.1-22.base.el7 + if (NOT "${LINKER_OUTPUT}" MATCHES "GNU ld version (([0-9]+\\.?)+)" AND + NOT "${LINKER_OUTPUT}" MATCHES "GNU ld \\([^\\)]*\\) (([0-9]+\\.?)+)") + message(SEND_ERROR "Could not extract GNU ld version. " + "Linker version output: ${LINKER_OUTPUT}") + endif() + set(LINKER_FAMILY "ld") + set(LINKER_VERSION "${CMAKE_MATCH_1}") + set(LINKER_FOUND TRUE) + elseif (LINKER_OUTPUT MATCHES "LLD") + # Sample: + # LLD 9.0.0 (example.com:kudu.git a5848a4c3c8c72a1ac823182e87cd1f6c31ddc15) (compatible with GNU linkers) + if (NOT "${LINKER_OUTPUT}" MATCHES "LLD (([0-9]+\\.?)+)") + message(SEND_ERROR "Could not extract lld version. " + "Linker version output: ${LINKER_OUTPUT}") + endif() + set(LINKER_FAMILY "lld") + set(LINKER_VERSION "${CMAKE_MATCH_1}") + set(LINKER_FOUND TRUE) + else() + set(LINKER_FAMILY "") + set(LINKER_VERSION "") + set(LINKER_FOUND FALSE) + endif() + + if(LINKER_FOUND) + message(STATUS "Found linker: ${LINKER_FAMILY} version ${LINKER_VERSION}") + else() + message(STATUS "Linker not found") + endif() + # Propagate the results to the caller. + set(LINKER_FOUND "${LINKER_FOUND}" PARENT_SCOPE) + set(LINKER_FAMILY "${LINKER_FAMILY}" PARENT_SCOPE) + set(LINKER_VERSION "${LINKER_VERSION}" PARENT_SCOPE) +endfunction()
