mach: Add TSAN support (#37195)

Add ThreadSanitizer support to mach (`--with-tsan`).

This refactors the current infrastructure to support `--with-asan` to a
more generic `Santiizer`.
If the need for adding multiple sanitizers at once (e.g. address + leak
sanitizer) arises, we can easily change the `SanitizerKind` enum to a
`enum.Flag` instead.

Testing: Manually run `./mach build --with-tsan` and `./mach run
--with-tsan` (on macos) and observe TSAN reporting data races.

Closes: #14869

---------

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>

python/servo/build_commands.py

@@ -17,7 +17,7 @@ import subprocess
 import sys
 
 from time import time
-from typing import Optional, List
+from typing import Optional, List, Dict
 
 import notifypy
 
@@ -37,6 +37,8 @@ from servo.command_base import BuildType, CommandBase, call, check_call
 from servo.gstreamer import windows_dlls, windows_plugins, package_gstreamer_dylibs
 from servo.platform.build_target import BuildTarget
 
+from python.servo.platform.build_target import SanitizerKind
+
 SUPPORTED_ASAN_TARGETS = [
     "aarch64-apple-darwin",
     "aarch64-unknown-linux-gnu",
@@ -45,6 +47,13 @@ SUPPORTED_ASAN_TARGETS = [
     "x86_64-unknown-linux-gnu",
 ]
 
+SUPPORTED_TSAN_TARGETS = [
+    "aarch64-apple-darwin",
+    "aarch64-unknown-linux-gnu",
+    "x86_64-apple-darwin",
+    "x86_64-unknown-linux-gnu",
+]
+
 
 def get_rustc_llvm_version() -> Optional[List[int]]:
     """Determine the LLVM version of `rustc` and return it as a List[major, minor, patch, ...]
@@ -91,7 +100,7 @@ class MachCommands(CommandBase):
         no_package=False,
         verbose=False,
         very_verbose=False,
-        with_asan=False,
+        sanitizer: SanitizerKind = SanitizerKind.NONE,
         flavor=None,
         **kwargs,
     ):
@@ -110,8 +119,7 @@ class MachCommands(CommandBase):
             opts += ["-v"]
         if very_verbose:
             opts += ["-vv"]
-        if with_asan:
-            self.config["build"]["with_asan"] = True
+        self.config["build"]["sanitizer"] = sanitizer
 
         env = self.build_env()
         self.ensure_bootstrapped()
@@ -120,58 +128,8 @@ class MachCommands(CommandBase):
         host = servo.platform.host_triple()
         target_triple = self.target.triple()
 
-        if with_asan:
-            if target_triple not in SUPPORTED_ASAN_TARGETS:
-                print(
-                    "AddressSanitizer is currently not supported on this platform\n",
-                    "See https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html",
-                )
-                sys.exit(1)
-
-            # do not use crown (clashes with different rust version)
-            env["RUSTC"] = "rustc"
-
-            # Enable usage of unstable rust flags
-            env["RUSTC_BOOTSTRAP"] = "1"
-
-            # Enable asan
-            env["RUSTFLAGS"] = env.get("RUSTFLAGS", "") + " -Zsanitizer=address"
-            opts += ["-Zbuild-std"]
-            kwargs["target_override"] = target_triple
-
-            # With asan we also want frame pointers
-            if "force-frame-pointers" not in env["RUSTFLAGS"]:
-                env["RUSTFLAGS"] += " -C force-frame-pointers=yes"
-
-            # Note: We want to use the same clang/LLVM version as rustc.
-            rustc_llvm_version = get_rustc_llvm_version()
-            if rustc_llvm_version is None:
-                raise RuntimeError("Unable to determine necessary clang version for ASAN support")
-            llvm_major: int = rustc_llvm_version[0]
-            target_clang = f"clang-{llvm_major}"
-            target_cxx = f"clang++-{llvm_major}"
-            if shutil.which(target_clang) is None or shutil.which(target_cxx) is None:
-                env.setdefault("TARGET_CC", target_clang)
-                env.setdefault("TARGET_CXX", target_cxx)
-            else:
-                # libasan can be compatible across multiple compiler versions and has a
-                # runtime check, which would fail if we used incompatible compilers, so
-                # we can try and fallback to the default clang.
-                env.setdefault("TARGET_CC", "clang")
-                env.setdefault("TARGET_CXX", "clang++")
-
-            # We need to use `TARGET_CFLAGS`, since we don't want to compile host dependencies with ASAN,
-            # since that causes issues when building build-scripts / proc macros.
-            env.setdefault("TARGET_CFLAGS", "")
-            env.setdefault("TARGET_CXXFLAGS", "")
-            env["TARGET_CFLAGS"] += " -fsanitize=address"
-            env["TARGET_CXXFLAGS"] += " -fsanitize=address"
-            # By default build mozjs from source to enable ASAN with mozjs.
-            env.setdefault("MOZJS_FROM_SOURCE", "1")
-
-            # asan replaces system allocator with asan allocator
-            # we need to make sure that we do not replace it with jemalloc
-            self.features.append("servo_allocator/use-system-allocator")
+        if sanitizer.is_some():
+            self.build_sanitizer_env(env, opts, kwargs, target_triple, sanitizer)
 
         build_start = time()
 
@@ -196,11 +154,11 @@ class MachCommands(CommandBase):
         status = self.run_cargo_build_like_command("rustc", opts, env=env, verbose=verbose, **kwargs)
 
         if status == 0:
-            built_binary = self.get_binary_path(build_type, asan=with_asan)
+            built_binary = self.get_binary_path(build_type, sanitizer=sanitizer)
 
             if not no_package and self.target.needs_packaging():
                 rv = Registrar.dispatch(
-                    "package", context=self.context, build_type=build_type, flavor=flavor, with_asan=with_asan
+                    "package", context=self.context, build_type=build_type, flavor=flavor, sanitizer=sanitizer
                 )
                 if rv:
                     return rv
@@ -258,6 +216,76 @@ class MachCommands(CommandBase):
         opts += params
         return check_call(["cargo", "clean"] + opts, env=self.build_env(), verbose=verbose)
 
+    def build_sanitizer_env(
+        self, env: Dict, opts: List[str], kwargs, target_triple, sanitizer: SanitizerKind = SanitizerKind.NONE
+    ):
+        if sanitizer.is_none():
+            return
+        # do not use crown (clashes with different rust version)
+        env["RUSTC"] = "rustc"
+        # Enable usage of unstable rust flags
+        env["RUSTC_BOOTSTRAP"] = "1"
+        # std library should also be instrumented
+        opts += ["-Zbuild-std"]
+        # We need to always set the target triple, even when building for host.
+        kwargs["target_override"] = target_triple
+        # When sanitizers are used we also want framepointers to help with backtraces.
+        if "force-frame-pointers" not in env["RUSTFLAGS"]:
+            env["RUSTFLAGS"] += " -C force-frame-pointers=yes"
+
+        # Note: We want to use the same clang/LLVM version as rustc.
+        rustc_llvm_version = get_rustc_llvm_version()
+        if rustc_llvm_version is None:
+            raise RuntimeError("Unable to determine necessary clang version for Sanitizer support")
+        llvm_major: int = rustc_llvm_version[0]
+        target_clang = f"clang-{llvm_major}"
+        target_cxx = f"clang++-{llvm_major}"
+        if shutil.which(target_clang) is None or shutil.which(target_cxx) is None:
+            env.setdefault("TARGET_CC", "clang")
+            env.setdefault("TARGET_CXX", "clang++")
+        else:
+            # libasan can be compatible across multiple compiler versions and has a
+            # runtime check, which would fail if we used incompatible compilers, so
+            # we can try and fallback to the default clang.
+            env.setdefault("TARGET_CC", target_clang)
+            env.setdefault("TARGET_CXX", target_cxx)
+        # By default, build mozjs from source to enable Sanitizers in mozjs.
+        env.setdefault("MOZJS_FROM_SOURCE", "1")
+
+        # We need to use `TARGET_CFLAGS`, since we don't want to compile host dependencies with ASAN,
+        # since that causes issues when building build-scripts / proc macros.
+        # The actual flags will be appended below depending on the sanitizer kind.
+        env.setdefault("TARGET_CFLAGS", "")
+        env.setdefault("TARGET_CXXFLAGS", "")
+        env.setdefault("RUSTFLAGS", "")
+
+        if sanitizer.is_asan():
+            if target_triple not in SUPPORTED_ASAN_TARGETS:
+                print(
+                    "AddressSanitizer is currently not supported on this platform\n",
+                    "See https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html",
+                )
+                sys.exit(1)
+
+            # Enable asan
+            env["RUSTFLAGS"] += " -Zsanitizer=address"
+            env["TARGET_CFLAGS"] += " -fsanitize=address"
+            env["TARGET_CXXFLAGS"] += " -fsanitize=address"
+
+            # asan replaces system allocator with asan allocator
+            # we need to make sure that we do not replace it with jemalloc
+            self.features.append("servo_allocator/use-system-allocator")
+        elif sanitizer.is_tsan():
+            if target_triple not in SUPPORTED_TSAN_TARGETS:
+                print(
+                    "ThreadSanitizer is currently not supported on this platform\n",
+                    "See https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html",
+                )
+                sys.exit(1)
+            env["RUSTFLAGS"] += " -Zsanitizer=thread"
+            env["TARGET_CFLAGS"] += " -fsanitize=thread"
+            env["TARGET_CXXFLAGS"] += " -fsanitize=thread"
+
     def notify(self, title: str, message: str):
         """Generate desktop notification when build is complete and the
         elapsed build time was longer than 30 seconds.

python/servo/command_base.py

@@ -43,6 +43,8 @@ from servo.util import download_file, get_default_cache_dir
 import servo.platform
 import servo.util as util
 
+from python.servo.platform.build_target import SanitizerKind
+
 NIGHTLY_REPOSITORY_URL = "https://servo-builds2.s3.amazonaws.com/"
 ASAN_LEAK_SUPPRESSION_FILE = "support/suppressed_leaks_for_asan.txt"
 
@@ -305,7 +307,7 @@ class CommandBase(object):
         self.config["build"].setdefault("incremental", None)
         self.config["build"].setdefault("webgl-backtrace", False)
         self.config["build"].setdefault("dom-backtrace", False)
-        self.config["build"].setdefault("with_asan", False)
+        self.config["build"].setdefault("sanitizer", SanitizerKind.NONE)
 
         self.config.setdefault("android", {})
         self.config["android"].setdefault("sdk", "")
@@ -328,9 +330,9 @@ class CommandBase(object):
     def get_top_dir(self):
         return self.context.topdir
 
-    def get_binary_path(self, build_type: BuildType, asan: bool = False):
+    def get_binary_path(self, build_type: BuildType, sanitizer: SanitizerKind = SanitizerKind.NONE):
         base_path = util.get_target_dir()
-        if asan or self.target.is_cross_build():
+        if sanitizer.is_some() or self.target.is_cross_build():
             base_path = path.join(base_path, self.target.triple())
         binary_name = self.target.binary_name()
         binary_path = path.join(base_path, build_type.directory_name(), binary_name)
@@ -537,7 +539,24 @@ class CommandBase(object):
                     help="Build in release mode without debug assertions",
                 ),
                 CommandArgument("--profile", group="Build Type", help="Build with custom Cargo profile"),
-                CommandArgument("--with-asan", action="store_true", help="Build with AddressSanitizer"),
+                CommandArgumentGroup("Sanitizer"),
+                CommandArgument(
+                    "--with-asan",
+                    group="Sanitizer",
+                    dest="sanitizer",
+                    action="store_const",
+                    const=SanitizerKind.ASAN,
+                    default=SanitizerKind.NONE,
+                    help="Build with AddressSanitizer",
+                ),
+                CommandArgument(
+                    "--with-tsan",
+                    group="Sanitizer",
+                    dest="sanitizer",
+                    action="store_const",
+                    const=SanitizerKind.TSAN,
+                    help="Build with ThreadSanitizer",
+                ),
             ]
 
         if build_configuration:
