Skip to content

build: add runtime sanitizer support and integration test wrapper helper - #22225

Draft
da-phil wants to merge 8 commits into
darktable-org:masterfrom
da-phil:pl/add_runtime_sanitizer_support_and_integration_test_wrapper_helper
Draft

build: add runtime sanitizer support and integration test wrapper helper#22225
da-phil wants to merge 8 commits into
darktable-org:masterfrom
da-phil:pl/add_runtime_sanitizer_support_and_integration_test_wrapper_helper

Conversation

@da-phil

@da-phil da-phil commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Run darktable's integration tests under sanitizers

Adds the build support, a test driver, and two static checks derived from what the sanitizers found.

Build

DT_SANITIZE takes a comma-separated list of address, undefined, leak, thread.
It is applied as compile/link options, so it composes with RelWithDebInfo and does not collide with rawspeed's own Sanitize/TSan build types.
Invalid combinations and MSan are rejected at configure time and the runtime is link-checked, so a missing libasan fails early.
build.sh gains --sanitize; --asan stays as an alias.

ASan is built with -fsanitize-recover=address and run with halt_on_error=0 so one run collects every finding instead of stopping at whichever comes first.

Each runtime's *_OPTIONS is exported only when that sanitizer is built in.
All four share sanitizer_common's log_path, so whichever set is parsed last decides where reports land, and which that is differs by runtime: a GCC address,undefined build wrote ASan reports to ubsan.<pid>, and a clang thread build wrote ThreadSanitizer reports to ubsan-<pass>.<pid>.

DEBUGINFOD_URLS is cleared. Distributions set it globally, and llvm-symbolizer then blocks on network lookups for modules it cannot satisfy locally - 45s per module against 0.02s with it cleared.

Driver

tools/run-integration-tests.sh drives src/tests/integration from outside the submodule, without patching anything in it. The suite runs darktable-cli as $* 1> /dev/null 2> /dev/null, so every sanitizer report would otherwise be discarded.

It:

  • supplies the per-invocation timeout the suite has nowhere
  • rescues the reports, via log_path for the runtimes that honour it and a stderr-capturing shim for GCC's UBSan, which ignores it
  • files reports under the test that produced them, so a ~180-test run stays traceable
  • preflights the binary, because a runtime that cannot map its shadow memory dies before main() and every test then fails identically in a fraction of a second, which reads like a darktable bug. On kernels defaulting vm.mmap_rnd_bits to 32 this affects TSan, and the driver falls back to setarch -R on its own
  • offers --no-openmp. GCC's libgomp carries no TSan annotations, so TSan cannot see the happens-before edges OpenMP barriers establish and reports a race on nearly every parallel loop: 97% of the findings in a full-suite run. The tsan.supp entries cannot catch these - called_from_lib only applies to interceptors, and race: needs symbolized names a stripped libgomp does not have. Serialising removes them at the source. It rewrites the suite's -t rather than setting OMP_NUM_THREADS, because dt_init() derives its thread count from -t and then calls omp_set_num_threads(), overriding the environment. On 0000-nop under TSan: 326s to 17s, 1.3MB of reports to 88KB, 119 unique findings to 18.

tools/sanitizer/aggregate-reports.py deduplicates the findings and names the tests each one appeared in.
Given --build-dir it also resolves the frames the runtimes leave as module+offset: TSan symbolizes the binary and its link-time libraries but not the dlopen'd iop plugins, which are the frames worth reading.
That runs after deduplication, on the frames about to be printed - a full-suite run is gigabytes of reports collapsing to a few hundred findings.

Static checks

Two invariants the sanitizers found the hard way are decidable without running anything, because both sides are in the DWARF of a build with debug info:

  • tools/check-iop-pipe-data-sizes.py - a module without its own init_pipe() gets default_init_pipe(), which allocates piece->data as calloc(1, params_size). Sound only while the data struct is no larger than the params struct.
  • tools/check-iop-legacy-params.py - dt_iop_<op>_params_v<N>_t structs must still match what those darktable versions wrote. Ground truth is the history entries in the integration test XMPs.

Neither can be a static_assert: params_size is a runtime field and the data and legacy structs are module-local, so the two sides are never visible to the compiler together.
Both read DWARF through tools/dwarf_types.py, which uses objdump - gdb cannot reach the function-local legacy typedefs - so they need only binutils and any RelWithDebInfo tree.

How to run it - Quickstart

Here is an example with ASAN and UBSAN instrumentation:

# build (a dedicated build dir, RelWithDebInfo, OpenCL off)
./build.sh --build-dir "$PWD/build-sanitize" \
           --sanitize address,undefined \
           --build-type RelWithDebInfo \
           --build-generator Ninja \
           --disable-opencl

# confirm the binary really is instrumented
ldd build-sanitize/bin/darktable-cli | grep -E 'libasan|libubsan'

# run the integration test suite against it
./tools/run-integration-tests.sh --build-dir "$PWD/build-sanitize"

What it found

Three heap-buffer-overflows, each fixed in its own PR:

