kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [2/2] kudu git commit: KUDU-2068: pass --gcc-toolchain into clang codegen build
Date Wed, 19 Jul 2017 17:59:42 GMT
KUDU-2068: pass --gcc-toolchain into clang codegen build

As a brief refresher: Kudu can be built using various versions of gcc and
clang, but the codegen build always uses clang from thirdparty.

Without this patch, we delegate finding the system header/library prefix to
clang. The problem is that the result isn't guaranteed to match the prefix
used by the compiler building the rest of Kudu, which may lead to obscure
runtime errors due to std:: class layout mismatches. Kudu consumers using
custom toolchains have been forced to work around this issue [1].

There were no build issues in the following environments:
- gcc 5.4 system compiler on Ubuntu 16 (--prefix=/usr).
- gcc 4.9 devtoolset-3 compiler on CentOS 6.6
  (--prefix=/opt/rh/devtoolset-3/root/usr).
- clang 4.0 thirdparty compiler on Ubuntu 16 (no --prefix).

Additionally, I built Kudu in the following environments:
- gcc 4.8 system compiler on CentOS 7.3 (--prefix=/usr) with devtoolset-3
  also installed.
- gcc 6.2 devtoolset-6 compiler on CentOS 7.3
  (--prefix=/opt/rh/devtoolset-6/root/usr).

In these cases codegen-test crashed without the patch, and stopped crashing
after the patch was applied.

1. https://github.com/cloudera/native-toolchain/blob/f0105fd35527f416799acb4aa92a887cbf511ce5/source/kudu/build.sh

Change-Id: Id3ccb129432e3915401ca7f1f815ddb355faf597
Reviewed-on: http://gerrit.cloudera.org:8080/7463
Reviewed-by: Dan Burkert <danburkert@apache.org>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/bd61f2d5
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/bd61f2d5
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/bd61f2d5

Branch: refs/heads/master
Commit: bd61f2d57f195baa5b88c7f3183a50099b828eaf
Parents: e4e1b8e
Author: Adar Dembo <adar@cloudera.com>
Authored: Tue Jul 18 17:25:25 2017 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Wed Jul 19 17:59:03 2017 +0000

----------------------------------------------------------------------
 CMakeLists.txt                   |  2 +-
 cmake_modules/CompilerInfo.cmake |  8 +++++++-
 src/kudu/codegen/CMakeLists.txt  | 18 +++++++++++++-----
 3 files changed, 21 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/bd61f2d5/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f58a6df..5b93506 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -25,7 +25,7 @@ cmake_minimum_required(VERSION 3.4.3)
 # Prevent builds from the top-level source directory. This ensures that build
 # output is well isolated from the source tree.
 #
-# May be overridden by setting KUDU_ALLOW_IN_SOURCE_BUILDS; this is only
+# May be overridden by setting KUDU_ALLOW_IN_SOURCE_BUILD; this is only
 # recommended for experts!
 if("${CMAKE_CURRENT_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_BINARY_DIR}" AND
     NOT "${KUDU_ALLOW_IN_SOURCE_BUILD}")

http://git-wip-us.apache.org/repos/asf/kudu/blob/bd61f2d5/cmake_modules/CompilerInfo.cmake
----------------------------------------------------------------------
diff --git a/cmake_modules/CompilerInfo.cmake b/cmake_modules/CompilerInfo.cmake
index f58893a..7e1812c 100644
--- a/cmake_modules/CompilerInfo.cmake
+++ b/cmake_modules/CompilerInfo.cmake
@@ -19,7 +19,7 @@
 # Sets COMPILER_VERSION to the version
 execute_process(COMMAND env LANG=C "${CMAKE_CXX_COMPILER}" -v
                 ERROR_VARIABLE COMPILER_VERSION_FULL)
-message(INFO " ${COMPILER_VERSION_FULL}")
+message(${COMPILER_VERSION_FULL})
 
 # clang on Linux and Mac OS X before 10.9
 if("${COMPILER_VERSION_FULL}" MATCHES ".*clang version.*")
@@ -52,3 +52,9 @@ else()
 endif()
 message("Selected compiler ${COMPILER_FAMILY} ${COMPILER_VERSION}")
 
+# gcc (and some varieties of clang) mention the path prefix where system headers
+# and libraries are located.
+if("${COMPILER_VERSION_FULL}" MATCHES "Configured with: .* --prefix=([^ ]*)")
+  set(COMPILER_SYSTEM_PREFIX_PATH ${CMAKE_MATCH_1})
+  message("Selected compiler built with --prefix=${COMPILER_SYSTEM_PREFIX_PATH}")
+endif()

http://git-wip-us.apache.org/repos/asf/kudu/blob/bd61f2d5/src/kudu/codegen/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/codegen/CMakeLists.txt b/src/kudu/codegen/CMakeLists.txt
index 275de47..3cd3300 100644
--- a/src/kudu/codegen/CMakeLists.txt
+++ b/src/kudu/codegen/CMakeLists.txt
@@ -50,13 +50,13 @@ llvm_map_components_to_libnames(llvm_LIBRARIES "${LLVM_REQ_COMPONENTS}")
 # Precompiling to LLVM bytecode
 #######################################
 
-## Create .ll file for precompiled functions (and their dependencies)
+## Create .ll file for precompiled functions (and their dependencies).
 set(CLANG_EXEC ${THIRDPARTY_DIR}/clang-toolchain/bin/clang++)
 set(IR_SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/precompiled.cc)
 set(IR_OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/precompiled.ll)
 set(IR_OUTPUT_CC ${IR_OUTPUT}.cc)
 
-# Retrieve all includes directories needed for precompilation
+# Retrieve all includes directories needed for precompilation.
 get_directory_property(IR_INCLUDES
   DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
   INCLUDE_DIRECTORIES)
@@ -72,7 +72,7 @@ if (APPLE)
     -cxx-isystem "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1")
 endif()
 
-# Get preprocessing definitions, which enable directives for glog and gtest
+# Get preprocessing definitions, which enable directives for glog and gtest.
 get_directory_property(IR_PP_DEFINITIONS
   DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
   COMPILE_DEFINITIONS)
@@ -80,13 +80,21 @@ foreach(noprefix ${IR_PP_DEFINITIONS})
     set(PREFIXED_IR_PP_DEFS ${PREFIXED_IR_PP_DEFS} -D${noprefix})
 endforeach()
 
-# Get flags related to actually compiling the source
+# Get flags related to actually compiling the source.
 set(IR_FLAGS
-  -S -emit-llvm
+  -S
+  -emit-llvm
   -DIR_BUILD
   ${CMAKE_CXX_FLAGS}
   ${PREFIXED_IR_PP_DEFS}
   ${PREFIXED_IR_INCLUDES})
+
+# Provide clang with any explicitly defined system prefix path. If there isn't
+# one, clang will search for it on its own.
+if(COMPILER_SYSTEM_PREFIX_PATH)
+  set(IR_FLAGS ${IR_FLAGS} --gcc-toolchain=${COMPILER_SYSTEM_PREFIX_PATH})
+endif()
+
 separate_arguments(IR_FLAGS)
 
 # Avoid enabling ASAN in the precompiled IR.


Mime
View raw message