ci: Address PR review feedback on the TSan matrix and instrumentation check

Drop the readelf/objdump fallback in the instrumentation check, since that
step runs only in the nix image, whose binutils always provides nm. Rename
the minimal/full/extended matrix tiers to an event-based pull_request,
ready_to_merge, schedule scheme in generate.py and its config files, which
also lets a single --event flag replace the old mutually exclusive pair and
drops the now-impossible "both flagged" assertion. Trim the verbose,
AI-reviewer-style rationale comments this PR added across the suppression
files, docs, and workflow steps down to their load-bearing content: keep
every measurement, command, and rejected alternative, cut the restatement.

Verified generate.py emits the same config-name sets for every event as the
old --minimal/--extended flags did, for all three platforms and packaging.

.github/scripts/strategy-matrix/generate.py

@@ -52,13 +52,25 @@ def get_cmake_args(build_type: str, extra_args: str) -> str:
     return " ".join(args)
 
 
+# The three matrix tiers, in nesting order: a config with an earlier event
+# also appears in every later event's matrix.
+_EVENTS = ("pull_request", "ready_to_merge", "schedule")
+
+
+def _event_index(event: str) -> int:
+    """The position of 'event' in the nesting order, smallest matrix first."""
+    return _EVENTS.index(event)
+
+
 # ---------------------------------------------------------------------------
 # Input types — shapes of the JSON config files
 # ---------------------------------------------------------------------------
 
 
-# Every config must declare 'minimal'. Minimal configs form the reduced matrix
-# built for pull requests by default; the full matrix adds the rest.
+# Every config must declare 'event': the earliest of 'pull_request',
+# 'ready_to_merge', or 'schedule' that builds it. Each later event's matrix
+# includes every config an earlier one does, and adds more. Use 'schedule' for
+# a Linux config too expensive to run per pull request; only Linux has one yet.
 #
 # Configs may also opt into 'benchmark' to smoke-run the benchmarks, or carry a
 # 'package' map to be packaged as well. Note that either applies to every entry
@@ -93,7 +105,7 @@ class LinuxConfig:
     compiler: list[str]
     build_type: list[str]
     arch: list[str]
-    minimal: bool
+    event: str
     benchmark: bool = False  # if true, smoke-run the benchmarks after testing
     sanitizers: list[str] = dataclasses.field(default_factory=list)
     suffix: str = ""
@@ -103,6 +115,9 @@ class LinuxConfig:
     def __post_init__(self) -> None:
         if isinstance(self.package, dict):
             self.package = PackageConfig(**self.package)
+        assert (
+            self.event in _EVENTS
+        ), f"unsupported event {self.event!r}: use one of {', '.join(_EVENTS)}."
 
 
 @dataclasses.dataclass
@@ -129,7 +144,7 @@ class PlatformConfig:
     """One entry in macos.json's or windows.json's 'configs' array."""
 
     build_type: list[str]
-    minimal: bool
+    event: str
     build_only: bool = False  # if true, skip tests (e.g. macos/Windows Debug)
     benchmark: bool = False  # if true, smoke-run the benchmarks after testing
     extra_cmake_args: str = ""
@@ -140,6 +155,11 @@ class PlatformConfig:
     def __post_init__(self) -> None:
         if isinstance(self.build_type, str):
             self.build_type = [self.build_type]
+        # Neither platform has a config expensive enough to reserve for the
+        # nightly/manual 'schedule' tier.
+        assert (
+            self.event in _EVENTS[:2]
+        ), f"unsupported event {self.event!r}: use one of {', '.join(_EVENTS[:2])}."
 
 
 @dataclasses.dataclass
@@ -215,50 +235,125 @@ _ARCHS: dict[str, Architecture] = {
 }
 
 
-def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]:
-    """Expand a LinuxFile into a flat list of matrix entries.
+def expand_linux_config(
+    distro: str, cfg: LinuxConfig, image_tag: str
+) -> list[MatrixEntry]:
+    """Expand one Linux config over the cross-product of its lists.
 
