kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [1/3] kudu git commit: thirdparty: clean up unused argument warnings
Date Thu, 31 May 2018 18:18:20 GMT
Repository: kudu
Updated Branches:
  refs/heads/master f7d1f39da -> 5f9a2f523


thirdparty: clean up unused argument warnings

Commit ba3cdea53 revved breakpad to a hash that includes -Werror in its
build. This exposed an issue with non-ccache thirdparty builds: it turns out
that clang has been emitting a lot of warnings about unused arguments when
building TSAN dependencies, and those warnings now break breakpad when built
for TSAN. Builds that use ccache don't emit those warnings because the
build-support/ccache-clang scripts add -Qunused-arguments to the clang
command line.

We could fix this by patching out -Werror in breakpad, but that's not
particularly robust; whose to say that -Werror won't show up in another
dependency in the future? So instead, this patch tries to clean up those
unused argument warnings.

The core issue is the use of -stdlib=libc++ and -nostdinc++. Apparently the
former is intended to be a linker option and thus shouldn't be part of
CXXFLAGS, while the latter is a compiler option. So this patch moves
-stdlib=libc++ to LDFLAGS. We're also now passing LDFLAGS into
CMAKE_SHARED_LINKER_FLAGS and CMAKE_MODULE_LINKER_FLAGS; this allowed
LLVMHello.so to find libc++ in the LLVM build.

Sadly, I couldn't get rid of all the unused argument warnings. The problem
is that clang warns about -nostdinc++ when it's passed to the linker, and
setting it in CMAKE_CXX_FLAGS does just that. So I ended up adding
-Qunused-arguments anyway.

I tested this with a full thirdparty build without ccache on Ubuntu 16.04
as well as full builds on CentOS 6.6 and Ubuntu 18.04 with ccache.

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


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

Branch: refs/heads/master
Commit: d7af95bd1ffb62338c051343dece68e7d849a718
Parents: f7d1f39
Author: Adar Dembo <adar@cloudera.com>
Authored: Fri May 18 15:01:30 2018 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Wed May 30 22:41:40 2018 +0000

----------------------------------------------------------------------
 thirdparty/build-definitions.sh | 33 +++++++++++++++++++++------------
 thirdparty/build-thirdparty.sh  | 21 +++++++++++++++------
 2 files changed, 36 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d7af95bd/thirdparty/build-definitions.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-definitions.sh b/thirdparty/build-definitions.sh