@@ -649,13 +668,13 @@ class CommandBase(object):
                         kwargs["servo_binary"] = (
                             kwargs.get("bin")
                             or self.get_nightly_binary_path(kwargs.get("nightly"))
-                            or self.get_binary_path(kwargs.get("build_type"), asan=kwargs.get("with_asan"))
+                            or self.get_binary_path(kwargs.get("build_type"), sanitizer=kwargs.get("sanitizer"))
                         )
                     kwargs.pop("bin")
                     kwargs.pop("nightly")
                     if not build_type:
                         kwargs.pop("build_type")
-                        kwargs.pop("with_asan")
+                        kwargs.pop("sanitizer")
 
                 return original_function(self, *args, **kwargs)
 

python/servo/gstreamer.py

@@ -237,7 +237,7 @@ def find_non_system_dependencies_with_otool(binary_path: str) -> Set[str]:
 
         # No need to do any processing for system libraries. They should be
         # present on all macOS systems.
-        if not is_macos_system_library(dependency):
+        if not (is_macos_system_library(dependency) or "librustc-stable_rt" in dependency):
             output.add(dependency)
     return output
 

python/servo/package_commands.py

@@ -41,6 +41,8 @@ from servo.command_base import (
 )
 from servo.util import delete, get_target_dir
 
+from python.servo.platform.build_target import SanitizerKind
+
 PACKAGES = {
     "android": [
         "android/aarch64-linux-android/release/servoapp.apk",
@@ -108,9 +110,9 @@ class PackageCommands(CommandBase):
     @CommandArgument("--target", "-t", default=None, help="Package for given target platform")
     @CommandBase.common_command_arguments(build_configuration=False, build_type=True, package_configuration=True)
     @CommandBase.allow_target_configuration
-    def package(self, build_type: BuildType, flavor=None, with_asan=False):
+    def package(self, build_type: BuildType, flavor=None, sanitizer: SanitizerKind = SanitizerKind.NONE):
         env = self.build_env()
-        binary_path = self.get_binary_path(build_type, asan=with_asan)
+        binary_path = self.get_binary_path(build_type, sanitizer=sanitizer)
         dir_to_root = self.get_top_dir()
         target_dir = path.dirname(binary_path)
         if self.is_android():
@@ -184,8 +186,10 @@ class PackageCommands(CommandBase):
                 "-p",
                 f"buildMode={build_mode}",
             ]
-            if with_asan:
+            if sanitizer.is_asan():
                 hvigor_command.extend(["-p", "ohos-debug-asan=true"])
+            elif sanitizer.is_tsan():
+                hvigor_command.extend(["-p", "ohos-enable-tsan=true"])
 
             # Detect if PATH already has hvigor, or else fallback to npm installation
             # provided via HVIGOR_PATH
@@ -391,17 +395,24 @@ class PackageCommands(CommandBase):
     @CommandArgument("--target", "-t", default=None, help="Install the given target platform")
     @CommandBase.common_command_arguments(build_configuration=False, build_type=True, package_configuration=True)
     @CommandBase.allow_target_configuration
-    def install(self, build_type: BuildType, emulator=False, usb=False, with_asan=False, flavor=None):
+    def install(
+        self,
+        build_type: BuildType,
+        emulator=False,
+        usb=False,
+        sanitizer: SanitizerKind = SanitizerKind.NONE,
+        flavor=None,
+    ):
         env = self.build_env()
         try:
-            binary_path = self.get_binary_path(build_type, asan=with_asan)
+            binary_path = self.get_binary_path(build_type, sanitizer=sanitizer)
         except BuildNotFound:
             print("Servo build not found. Building servo...")
             result = Registrar.dispatch("build", context=self.context, build_type=build_type, flavor=flavor)
             if result:
                 return result
             try:
-                binary_path = self.get_binary_path(build_type, asan=with_asan)
+                binary_path = self.get_binary_path(build_type, sanitizer=sanitizer)
             except BuildNotFound:
                 print("Rebuilding Servo did not solve the missing build problem.")
                 return 1

python/servo/platform/build_target.py

@@ -15,6 +15,7 @@ import platform
 import shutil
 import subprocess
 import sys
+from enum import Enum
 
 from os import path
 from packaging.version import parse as parse_version
@@ -24,6 +25,28 @@ import servo.platform
 import servo.util as util
 
 
+class SanitizerKind(Enum):
+    NONE = 0
+    ASAN = 1
+    TSAN = 2
+
+    # Apparently enums don't always compare across modules, so we define
+    # helper methods.
+    def is_asan(self) -> bool:
+        return self is self.ASAN
+
+    def is_tsan(self) -> bool:
+        return self is self.TSAN
+
+    # Returns true if no sanitizer is enabled.
+    def is_none(self) -> bool:
+        return self is self.NONE
+
+    # Returns true if a sanitizer is enabled.
+    def is_some(self) -> bool:
+        return not self.is_none()
+
+
 class BuildTarget(object):
     def __init__(self, target_triple: str):
         self.target_triple = target_triple
@@ -397,8 +420,9 @@ class OpenHarmonyTarget(CrossBuildTarget):
         bindgen_extra_clangs_args = bindgen_extra_clangs_args + " " + ohos_cflags_str
         env[bindgen_extra_clangs_args_var] = bindgen_extra_clangs_args
 
-        # On OpenHarmony we add some additional flags when asan is enabled
-        if config["build"]["with_asan"]:
+        sanitizer: SanitizerKind = config["build"]["sanitizer"]
+        san_compile_flags = []
+        if sanitizer.is_some():
             # Lookup `<sdk>/native/llvm/lib/clang/15.0.4/lib/aarch64-linux-ohos/libclang_rt.asan.so`
             lib_clang = llvm_toolchain.joinpath("lib", "clang")
             children = [f.path for f in os.scandir(lib_clang) if f.is_dir()]
@@ -406,6 +430,12 @@ class OpenHarmonyTarget(CrossBuildTarget):
                 raise RuntimeError(f"Expected exactly 1 libclang version: `{children}`")
             lib_clang_version_dir = pathlib.Path(children[0])
             libclang_arch = lib_clang_version_dir.joinpath("lib", clang_target_triple).resolve()
+            # Use the clangrt from the NDK to use the same library for both C++ and Rust.
+            env["RUSTFLAGS"] += " -Zexternal-clangrt"
+            san_compile_flags.append("-fno-omit-frame-pointer")
+
+        # On OpenHarmony we add some additional flags when asan is enabled
+        if sanitizer.is_asan():
             libasan_so_path = libclang_arch.joinpath("libclang_rt.asan.so")
             libasan_preinit_path = libclang_arch.joinpath("libclang_rt.asan-preinit.a")
             if not libasan_so_path.exists():
@@ -422,23 +452,26 @@ class OpenHarmonyTarget(CrossBuildTarget):
                 ]
             )
 
-            # Use the clangrt from the NDK to use the same library for both C++ and Rust.
-            env["RUSTFLAGS"] += " -Zexternal-clangrt"
-
-            asan_compile_flags = (
-                " -fsanitize=address -shared-libasan -fno-omit-frame-pointer -fsanitize-recover=address"
-            )
+            san_compile_flags.extend(["-fsanitize=address", "-shared-libasan", "-fsanitize-recover=address"])
 
             arch_asan_ignore_list = lib_clang_version_dir.joinpath("share", "asan_ignorelist.txt")
             if arch_asan_ignore_list.exists():
-                asan_compile_flags += " -fsanitize-system-ignorelist=" + str(arch_asan_ignore_list)
+                san_compile_flags.append("-fsanitize-system-ignorelist=" + str(arch_asan_ignore_list))
             else:
                 print(f"Warning: Couldn't find system ASAN ignorelist at `{arch_asan_ignore_list}`")
-            env["TARGET_CFLAGS"] += asan_compile_flags
-            env["TARGET_CXXFLAGS"] += asan_compile_flags
+        elif sanitizer.is_tsan():
+            libtsan_so_path = libclang_arch.joinpath("libclang_rt.tsan.so")
+            builtins_path = libclang_arch.joinpath("libclang_rt.builtins.a")
+
+            link_args.extend(
+                ["-fsanitize=thread", "--rtlib=compiler-rt", "-shared-libsan", str(libtsan_so_path), str(builtins_path)]
+            )
+            san_compile_flags.append("-shared-libsan")
 
         link_args = [f"-Clink-arg={arg}" for arg in link_args]
         env["RUSTFLAGS"] += " " + " ".join(link_args)
+        env["TARGET_CFLAGS"] += " " + " ".join(san_compile_flags)
+        env["TARGET_CXXFLAGS"] += " " + " ".join(san_compile_flags)
 
     def binary_name(self) -> str:
         return "libservoshell.so"