-    Each config entry is expanded over the cross-product of its
-    compiler, build_type, sanitizers, and architecture lists. When 'minimal' is
-    true, only configs flagged as minimal are included.
+    Kept apart from the event filtering in expand_linux_matrix so that
+    validate_linux_matrices can ask what a single config expands to without
+    repeating the cross-product.
+    """
+    # An empty sanitizers list means "one entry with no sanitizer".
+    effective_sanitizers = cfg.sanitizers or [""]
+    effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch}
+
+    return [
+        MatrixEntry(
+            config_name=config_name(
+                distro, compiler, build_type, arch, cfg.suffix, sanitizer
+            ),
+            image=f"ghcr.io/xrplf/xrpld/nix-{distro}:{image_tag}",
+            cmake_args=get_cmake_args(build_type, cfg.extra_cmake_args),
+            cmake_target="all",
+            build_only=False,
+            benchmark=cfg.benchmark,
+            build_type=build_type,
+            architecture=arch_info,
+            sanitizers=sanitizer,
+            compiler=compiler,
+        )
+        for compiler, build_type, sanitizer, (arch, arch_info) in itertools.product(
+            cfg.compiler,
+            cfg.build_type,
+            effective_sanitizers,
+            effective_archs.items(),
+        )
+    ]
+
+
+def expand_linux_matrix(linux: LinuxFile, event: str) -> list[MatrixEntry]:
+    """Expand a LinuxFile into the flat matrix 'event' builds.
+
+    Each config entry is expanded over the cross-product of its compiler,
+    build_type, sanitizers, and architecture lists. A config is included when
+    its own event is no later than 'event', per the nesting order in _EVENTS.
     """
     entries: list[MatrixEntry] = []
 
     for distro, configs in linux.configs.items():
         for cfg in configs:
-            if minimal and not cfg.minimal:
+            if _event_index(cfg.event) > _event_index(event):
                 continue
-            # An empty sanitizers list means "one entry with no sanitizer".
-            effective_sanitizers = cfg.sanitizers or [""]
-            effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch}
-
-            for compiler, build_type, sanitizer, (arch, arch_info) in itertools.product(
-                cfg.compiler,
-                cfg.build_type,
-                effective_sanitizers,
-                effective_archs.items(),
-            ):
-                name = config_name(
-                    distro, compiler, build_type, arch, cfg.suffix, sanitizer
-                )
-                entries.append(
-                    MatrixEntry(
-                        config_name=name,
-                        image=f"ghcr.io/xrplf/xrpld/nix-{distro}:{linux.image_tag}",
-                        cmake_args=get_cmake_args(build_type, cfg.extra_cmake_args),
-                        cmake_target="all",
-                        build_only=False,
-                        benchmark=cfg.benchmark,
-                        build_type=build_type,
-                        architecture=arch_info,
-                        sanitizers=sanitizer,
-                        compiler=compiler,
-                    )
-                )
+            entries += expand_linux_config(distro, cfg, linux.image_tag)
 
     return entries
 
 
+def validate_linux_matrices(linux: LinuxFile) -> None:
+    """Check that the three event matrices nest, and hold the configs they should.
+
+    CI runs only the jobs this script emits, so a config that falls out of the
+    event it belongs to takes its coverage with it and fails nothing. These
+    checks run on every invocation, so the drop fails matrix generation instead.
+
+    The checks name no config, only the events, so adding or removing a config
+    needs no edit here.
+    """
+    names_by_event = {
+        event: {e.config_name for e in expand_linux_matrix(linux, event)}
+        for event in _EVENTS
+    }
+
+    # The names each config expands to, paired with the config, so that the
+    # checks below expand every config once.
+    per_config = [
+        (
+            distro,
+            cfg,
+            {e.config_name for e in expand_linux_config(distro, cfg, linux.image_tag)},
+        )
+        for distro, configs in linux.configs.items()
+        for cfg in configs
+    ]
+
+    # A config name is also the name of the artifacts the job uploads, so two
+    # configs that expand to the same name overwrite each other. The per-config
+    # checks below also need a name to belong to one config only.
+    all_names = [n for _, _, names in per_config for n in names]
+    duplicates = sorted({n for n in all_names if all_names.count(n) > 1})
+    assert not duplicates, f"configs expand to duplicate names: {duplicates}."
+
+    # A later event's matrix only ever adds to an earlier one's.
+    for earlier, later in zip(_EVENTS, _EVENTS[1:]):
+        assert names_by_event[earlier] <= names_by_event[later], (
+            f"the {earlier} matrix is not part of the {later} one, missing: "
+            f"{sorted(names_by_event[earlier] - names_by_event[later])}."
+        )
+
+    # Every config reaches the events its own event asks for, and no earlier one.
+    for distro, cfg, names in per_config:
+        for event in _EVENTS[_event_index(cfg.event) :]:
+            assert names <= names_by_event[event], (
+                f"{distro} config declaring {cfg.event!r} is missing from the "
+                f"{event} matrix: {sorted(names - names_by_event[event])}."
+            )
+        for event in _EVENTS[: _event_index(cfg.event)]:
+            assert not names & names_by_event[event], (
+                f"{distro} config declaring {cfg.event!r} also reaches the "
+                f"{event} matrix: {sorted(names & names_by_event[event])}."
+            )
+
+    # The 'schedule' tier costs a branch here, a condition in
+    # reusable-strategy-matrix.yml, and the check after it. An empty tier leaves
+    # all three as dead weight that still reads as working, so require a holder.
+    # Checked last, because the checks above name the config that went missing.
+    assert names_by_event["schedule"] > names_by_event["ready_to_merge"], (
+        "no config declares 'schedule', so its matrix is the ready_to_merge one. "
+        "Either give the config that needs the tier that event, or remove the tier."
+    )
+
+
 def expand_linux_packaging(linux: LinuxFile) -> list[PackagingEntry]:
     """Generate the packaging matrix from the configs that carry a 'package' map.
 
