Allow building with Clang 13 (#99)

Changes that were necessary to build the third-party package with Clang 13: 
- libc++ and libc++abi now need to be built together because libc++abi needs access to libc++ headers
- For building libc++ and libc++abi under TSAN, a compiler wrapper hack turned out to be necessary to allow building shared objects that still have undefined TSAN-related symbols.

Other improvements:
- Better display of long CMake command lines (breaking CMake arguments that are themselves compiler or argument lists into separate lines).

.env

@@ -0,0 +1,3 @@
+# Used for VSCode.
+# See https://code.visualstudio.com/docs/python/environments.
+PYTHONPATH=python

.github/workflows/ci.yml

@@ -62,6 +62,13 @@ jobs:
               --toolchain=llvm12
               --expected-major-compiler-version=12
 
+          - name: centos7-x86_64-clang13
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_centos7:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13
+              --expected-major-compiler-version=13
+
           # ---------------------------------------------------------------------------------------
           # Ubuntu 18.04
           # ---------------------------------------------------------------------------------------
@@ -118,6 +125,8 @@ jobs:
               --devtoolset=9
               --expected-major-compiler-version=9
 
+          # Clang/LLVM 12
+
           - name: almalinux8-x86_64-clang12
             os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
             docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
@@ -164,6 +173,53 @@ jobs:
               --expected-major-compiler-version=12
               --lto=full
 
+          # Clang/LLVM 13
+          - name: almalinux8-x86_64-clang13
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13
+              --expected-major-compiler-version=13
+
+          - name: almalinux8-x86_64-clang13-thin-lto
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13
+              --expected-major-compiler-version=13
+              --lto=thin
+
+          - name: almalinux8-x86_64-clang13-full-lto
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13
+              --expected-major-compiler-version=13
+              --lto=full
+
+          - name: almalinux8-x86_64-clang13-linuxbrew
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13_linuxbrew
+              --expected-major-compiler-version=13
+
+          - name: almalinux8-x86_64-clang13-linuxbrew-thin-lto
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13_linuxbrew
+              --expected-major-compiler-version=13
+              --lto=thin
+
+          - name: almalinux8-x86_64-clang13-linuxbrew-full-lto
+            os: ubuntu-20.04  # Ubuntu 20.04 is for the top-level VM only. We use Docker in it.
+            docker_image: yugabyteci/yb_build_infra_almalinux8:v2021-08-27T03_10_19
+            build_thirdparty_args: >-
+              --toolchain=llvm13_linuxbrew
+              --expected-major-compiler-version=13
+              --lto=full
+
           # ---------------------------------------------------------------------------------------
           # macOS
           # ---------------------------------------------------------------------------------------

python/build_definitions/llvm1x_libcxx.py

@@ -90,11 +90,12 @@ def get_source_subdir_name(self) -> str:
 
     def get_additional_cmake_args(self, builder: BuilderInterface) -> List[str]:
         llvm_src_path = builder.fs_layout.get_source_path(self)
-        return [
+        args = [
             '-DLIBCXXABI_LIBCXX_PATH=%s' % os.path.join(llvm_src_path, 'libcxx'),
             '-DLIBCXXABI_USE_COMPILER_RT=ON',
             '-DLIBCXXABI_USE_LLVM_UNWINDER=ON',
         ]
+        return args
 
     def build(self, builder: BuilderInterface) -> None:
         super().build(builder)
@@ -125,3 +126,46 @@ def get_additional_cmake_args(self, builder: BuilderInterface) -> List[str]:
             '-DLIBCXX_CXX_ABI=libcxxabi',
             '-DLIBCXXABI_USE_LLVM_UNWINDER=ON',
         ]
+
+
+class LibCxxWithAbiDependency(Llvm1xLibCxxDependencyBase):
+    """
+    A combined dependency for libc++ and libc++abi.
+
+    Using the approach described at:
+
+    https://libcxx.llvm.org/BuildingLibcxx.html
+
+    Based on the following instructions:
+
+    $ git clone https://github.com/llvm/llvm-project.git
+    $ cd llvm-project
+    $ mkdir build
+    $ cmake -G Ninja -S runtimes -B build -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
+    $ ninja -C build cxx cxxabi unwind
+    $ ninja -C build check-cxx check-cxxabi check-unwind
+    $ ninja -C build install-cxx install-cxxabi install-unwind
+
+    Using this with LLVM/Clang 13 or later.
+    """
+
+    def __init__(self, version: str) -> None:
+        super(LibCxxWithAbiDependency, self).__init__(
+            name='llvm1x_libcxx_with_abi',
+            version=version)
+
+    def get_source_subdir_name(self) -> str:
+        return 'runtimes'
+
+    def get_additional_cmake_args(self, builder: BuilderInterface) -> List[str]:
+        return [
+            '-DLLVM_ENABLE_RUNTIMES=libcxx;libcxxabi',
+        ]
+
+    def get_compiler_wrapper_ld_flags_to_append(self, builder: 'BuilderInterface') -> List[str]:
+        extra_ld_flags = super().get_compiler_wrapper_ld_flags_to_append(builder)
+        if builder.build_type == BUILD_TYPE_TSAN:
+            # It is not clear why in Clang 13 this suddenly becomes necessary in order to avoid
+            # failing with undefined TSAN-related symbols while linking shared libraries.
+            extra_ld_flags.append('-Wl,--unresolved-symbols=ignore-all')
+        return extra_ld_flags

python/yugabyte_db_thirdparty/builder.py

@@ -35,7 +35,7 @@
     get_build_def_module,
 )
 from yugabyte_db_thirdparty.builder_helpers import PLACEHOLDER_RPATH, get_make_parallelism, \