Still open: a lock-order-inversion between dt_cache_get_with_caller (cache.c:150, :233) and dt_rawspeed_load_meta (imageio_rawspeed.cc:68), closed by dt_variables_expand re-entering the cache.
It reproduces in every test under GCC with and without OpenMP, and under clang, so it is not a runtime artifact.

Here are example log outputs of running all integration tests with sanitizer instrumentation:

Disclaimer: this change was co-created with Claude.

Add DT_SANITIZE, a CMake option taking a comma separated list of
address, undefined, leak and thread. It applies the instrumentation as
compile/link options rather than as a build type, so it composes with
RelWithDebInfo and does not collide with rawspeed's own Sanitize/TSan
build types. Invalid combinations and MSan are rejected at configure
time, and the runtime is link-checked so a missing libasan fails early.

build.sh gains --sanitize; --asan is kept as an alias. The old --asan
passed CFLAGS/LDFLAGS as environment prefixes to cmake, which CMake only
consumes on the first configure into an empty cache, so re-running it on
an existing build dir silently produced an uninstrumented binary.

Drop -Werror for sanitizer builds: instrumentation perturbs the
optimizer and produces fresh -Wmaybe-uninitialized / -Wstringop-overflow
diagnostics that would otherwise abort the build.

Add tools/run-integration-tests.sh and tools/sanitizer/aggregate-reports.py,
which drive src/tests/integration from outside the submodule. They supply
the per-invocation timeout the suite lacks, and rescue the sanitizer
reports the suite would otherwise discard via its "2> /dev/null":
log_path covers the runtimes reporting through sanitizer_common, and a
darktable-cli shim captures stderr, where GCC's UBSan prints its
non-fatal diagnostics regardless of log_path.
Both were found by AddressSanitizer during an integration run, and both
are decidable without executing anything: the sizes involved are all in
the DWARF of a build with debug info.

check-iop-pipe-data-sizes.py: a module without its own init_pipe() gets
default_init_pipe(), which allocates piece->data as
calloc(1, params_size). That is only sound while the data struct is no
larger than the params struct, and nothing enforces it.

check-iop-legacy-params.py: a module's old parameter layouts survive as
dt_iop_<op>_params_v<N>_t inside legacy_params(), and nothing checks they
still match what those darktable versions wrote. Ground truth comes from
the history entries in the integration test XMPs. A size mismatch is an
error; a version with no matching struct is a warning, since the compiler
need not emit a local type it had no use for.

Neither invariant can be a static_assert: params_size is a runtime field
and the data and legacy structs are module-local, so the two sides are
never visible to the compiler together.

Both read DWARF through tools/dwarf_types.py, which uses objdump rather
than gdb, gdb resolves types through a lookup scope and cannot reach
the function-local legacy typedefs.
GCC's libgomp carries no TSan annotations, so TSan cannot see the
happens-before edges OpenMP barriers establish and reports a race on
nearly every parallel loop: 97% of the reports in a full-suite run.
The tsan.supp entries cannot catch these - called_from_lib only applies
to interceptors, and race: needs symbolized names that a stripped libgomp does not have.
Option --no-openmp runs the CLI with OMP_NUM_THREADS=1 instead,
removing them at the source and leaving darktable's own threading visible.

Distributions set DEBUGINFOD_URLS globally (/etc/debuginfod), which makes
llvm-symbolizer block on network lookups -- 45s per module here against
0.02s with it cleared. That is why the runtimes gave up and printed
unsymbolized "<null>" frames for the dlopen'd iop plugins, which are the
frames worth reading. Clear it in the generated environment.

aggregate-reports.py resolves any that still arrive as module+offset,
given --build-dir. It runs after deduplication, on the frames about to be
printed: a full-suite run is gigabytes of reports collapsing to a few
hundred findings, so resolving during parsing would symbolize the same
addresses tens of thousands of times.
Setting OMP_NUM_THREADS had no effect: dt_init() derives its thread count
from -t/--threads and then calls omp_set_num_threads()
(src/common/darktable.c:1618), overriding the environment. The suite always
passes -t 4, so the flag was silently ignored.

Rewrite -t to 1 in the darktable-cli shim instead, which owns the argument
list. The environment variable stays for the OpenMP users that do honour
it, rawspeed among them.

On 0000-nop under ThreadSanitizer: 326s to 17s, 1.3MB of reports to 88KB,
119 unique findings to 18.
All four *_OPTIONS share sanitizer_common's log_path, so whichever set is
parsed last decides where reports land -- and which that is differs by
runtime. A GCC address,undefined build wrote its ASan reports to
ubsan.<pid>, and a clang thread-only build wrote ThreadSanitizer reports
to ubsan-<pass>.<pid>. Both send you looking in the wrong file.

Gate each block on DT_SANITIZERS so the file name says what the report is.
LSAN_OPTIONS is kept for 'address' as well, since ASan embeds
LeakSanitizer.
@da-phil

da-phil commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

I'm pushing those changes in case somebody wants to play while I'm on vacation the next 2 weeks, have fun ;)

@da-phil da-phil changed the title Add runtime sanitizer support and integration test wrapper helper build: add runtime sanitizer support and integration test wrapper helper Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant