Merge 1f3e3af432 into c94605b13e

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,9 +37,12 @@ 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",
+    "aarch64-unknown-linux-ohos",
     "x86_64-apple-darwin",
     "x86_64-unknown-linux-gnu",
 ]
@@ -90,7 +93,7 @@ class MachCommands(CommandBase):
         no_package=False,
         verbose=False,
         very_verbose=False,
-        with_asan=False,
+        sanitizer: SanitizerKind = SanitizerKind.NONE,
         flavor=None,
         **kwargs,
     ):
@@ -109,6 +112,8 @@ class MachCommands(CommandBase):
             opts += ["-v"]
         if very_verbose:
             opts += ["-vv"]
+        self.config["build"]["sanitizer"] = sanitizer
+        assert sanitizer.is_tsan()
 
         env = self.build_env()
         self.ensure_bootstrapped()
@@ -117,52 +122,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
-
-            # 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:
-                raise RuntimeError(f"--with-asan requires `{target_clang}` and `{target_cxx}` to be in PATH")
-            env.setdefault("TARGET_CC", target_clang)
-            env.setdefault("TARGET_CXX", target_cxx)
-            # TODO: We should also parse the LLVM version from the clang compiler we chose.
-            # It's unclear if the major version being the same is sufficient.
-
-            # 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"
-            env["TARGET_LDFLAGS"] = "-static-libasan"
-            # 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 not None:
+            self.build_sanitizer_env(env, opts, kwargs, target_triple, sanitizer)
 
         build_start = time()
 
@@ -187,10 +148,12 @@ 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)
+                rv = Registrar.dispatch(
+                    "package", context=self.context, build_type=build_type, flavor=flavor, sanitizer=sanitizer
+                )
                 if rv:
                     return rv
 
@@ -247,6 +210,72 @@ 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: Optional[SanitizerKind] = None
+    ):
+        if sanitizer is None:
+            return None
+        # 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 sanitizer.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 SanitizerKind.TSAN:
+            env["RUSTFLAGS"] += " -Zsanitizer=thread"
+            env["TARGET_CFLAGS"] += " -fsanitize=thread"
+            env["TARGET_CXXFLAGS"] += " -fsanitize=thread"
+
+        return None
+
     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,6 +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("sanitizer", SanitizerKind.NONE)
 
         self.config.setdefault("android", {})
         self.config["android"].setdefault("sdk", "")
@@ -327,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=None):
         base_path = util.get_target_dir()
-        if asan or self.target.is_cross_build():
+        if sanitizer is not None 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)
@@ -536,7 +539,23 @@ 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,
+                    help="Build with AddressSanitizer",
+                ),
+                CommandArgument(
+                    "--with-tsan",
+                    group="Sanitizer",
+                    dest="sanitizer",
+                    action="store_const",
+                    const=SanitizerKind.TSAN,
+                    help="Build with ThreadSanitizer",
+                ),
             ]
 
         if build_configuration:
@@ -648,13 +667,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)
 
@@ -803,15 +822,16 @@ class CommandBase(object):
                 "--manifest-path",
                 path.join(self.context.topdir, "ports", "servoshell", "Cargo.toml"),
             ]
-        if target_override:
-            args += ["--target", target_override]
-        elif self.target.is_cross_build():
+
+        if self.target.is_cross_build():
             args += ["--target", self.target.triple()]
             if type(self.target) in [AndroidTarget, OpenHarmonyTarget]:
                 # Note: in practice `cargo rustc` should just be used unconditionally.
                 assert command != "build", "For Android / OpenHarmony `cargo rustc` must be used instead of cargo build"
                 if command == "rustc":
                     args += ["--lib", "--crate-type=cdylib"]
+        elif target_override:
+            args += ["--target", target_override]
 
         features = []
 

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

@@ -10,7 +10,7 @@
 from datetime import datetime
 import random
 import time
-from typing import List
+from typing import List, Optional
 from github import Github
 
 import hashlib
@@ -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: Optional[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,6 +186,11 @@ class PackageCommands(CommandBase):
                 "-p",
                 f"buildMode={build_mode}",
             ]
+            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
             if "HVIGOR_PATH" not in env:
@@ -388,17 +395,19 @@ 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: Optional[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,24 @@ 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 a sanitizer is enabled.
+    def is_some(self) -> bool:
+        return self is not self.NONE
+
+
 class BuildTarget(object):
     def __init__(self, target_triple: str):
         self.target_triple = target_triple
@@ -350,9 +369,8 @@ class OpenHarmonyTarget(CrossBuildTarget):
         env[f"CXX_{clang_target_triple_underscore}"] = ndk_clangxx
         # rustc linker
         env[f"CARGO_TARGET_{rust_target_triple.upper()}_LINKER"] = ndk_clang
-        # We could also use a cross-compile wrapper
-        env["RUSTFLAGS"] += f" -Clink-arg=--target={clang_target_triple}"
-        env["RUSTFLAGS"] += f" -Clink-arg=--sysroot={ohos_sysroot_posix}"
+
+        link_args = ["-fuse-ld=lld", f"--target={clang_target_triple}", f"--sysroot={ohos_sysroot_posix}"]
 
         env["HOST_CFLAGS"] = ""
         env["HOST_CXXFLAGS"] = ""
@@ -398,6 +416,65 @@ class OpenHarmonyTarget(CrossBuildTarget):
         bindgen_extra_clangs_args = bindgen_extra_clangs_args + " " + ohos_cflags_str
         env[bindgen_extra_clangs_args_var] = bindgen_extra_clangs_args
 
+        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()]
+            if len(children) != 1:
+                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():
+                raise RuntimeError(f"Couldn't find ASAN runtime library at {libasan_so_path}")
+            link_args.extend(
+                [
+                    "-fsanitize=address",
+                    "--rtlib=compiler-rt",
+                    "-shared-libasan",
+                    str(libasan_so_path),
+                    "-Wl,--whole-archive",
+                    "-Wl," + str(libasan_preinit_path),
+                    "-Wl,--no-whole-archive",
+                ]
+            )
+
+            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():
+                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}`")
+        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"