@@ -313,17 +408,14 @@ def package_names_by_type(entries: list[PackagingEntry]) -> dict[str, list[str]]
     }
 
 
-def expand_platform_matrix(pf: PlatformFile, minimal: bool) -> list[MatrixEntry]:
-    """Expand a PlatformFile (macOS or Windows) into matrix entries.
-
-    When 'minimal' is true, only configs flagged as minimal are included.
-    """
+def expand_platform_matrix(pf: PlatformFile, event: str) -> list[MatrixEntry]:
+    """Expand a PlatformFile (macOS or Windows) into the matrix 'event' builds."""
     platform_name, arch = pf.platform.split("/")
     is_windows = platform_name == "windows"
 
     entries: list[MatrixEntry] = []
     for cfg in pf.configs:
-        if minimal and not cfg.minimal:
+        if _event_index(cfg.event) > _event_index(event):
             continue
         for build_type in cfg.build_type:
             name = f"{platform_name}-{arch}-{build_type.lower()}"
@@ -368,35 +460,40 @@ if __name__ == "__main__":
         action="store_true",
     )
     parser.add_argument(
-        "-m",
-        "--minimal",
-        help="Emit only the minimal matrix (the configs flagged 'minimal'), "
-        "used for pull requests by default. If omitted, the full matrix is "
-        "emitted.",
-        action="store_true",
+        "-e",
+        "--event",
+        help="The event driving generation, which picks the matrix: "
+        "'pull_request' emits the smallest, 'ready_to_merge' (the default) "
+        "widens it for a labeled pull request, and 'schedule' widens it "
+        "further for the nightly run and a manual dispatch.",
+        choices=_EVENTS,
+        default="ready_to_merge",
     )
     args = parser.parse_args()
 
     matrix: list[MatrixEntry] | list[PackagingEntry] = []
 
+    # Checked on every invocation, including the ones that emit another platform
+    # or the packaging matrix, so that no call can pass a broken linux.json.
+    linux = LinuxFile.load(THIS_DIR / "linux.json")
+    validate_linux_matrices(linux)
+
     if args.packaging:
-        matrix = expand_linux_packaging(LinuxFile.load(THIS_DIR / "linux.json"))
+        matrix = expand_linux_packaging(linux)
         # One list per format, so each install-test job installs the packages its
         # own format produced.
         for package_type, names in package_names_by_type(matrix).items():
             print(f"{package_type}_package_names={json.dumps(names)}")
     else:
         if args.config in ("linux", None):
-            matrix += expand_linux_matrix(
-                LinuxFile.load(THIS_DIR / "linux.json"), args.minimal
-            )
+            matrix += expand_linux_matrix(linux, args.event)
         if args.config in ("macos", None):
             matrix += expand_platform_matrix(
-                PlatformFile.load(THIS_DIR / "macos.json"), args.minimal
+                PlatformFile.load(THIS_DIR / "macos.json"), args.event
             )
         if args.config in ("windows", None):
             matrix += expand_platform_matrix(
-                PlatformFile.load(THIS_DIR / "windows.json"), args.minimal
+                PlatformFile.load(THIS_DIR / "windows.json"), args.event
             )
 
     print(f"matrix={json.dumps({'include': [dataclasses.asdict(e) for e in matrix]})}")

.github/scripts/strategy-matrix/linux.json

@@ -6,7 +6,7 @@
         "compiler": ["gcc"],
         "build_type": ["Debug"],
         "arch": ["amd64"],
-        "minimal": true,
+        "event": "pull_request",
         "suffix": "coverage",
         "extra_cmake_args": "-DUNIT_TEST_REFERENCE_FEE=500 -Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_C_FLAGS=-O0 -DCMAKE_CXX_FLAGS=-O0"
       },
@@ -14,7 +14,7 @@
         "compiler": ["clang"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": true,
+        "event": "pull_request",
         "benchmark": true
       },
 
@@ -22,28 +22,35 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": false
+        "event": "ready_to_merge"
       },
       {
         "compiler": ["gcc", "clang"],
         "build_type": ["Debug", "Release"],
         "arch": ["arm64"],
-        "minimal": false
+        "event": "ready_to_merge"
       },
 
       {
         "compiler": ["gcc", "clang"],
         "build_type": ["Debug", "Release"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "sanitizers": ["address", "undefinedbehavior"]
       },
+      {
+        "compiler": ["clang"],
+        "build_type": ["Debug"],
+        "arch": ["amd64"],
+        "event": "schedule",
+        "sanitizers": ["thread"]
+      },
 
       {
         "compiler": ["clang"],
         "build_type": ["Debug"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "suffix": "voidstar",
         "extra_cmake_args": "-Dvoidstar=ON"
       },
@@ -51,7 +58,7 @@
         "compiler": ["clang"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "suffix": "reffee",
         "extra_cmake_args": "-DUNIT_TEST_REFERENCE_FEE=1000"
       },
@@ -59,7 +66,7 @@
         "compiler": ["gcc"],
         "build_type": ["Debug"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "suffix": "unity",
         "extra_cmake_args": "-Dunity=ON"
       }
@@ -70,7 +77,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "extra_cmake_args": "-Dvalidator_keys=ON",
         "package": {
           "type": "deb",
@@ -81,7 +88,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "suffix": "assert",
         "extra_cmake_args": "-Dvalidator_keys=ON -Dassert=ON",
         "package": {
@@ -97,7 +104,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "minimal": false,
+        "event": "ready_to_merge",
         "extra_cmake_args": "-Dvalidator_keys=ON",
         "package": {
           "type": "rpm",

.github/scripts/strategy-matrix/macos.json

@@ -5,26 +5,26 @@
     {
       "build_type": "Release",
       "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5",
-      "minimal": true
+      "event": "pull_request"
     },
     {
       "build_type": "Debug",
       "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5",
       "build_only": true,
-      "minimal": false
+      "event": "ready_to_merge"
     },
     {
       "build_type": "Release",
       "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5",
       "toolchain": "nix",
-      "minimal": false
+      "event": "ready_to_merge"
     },
     {
       "build_type": "Debug",
       "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5",
       "toolchain": "nix",
       "build_only": true,
-      "minimal": false
+      "event": "ready_to_merge"
     }
   ]
 }

.github/scripts/strategy-matrix/windows.json

@@ -2,11 +2,11 @@
   "platform": "windows/amd64",
   "runner": ["self-hosted", "Windows", "dev-box-windows-2026"],
   "configs": [
-    { "build_type": "Release", "minimal": true },
+    { "build_type": "Release", "event": "pull_request" },
     {
       "build_type": "Debug",
       "build_only": true,
-      "minimal": false
+      "event": "ready_to_merge"
     }
   ]
 }

.github/workflows/reusable-build-test-config.yml

@@ -117,6 +117,12 @@ jobs:
       VOIDSTAR_ENABLED: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }}
       VALIDATOR_KEYS_ENABLED: ${{ contains(inputs.cmake_args, '-Dvalidator_keys=ON') }}
       SANITIZERS_ENABLED: ${{ inputs.sanitizers != '' }}
+      # One flag per sanitizer, so that a step needing only one of them does not
+      # have to parse the list. conan/profiles/sanitizers rejects any name that is
+      # not one of these three, so an unrecognized one never reaches a step.
+      ASAN_ENABLED: ${{ contains(inputs.sanitizers, 'address') }}
+      TSAN_ENABLED: ${{ contains(inputs.sanitizers, 'thread') }}
+      UBSAN_ENABLED: ${{ contains(inputs.sanitizers, 'undefinedbehavior') }}
       # The binaries reusable-package.yml consumes. A private repository skips
       # them except on a tag push, which is what produces its release packages.
       PACKAGING_ARTIFACTS_ENABLED: ${{ github.event.repository.visibility == 'public' || startsWith(github.ref, 'refs/tags/') }}
@@ -234,7 +240,10 @@ jobs:
               ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
           fi
           echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV}
-          echo "TSAN_OPTIONS=include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp" >>${GITHUB_ENV}
+          # exitcode=0 is set here rather than in runtime-tsan-options.txt, which
+          # the documented local command also reads and must still fail on a finding.
+          TSAN_OPTS="include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp"
+          echo "TSAN_OPTIONS=${TSAN_OPTS}:exitcode=0" >>${GITHUB_ENV}
           echo "UBSAN_OPTIONS=include=${SUPP}/runtime-ubsan-options.txt:suppressions=${SUPP}/ubsan.supp" >>${GITHUB_ENV}
           echo "LSAN_OPTIONS=include=${SUPP}/runtime-lsan-options.txt:suppressions=${SUPP}/lsan.supp" >>${GITHUB_ENV}
 
@@ -343,11 +352,82 @@ jobs:
               exit 1
           fi
 
