From 224e849e8d4b0da92cb99e4a78d130ad9b88dea2 Mon Sep 17 00:00:00 2001 From: xingbowang Date: Thu, 7 May 2026 16:40:56 -0700 Subject: [PATCH] env: suppress liburing TSAN false positives (#14710) Summary: - Use the liburing TSAN suppressions in `tools/tsan_suppressions.txt` instead of defining the process-wide `__tsan_default_suppressions()` hook, avoiding conflicts with downstream applications. - Wire RocksDB TSAN make and crash-test flows to use that suppressions file by default without overriding caller-provided `TSAN_OPTIONS`. - Cover direct `db_crashtest.py` launches by passing the default suppressions to `db_stress` subprocesses. - Fix the GCC 16 unity-build warning in `CacheItemHelper` by directly initializing the no-secondary-cache helper fields instead of delegating with `this`. Imported from D101303486. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14710 Test Plan: - `COMPILE_WITH_TSAN=1 make -j128 env_test` - `timeout 60s ./env_test --gtest_filter=EnvPosixTest.IOUringAddressReuseNoTsanFalsePositive` - `python3 tools/db_crashtest_test.py` - `python3 -m py_compile tools/db_crashtest.py tools/db_crashtest_test.py` - CI: `build-linux-unity-and-headers` passed after the `CacheItemHelper` fix Reviewed By: hx235 Differential Revision: D104103800 Pulled By: xingbowang fbshipit-source-id: 6066d9abe02a3c44d75f9ce449889468c927ce56 --- Makefile | 4 ++++ crash_test.mk | 7 +++++++ tools/db_crashtest.py | 20 ++++++++++++++++++-- tools/db_crashtest_test.py | 29 +++++++++++++++++++++++++++++ tools/tsan_suppressions.txt | 20 ++++++++++++++++++++ 5 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 tools/tsan_suppressions.txt diff --git a/Makefile b/Makefile index f4a07cbf3d..76f39f4cbb 100644 --- a/Makefile +++ b/Makefile @@ -384,6 +384,10 @@ endif # TSAN doesn't work well with jemalloc. If we're compiling with TSAN, we should use regular malloc. ifdef COMPILE_WITH_TSAN DISABLE_JEMALLOC=1 + # Use a suppressions file instead of the process-wide TSAN default + # suppressions hook, which belongs to the final application. + TSAN_OPTIONS?=suppressions=$(CURDIR)/tools/tsan_suppressions.txt + export TSAN_OPTIONS EXEC_LDFLAGS += -fsanitize=thread PLATFORM_CCFLAGS += -fsanitize=thread -fPIC -DFOLLY_SANITIZE_THREAD PLATFORM_CXXFLAGS += -fsanitize=thread -fPIC -DFOLLY_SANITIZE_THREAD diff --git a/crash_test.mk b/crash_test.mk index 02e15a862a..07bf2ff66a 100644 --- a/crash_test.mk +++ b/crash_test.mk @@ -7,6 +7,13 @@ DB_STRESS_CMD?=./db_stress include common.mk +ifdef COMPILE_WITH_TSAN + # Keep direct `make -f crash_test.mk COMPILE_WITH_TSAN=1 ...` runs + # aligned with the main Makefile's TSAN runtime options. + TSAN_OPTIONS?=suppressions=$(CURDIR)/tools/tsan_suppressions.txt + export TSAN_OPTIONS +endif + CRASHTEST_MAKE=$(MAKE) -f crash_test.mk CRASHTEST_PY=$(PYTHON) -u tools/db_crashtest.py --stress_cmd=$(DB_STRESS_CMD) --cleanup_cmd='$(DB_CLEANUP_CMD)' --destroy_db_initially=1 diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index d014b04a31..c694173d00 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -30,6 +30,10 @@ _NO_SPACE_SUBSTRINGS = ( "enospc", ) _OUTPUT_PATH_RE = re.compile(r"(/[^\s]+)") +_TSAN_OPTIONS_ENV_VAR = "TSAN_OPTIONS" +_TSAN_SUPPRESSIONS_FILE = os.path.abspath( + os.path.join(os.path.dirname(__file__), "tsan_suppressions.txt") +) def get_random_seed(override): @@ -51,6 +55,16 @@ def quote_arg_for_display(arg): return f"{flag}={shlex.quote(value)}" +def stress_cmd_env(): + env = os.environ.copy() + if ( + _TSAN_OPTIONS_ENV_VAR not in env + and os.path.exists(_TSAN_SUPPRESSIONS_FILE) + ): + env[_TSAN_OPTIONS_ENV_VAR] = "suppressions=" + _TSAN_SUPPRESSIONS_FILE + return env + + def early_argument_parsing_before_main(): parser = argparse.ArgumentParser() parser.add_argument( @@ -1905,7 +1919,9 @@ def diagnostic_paths(finalized_params): def execute_cmd(cmd, timeout=None, timeout_pstack=False): - child = subprocess.Popen(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) + child = subprocess.Popen( + cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE, env=stress_cmd_env() + ) print( "Running db_stress with pid=%d: %s\n\n" % (child.pid, " ".join(quote_arg_for_display(arg) for arg in cmd)) @@ -2006,7 +2022,7 @@ def cleanup_after_success(dbname): if parts[0] in ["--env_uri", "--fs_uri"]: cleanup_cmd_parts.append(arg) print("Running DB cleanup command - %s\n" % " ".join(cleanup_cmd_parts)) - ret = subprocess.call(cleanup_cmd_parts) + ret = subprocess.call(cleanup_cmd_parts, env=stress_cmd_env()) if ret != 0: print("ERROR: DB cleanup returned error %d\n" % ret) sys.exit(2) diff --git a/tools/db_crashtest_test.py b/tools/db_crashtest_test.py index 4c165257a4..1a1a37ae4f 100644 --- a/tools/db_crashtest_test.py +++ b/tools/db_crashtest_test.py @@ -16,6 +16,7 @@ import unittest _DB_CRASHTEST_PATH = os.path.join(os.path.dirname(__file__), "db_crashtest.py") _TEST_DIR_ENV_VAR = "TEST_TMPDIR" _TEST_EXPECTED_DIR_ENV_VAR = "TEST_TMPDIR_EXPECTED" +_TSAN_OPTIONS_ENV_VAR = "TSAN_OPTIONS" def load_db_crashtest_module(): @@ -40,6 +41,7 @@ class DBCrashTestTest(unittest.TestCase): ) self.old_test_tmpdir = os.environ.get(_TEST_DIR_ENV_VAR) self.old_test_expected_tmpdir = os.environ.get(_TEST_EXPECTED_DIR_ENV_VAR) + self.old_tsan_options = os.environ.get(_TSAN_OPTIONS_ENV_VAR) os.environ[_TEST_DIR_ENV_VAR] = self.test_tmpdir os.environ.pop(_TEST_EXPECTED_DIR_ENV_VAR, None) @@ -54,6 +56,11 @@ class DBCrashTestTest(unittest.TestCase): else: os.environ[_TEST_EXPECTED_DIR_ENV_VAR] = self.old_test_expected_tmpdir + if self.old_tsan_options is None: + os.environ.pop(_TSAN_OPTIONS_ENV_VAR, None) + else: + os.environ[_TSAN_OPTIONS_ENV_VAR] = self.old_tsan_options + shutil.rmtree(self.test_tmpdir) def load_db_crashtest(self): @@ -66,6 +73,28 @@ class DBCrashTestTest(unittest.TestCase): params.update(overrides) return params + def test_stress_cmd_env_defaults_tsan_suppressions(self): + os.environ.pop(_TSAN_OPTIONS_ENV_VAR, None) + db_crashtest = self.load_db_crashtest() + + env = db_crashtest.stress_cmd_env() + + self.assertEqual( + "suppressions=" + + os.path.abspath( + os.path.join(os.path.dirname(__file__), "tsan_suppressions.txt") + ), + env[_TSAN_OPTIONS_ENV_VAR], + ) + + def test_stress_cmd_env_preserves_tsan_options(self): + os.environ[_TSAN_OPTIONS_ENV_VAR] = "halt_on_error=1" + db_crashtest = self.load_db_crashtest() + + env = db_crashtest.stress_cmd_env() + + self.assertEqual("halt_on_error=1", env[_TSAN_OPTIONS_ENV_VAR]) + def test_setup_expected_values_dir_preserves_existing_contents(self): os.makedirs(self.expected_dir) marker = os.path.join(self.expected_dir, "marker") diff --git a/tools/tsan_suppressions.txt b/tools/tsan_suppressions.txt new file mode 100644 index 0000000000..ddefc27103 --- /dev/null +++ b/tools/tsan_suppressions.txt @@ -0,0 +1,20 @@ +# ThreadSanitizer suppressions for known third-party false positives. +# +# Use with: +# TSAN_OPTIONS="suppressions=/path/to/rocksdb/tools/tsan_suppressions.txt" +# +# If the final application already has a TSAN suppressions file, merge these +# entries into that application-owned file. +# +# liburing's io_uring_mmap() uses __sys_mmap, which bypasses TSAN's mmap +# interceptor, then immediately reads from the mapped memory in +# io_uring_setup_ring_pointers(). When the kernel reuses a virtual address that +# was previously mmap'd through libc and tracked by TSAN, TSAN can report a race +# between the old mapping's write and the new io_uring read. RocksDB annotates +# the io_uring mappings after io_uring_queue_init() returns, but that is too +# late for reads that happen inside liburing during initialization. +# +# Revisit these suppressions when RocksDB upgrades to a liburing version that no +# longer uses the raw mmap syscall for io_uring_mmap(). +race:io_uring_mmap +race:io_uring_setup_ring_pointers