index ebb4f72..203d84d 100644
--- a/thirdparty/build-definitions.sh
+++ b/thirdparty/build-definitions.sh
@@ -64,7 +64,7 @@ restore_env() {
 #
 # 1. https://debbugs.gnu.org/db/10/10579.html
 fixup_libtool() {
-  if [[ ! "$EXTRA_CXXFLAGS" =~ "-stdlib=libc++" ]]; then
+  if [[ ! "$EXTRA_LDFLAGS" =~ "-stdlib=libc++" ]]; then
     echo "libtool does not need to be fixed up: not using libc++"
     return
   fi
@@ -136,10 +136,14 @@ build_libcxx() {
   cmake \
     -DCMAKE_BUILD_TYPE=Release \
     -DCMAKE_INSTALL_PREFIX=$PREFIX \
-    -DCMAKE_CXX_FLAGS="$EXTRA_CXXFLAGS $EXTRA_LDFLAGS" \
+    -DCMAKE_CXX_FLAGS="$EXTRA_CXXFLAGS" \
+    -DCMAKE_EXE_LINKER_FLAGS="$EXTRA_LDFLAGS" \
+    -DCMAKE_MODULE_LINKER_FLAGS="$EXTRA_LDFLAGS" \
+    -DCMAKE_SHARED_LINKER_FLAGS="$EXTRA_LDFLAGS" \
     -DLLVM_PATH=$LLVM_SOURCE \
     -DLIBCXX_CXX_ABI=libcxxabi \
     -DLIBCXX_CXX_ABI_INCLUDE_PATHS=$LLVM_SOURCE/projects/libcxxabi/include \
+    -DLIBCXX_CXX_ABI_LIBRARY_PATH=$PREFIX/lib \
     -DLLVM_USE_SANITIZER=$SANITIZER_TYPE \
     $EXTRA_CMAKE_FLAGS \
     $LLVM_SOURCE/projects/libcxx
@@ -252,11 +256,8 @@ build_llvm() {
     TOOLS_ARGS="$TOOLS_ARGS -D${arg}=OFF"
   done
 
-  # Remove '-nostdinc++' from the cflags for clang, since it already
-  # handles this when passing --stdlib=libc++ and passing this confuses
-  # the check for -fPIC.
-  CLANG_CXXFLAGS=$(echo "$EXTRA_CXXFLAGS" | sed -e 's,-nostdinc++,,g;')
-
+  CLANG_CXXFLAGS="$EXTRA_CXXFLAGS"
+  CLANG_LDFLAGS="$EXTRA_LDFLAGS"
   case $BUILD_TYPE in
     "normal")
       # Default build: core LLVM libraries, clang, compiler-rt, and all tools.
@@ -326,8 +327,10 @@ build_llvm() {
     -DLLVM_INCLUDE_UTILS=OFF \
     -DLLVM_TARGETS_TO_BUILD=X86 \
     -DLLVM_ENABLE_RTTI=ON \
-    -DCMAKE_CXX_FLAGS="$CLANG_CXXFLAGS $EXTRA_LDFLAGS" \
+    -DCMAKE_CXX_FLAGS="$CLANG_CXXFLAGS" \
     -DCMAKE_EXE_LINKER_FLAGS="$CLANG_LDFLAGS" \
+    -DCMAKE_MODULE_LINKER_FLAGS="$CLANG_LDFLAGS" \
+    -DCMAKE_SHARED_LINKER_FLAGS="$CLANG_LDFLAGS" \
     -DPYTHON_EXECUTABLE=$PYTHON_EXECUTABLE \
     $TOOLS_ARGS \
     $EXTRA_CMAKE_FLAGS \
@@ -351,11 +354,14 @@ build_gflags() {
   mkdir -p $GFLAGS_BDIR
   pushd $GFLAGS_BDIR
   rm -rf CMakeCache.txt CMakeFiles/
-  CXXFLAGS="$EXTRA_CFLAGS $EXTRA_CXXFLAGS $EXTRA_LDFLAGS $EXTRA_LIBS" \
-    cmake \
+  cmake \
     -DCMAKE_BUILD_TYPE=Release \
     -DCMAKE_POSITION_INDEPENDENT_CODE=On \
     -DCMAKE_INSTALL_PREFIX=$PREFIX \
+    -DCMAKE_CXX_FLAGS="$EXTRA_CXXFLAGS" \
+    -DCMAKE_EXE_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
+    -DCMAKE_MODULE_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
+    -DCMAKE_SHARED_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
     -DBUILD_SHARED_LIBS=On \
     -DBUILD_STATIC_LIBS=On \
     -DREGISTER_INSTALL_PREFIX=Off \
@@ -437,10 +443,13 @@ build_gmock() {
     mkdir -p $GMOCK_BDIR
     pushd $GMOCK_BDIR
     rm -rf CMakeCache.txt CMakeFiles/
-    CXXFLAGS="$EXTRA_CXXFLAGS $EXTRA_LDFLAGS $EXTRA_LIBS" \
-      cmake \
+    cmake \
       -DCMAKE_BUILD_TYPE=Debug \
       -DCMAKE_POSITION_INDEPENDENT_CODE=On \
+      -DCMAKE_CXX_FLAGS="$EXTRA_CXXFLAGS" \
+      -DCMAKE_EXE_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
+      -DCMAKE_MODULE_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
+      -DCMAKE_SHARED_LINKER_FLAGS="$EXTRA_LDFLAGS $EXTRA_LIBS" \
       -DBUILD_SHARED_LIBS=$SHARED \
       $EXTRA_CMAKE_FLAGS \
       $GMOCK_SOURCE/googlemock

http://git-wip-us.apache.org/repos/asf/kudu/blob/d7af95bd/thirdparty/build-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-thirdparty.sh b/thirdparty/build-thirdparty.sh
index 8abc948..8c2a44d 100755
--- a/thirdparty/build-thirdparty.sh
+++ b/thirdparty/build-thirdparty.sh
@@ -462,10 +462,6 @@ fi
 
 save_env
 
-# libc++ (and its dependents) need to find libc++abi at link and run time.
-EXTRA_LDFLAGS="-L$PREFIX/lib $EXTRA_LDFLAGS"
-EXTRA_LDFLAGS="-Wl,-rpath,$PREFIX/lib $EXTRA_LDFLAGS"
-
 # Build libc++ with TSAN enabled.
 if [ -n "$F_TSAN" -o -n "$F_LLVM" ]; then
   build_libcxx tsan
@@ -473,9 +469,9 @@ fi
 
 # Build the rest of the dependencies against the TSAN-instrumented libc++
 # instead of the system's C++ standard library.
-EXTRA_CXXFLAGS="-nostdinc++ $EXTRA_CXXFLAGS"
-EXTRA_CXXFLAGS="-stdlib=libc++ $EXTRA_CXXFLAGS"
 EXTRA_CXXFLAGS="-isystem $PREFIX/include/c++/v1 $EXTRA_CXXFLAGS"
+EXTRA_LDFLAGS="-L$PREFIX/lib $EXTRA_LDFLAGS"
+EXTRA_LDFLAGS="-Wl,-rpath,$PREFIX/lib $EXTRA_LDFLAGS"
 
 # Build the rest of the dependencies with TSAN instrumentation.
 EXTRA_CFLAGS="-fsanitize=thread $EXTRA_CFLAGS"
@@ -486,6 +482,19 @@ if [ -n "$F_TSAN" -o -n "$F_LLVM" ]; then
   build_llvm tsan
 fi
 
+# LLVM is told to use libc++ explicitly and thus doesn't need these, but the
+# rest of the dependencies need them.
+#
+# Note: -nostdinc++ is necessary to prevent C++ headers from using #include_next
+# to chain the host's C++ headers. However, using it means we need to also use
+# -Qunused-arguments because clang raises an unused argument warning when it
+# detects -nostdinc++ on a link line, and there's no way to prevent that when
+# passing -nostdinc++ to cmake via -DCMAKE_CXX_FLAGS [1].
+#
+# 1. https://gitlab.kitware.com/cmake/cmake/issues/12652
+EXTRA_CXXFLAGS="-Qunused-arguments -nostdinc++ $EXTRA_CXXFLAGS"
+EXTRA_LDFLAGS="-stdlib=libc++ $EXTRA_LDFLAGS"
+
 # Enable debug symbols so that stacktraces and linenumbers are available at
 # runtime. LLVM is compiled without debug symbols because the LLVM debug symbols
 # take up more than 20GiB of disk space.


Mime
View raw message