+      # Catch a build configured without the instrumentation it was asked for,
+      # which would otherwise run every test, pass, and report nothing.
+      #
+      # Checked by the symbols the binary carries: instrumented code calls into
+      # its runtime whether that runtime is linked statically or as a shared
+      # library, so those symbols prove the -fsanitize flags reached the
+      # compiler, which the version string alone cannot (cmake sets the
+      # SANITIZERS macro independently of those flags). The version string is
+      # checked too, since BuildInfo.cpp is what a bug report quotes and should
+      # agree with the binary. voidstar has no compile-time symbol to check, so
+      # the version string is all it gets.
+      #
+      # The version call's exit code is ignored: ASan's leak reporting and
+      # UBSan's halt_on_error can both end it nonzero on a healthy binary. Only
+      # the printed text decides.
       - name: Verify presence of instrumentation (Linux)
-        if: ${{ runner.os == 'Linux' && env.VOIDSTAR_ENABLED == 'true' }}
+        if: ${{ runner.os == 'Linux' && (env.VOIDSTAR_ENABLED == 'true' || env.SANITIZERS_ENABLED == 'true') }}
         working-directory: ${{ env.BUILD_DIR }}
         run: |
-          ./xrpld --version | grep libvoidstar
+          version="$(./xrpld --version 2>&1 || true)"
+          echo "${version}"
+          status=0
+
+          named() {
+              if grep -q "$2" <<<"${version}"; then
+                  echo "${1}: the version string names '${2}'."
+              else
+                  echo "${1} is enabled, but the version string does not name '${2}'."
+                  status=1
+              fi
+          }
+
+          if [ "${SANITIZERS_ENABLED}" = 'true' ]; then
+              # This step runs only inside the nix image, whose binutils is
+              # rebuilt as part of the toolchain, so nm is always present.
+              symbols="$(mktemp)"
+              nm ./xrpld >"${symbols}" 2>/dev/null || true
+              if [ ! -s "${symbols}" ]; then
+                  echo 'Read no symbols from ./xrpld, so the instrumentation cannot be confirmed.'
+                  exit 1
+              fi
+
+              # Each pattern names a call a sanitizer plants at an instrumented
+              # operation, not one that only proves its runtime is present:
+              # __asan_init/__tsan_init appear as soon as the flags reach the
+              # linker, and __tsan_func_entry survives an ignorelist, so any of
+              # those would pass a build whose compile step never saw them.
+              #
+              # GCC links libasan/libubsan dynamically, so an instrumented call
+              # site is the only way these symbols reach the binary, making the
+              # check exact. Clang links TSan's runtime statically, so its own
+              # symbols are there regardless, narrowing this to "reached the
+              # link step" rather than "reached the compiler."
+              instrumented() {
+                  if grep -qE "$2" "${symbols}"; then
+                      echo "${1}: the binary calls into its runtime, matching '${2}'."
+                  else
+                      echo "${1} is enabled, but no instrumented call site matches '${2}'."
+                      status=1
+                  fi
+              }
+
+              [ "${ASAN_ENABLED}" = 'true' ] &&
+                  instrumented address '__asan_report|__asan_memcpy|__asan_stack_malloc'
+              [ "${TSAN_ENABLED}" = 'true' ] &&
+                  instrumented thread '__tsan_read|__tsan_write|__tsan_atomic'
+              [ "${UBSAN_ENABLED}" = 'true' ] &&
+                  instrumented undefinedbehavior '__ubsan_handle'
+          fi
+
+          [ "${VOIDSTAR_ENABLED}" = 'true' ] && named voidstar libvoidstar
+          [ "${ASAN_ENABLED}" = 'true' ] && named address ASAN
+          [ "${TSAN_ENABLED}" = 'true' ] && named thread TSAN
+          [ "${UBSAN_ENABLED}" = 'true' ] && named undefinedbehavior UBSAN
+
+          exit "${status}"
 
       - name: Run the separate tests
         if: ${{ !inputs.build_only }}
