diff --git a/.bazelrc b/.bazelrc index c76728149ae..fe4881519ae 100644 --- a/.bazelrc +++ b/.bazelrc @@ -244,6 +244,24 @@ build:asan --test_env=KJ_CLEAN_SHUTDOWN=1 build:asan --copt="-DV8_USE_ADDRESS_SANITIZER" build:asan --per_file_copt='external/.*v8@-DADDRESS_SANITIZER,-DLEAK_SANITIZER' +# thread sanitizer (https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual) +build:tsan --config=sanitizer-common +build:tsan --copt="-fsanitize=thread" --linkopt="-fsanitize=thread" +# A statically-linked unwinder in each shared library cannot propagate exceptions across DSOs. +build:tsan --linkopt="-l:libunwind.so.8" +# Rust's stable compiler cannot instrument the statically-linked Rust dependencies. Ignore accesses +# observed only through libc interceptors to avoid false positives from their synchronization. +build:tsan --action_env=TSAN_OPTIONS=halt_on_error=true:ignore_interceptors_accesses=true +build:tsan --test_env=TSAN_OPTIONS=halt_on_error=true:ignore_interceptors_accesses=true +# Clang-tidy loads this shared object into an unsanitized process, which cannot resolve TSan hooks. +build:tsan --per_file_copt='tools/clang-tidy@-fno-sanitize=thread' +# TSan significantly increases test runtime. +build:tsan --test_timeout=30,150,600,2400 +# Enable V8's TSan-specific generated code and synchronization annotations. V8's Bazel build does +# not currently derive these defines from the compiler's sanitizer flags. +build:tsan --copt="-DV8_IS_TSAN" +build:tsan --per_file_copt='external/.*v8@-DTHREAD_SANITIZER' + # fuzzilli (https://github.com/googleprojectzero/fuzzilli/) build:fuzzilli --config=asan build:fuzzilli --copt="-DWORKERD_FUZZILLI" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eadffc9393d..1a1c624ab38 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,9 +45,11 @@ jobs: { suffix: -debug }, ] include: - # Add an Address Sanitizer (ASAN) build on Linux for additional checking. + # Add sanitizer builds on Linux for additional checking. - os: { name: linux, arch: X64, image: ubuntu-22.04-16core } config: { suffix: -asan } + - os: { name: linux, arch: X64, image: ubuntu-22.04-16core } + config: { suffix: -tsan } # TODO (later): The custom Windows-debug configuration consistently runs out of disk # space on CI, disable it for now. Once https://github.com/bazelbuild/bazel/issues/21615 # has been resolved we can likely re-enable it and possibly fold up the custom diff --git a/build/ci.bazelrc b/build/ci.bazelrc index 2d17e05081d..3d7a6dd85ee 100644 --- a/build/ci.bazelrc +++ b/build/ci.bazelrc @@ -73,6 +73,8 @@ build:ci-linux-asan --config=ci-linux-common --config=ci-limit-storage build:ci-linux-asan --config=asan --copt="-g0" --strip=always build:ci-linux-arm-asan --config=ci-linux-asan +build:ci-linux-tsan --config=ci-linux-common --config=ci-limit-storage --config=tsan + # Build container tests on Linux CI build:ci-linux --build_tag_filters=-off-by-default build:ci-linux-arm --build_tag_filters=-off-by-default diff --git a/build/deps/gen/deps.MODULE.bazel b/build/deps/gen/deps.MODULE.bazel index b4965010eeb..5ee6b102a8e 100644 --- a/build/deps/gen/deps.MODULE.bazel +++ b/build/deps/gen/deps.MODULE.bazel @@ -27,10 +27,11 @@ bazel_dep(name = "brotli", version = "1.2.0.bcr.1") # capnp-cpp http.archive( name = "capnp-cpp", - sha256 = "6753378bd099029cb2830fecd32dd158218019e459ffd3c8e379cbf025906eb8", - strip_prefix = "capnproto-capnproto-a1cd1c4/c++", + patch_args = ["-p1"], + sha256 = "2af77499a11d545186635031ca00cd592d937d7822b7418cb98b6ddc23aa2ed0", + strip_prefix = "capnproto-capnproto-671a421/c++", type = "tgz", - url = "https://github.com/capnproto/capnproto/tarball/a1cd1c4b3d241b77478035a6ccad8b0fb587d444", + url = "https://github.com/capnproto/capnproto/tarball/671a421bf5f6eb0191781c63c03be8121377b5b8", ) use_repo(http, "capnp-cpp") diff --git a/build/deps/v8.MODULE.bazel b/build/deps/v8.MODULE.bazel index 18db5f3df94..9c4ac5a8fa7 100644 --- a/build/deps/v8.MODULE.bazel +++ b/build/deps/v8.MODULE.bazel @@ -61,6 +61,7 @@ PATCHES = [ "0036-Fix-ExtendedMap-layout-on-Windows.patch", "0037-Fix-CFunction-MemorySpan-declarations-on-Windows.patch", "0038-Properly-depend-on-llvm-libc.patch", + "0039-Fix-TSan-build-without-the-V8-sandbox.patch", ] http_archive( diff --git a/cfsetup.yaml b/cfsetup.yaml index 90e55098c6e..8167d495216 100644 --- a/cfsetup.yaml +++ b/cfsetup.yaml @@ -34,6 +34,15 @@ trixie: &default-build - *pre-bazel-write-gcp-creds - bazel test -k --config=ci --config=ci-test --config=ci-linux-asan //... --announce_rc --remote_cache=https://storage.googleapis.com/cloudflare-edgeworker-bazel-build-cache --google_credentials=/tmp/bazel_cache_gcp_creds.json --remote_local_fallback=True --remote_timeout=10 + ci-bazel-x64-tsan: + nosubmodule: true + base_image: *ci-image-bazel-amd64 + tmpfs_tmp: true + post-cache: + - *pre-bazel-install-deps + - *pre-bazel-write-gcp-creds + - bazel test -k --config=ci --config=ci-test --config=ci-linux-tsan //... --announce_rc --remote_cache=https://storage.googleapis.com/cloudflare-edgeworker-bazel-build-cache --google_credentials=/tmp/bazel_cache_gcp_creds.json --remote_local_fallback=True --remote_timeout=10 + ci-bazel-x64-lint: nosubmodule: true base_image: *ci-image-bazel-amd64 diff --git a/ci/build.yml b/ci/build.yml index 3cd90f51408..a9a96d3b0ef 100644 --- a/ci/build.yml +++ b/ci/build.yml @@ -44,6 +44,12 @@ include: jobPrefix: "linux-x64-asan" CFSETUP_TARGET: "ci-bazel-x64-asan" + - component: $CI_SERVER_FQDN/cloudflare/ci/cfsetup/build@~latest + inputs: + <<: *cfsetup-input-template + jobPrefix: "linux-x64-tsan" + CFSETUP_TARGET: "ci-bazel-x64-tsan" + - component: $CI_SERVER_FQDN/cloudflare/ci/cfsetup/build@~latest inputs: <<: *cfsetup-input-template @@ -104,6 +110,9 @@ linux-x64-build: linux-x64-asan-build: <<: *job-template +linux-x64-tsan-build: + <<: *job-template + linux-x64-release-build: <<: *job-template diff --git a/justfile b/justfile index 634343fd747..402d460cb4c 100644 --- a/justfile +++ b/justfile @@ -44,12 +44,18 @@ run *args="-- --help": build-asan *args="//...": just build {{args}} --config=asan +build-tsan *args="//...": + just build {{args}} --config=tsan + test *args="//...": bazel test {{args}} test-asan *args="//...": just test {{args}} --config=asan +test-tsan *args="//...": + just test {{args}} --config=tsan + # e.g. just stream-test //src/cloudflare:cloudflare.capnp@eslint stream-test *args: bazel test {{args}} --test_output=streamed --nocache_test_results --test_tag_filters= --test_size_filters= diff --git a/patches/v8/0039-Fix-TSan-build-without-the-V8-sandbox.patch b/patches/v8/0039-Fix-TSan-build-without-the-V8-sandbox.patch new file mode 100644 index 00000000000..69effbb346e --- /dev/null +++ b/patches/v8/0039-Fix-TSan-build-without-the-V8-sandbox.patch @@ -0,0 +1,44 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Cloudflare Workers Team +Date: Mon, 10 Aug 2026 18:00:00 +0000 +Subject: Fix TSan build without the V8 sandbox + +The TSan store helpers reference ExposedTrustedObject's indirect pointer +field, which only exists when the V8 sandbox is enabled. Workerd builds V8 +without the sandbox, so compile the indirect-pointer handling only when the +field is present. + +diff --git a/src/compiler/backend/x64/code-generator-x64.cc b/src/compiler/backend/x64/code-generator-x64.cc +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/compiler/backend/x64/code-generator-x64.cc ++++ b/src/compiler/backend/x64/code-generator-x64.cc +@@ -814,6 +814,7 @@ Register GetTSANValueRegister(MacroAssembler* masm, Register value, + masm->movq(value_reg, value); + masm->EncodeSandboxedPointer(value_reg); + return value_reg; ++#if V8_ENABLE_SANDBOX + } else if (rep == MachineRepresentation::kIndirectPointer) { + // Indirect pointer fields contain an index to a pointer table entry, which + // is obtained from the referenced object. +@@ -823,6 +824,7 @@ Register GetTSANValueRegister(MacroAssembler* masm, Register value, + FieldOperand(value, offsetof(ExposedTrustedObject, + self_indirect_pointer_))); + return value_reg; ++#endif // V8_ENABLE_SANDBOX + } + return value; + } +@@ -839,5 +841,6 @@ Register GetTSANValueRegister( + // SandboxedPointers need to be encoded. + masm->EncodeSandboxedPointer(value_reg); ++#if V8_ENABLE_SANDBOX + } else if (rep == MachineRepresentation::kIndirectPointer) { + // Indirect pointer fields contain an index to a pointer table entry, which + // is obtained from the referenced object. +@@ -848,5 +851,6 @@ Register GetTSANValueRegister( + FieldOperand(value_reg, offsetof(ExposedTrustedObject, + self_indirect_pointer_))); ++#endif // V8_ENABLE_SANDBOX + } + return value_reg; + } diff --git a/src/rust/cxx-integration-test/cxx-rust-integration-test.h b/src/rust/cxx-integration-test/cxx-rust-integration-test.h index 241a6cc8a43..091409b6d76 100644 --- a/src/rust/cxx-integration-test/cxx-rust-integration-test.h +++ b/src/rust/cxx-integration-test/cxx-rust-integration-test.h @@ -2,6 +2,10 @@ #include +#if (defined(__has_feature) && __has_feature(thread_sanitizer)) || defined(__SANITIZE_THREAD__) +#include +#endif + #define operatorCALL operator() namespace workerd::rust::test { diff --git a/src/rust/cxx/kj-rs/tests/lib.rs b/src/rust/cxx/kj-rs/tests/lib.rs index 9a9aa7ae39d..cf78700857c 100644 --- a/src/rust/cxx/kj-rs/tests/lib.rs +++ b/src/rust/cxx/kj-rs/tests/lib.rs @@ -80,6 +80,10 @@ pub mod ffi { fn get_data(&self) -> u64; #[cxx_name = "setData"] fn set_data(self: Pin<&mut OpaqueCxxClass>, val: u64); + #[cxx_name = "tsanReleaseOpaque"] + fn tsan_release_opaque(value: &OpaqueCxxClass); + #[cxx_name = "tsanAcquireOpaque"] + fn tsan_acquire_opaque(value: &OpaqueCxxClass); fn cxx_kj_own() -> KjOwn; fn null_kj_own() -> KjOwn; diff --git a/src/rust/cxx/kj-rs/tests/test-own.h b/src/rust/cxx/kj-rs/tests/test-own.h index a79705ebe7b..a15af533db1 100644 --- a/src/rust/cxx/kj-rs/tests/test-own.h +++ b/src/rust/cxx/kj-rs/tests/test-own.h @@ -2,6 +2,10 @@ #include +#if (defined(__has_feature) && __has_feature(thread_sanitizer)) || defined(__SANITIZE_THREAD__) +#include +#endif + #include #include @@ -24,6 +28,18 @@ class OpaqueCxxClass { uint64_t data; }; +inline void tsanReleaseOpaque(const OpaqueCxxClass& value) { +#if (defined(__has_feature) && __has_feature(thread_sanitizer)) || defined(__SANITIZE_THREAD__) + __tsan_release(const_cast(&value)); +#endif +} + +inline void tsanAcquireOpaque(const OpaqueCxxClass& value) { +#if (defined(__has_feature) && __has_feature(thread_sanitizer)) || defined(__SANITIZE_THREAD__) + __tsan_acquire(const_cast(&value)); +#endif +} + // Forward declaration for Rust function, including the lib.rs.h caused problems kj::Own modify_own_return(kj::Own cpp_own); // Rust function that takes in a cpp_own. Should cause C++ exception if the own is NULL diff --git a/src/rust/cxx/kj-rs/tests/test_own.rs b/src/rust/cxx/kj-rs/tests/test_own.rs index 61d0235b08c..37031ac4145 100644 --- a/src/rust/cxx/kj-rs/tests/test_own.rs +++ b/src/rust/cxx/kj-rs/tests/test_own.rs @@ -273,7 +273,9 @@ pub mod tests { let value = thread_id * items_per_thread + i; own.pin_mut().set_data(value); - // Send the Own across thread boundary + // Rust's standard library is not TSan-instrumented, so describe the channel + // handoff to TSan explicitly. + ffi::tsan_release_opaque(&own); tx_clone.send(own).unwrap(); } }); @@ -283,6 +285,7 @@ pub mod tests { // Collect all Owns from all threads let mut received_owns = Vec::new(); while let Ok(own) = rx.recv() { + ffi::tsan_acquire_opaque(&own); received_owns.push(own); } diff --git a/src/workerd/jsg/modules-new-test.c++ b/src/workerd/jsg/modules-new-test.c++ index 4049d72f902..ab3cf537355 100644 --- a/src/workerd/jsg/modules-new-test.c++ +++ b/src/workerd/jsg/modules-new-test.c++ @@ -2673,27 +2673,29 @@ KJ_TEST("Using a registry from multiple threads works") { kj::mv(registry), compilationObserver, successfulResolutions, kj::mv(paf.fulfiller))}; }; - auto [paf1, task1] = - makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); - kj::Thread(kj::mv(task1)).detach(); - auto [paf2, task2] = - makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); - kj::Thread(kj::mv(task2)).detach(); - auto [paf3, task3] = - makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); - kj::Thread(kj::mv(task3)).detach(); - auto [paf4, task4] = - makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); - kj::Thread(kj::mv(task4)).detach(); - auto [paf5, task5] = - makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); - kj::Thread(kj::mv(task5)).detach(); - - tasks.add(kj::mv(paf1)); - tasks.add(kj::mv(paf2)); - tasks.add(kj::mv(paf3)); - tasks.add(kj::mv(paf4)); - tasks.add(kj::mv(paf5)); + { + auto [paf1, task1] = + makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); + kj::Thread thread1(kj::mv(task1)); + auto [paf2, task2] = + makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); + kj::Thread thread2(kj::mv(task2)); + auto [paf3, task3] = + makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); + kj::Thread thread3(kj::mv(task3)); + auto [paf4, task4] = + makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); + kj::Thread thread4(kj::mv(task4)); + auto [paf5, task5] = + makeRunnableAndPromise(registry.addRef(), compilationObserver, successfulResolutions); + kj::Thread thread5(kj::mv(task5)); + + tasks.add(kj::mv(paf1)); + tasks.add(kj::mv(paf2)); + tasks.add(kj::mv(paf3)); + tasks.add(kj::mv(paf4)); + tasks.add(kj::mv(paf5)); + } tasks.onEmpty().wait(io.waitScope); KJ_IF_SOME(exception, errorHandler.error) { kj::throwRecoverableException(kj::mv(exception)); diff --git a/src/workerd/jsg/setup.c++ b/src/workerd/jsg/setup.c++ index de2d12bbeb5..1444b62b7a6 100644 --- a/src/workerd/jsg/setup.c++ +++ b/src/workerd/jsg/setup.c++ @@ -124,6 +124,9 @@ void V8System::init(kj::Own platformParam, v8::V8::SetDcheckErrorHandler(&v8DcheckError); v8::V8::SetFatalErrorHandler(&v8DcheckError); + // Sandbox allocation can fail when there is no current isolate, so also install the global OOM + // handler rather than relying only on Isolate::SetOOMErrorHandler(). + v8::V8::SetFatalMemoryErrorCallback(&IsolateBase::oomError); // Note that v8::V8::SetFlagsFromString() simply ignores flags it doesn't recognize, which means // typos don't generate any error. SetFlagsFromCommandLine() has the `remove_flags` option which @@ -421,11 +424,6 @@ IsolateBase::IsolateBase(V8System& system, ptr->SetFatalErrorHandler(&fatalError); ptr->SetOOMErrorHandler(&oomError); - // We also set the global OOM error handler. This is a bit of - // a hack: Later in the run the allocation of a sandbox may fail - // due to OOM. In that case we want our handler to be called - // even though there is no current isolate. - v8::V8::SetFatalMemoryErrorCallback(&oomError); ptr->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit); ptr->SetData(SET_DATA_ISOLATE_BASE, this); diff --git a/src/workerd/jsg/setup.h b/src/workerd/jsg/setup.h index aab2d9e99d8..f6969e24429 100644 --- a/src/workerd/jsg/setup.h +++ b/src/workerd/jsg/setup.h @@ -388,6 +388,7 @@ class IsolateBase { private: template friend class Isolate; + friend class V8System; static void buildEmbedderGraph(v8::Isolate* isolate, v8::EmbedderGraph* graph, void* data);