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


The following commit(s) were added to refs/heads/master by this push:
     new 3a856f450 [codegen] fix compilation with CLANG 15 on macOS
3a856f450 is described below

commit 3a856f4502a29f26bb8c9b10a17b06eb16892d02
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Mon Apr 1 19:29:26 2024 -0700

    [codegen] fix compilation with CLANG 15 on macOS
    
    Prior to this fix, compiling Kudu with CLANG 15 on macOS Sonoma
    would fail with an error like below:
    
      [..%] Generating precompiled.ll
      In file included from src/kudu/codegen/precompiled.cc:39:
      In file included from src/kudu/common/rowblock.h:21:
      ...
      
/Applications/Xcode-15.3.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/cstdlib:144:9:
 error: no member named 'at_quick_exit' in the global namespace
      using ::at_quick_exit _LIBCPP_USING_IF_EXISTS;
            ~~^
      
/Applications/Xcode-15.3.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/cstdlib:145:9:
 error: no member named 'quick_exit' in the global namespace
      using ::quick_exit _LIBCPP_USING_IF_EXISTS;
    
    Change-Id: Ibe92b7f00fdd446010cea1bda5fd25868a9acabc
    Reviewed-on: http://gerrit.cloudera.org:8080/21232
    Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
    Tested-by: Alexey Serbin <ale...@apache.org>
---
 src/kudu/codegen/CMakeLists.txt | 98 +++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 58 deletions(-)

diff --git a/src/kudu/codegen/CMakeLists.txt b/src/kudu/codegen/CMakeLists.txt
index eb32a41d5..6ffa499e8 100644
--- a/src/kudu/codegen/CMakeLists.txt
+++ b/src/kudu/codegen/CMakeLists.txt
@@ -56,57 +56,50 @@ 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.
-get_directory_property(IR_INCLUDES
-  DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
-  INCLUDE_DIRECTORIES)
-foreach(noprefix ${IR_INCLUDES})
-  set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES} -I${noprefix})
-endforeach()
-
+# Set proper include directories to run CLANG from the Kudu's thirdparty
+# when generating the pre-compiled.ll target (a.k.a. IR_OUTPUT). To avoid
+# mixing third-party CLANG's headers with the headers of the C++ compiler
+# used to build the top-level project, one shouldn't rely on the cmake's
+# INCLUDE_DIRECTORIES property, but instead explicitly specify
+# only the required paths with the '-cxx-isystem', '-isystem', and '-I' flags.
+# Otherwise, the contaminated header file space might lead either to
+# compilation errors or inconsistencies in the built binaries
+# if the compilation succeeds. The latter, in its turn, might result
+# in undefined behavior of the LLVM-generated code.
 if (APPLE)
-  # The macOS keeps the libc++ headers in a non-standard location so
-  # that the thirdparty CLANG does not know about by default.
-  #
-  # Xcode starting with version 12.5 has the libc++ headers under
-  # Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk,
-  # which corresponds to CMAKE_OSX_SYSROOT, so it's enough to add -isysroot
-  # pointing to CMAKE_OSX_SYSROOT.
-  #
-  # Xcode prior to version 12.5 (12.4 and earlier, inclusive) doesn't have the
-  # libc++ headers under CMAKE_OSX_SYSROOT, but instead keeps those under
-  # Contents/Developer/Toolchains/XcodeDefault.xctoolchain: with that,
-  # it's easy to deduce the path to the libc++ headers from the output
-  # produced by `clang++ --version`.
-  #
-  # For non-clang compilers, assume the libc++ include directory provided
-  # with the Xcode command line tools.
-  if (NOT "${COMPILER_FAMILY}" STREQUAL "clang")
-    set(PREFIXED_IR_INCLUDES
-      ${PREFIXED_IR_INCLUDES}
-      -cxx-isystem "/Library/Developer/CommandLineTools/usr/include/c++/v1")
-  elseif (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 12.0.5)
-    execute_process(
-      COMMAND ${CMAKE_CXX_COMPILER} --version
-      COMMAND grep -E "^InstalledDir: "
-      COMMAND sed "s/^InstalledDir: \\(.*\\)$/\\1/"
-      RESULT_VARIABLE CXX_INSTALLED_DIR_CMD_EXIT_CODE
-      OUTPUT_VARIABLE CXX_INSTALLED_DIR_CMD_OUT
-      OUTPUT_STRIP_TRAILING_WHITESPACE)
-    if (${CXX_INSTALLED_DIR_CMD_EXIT_CODE} EQUAL 0 AND
-        NOT ${CXX_INSTALLED_DIR_CMD_OUT} STREQUAL "")
-      set(PREFIXED_IR_INCLUDES
-        ${PREFIXED_IR_INCLUDES}
-        -cxx-isystem "${CXX_INSTALLED_DIR_CMD_OUT}/../include/c++/v1")
-    else()
-      message(FATAL_ERROR "failed to deduce path to libc++ headers")
-    endif()
+  # On macOS, the built-in C++ header search directories in the 3rd-party CLANG
+  # disappear upon adding "-isystem ${CMAKE_OSX_SYSROOT}/usr/include" below,
+  # so let's add them back.
+  if (${KUDU_USE_TSAN})
+    # NOTE: TSAN isn't yet supported on macOS, but it's better to keep
+    #       paths consistent if that changes in the future.
+    set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+      -isystem "${THIRDPARTY_INSTALL_TSAN_DIR}/include/c++/v1")
   else()
-    set(PREFIXED_IR_INCLUDES
-      ${PREFIXED_IR_INCLUDES}
-      -isysroot "${CMAKE_OSX_SYSROOT}")
+    set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+      -isystem "${THIRDPARTY_INSTALL_UNINSTRUMENTED_DIR}/include/c++/v1")
   endif()
+
+  # A contemporary macOS does not keep its system header files under
+  # /usr/include anymore. It's necessary to explicitly point to proper
+  # location so that the CLANG from the Kudu's third-party can pick them up.
+  set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+    -isystem ${CMAKE_OSX_SYSROOT}/usr/include)
+endif()
+
+if (${KUDU_USE_TSAN})
+  set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+    -I${THIRDPARTY_INSTALL_TSAN_DIR}/include)
+else()
+  set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+    -I${THIRDPARTY_INSTALL_UNINSTRUMENTED_DIR}/include)
 endif()
+set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+  -I${THIRDPARTY_INSTALL_COMMON_DIR}/include)
+set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+  -I${CMAKE_SOURCE_DIR}/src)
+set(PREFIXED_IR_INCLUDES ${PREFIXED_IR_INCLUDES}
+  -I${CMAKE_BINARY_DIR}/src)
 
 # Get preprocessing definitions, which enable directives for glog and gtest.
 get_directory_property(IR_PP_DEFINITIONS
@@ -125,17 +118,6 @@ set(IR_FLAGS
   ${PREFIXED_IR_PP_DEFS}
   ${PREFIXED_IR_INCLUDES})
 
-# /usr/include doesn't have the necessary headers on macOS 10.14 (Mojave) with
-# Xcode 10[1]+. In this case the --sysroot has to be set to the output of 
"xcrun
-# --show-sdk-path" for the thirdparty clang-6.0 to work correctly.
-#
-# [1] 
https://developer.apple.com/documentation/xcode_release_notes/xcode_10_release_notes
-if (APPLE AND NOT EXISTS /usr/include/libc.h)
-  set(IR_FLAGS
-    ${IR_FLAGS}
-    --sysroot="${CMAKE_OSX_SYSROOT}")
-endif()
-
 # 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)

Reply via email to