@@ -366,6 +446,12 @@ jobs:
           # Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness
           [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$((BUILD_NPROC - 2))
 
+          # TSan's shadow memory makes each job cost several times what it does
+          # uninstrumented, so cap the count rather than subtract from it: the
+          # ceiling is total memory, not core count. Raising it risks the OOM
+          # killer (exit 137) or, worse, a silent loss of contact with the runner.
+          [ "$TSAN_ENABLED" = "true" ] && BUILD_NPROC=4
+
           # The resolver/preload workaround is only correct for the ASan build:
           # a regular build doesn't hit the __dn_expand interceptor bug, and must
           # NOT have libasan injected. So only preload when xrpld is ASan-built.

.github/workflows/reusable-strategy-matrix.yml

@@ -21,6 +21,25 @@ jobs:
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.generate.outputs.matrix }}
+    # Set at job level rather than on the step, so that the generate step and the
+    # check after it read the same values and cannot disagree.
+    env:
+      GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }}
+      # Pick the smallest matrix for a pull request not yet labeled "Ready to
+      # merge" or "Full CI build", the largest for the nightly schedule or a
+      # manual run (otherwise the only way to exercise it is to wait for the
+      # night), and generate.py's own default, ready_to_merge, for anything
+      # else (merge queue, push).
+      GENERATE_EVENT: >-
+        ${{
+          (github.event_name == 'pull_request'
+            && !contains(github.event.pull_request.labels.*.name, 'Ready to merge')
+            && !contains(github.event.pull_request.labels.*.name, 'Full CI build'))
+          && '--event=pull_request'
+          || ((github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')
+            && '--event=schedule'
+            || '')
+        }}
     steps:
       - name: Checkout repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
@@ -33,10 +52,22 @@ jobs:
       - name: Generate strategy matrix
         working-directory: .github/scripts/strategy-matrix
         id: generate
+        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_EVENT} >>"${GITHUB_OUTPUT}"
+
+      # generate.py checks that the schedule matrix holds every config that
+      # declares it, but it never sees the event, so it cannot tell whether
+      # this run asked for that matrix. A GENERATE_EVENT that stops matching
+      # the events would drop those configs and fail nothing, because CI runs
+      # only the jobs the matrix names. So compare what was generated against
+      # the schedule matrix, on the events that must produce it.
+      - name: Check the schedule configs reached the matrix
+        if: ${{ env.GENERATE_EVENT == '--event=schedule' }}
+        working-directory: .github/scripts/strategy-matrix
         env:
-          GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }}
-          # Run only the minimal matrix for pull requests that are not yet
-          # labeled "Ready to merge" or "Full CI build". Any other event (merge
-          # queue, push, schedule, manual dispatch) runs the full matrix.
-          GENERATE_MINIMAL: ${{ (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'Ready to merge') && !contains(github.event.pull_request.labels.*.name, 'Full CI build')) && '--minimal' || '' }}
-        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} >>"${GITHUB_OUTPUT}"
+          GENERATED: ${{ steps.generate.outputs.matrix }}
+        run: |
+          expected="$(./generate.py ${GENERATE_CONFIG} --event=schedule)"
+          if [[ "matrix=${GENERATED}" != "${expected}" ]]; then
+              echo "::error::the ${GITHUB_EVENT_NAME} matrix is not the schedule one"
+              exit 1
+          fi

BUILD.md

@@ -277,7 +277,7 @@ Example use with some cmake variables set:
 ```
 cd .build
 conan install .. --output-folder . --build missing --settings build_type=Debug
-cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_test_parallelism=2 -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
+cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
 cmake --build . --target coverage
 ```
 

cmake/XrplSanitizers.cmake

@@ -77,19 +77,24 @@ if(is_clang)
     message(STATUS "  Ignorelist: ${ignorelist_path}")
 endif()
 
-# Define SANITIZERS macro for BuildInfo.cpp
+# Define the SANITIZERS macro for BuildInfo.cpp, plus one of XRPL_ASAN,
+# XRPL_TSAN and XRPL_UBSAN per active sanitizer, so that code can test for a
+# specific one with #ifdef instead of parsing the dot-joined SANITIZERS string.
 set(sanitizers_list)
 if(SANITIZERS MATCHES "address")
     set(enable_asan ON)
     list(APPEND sanitizers_list "ASAN")
+    target_compile_definitions(common INTERFACE XRPL_ASAN)
 endif()
 if(SANITIZERS MATCHES "thread")
     set(enable_tsan ON)
     list(APPEND sanitizers_list "TSAN")
+    target_compile_definitions(common INTERFACE XRPL_TSAN)
 endif()
 if(SANITIZERS MATCHES "undefinedbehavior")
     set(enable_ubsan ON)
     list(APPEND sanitizers_list "UBSAN")
+    target_compile_definitions(common INTERFACE XRPL_UBSAN)
 endif()
 
 if(sanitizers_list)

docs/build/sanitizers.md

@@ -99,6 +99,30 @@ export TSAN_OPTIONS="include=sanitizers/suppressions/runtime-tsan-options.txt:su
 
 More details [here](https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual).
 