-    get_rpath_flag, sanitize_flags_line_for_log, log_and_set_env_var_to_list
+    get_rpath_flag, log_and_set_env_var_to_list, format_cmake_args_for_log
 from yugabyte_db_thirdparty.builder_helpers import is_ninja_available
 from yugabyte_db_thirdparty.builder_interface import BuilderInterface
 from yugabyte_db_thirdparty.cmd_line_args import parse_cmd_line_args
@@ -74,6 +74,8 @@
 )
 from yugabyte_db_thirdparty.macos import get_min_supported_macos_version
 from yugabyte_db_thirdparty.linuxbrew import get_linuxbrew_dir, using_linuxbrew, set_linuxbrew_dir
+from yugabyte_db_thirdparty.constants import COMPILER_WRAPPER_LD_FLAGS_TO_APPEND_ENV_VAR_NAME
+
 
 ASAN_FLAGS = [
     '-fsanitize=address',
@@ -191,11 +193,6 @@ def parse_args(self) -> None:
             compiler_prefix = self.toolchain.toolchain_root
             single_compiler_type = self.toolchain.get_compiler_type()
             self.toolchain.write_url_and_path_files()
-            if single_compiler_type == 'clang' and using_linuxbrew():
-                log("Automatically enabling compiler wrapper for a Clang Linuxbrew-targeting build "
-                    "and configuring it to disallow headers from /usr/include.")
-                self.args.use_compiler_wrapper = True
-                os.environ['YB_DISALLOWED_INCLUDE_DIRS'] = '/usr/include'
         else:
             compiler_prefix = self.args.compiler_prefix
             single_compiler_type = self.args.single_compiler_type
@@ -209,6 +206,17 @@ def parse_args(self) -> None:
             use_ccache=self.args.use_ccache,
             expected_major_compiler_version=self.args.expected_major_compiler_version
         )
+        llvm_major_version: Optional[int] = self.compiler_choice.get_llvm_major_version()
+        if llvm_major_version:
+            if using_linuxbrew():
+                log("Automatically enabling compiler wrapper for a Clang Linuxbrew-targeting build")
+                log("Disallowing the use of headers in /usr/include")
+                os.environ['YB_DISALLOWED_INCLUDE_DIRS'] = '/usr/include'
+                self.args.use_compiler_wrapper = True
+            if llvm_major_version >= 13:
+                log("Automatically enabling compiler wrapper for Clang major version 13 or higher")
+                self.args.use_compiler_wrapper = True
+        self.compiler_choice.use_compiler_wrapper = self.args.use_compiler_wrapper
 
         self.lto_type = self.args.lto
 
@@ -258,18 +266,23 @@ def populate_dependencies(self) -> None:
                     llvm_version_str = self.toolchain.get_llvm_version_str()
                 else:
                     llvm_version_str = self.compiler_choice.get_llvm_version_str()
-                self.dependencies += [
-                    # New LLVM. We will keep supporting new LLVM versions here.
+
+                self.dependencies.append(
                     get_build_def_module('llvm1x_libunwind').Llvm1xLibUnwindDependency(
                         version=llvm_version_str
-                    ),
-                    get_build_def_module('llvm1x_libcxx').Llvm1xLibCxxAbiDependency(
-                        version=llvm_version_str
-                    ),
-                    get_build_def_module('llvm1x_libcxx').Llvm1xLibCxxDependency(
-                        version=llvm_version_str
-                    ),
-                ]
+                    ))
+                libcxx_dep_module = get_build_def_module('llvm1x_libcxx')
+                if llvm_major_version >= 13:
+                    self.dependencies.append(
+                        libcxx_dep_module.LibCxxWithAbiDependency(version=llvm_version_str))
+                else:
+                    # It is important that we build libc++abi first, and only then build libc++.
+                    self.dependencies += [
+                        libcxx_dep_module.Llvm1xLibCxxAbiDependency(version=llvm_version_str),
+                        libcxx_dep_module.Llvm1xLibCxxDependency(version=llvm_version_str),
+                    ]
+                self.additional_allowed_shared_lib_paths.add(
+                    get_clang_library_dir(self.compiler_choice.get_c_compiler()))
             else:
                 self.dependencies.append(get_build_def_module('libunwind').LibUnwindDependency())
 