+> [!IMPORTANT]
+> The `ubuntu-clang-debug-amd64-tsan` CI config runs TSan only on the
+> `schedule` event (the nightly run and a manual dispatch), and it reports
+> nothing back yet: `runtime-tsan-options.txt` sets `halt_on_error=false` so
+> the run continues past a finding, and the workflow's `TSAN_OPTIONS` adds
+> `exitcode=0` so the finding does not fail the job. Read the job log to see
+> findings. `exitcode=0` lives in the workflow rather than in
+> `runtime-tsan-options.txt` so that the local command above still fails on a
+> finding. This is a first stage: once the job's findings are known and
+> stable, drop `exitcode=0` so a new one fails the job, and suppress the rest.
+
+> [!IMPORTANT]
+> Run TSan on Linux to check lock order. Linux reports an inversion as
+> `WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)`, with no
+> `TSAN_OPTIONS` needed. macOS arm64 reports nothing, even for a genuine double
+> lock of a non-recursive mutex. That is a platform limit, not a configuration
+> mistake. Data race detection does work on macOS.
+
+> [!TIP]
+> The build defines `XRPL_TSAN` when TSan is active, and `XRPL_ASAN` and
+> `XRPL_UBSAN` for the other two. Use them to skip a test that only means
+> something under one sanitizer, at run time rather than with `#ifdef` around
+> the body, so every build still compiles the test.
+
 ### LeakSanitizer (LSan)
 
 LSan is automatically enabled with ASAN. To disable it:
@@ -150,15 +174,30 @@ More details [here](https://clang.llvm.org/docs/undefinedbehaviorSanitizer.html)
 
 ### [`tsan.supp`](../../sanitizers/suppressions/tsan.supp)
 
-- **Purpose**: Suppress ThreadSanitizer data race warnings
-- **Format**: `race:<pattern>` where pattern matches function/file names
+- **Purpose**: Suppress ThreadSanitizer warnings
+- **Format**: `<type>:<pattern>` where pattern matches function/file names, and
+  type is `race`, `deadlock`, `signal`, `mutex` or `called_from_lib`
 - **More info**: [ThreadSanitizer suppressions](https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions)
 
 ### [`sanitizer-ignorelist.txt`](../../sanitizers/suppressions/sanitizer-ignorelist.txt)
 
 - **Purpose**: Compile-time ignorelist for all sanitizers
 - **Usage**: Passed via `-fsanitize-ignorelist=absolute/path/to/sanitizer-ignorelist.txt`
-- **Format**: `<level>:<pattern>` (e.g., `src:Workers.cpp`)
+- **Format**: `<entity>:<glob>` (e.g. `src:*Workers.cpp`)
+- **Note**: Not a suppressions file; the syntax differs. Clang looks up only
+  the entities `src`, `fun`, `global`, `type` and `mainfile`. A `race:`,
+  `deadlock:` or `signal:` entry parses but is never consulted, so it does
+  nothing. Those types belong in `tsan.supp`.
+- **Note**: The glob must match the whole absolute path the compiler receives.
+  `src:core/detail/Workers.cpp` matches nothing; `src:*core/detail/Workers.cpp`
+  does.
+- **Note**: To confirm an entry works, compile the file with and without it and
+  compare how many times the object references the calls a sanitizer plants at
+  an instrumented operation (`__tsan_read`/`__tsan_write`, `__asan_report`, or
+  `__ubsan_handle`), e.g. `nm -u <file>.o | grep -cE '__tsan_(read|write)'`. A
+  working entry lowers the count, usually to zero (inlined code still counts).
+  Ignore `__tsan_func_entry` (survives an ignorelist) and `__tsan_init` (only
+  shows the runtime is present); both make an ignored file look instrumented.
 
 ## Troubleshooting
 

sanitizers/suppressions/runtime-tsan-options.txt

@@ -1,3 +1,4 @@
 halt_on_error=false
 verbosity=1
+detect_deadlocks=1
 second_deadlock_stack=1

sanitizers/suppressions/sanitizer-ignorelist.txt

@@ -1,32 +1,17 @@
-# We were seeing some false positives and some repeated errors(since these are library files) in following files.
-# Clang will skip instrumenting the files added here.
-# We should fix the underlying issues(if any) and remove these entries.
-
-deadlock:libxrpl/beast/utility/beast_Journal.cpp
-deadlock:libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:test/beast/beast_PropertyStream_test.cpp
-deadlock:xrpld/core/detail/Workers.cpp
-deadlock:xrpld/core/JobQueue.cpp
-
-race:libxrpl/beast/utility/beast_Journal.cpp
-race:libxrpl/beast/utility/beast_PropertyStream.cpp
-race:test/beast/beast_PropertyStream_test.cpp
-race:xrpld/core/detail/Workers.cpp
-race:xrpld/core/JobQueue.cpp
-
-signal:libxrpl/beast/utility/beast_Journal.cpp
-signal:libxrpl/beast/utility/beast_PropertyStream.cpp
-signal:test/beast/beast_PropertyStream_test.cpp
-signal:xrpld/core/detail/Workers.cpp
-signal:xrpld/core/JobQueue.cpp
-
-src:beast/utility/beast_Journal.cpp
-src:beast/utility/beast_PropertyStream.cpp
-src:core/detail/Workers.cpp
-src:core/JobQueue.cpp
-src:libxrpl/beast/utility/beast_Journal.cpp
-src:test/beast/beast_PropertyStream_test.cpp
-src:src/test/app/Invariants_test.cpp
+# Clang skips instrumenting the files listed here. Keep the list as short as it
+# can be: an entry here removes a file from every sanitizer's view, so it hides
+# real findings along with the false positive it was added for.
+#
+# This is a clang ignorelist, not a runtime suppressions file: an entry reads
+# <entity>:<glob>, only src/fun/global/type/mainfile are looked up (a `race:`,
+# `deadlock:` or `signal:` entry parses but does nothing, so those belong in
+# tsan.supp), and the glob needs a leading `*` to match the absolute path the
+# compiler receives. See docs/build/sanitizers.md.
+#
+# This list once held seven `src:` entries, none with the leading `*`, so none
+# ever matched and those files stayed instrumented the whole time; removed
+# rather than repaired, since the jobs already pass without them. Restore one
+# only with a finding that shows it is needed, checked by the method above.
 
 # ASan false positive: stack-use-after-scope in ErrorCodes.h inline functions.
 # When Clang inlines the StaticString overloads (e.g. invalidFieldError(StaticString)),

sanitizers/suppressions/tsan.supp

@@ -35,43 +35,42 @@ race:src/xrpld/app/main/BasicApp.cpp
 race:src/xrpld/app/main/GRPCServer.cpp
 race:src/xrpld/app/misc/detail/AmendmentTable.cpp
 race:src/xrpld/app/misc/FeeVoteImpl.cpp
-race:src/xrpld/app/rdb/detail/Wallet.cpp
+race:src/libxrpl/server/Wallet.cpp
 race:src/xrpld/overlay/detail/OverlayImpl.cpp
-race:src/xrpld/peerfinder/detail/PeerfinderManager.cpp
-race:src/xrpld/peerfinder/detail/SourceStrings.cpp
+race:src/libxrpl/peerfinder/PeerfinderManager.cpp
+race:src/libxrpl/peerfinder/SourceStrings.cpp
 race:src/xrpld/rpc/detail/ServerHandler.cpp
 race:xrpl/server/detail/Door.h
 race:xrpl/server/detail/Spawn.h
 race:xrpl/server/detail/ServerImpl.h
 race:xrpl/nodestore/detail/DatabaseNodeImp.h
 race:src/libxrpl/beast/utility/beast_Journal.cpp
-race:src/test/beast/LexicalCast_test.cpp
+race:src/tests/libxrpl/beast/LexicalCast.cpp
 race:ServerHandler
 
 # More suppressions in external library code.
 race:crtstuff.c
 race:pipe
 
-# Deadlock / lock-order-inversion suppressions
-# Note: GCC's TSAN may not fully support all deadlock suppression patterns
-deadlock:src/libxrpl/beast/utility/beast_Journal.cpp
-deadlock:src/libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:src/test/beast/beast_PropertyStream_test.cpp
-deadlock:src/xrpld/core/detail/Workers.cpp
-deadlock:src/xrpld/app/misc/detail/Manifest.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorList.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorSite.cpp
+# Deadlock / lock-order-inversion suppressions: deliberately none.
+#
+# The TSan job exists to find lock-order inversions and cannot fail a build, so
+# a suppression here costs a finding and buys nothing. Ten used to sit below,
+# naming either a locking primitive (turning off checking for every lock of that
+# kind in the tree, not just the one call site) or a source file. Removing the
+# Manifest.cpp one surfaced a real deadlock 52 times in one run: ManifestCache::
+# save() calls the caller's predicate while holding its own lock, and the
+# validator list takes the same pair the other way.
+#
+# If one has to come back, it must name a source file that still exists at that
+# path, because a suppression naming a moved file protects nothing. GCC's TSan
+# may not fully honor deadlock suppression patterns.
 
 signal:src/libxrpl/beast/utility/beast_Journal.cpp
-signal:src/xrpld/core/detail/Workers.cpp
-signal:src/xrpld/core/JobQueue.cpp
+signal:src/libxrpl/core/detail/Workers.cpp
+signal:src/libxrpl/core/detail/JobQueue.cpp
 signal:Workers::Worker
 
-# Aggressive suppressing of deadlock tsan errors
-deadlock:pthread_create
-deadlock:pthread_rwlock_rdlock
-deadlock:boost::asio
-
 # Suppress SEGV crashes in TSAN itself during stringbuf operations
 # This appears to be a GCC-15 TSAN instrumentation issue with basic_stringbuf::str()
 # Commonly triggered in beast::Journal::ScopedStream destructor