@@ -589,8 +602,7 @@ def build_with_cmake(
         def do_build_with_cmake(additional_cmake_args: List[str] = []) -> None:
             final_cmake_args = args + additional_cmake_args
             log("CMake command line (one argument per line):\n%s" %
-                "\n".join([(" " * 4 + sanitize_flags_line_for_log(line))
-                           for line in final_cmake_args]))
+                format_cmake_args_for_log(final_cmake_args))
             cmake_configure_script_path = os.path.abspath('yb_build_with_cmake.sh')
 
             build_tool_cmd = [
@@ -786,6 +798,9 @@ def init_linux_clang1x_flags(self, dep: Dependency) -> None:
         Flags for Clang 10 and beyond. We are using LLVM-supplied libunwind and compiler-rt in this
         configuration.
         """
+        llvm_major_version = self.compiler_choice.get_llvm_major_version()
+        assert llvm_major_version is not None
+
         if not using_linuxbrew():
             # We don't build compiler-rt for Linuxbrew yet.
             # TODO: we can build compiler-rt here the same way we build other LLVM components,
@@ -823,35 +838,53 @@ def init_linux_clang1x_flags(self, dep: Dependency) -> None:
         # TODO mbautin: refactor to polymorphism
         is_libcxxabi = dep.name.endswith('_libcxxabi')
         is_libcxx = dep.name.endswith('_libcxx')
+
+        is_libcxx_with_abi = dep.name.endswith('_libcxx_with_abi')
+
         log("Dependency name: %s, is_libcxxabi: %s, is_libcxx: %s",
             dep.name, is_libcxxabi, is_libcxx)
 
         if self.build_type == BUILD_TYPE_ASAN:
             self.compiler_flags.append('-shared-libasan')
 
-            if is_libcxxabi:
+            if is_libcxxabi or is_libcxx_with_abi:
                 # To avoid an infinite loop in UBSAN.
                 # https://monorail-prod.appspot.com/p/chromium/issues/detail?id=609786
                 # This comment:
                 # https://gist.githubusercontent.com/mbautin/ad9ea4715669da3b3a5fb9495659c4a9/raw
                 self.compiler_flags.append('-fno-sanitize=vptr')
 
+                # Unfortunately, for the combined libc++ and libc++abi build in Clang 13 or later,
+                # we also disable this check in libc++, where in theory it could have been
+                # enabled.
+
+                # The description of this check from
+                # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html:
+                #
+                # -fsanitize=vptr: Use of an object whose vptr indicates that it is of the wrong
+                # dynamic type, or that its lifetime has not begun or has ended. Incompatible with
+                # -fno-rtti. Link must be performed by clang++, not clang, to make sure C++-specific
+                # parts of the runtime library and C++ standard libraries are present.
+
             assert self.compiler_choice.cc is not None
-            compiler_rt_lib_dir = get_clang_library_dir(self.compiler_choice.cc)
+            compiler_rt_lib_dir = get_clang_library_dir(self.compiler_choice.get_c_compiler())
             self.add_lib_dir_and_rpath(compiler_rt_lib_dir)
             ubsan_lib_name = f'clang_rt.ubsan_minimal-{platform.processor()}'
             ubsan_lib_so_path = os.path.join(compiler_rt_lib_dir, f'lib{ubsan_lib_name}.so')
             if not os.path.exists(ubsan_lib_so_path):
                 raise IOError(f"UBSAN library not found at {ubsan_lib_so_path}")
             self.ld_flags.append(f'-l{ubsan_lib_name}')
 
+        if self.build_type == BUILD_TYPE_TSAN and llvm_major_version >= 13:
+            self.executable_only_ld_flags.extend(['-fsanitize=thread'])
+
         self.ld_flags += ['-lunwind']
 
         libcxx_installed_include, libcxx_installed_lib = self.get_libcxx_dirs(self.build_type)
         log("libc++ include directory: %s", libcxx_installed_include)
         log("libc++ library directory: %s", libcxx_installed_lib)
 
-        if not is_libcxx and not is_libcxxabi:
+        if not is_libcxx and not is_libcxxabi and not is_libcxx_with_abi:
             log("Adding special compiler/linker flags for Clang 10+ for dependencies other than "
                 "libc++")
             self.ld_flags += ['-lc++', '-lc++abi']
@@ -871,7 +904,7 @@ def init_linux_clang1x_flags(self, dep: Dependency) -> None:
             # libc++ build needs to be able to find libc++abi library installed here.
             self.ld_flags.append('-L%s' % libcxx_installed_lib)
 
-        if is_libcxx or is_libcxxabi:
+        if is_libcxx or is_libcxxabi or is_libcxx_with_abi:
             log("Adding special linker flags for Clang 10 or newer for libc++ or libc++abi")
             # libc++abi needs to be able to find libcxx at runtime, even though it can't always find
             # it at build time because libc++abi is built first.
@@ -991,6 +1024,19 @@ def build_dependency(self, dep: Dependency, only_process_flags: bool = False) ->
         log_and_set_env_var_to_list(
             env_vars, 'CPPFLAGS', self.get_effective_preprocessor_flags(dep))
 
+        compiler_wrapper_extra_ld_flags = dep.get_compiler_wrapper_ld_flags_to_append(self)
+        if compiler_wrapper_extra_ld_flags:
+            if not self.compiler_choice.use_compiler_wrapper:
+                raise RuntimeError(
+                    "Need to add extra linker arguments in the compiler wrapper, but compiler "
+                    "wrapper is not being used: %s" % compiler_wrapper_extra_ld_flags)
+            log_and_set_env_var_to_list(
+                env_vars, COMPILER_WRAPPER_LD_FLAGS_TO_APPEND_ENV_VAR_NAME,
+                compiler_wrapper_extra_ld_flags)
+
+        for k, v in env_vars.items():
+            log("Setting environment variable %s to: %s" % (k, v))
+
         if self.build_type == BUILD_TYPE_ASAN:
             # To avoid errors similar to:
             # https://gist.githubusercontent.com/mbautin/4b8eec566f54bcc35706dcd97cab1a95/raw

python/yugabyte_db_thirdparty/builder_helpers.py

@@ -14,6 +14,8 @@
 
 import multiprocessing
 import os
+import re
+
 from typing import Dict, Optional, List
 
 from yugabyte_db_thirdparty.custom_logging import log
@@ -24,6 +26,8 @@
     "/tmp/making_sure_we_have_enough_room_to_set_rpath_later_{}_end_of_rpath".format('_' * 256))
 PLACEHOLDER_RPATH_FOR_LOG = '/tmp/long_placeholder_rpath'
 
+CMAKE_VAR_RE = re.compile(r'^(-D[A-Z_]+)=(.*)$')
+
 
 def get_make_parallelism() -> int:
     return int(os.environ.get('YB_MAKE_PARALLELISM', multiprocessing.cpu_count()))
@@ -62,3 +66,25 @@ def log_and_set_env_var_to_list(
         log('Unsetting env var %s', env_var_name)
         # When used with EnvVarContext, this will cause the environment variable to be unset.
         env_var_map[env_var_name] = None
+
+
+def format_cmake_args_for_log(args: List[str]) -> str:
+    lines = []
+    for arg in args:
+        match = CMAKE_VAR_RE.match(arg)
+        if match:
+            cmake_var_name = match.group(1)
+            cmake_var_value = match.group(2)
+            cmake_var_value_parts = cmake_var_value.split()
+            if len(cmake_var_value_parts) > 1:
+                lines.append('%s="%s' % (cmake_var_name, cmake_var_value_parts[0]))
+                current_indent = ' ' * (len(cmake_var_name) + 2)
+                for cmake_var_value_part in cmake_var_value_parts[1:-1]:
+                    lines.append(current_indent + cmake_var_value_part)
+                lines.append('%s%s"' % (current_indent, cmake_var_value_parts[-1]))
+                continue
+
+        lines.append(arg)
+
+    indent = " " * 4
+    return indent + "\n".join([(indent + sanitize_flags_line_for_log(line)) for line in lines])

python/yugabyte_db_thirdparty/clang_util.py

@@ -53,6 +53,9 @@ def get_clang_library_dir(clang_executable_path: str) -> str:
 
 
 def get_clang_include_dir(clang_executable_path: str) -> str:
+    """
+    Returns a directory such as lib/clang/13.0.1/include relative to the LLVM installation path.
+    """
     library_dirs = get_clang_library_dirs(clang_executable_path)
     for library_dir in library_dirs:
         include_dir = os.path.join(library_dir, 'include')