diff --git a/.gitignore b/.gitignore index 4fd5d97fd1a..30e8f694deb 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,7 @@ AppDir/ # personal, per-project agent instructions (see AGENTS.md) AGENTS.local.md + +# Python bytecode from the tools/ scripts +__pycache__/ +*.pyc diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f2a23b0506..1ebf3882180 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -251,6 +251,11 @@ endif() include(compiler-warnings) include(windows-macros) +# Optional runtime sanitizers, a no-op unless DT_SANITIZE is set. +# Must come before add_subdirectory(src) so that the compile/link options +# are inherited by darktable and by everything under src/external. +include(sanitizers) + # we need some external programs for building darktable message(STATUS "Looking for external programs") diff --git a/DefineOptions.cmake b/DefineOptions.cmake index 8806a7ed792..4d4eb6e2e0c 100644 --- a/DefineOptions.cmake +++ b/DefineOptions.cmake @@ -40,6 +40,12 @@ option(USE_OPENCV "Use OpenCV for HDR exposure-bracket auto-alignment." ON) option(FORCE_COLORED_OUTPUT "Always produce ANSI-colored output (GNU/Clang only)." OFF) option(USE_SDL2 "Enable SDL2 support" ON) +# Runtime sanitizers. Empty (the default) means no instrumentation at all and +# leaves every other build flag untouched. See cmake/sanitizers.cmake. +set(DT_SANITIZE "" CACHE STRING + "Runtime sanitizers to build with, comma separated: address, undefined, leak, thread") +option(DT_SANITIZE_EXTRA_CHECKS "Add the noisy clang-only UBSan checks (integer, implicit-conversion)" OFF) + if (USE_OPENCL) option(TESTBUILD_OPENCL_PROGRAMS "Test-compile OpenCL programs (needs LLVM and Clang 7+)" ON) else () diff --git a/build.sh b/build.sh index 4f6f30c7212..522b0874c1c 100755 --- a/build.sh +++ b/build.sh @@ -22,7 +22,7 @@ BUILD_DIR="$BUILD_DIR_DEFAULT" BUILD_GENERATOR_DEFAULT="Unix Makefiles" BUILD_GENERATOR="$BUILD_GENERATOR_DEFAULT" MAKE_TASKS=-1 -ADDRESS_SANITIZER=0 +SANITIZE="" DO_CLEAN_BUILD=0 DO_CLEAN_INSTALL=0 MANIFEST_FILE="$BUILD_DIR/install_manifest.txt" @@ -103,8 +103,16 @@ parse_args() feature=${option#--disable-} parse_feature "$feature" 0 ;; + --sanitize) + SANITIZE="$2" + shift + ;; + --sanitize=*) + SANITIZE="${option#--sanitize=}" + ;; --asan) - ADDRESS_SANITIZER=1 + # deprecated spelling, kept so existing muscle memory keeps working + SANITIZE="address,undefined" ;; --skip-config) DO_CONFIG=0 @@ -160,8 +168,15 @@ Build: -j --jobs Number of tasks (default: number of CPUs) - --asan Enable address sanitizer options + --sanitize Build with runtime sanitizers. Comma separated + list of: address, undefined, leak, thread. + 'address' implies leak checking; 'thread' cannot + be combined with 'address' or 'leak'. + Use a dedicated --build-dir, and prefer + --build-type RelWithDebInfo. + e.g. --sanitize address,undefined (default: disabled) + --asan Deprecated alias for --sanitize address,undefined Actual actions: --skip-build Configure but exit before building the binaries @@ -332,6 +347,7 @@ Installation prefix: $INSTALL_PREFIX Build type: $BUILD_TYPE Build generator: $BUILD_GENERATOR Build tasks: $MAKE_TASKS +Sanitizers: ${SANITIZE:-none} EOF @@ -364,14 +380,21 @@ fi mkdir -p "$BUILD_DIR" -if [ $ADDRESS_SANITIZER -ne 0 ] ; then - ASAN_FLAGS="CFLAGS=\"-fsanitize=address -fno-omit-frame-pointer\"" - ASAN_FLAGS="$ASAN_FLAGS CXXFLAGS=\"-fsanitize=address -fno-omit-frame-pointer\"" - ASAN_FLAGS="$ASAN_FLAGS LDFLAGS=\"-fsanitize=address\" " +if [ -n "$SANITIZE" ] ; then + # Passed as a -D option rather than as CFLAGS/CXXFLAGS/LDFLAGS environment + # variables: CMake only picks those up on the very first configure into an + # empty cache, so the old env-prefix approach silently produced an + # uninstrumented binary whenever an existing build dir was reused. + CMAKE_MORE_OPTIONS="$CMAKE_MORE_OPTIONS -DDT_SANITIZE=$SANITIZE" + + if [ "$BUILD_DIR" = "$BUILD_DIR_DEFAULT" ] ; then + log warn "Building with sanitizers into the default build directory." + log warn "Consider a dedicated one, e.g. --build-dir $DT_SRC_DIR/build-sanitize" + fi fi -cmd_config="${ASAN_FLAGS}cmake -G \"$BUILD_GENERATOR\" -DCMAKE_INSTALL_PREFIX=\"${INSTALL_PREFIX}\" -DCMAKE_BUILD_TYPE=${BUILD_TYPE} ${CMAKE_MORE_OPTIONS} ${CMAKE_OPTIONS_FROM_CMDLINE} \"$DT_SRC_DIR\"" +cmd_config="cmake -G \"$BUILD_GENERATOR\" -DCMAKE_INSTALL_PREFIX=\"${INSTALL_PREFIX}\" -DCMAKE_BUILD_TYPE=${BUILD_TYPE} ${CMAKE_MORE_OPTIONS} ${CMAKE_OPTIONS_FROM_CMDLINE} \"$DT_SRC_DIR\"" cmd_build="cmake --build \"$BUILD_DIR\" -- -j$MAKE_TASKS" cmd_install="${SUDO}cmake --build \"$BUILD_DIR\" --target install -- -j$MAKE_TASKS" diff --git a/cmake/sanitizers.cmake b/cmake/sanitizers.cmake new file mode 100644 index 00000000000..700f2ab5b8e --- /dev/null +++ b/cmake/sanitizers.cmake @@ -0,0 +1,193 @@ +# Runtime sanitizer support for darktable. +# +# Driven by the DT_SANITIZE cache variable (see DefineOptions.cmake), which +# takes a comma or semicolon separated list of: +# +# address AddressSanitizer, implies LeakSanitizer (~2x slower, ~3x RSS) +# undefined UndefinedBehaviorSanitizer (~1.2x slower) +# leak LeakSanitizer standalone (~native speed) +# thread ThreadSanitizer (~5-15x slower) +# +# Unlike rawspeed, which models this as dedicated CMAKE_BUILD_TYPEs +# (Sanitize/TSan, see src/external/rawspeed/cmake/build-type.cmake), we apply +# the instrumentation as an orthogonal set of compile/link options. That way it +# composes with RelWithDebInfo, it does not need a remap in +# src/external/CMakeLists.txt for rawspeed's build type whitelist, and it does +# not collide with the CMAKE_C_FLAGS_SANITIZE cache entries rawspeed FORCEs. +# +# This file must be included from the top level CMakeLists.txt before +# add_subdirectory(src), so that the directory-inherited options reach both +# darktable itself and everything under src/external. + +if(NOT DT_SANITIZE) + return() +endif() + +include(CheckCSourceCompiles) + +# --------------------------------------------------------------------------- +# Parse and validate the requested set +# --------------------------------------------------------------------------- + +string(TOLOWER "${DT_SANITIZE}" _dt_san_input) +string(REPLACE "," ";" _dt_san_list "${_dt_san_input}") +list(REMOVE_ITEM _dt_san_list "") +list(REMOVE_DUPLICATES _dt_san_list) + +set(_dt_san_known address undefined leak thread) +foreach(_san IN LISTS _dt_san_list) + if(NOT _san IN_LIST _dt_san_known) + if(_san STREQUAL "memory") + message(FATAL_ERROR + "DT_SANITIZE: MemorySanitizer is not supported. It only produces usable " + "results when every dependency (glib, GTK, lcms2, exiv2, libjpeg, ...) is " + "MSan-instrumented too; against stock system libraries it reports nothing " + "but false positives.") + endif() + message(FATAL_ERROR + "DT_SANITIZE: unknown sanitizer '${_san}'. Known values: ${_dt_san_known}") + endif() +endforeach() + +if("thread" IN_LIST _dt_san_list AND "address" IN_LIST _dt_san_list) + message(FATAL_ERROR "DT_SANITIZE: 'thread' and 'address' are mutually exclusive") +endif() +if("thread" IN_LIST _dt_san_list AND "leak" IN_LIST _dt_san_list) + message(FATAL_ERROR "DT_SANITIZE: 'thread' and 'leak' are mutually exclusive") +endif() +if("address" IN_LIST _dt_san_list AND "leak" IN_LIST _dt_san_list) + message(FATAL_ERROR + "DT_SANITIZE: 'address' already includes LeakSanitizer, drop 'leak'. " + "Use 'leak' on its own if you want leak checking without ASan's slowdown.") +endif() + +# --------------------------------------------------------------------------- +# Assemble the flags +# --------------------------------------------------------------------------- + +# Keep stack traces readable and complete. +set(_dt_san_flags -fno-omit-frame-pointer -fno-optimize-sibling-calls) +# -g is already added unconditionally in src/CMakeLists.txt. + +if("address" IN_LIST _dt_san_list) + # -fno-common so that globals get redzones, -U_FORTIFY_SOURCE because the + # distro default conflicts with ASan's interceptors. + # + # -fsanitize-recover=address is what gives ASAN_OPTIONS' halt_on_error=0 any + # effect. Without it every ASan finding is a hard abort, so one recoverable + # over-read during start-up ends the run and hides everything behind it. It + # belongs here rather than only in the 'undefined' branch below, so that a + # plain -DDT_SANITIZE=address build recovers as well. Genuinely fatal faults + # (SIGSEGV, allocator failures) still terminate the process. + list(APPEND _dt_san_flags -fsanitize=address -fno-common -U_FORTIFY_SOURCE + -fsanitize-recover=address) + if(CMAKE_C_COMPILER_ID MATCHES "Clang") + list(APPEND _dt_san_flags -fsanitize-address-use-after-scope) + endif() +endif() + +if("undefined" IN_LIST _dt_san_list) + # -fsanitize-recover=all keeps the process alive after a finding, so a single + # run of the test suite collects every report instead of dying on the first. + # vptr is off because the dlopen'd C++ modules are built with hidden + # visibility (cmake/manage-symbol-visibility.cmake), which makes the check + # unreliable across module boundaries. + list(APPEND _dt_san_flags -fsanitize=undefined -fno-sanitize=vptr -fsanitize-recover=all) + if(DT_SANITIZE_EXTRA_CHECKS) + if(CMAKE_C_COMPILER_ID MATCHES "Clang") + list(APPEND _dt_san_flags + -fsanitize=integer,implicit-conversion + -fno-sanitize=unsigned-shift-base) + else() + message(WARNING + "DT_SANITIZE_EXTRA_CHECKS is clang-only and has no effect with " + "${CMAKE_C_COMPILER_ID}") + endif() + endif() +endif() + +if("leak" IN_LIST _dt_san_list) + list(APPEND _dt_san_flags -fsanitize=leak) +endif() + +if("thread" IN_LIST _dt_san_list) + list(APPEND _dt_san_flags -fsanitize=thread) +endif() + +# --------------------------------------------------------------------------- +# Fail early and clearly if the runtime libraries are missing +# --------------------------------------------------------------------------- + +string(REPLACE ";" " " _dt_san_flags_str "${_dt_san_flags}") +set(CMAKE_REQUIRED_FLAGS "${_dt_san_flags_str}") +set(CMAKE_REQUIRED_LINK_OPTIONS ${_dt_san_flags}) +check_c_source_compiles("int main(void) { return 0; }" DT_SANITIZE_USABLE) +unset(CMAKE_REQUIRED_FLAGS) +unset(CMAKE_REQUIRED_LINK_OPTIONS) + +if(NOT DT_SANITIZE_USABLE) + message(FATAL_ERROR + "DT_SANITIZE=${DT_SANITIZE}: the compiler accepted the flags but linking " + "failed. The sanitizer runtime libraries are probably not installed " + "(libasan/libubsan/libtsan/liblsan for GCC, compiler-rt for Clang). " + "Tried: ${_dt_san_flags_str}") +endif() + +# --------------------------------------------------------------------------- +# Apply +# --------------------------------------------------------------------------- + +add_compile_options(${_dt_san_flags}) +add_link_options(${_dt_san_flags}) + +# --------------------------------------------------------------------------- +# Warn about build type combinations that quietly weaken the instrumentation +# --------------------------------------------------------------------------- + +if("undefined" IN_LIST _dt_san_list AND CMAKE_BUILD_TYPE MATCHES "^[Rr][Ee][Ll][Ee][Aa][Ss][Ee]$") + message(WARNING + "DT_SANITIZE includes 'undefined' but CMAKE_BUILD_TYPE is Release, which " + "adds -ffast-math -fno-finite-math-only. The compiler is then allowed to " + "assume no NaN/Inf, making UBSan's floating point checks unreliable. " + "Use -DCMAKE_BUILD_TYPE=RelWithDebInfo instead.") +endif() + +if(CMAKE_BUILD_TYPE MATCHES "^[Dd][Ee][Bb][Uu][Gg]$" + AND ("address" IN_LIST _dt_san_list OR "leak" IN_LIST _dt_san_list)) + message(WARNING + "DT_SANITIZE includes 'address'/'leak' but CMAKE_BUILD_TYPE is Debug, which " + "defines _DEBUG. dt_alloc_aligned() then over-allocates by one cacheline and " + "hands out an interior pointer (src/common/darktable.c), so ASan's redzones " + "no longer sit next to the user buffer and small over/underflows go " + "undetected. Use -DCMAKE_BUILD_TYPE=RelWithDebInfo instead.") +endif() + +# --------------------------------------------------------------------------- +# Generate the runtime environment helper +# --------------------------------------------------------------------------- + +# Distributions commonly ship only the version-suffixed binary (Debian and +# Ubuntu put llvm-symbolizer-N in /usr/bin and leave the unsuffixed name to the +# llvm meta-package), so look for both. Without it the runtimes fall back to +# addr2line, which is far slower and resolves fewer frames. +find_program(DT_LLVM_SYMBOLIZER + NAMES llvm-symbolizer + llvm-symbolizer-21 llvm-symbolizer-20 llvm-symbolizer-19 + llvm-symbolizer-18 llvm-symbolizer-17 llvm-symbolizer-16 + llvm-symbolizer-15 llvm-symbolizer-14) +if(NOT DT_LLVM_SYMBOLIZER) + set(DT_LLVM_SYMBOLIZER "") + message(STATUS "llvm-symbolizer not found, sanitizer stack traces will be slower to symbolize") +endif() + +string(REPLACE ";" "," DT_SANITIZE_ACTIVE "${_dt_san_list}") +set(DT_SANITIZE_SUPPRESSION_DIR "${CMAKE_SOURCE_DIR}/tools/sanitizer") + +configure_file( + "${CMAKE_SOURCE_DIR}/tools/sanitizer/sanitizer-env.sh.in" + "${DARKTABLE_BINDIR}/sanitizer-env.sh" + @ONLY) + +message(STATUS "Sanitizers: ${DT_SANITIZE_ACTIVE}") +message(STATUS "Sanitizer flags: ${_dt_san_flags_str}") +message(STATUS "Sanitizer runtime env: ${DARKTABLE_BINDIR}/sanitizer-env.sh") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 671b64ca268..84e18928cf6 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -937,7 +937,11 @@ find_package(Pugixml 1.5 REQUIRED) include_directories(SYSTEM ${Pugixml_INCLUDE_DIRS}) list(APPEND LIBS ${Pugixml_LIBRARIES}) -if(NOT SOURCE_PACKAGE) +# Sanitizer instrumentation perturbs the optimizer and reliably produces fresh +# -Wmaybe-uninitialized / -Wstringop-* diagnostics. With -Werror -Wfatal-errors +# those abort the build before a single sanitizer has had a chance to run, so +# keep the warnings but drop the promotion to error for sanitizer builds. +if(NOT SOURCE_PACKAGE AND NOT DT_SANITIZE) add_definitions(-Werror -Wfatal-errors ) endif() diff --git a/tools/check-iop-legacy-params.py b/tools/check-iop-legacy-params.py new file mode 100755 index 00000000000..ebe8def0f92 --- /dev/null +++ b/tools/check-iop-legacy-params.py @@ -0,0 +1,168 @@ +#!/usr/bin/env python3 +"""Check each iop's versioned legacy parameter structs against real stored data. + +When a module's parameters change, the old layout is preserved as a +dt_iop__params_v_t struct inside legacy_params(), and history entries +written by older darktable versions are converted through it. Nothing checks +that the struct still matches what those versions actually wrote. When it drifts, +legacy_params() reads a struct's worth of bytes out of a blob that is shorter, +running off the end of the allocation dt_iop_legacy_params() sized from the +stored data. Both src/iop/highlights.c and src/iop/denoiseprofile.c did this. + +The two sides are never visible together at compile time -- the on-disk size is +a runtime property of user data -- but both are recoverable afterwards: + + * the struct sizes are in the DWARF of the built plugins, including the + function-local typedefs, which is why this reads objdump rather than gdb; + * the sizes actually written are in the history entries of the integration + test XMPs, which cover a wide range of darktable versions. + +Usage: + tools/check-iop-legacy-params.py [xmp-dir] + +xmp-dir defaults to src/tests/integration. Exits 1 on a size mismatch, 2 if +nothing could be checked. Missing structs are reported as warnings only, see +below. +""" + +import base64 +import collections +import glob +import os +import re +import sys +import zlib + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from dwarf_types import TypeSizes # noqa: E402 + +HISTORY_ENTRY = re.compile(r"]*?/>", re.S) +ATTR = {name: re.compile(r'%s="([^"]*)"' % name) + for name in ("operation", "modversion", "params")} +VERSIONED = re.compile(r"^dt_iop_[a-z0-9_]+_params_v(\d+)_t$") + + +def decode(payload): + """Undo dt_exif_xmp_encode: plain hex, or "gz" + factor + base64 + zlib.""" + if payload.startswith("gz"): + return zlib.decompress(base64.b64decode(payload[4:])) + return bytes.fromhex(payload) + + +def stored_sizes(xmp_dir): + """{(operation, version): {sizes seen}} across every test XMP.""" + corpus = collections.defaultdict(set) + for path in sorted(glob.glob(os.path.join(xmp_dir, "0*", "*.xmp"))): + text = open(path, errors="replace").read() + for entry in HISTORY_ENTRY.finditer(text): + fields = {k: r.search(entry.group(0)) for k, r in ATTR.items()} + if not all(fields.values()): + continue + try: + size = len(decode(fields["params"].group(1))) + version = int(fields["modversion"].group(1)) + except (ValueError, zlib.error, base64.binascii.Error): + continue + corpus[(fields["operation"].group(1), version)].add(size) + return corpus + + +def plugin_index(build_dir): + index = {} + for root, dirs, names in os.walk(build_dir): + dirs[:] = [d for d in dirs if d != "CMakeFiles"] + for name in names: + if name.startswith("lib") and name.endswith(".so"): + index.setdefault(name[3:-3], os.path.join(root, name)) + return index + + +def main(): + if not 2 <= len(sys.argv) <= 3: + sys.stderr.write("usage: %s [xmp-dir]\n" + % os.path.basename(sys.argv[0])) + return 2 + + build_dir = os.path.abspath(sys.argv[1]) + source_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir) + xmp_dir = os.path.abspath(sys.argv[2] if len(sys.argv) > 2 + else os.path.join(source_dir, "src/tests/integration")) + + if not os.path.isdir(build_dir): + sys.stderr.write("error: no such build directory: %s\n" % build_dir) + return 2 + if not os.path.isdir(xmp_dir): + sys.stderr.write( + "error: no XMP corpus at %s\n" + " git submodule update --init src/tests/integration\n" % xmp_dir) + return 2 + + print("build : %s" % build_dir) + print("XMPs : %s\n" % xmp_dir) + + corpus = stored_sizes(xmp_dir) + if not corpus: + sys.stderr.write("error: no history entries found under %s\n" % xmp_dir) + return 2 + + plugins = plugin_index(build_dir) + if not plugins: + sys.stderr.write("error: no plugins under %s -- not a built tree.\n" % build_dir) + return 2 + + errors, warnings, checked = [], [], 0 + + for operation in sorted({op for op, _ in corpus}): + plugin = plugins.get(operation) + if not plugin: + continue + types = TypeSizes(plugin) + if not types.usable(): + continue + compiled = {int(VERSIONED.match(n).group(1)): size + for n, size in types.matching(VERSIONED).items()} + if not compiled: + continue + + for (op, version), sizes in sorted(corpus.items()): + if op != operation: + continue + checked += 1 + if version in compiled: + if compiled[version] not in sizes: + errors.append( + "%s v%d: struct is %d bytes, but stored history is %s" + % (op, version, compiled[version], + " or ".join(str(s) for s in sorted(sizes)))) + else: + warnings.append( + "%s v%d: %s bytes stored, no _params_v%d_t in debug info" + % (op, version, " or ".join(str(s) for s in sorted(sizes)), + version)) + + for line in errors: + print("error: %s" % line) + if errors: + print(" legacy_params() reads sizeof(struct) from a blob that size,") + print(" so the conversion runs off the end of the allocation.\n") + + # A local type the compiler had no reason to emit is indistinguishable here + # from one that was never written, so this half cannot be an error. It still + # points at real defects: denoiseprofile's v10 struct was #if 0'd out, which + # is what sent its conversion through the current struct instead. + for line in warnings: + print("warning: %s" % line) + + print("\n%d (operation, version) pair(s) checked, %d error(s), %d warning(s)" + % (checked, len(errors), len(warnings))) + + if not checked: + sys.stderr.write("\nerror: nothing could be checked -- no plugin in %s\n" + " matched an operation in the corpus, or the\n" + " plugins carry no debug info.\n" % build_dir) + return 2 + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/check-iop-pipe-data-sizes.py b/tools/check-iop-pipe-data-sizes.py new file mode 100755 index 00000000000..ff11922ae20 --- /dev/null +++ b/tools/check-iop-pipe-data-sizes.py @@ -0,0 +1,165 @@ +#!/usr/bin/env python3 +"""Check that every iop can safely use the default pixelpipe data allocation. + +dt_iop_module_t modules that do not provide their own init_pipe() get +default_init_pipe() (src/develop/imageop.c), which allocates piece->data as + + calloc(1, self->params_size) + +that is, by the size of the module's *params* struct. Modules then use +piece->data as their *data* struct. That is only sound while + + sizeof(dt_iop__data_t) <= sizeof(dt_iop__params_t) + +and nothing in the build enforces it. When a module grows a data struct larger +than its params struct - an extra precomputed field is enough - commit_params() +writes past the end of the allocation and process() reads it back, silently, on +every pixelpipe run. src/iop/contrastntexture.c did exactly this. + +The two sizes are never both visible to the compiler in one place: params_size +is a runtime field and the data struct is module-local, so this cannot be a +static_assert in shared code. They are both in the DWARF of the built plugin, +which is what this script reads, via tools/dwarf_types.py. + +Usage: + tools/check-iop-pipe-data-sizes.py [build-dir] + +Exits non-zero if any module would overflow. +""" + +import glob +import os +import re +import sys + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from dwarf_types import TypeSizes # noqa: E402 + +INTROSPECTION = re.compile( + r"DT_MODULE_INTROSPECTION\(\s*\d+\s*,\s*(dt_iop_[A-Za-z0-9_]+_params_t)\s*\)") +# A module that defines init_pipe allocates piece->data itself and is exempt. +OWN_INIT_PIPE = re.compile(r"^\s*void\s+init_pipe\s*\(", re.MULTILINE) + +# The type a module actually reads piece->data as. Most follow the +# dt_iop__data_t convention, but not all -- blurs and primaries use their +# params struct directly -- so take the name from the code rather than assume. +PIECE_DATA_USES = ( + re.compile(r"(dt_[A-Za-z0-9_]+_t)\s*\*(?:\s*const)?\s*[A-Za-z0-9_]+\s*=\s*piece->data"), + re.compile(r"\(\s*(dt_[A-Za-z0-9_]+_t)\s*\*\s*\)\s*piece->data"), +) + + +def plugin_index(build_dir): + """Map plugin basename -> path, by searching the tree once. + + The usual location is /lib/darktable/plugins, but searching keeps + this working for other layouts instead of silently finding nothing. + """ + index = {} + for root, dirs, names in os.walk(build_dir): + dirs[:] = [d for d in dirs if d != "CMakeFiles"] + for name in names: + if name.startswith("lib") and name.endswith(".so"): + index.setdefault(name[3:-3], os.path.join(root, name)) + return index + + +def main(): + build_dir = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else "build") + source_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir) + + if not os.path.isdir(build_dir): + sys.stderr.write( + "error: no such build directory: %s\n" + "usage: %s [build-dir] (needs a tree built with debug info)\n" + % (build_dir, os.path.basename(sys.argv[0]))) + return 2 + + print("source: %s" % os.path.normpath(source_dir)) + print("build : %s\n" % build_dir) + + plugins = plugin_index(build_dir) + if not plugins: + sys.stderr.write( + "error: no plugins under %s -- that is not a built darktable tree.\n" + " Pass the build directory, e.g. build-sanitize.\n" % build_dir) + return 2 + + violations, checked, skipped = [], 0, [] + exempt = 0 + + for path in sorted(glob.glob(os.path.join(source_dir, "src/iop/*.c"))): + module = os.path.basename(path)[:-2] + text = open(path, errors="replace").read() + + params_match = INTROSPECTION.search(text) + if not params_match: + continue # not an introspected pixelpipe module + if OWN_INIT_PIPE.search(text): + exempt += 1 # allocates piece->data itself + continue + + params_type = params_match.group(1) + + # Every distinct type this module reads piece->data as, plus the + # conventional name in case the module only uses it indirectly. + candidates = {params_type[: -len("_params_t")] + "_data_t"} + for pattern in PIECE_DATA_USES: + candidates.update(pattern.findall(text)) + candidates.discard(params_type) # using params directly is always safe + + plugin = plugins.get(module) + if not plugin: + # Example modules such as useless.c are not built by default. + skipped.append("%s (not built)" % module) + continue + + types = TypeSizes(plugin) + params_size = types.size(params_type) + if params_size is None: + skipped.append("%s (no debug info for %s -- build with -g?)" + % (module, params_type)) + continue + + resolved = {t: types.size(t) for t in sorted(candidates)} + resolved = {t: n for t, n in resolved.items() if n is not None} + if not resolved: + # Reads piece->data as its params struct, or not at all. Safe. + skipped.append("%s (no pipe data struct of its own)" % module) + continue + + checked += 1 + for data_type, data_size in sorted(resolved.items()): + if data_size > params_size: + violations.append((module, data_type, data_size, + params_type, params_size)) + + for module, dtype, dsize, ptype, psize in violations: + print("%s: %s is %d bytes but piece->data is allocated as %d " + "(sizeof %s)" % (module, dtype, dsize, psize, ptype)) + print(" add an init_pipe()/cleanup_pipe() pair allocating %s," + % dtype) + print(" as src/iop/toneequal.c does.") + + print("%d module(s) checked, %d overflowing " + "(%d exempt: own init_pipe, %d skipped)" + % (checked, len(violations), exempt, len(skipped))) + if skipped and os.environ.get("VERBOSE"): + for entry in skipped: + print(" skipped: %s" % entry) + + # Checking nothing is not a clean result, it is no result. Say so rather + # than exiting 0, which reads as "nothing to fix". + sys.stdout.flush() + if not checked: + sys.stderr.write( + "\nerror: no module could be checked. Either %s is not a darktable\n" + " build, or its plugins carry no debug info -- build with\n" + " -DCMAKE_BUILD_TYPE=RelWithDebInfo.\n" % build_dir) + return 2 + + return 1 if violations else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/dwarf_types.py b/tools/dwarf_types.py new file mode 100644 index 00000000000..012be1bce33 --- /dev/null +++ b/tools/dwarf_types.py @@ -0,0 +1,105 @@ +"""Read C type sizes out of a build's DWARF, using only binutils objdump. + +Why not gdb: it resolves types through a lookup scope, which cannot reach the +function-local typedefs iop modules use for their legacy parameter versions +("No symbol ... in current context"). objdump dumps every DIE regardless of +scope, so those are readable. + +Why not pyelftools: objdump ships with binutils, which is already needed to +build darktable, and following DW_AT_type by offset is all the resolution +required here -- no new dependency for the callers. + +Needs a build with debug info; RelWithDebInfo is enough. +""" + +import re +import subprocess + +_DIE = re.compile(r"^\s*<\d+><([0-9a-f]+)>:.*\((DW_TAG_\w+)\)") +# The name is either an indirect string ("...offset: 0x1234): foo") or inline. +_NAME = re.compile(r"DW_AT_name\s*:\s*(?:\(indirect string[^)]*\):\s*)?(\S+)\s*$") +_SIZE = re.compile(r"DW_AT_byte_size\s*:\s*(\d+)") +_TYPE = re.compile(r"DW_AT_type\s*:\s*<0x([0-9a-f]+)>") + +# A named struct carries its own size; a typedef of an anonymous struct does +# not, and has to be followed. Both spellings occur across the iop modules. +_CARRIERS = ("DW_TAG_structure_type", "DW_TAG_typedef", + "DW_TAG_const_type", "DW_TAG_volatile_type") + +MAX_CHAIN = 16 + + +class TypeSizes: + """sizeof() for the named types in one shared object.""" + + def __init__(self, shared_object): + self.path = shared_object + self._dies = {} + self._by_name = {} + self._parse() + + def _parse(self): + try: + dump = subprocess.run(["objdump", "--dwarf=info", self.path], + capture_output=True, text=True, + timeout=600).stdout + except (OSError, subprocess.SubprocessError): + return + + current = None + for line in dump.splitlines(): + header = _DIE.match(line) + if header: + current = {"tag": header.group(2), "name": None, + "size": None, "type": None} + self._dies[int(header.group(1), 16)] = current + continue + if current is None: + continue + name = _NAME.search(line) + if name: + current["name"] = name.group(1) + # Several DIEs can share a name (a struct tag and its typedef); + # keep the first that can yield a size. + self._by_name.setdefault(name.group(1), []).append(current) + size = _SIZE.search(line) + if size: + current["size"] = int(size.group(1)) + ref = _TYPE.search(line) + if ref: + current["type"] = int(ref.group(1), 16) + + def _resolve(self, die): + """Follow typedef and cv-qualifier chains to whatever carries a size.""" + seen = 0 + while die is not None and seen < MAX_CHAIN: + if die["size"] is not None: + return die["size"] + if die["type"] is None: + return None + die = self._dies.get(die["type"]) + seen += 1 + return None + + def size(self, type_name): + """sizeof(type_name), or None if it is not in this object's DWARF.""" + for die in self._by_name.get(type_name, ()): + if die["tag"] in _CARRIERS: + resolved = self._resolve(die) + if resolved is not None: + return resolved + return None + + def matching(self, pattern): + """{name: size} for every type whose name matches a compiled regex.""" + found = {} + for name in self._by_name: + if pattern.match(name): + size = self.size(name) + if size is not None: + found[name] = size + return found + + def usable(self): + """False when the object carries no debug info worth reading.""" + return bool(self._dies) diff --git a/tools/run-integration-tests.sh b/tools/run-integration-tests.sh new file mode 100755 index 00000000000..736c0b581c5 --- /dev/null +++ b/tools/run-integration-tests.sh @@ -0,0 +1,401 @@ +#!/bin/bash +# +# Run the darktable integration test suite against a sanitizer build and +# collect the sanitizer findings. +# +# The suite itself lives in src/tests/integration, which is a separate upstream +# repository (darktable-org/darktable-tests). This driver therefore does its +# work from the outside, without patching anything in there: +# +# * it points DARKTABLE_CLI at the sanitizer build (the suite's 'run' honours +# that environment variable), +# * it sources the generated sanitizer-env.sh, whose log_path settings are +# what rescue the reports from the "2> /dev/null" in the suite's call(), +# * it interposes a shim so that a hung darktable-cli hits a timeout -- the +# suite has no timeout of its own anywhere, +# * it preflights the binary, so that a sanitizer runtime which cannot even +# start is reported as such instead of as every test failing, +# * and it aggregates and deduplicates the reports afterwards. +# +# Usage: +# ./tools/run-integration-tests.sh [options] [test names passed to ./run] +# +# Options: +# --build-dir sanitizer build tree (default: build-sanitize) +# --log-dir where reports land (default: /sanitizer-logs/) +# --timeout per darktable-cli call (default: 1800) +# --with-opencl also run the GPU pass (default: CPU pass only) +# --no-openmp serialise OpenMP (rewrites the suite's -t to 1) +# --no-aslr force ASLR off for the CLI (setarch -R) +# --keep-aslr never disable ASLR, even if the runtime needs it +# -h, --help this message +# +# --no-openmp is mostly for ThreadSanitizer. 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 -- in one full-suite +# run, 97% of all reports. Serialising the OpenMP loops removes that at the +# source instead of trying to suppress it afterwards, leaving darktable's own +# threading (pixelpipe, caches, lua) visible. +# +# ASLR handling defaults to auto: the preflight only falls back to setarch -R +# when the sanitizer runtime turns out to need it. +# +# Everything not recognised is passed straight through to the suite's ./run, +# so a single test can be selected by name: +# +# ./tools/run-integration-tests.sh --build-dir build-asan 0001-exposure + +set -u + +DT_SRC_DIR=$(dirname "$0") +DT_SRC_DIR=$(cd "$DT_SRC_DIR/.." && pwd -P) + +BUILD_DIR="$DT_SRC_DIR/build-sanitize" +LOG_DIR="" +CLI_TIMEOUT=1800 +WITH_OPENCL=0 +NO_OPENMP=0 +ASLR_MODE=auto +RUN_ARGS=() + +while [ "$#" -ge 1 ]; do + case "$1" in + --build-dir) BUILD_DIR="$2"; shift ;; + --build-dir=*) BUILD_DIR="${1#--build-dir=}" ;; + --log-dir) LOG_DIR="$2"; shift ;; + --log-dir=*) LOG_DIR="${1#--log-dir=}" ;; + --timeout) CLI_TIMEOUT="$2"; shift ;; + --timeout=*) CLI_TIMEOUT="${1#--timeout=}" ;; + --with-opencl) WITH_OPENCL=1 ;; + --no-openmp) NO_OPENMP=1 ;; + --no-aslr) ASLR_MODE=off ;; + --keep-aslr) ASLR_MODE=keep ;; + # Print the header block, however long it happens to be. + -h|--help) awk 'NR>2 && /^#/ { sub(/^# ?/, ""); print; next } + NR>2 { exit }' "$0"; exit 0 ;; + *) RUN_ARGS+=("$1") ;; + esac + shift +done + +case "$BUILD_DIR" in + /*) ;; + *) BUILD_DIR="$(cd "$(dirname "$BUILD_DIR")" 2>/dev/null && pwd -P)/$(basename "$BUILD_DIR")" ;; +esac + +SUITE_DIR="$DT_SRC_DIR/src/tests/integration" +REAL_CLI="$BUILD_DIR/bin/darktable-cli" +SAN_ENV="$BUILD_DIR/bin/sanitizer-env.sh" + +die() { echo "error: $*" >&2; exit 1; } + +[ -x "$REAL_CLI" ] || die "no darktable-cli in $BUILD_DIR/bin -- build it first, e.g. + ./build.sh --build-dir $BUILD_DIR --sanitize address,undefined --build-type RelWithDebInfo" +[ -d "$SUITE_DIR" ] && [ -x "$SUITE_DIR/run" ] || die "integration test suite not checked out. Run: + git submodule update --init src/tests/integration" + +if [ ! -f "$SAN_ENV" ]; then + die "$SAN_ENV is missing -- $BUILD_DIR was not configured with -DDT_SANITIZE" +fi + +if [ -z "$LOG_DIR" ]; then + LOG_DIR="$BUILD_DIR/sanitizer-logs/$(date +%Y%m%d-%H%M%S)" +fi +mkdir -p "$LOG_DIR" + +DT_SAN_LOGDIR="$LOG_DIR" +export DT_SAN_LOGDIR +# shellcheck source=/dev/null +. "$SAN_ENV" + +# The suite hardcodes --configdir /tmp/darktable-test and shares it across runs. +# A library.db left behind by a non-sanitized run can change what gets processed, +# so start from a clean one. +rm -rf /tmp/darktable-test + +# --------------------------------------------------------------------------- +# Preflight +# --------------------------------------------------------------------------- +# +# A sanitizer runtime that cannot lay out its shadow memory dies inside its own +# initialiser, before main(). Every test then fails in a fraction of a second +# with "darktable-cli errored", which reads like a darktable bug and is not one. +# Catch that here, up front, with a message that says what actually happened. +# +# The usual cause is ASLR entropy: kernels default vm.mmap_rnd_bits to 32 while +# GCC's runtimes support at most 28. Running the child with randomisation off +# works around it without root, and costs a test run nothing. + +# The preflight output has to come back on stderr, so drop the log_path that +# would otherwise divert it into a file and pollute the report directory. +strip_log_path() { + printf '%s' "$1" | sed -e 's/log_path=[^:]*://' -e 's/:log_path=[^:]*//' +} + +run_preflight() { + # shellcheck disable=SC2086 # $1 is a command prefix and must word-split + ASAN_OPTIONS=$(strip_log_path "${ASAN_OPTIONS:-}") \ + TSAN_OPTIONS=$(strip_log_path "${TSAN_OPTIONS:-}") \ + UBSAN_OPTIONS=$(strip_log_path "${UBSAN_OPTIONS:-}") \ + LSAN_OPTIONS=$(strip_log_path "${LSAN_OPTIONS:-}") \ + $1 "$REAL_CLI" --version 2>&1 +} + +startup_failed() { + case "$1" in + *"unexpected memory mapping"*) return 0 ;; + *"Shadow memory range interleaves"*) return 0 ;; + *"FATAL: "*"Sanitizer"*) return 0 ;; + esac + return 1 +} + +# Whether the runtime survives is probabilistic: it depends on where the kernel +# happened to place a mapping, so a single probe proves nothing. Measured on +# this 7.x kernel with the default vm.mmap_rnd_bits=32, TSan started in 1 run +# out of 40 -- an unlucky single preflight would wave through a configuration in +# which almost every test then dies. +PREFLIGHT_PROBES=5 + +# Succeeds only when every probe started cleanly. The last failing output is +# left in PREFLIGHT for the diagnostic below. +probe_startup() { + local prefix="$1" i out + PREFLIGHT="" + for i in $(seq "$PREFLIGHT_PROBES"); do + out=$(run_preflight "$prefix") + if startup_failed "$out"; then + PREFLIGHT="$out" + return 1 + fi + done + return 0 +} + +SETARCH_PREFIX="" +ASLR_NOTE="on" +HAVE_SETARCH=0 +command -v setarch >/dev/null 2>&1 && HAVE_SETARCH=1 + +if [ "$ASLR_MODE" = off ]; then + [ "$HAVE_SETARCH" -eq 1 ] || die "--no-aslr needs setarch (util-linux)" + SETARCH_PREFIX="setarch -R" + ASLR_NOTE="off (forced)" +fi + +if ! probe_startup "$SETARCH_PREFIX"; then + if [ "$ASLR_MODE" = auto ] && [ "$HAVE_SETARCH" -eq 1 ] \ + && probe_startup "setarch -R"; then + SETARCH_PREFIX="setarch -R" + ASLR_NOTE="off (auto: the runtime needs it)" + else + case "$ASLR_MODE" in + keep) ADVICE=" +--keep-aslr suppressed the 'setarch -R' fallback, which works around this +without root." ;; + off) ADVICE=" +'setarch -R' was already in use and did not help, so ASLR entropy is not the +only thing wrong here." ;; + *) if [ "$HAVE_SETARCH" -eq 1 ]; then + ADVICE=" +'setarch -R' did not help either." + else + ADVICE=" +Installing util-linux would let this script fall back to 'setarch -R', which +works around this without root." + fi ;; + esac + + die "the sanitizer runtime does not start reliably ($PREFLIGHT_PROBES probes): + + $PREFLIGHT + +This is an environment problem rather than a darktable one: the runtime could +not lay out its shadow memory, so the tests would have failed without ever +running. It is probabilistic -- an occasional run that does start does not +contradict it. + +Lower the kernel's ASLR entropy to something the runtime supports: + + sudo sysctl -w vm.mmap_rnd_bits=28 + +and make it permanent with a line in /etc/sysctl.d/. +$ADVICE" + fi +fi + +# darktable-cli shim. It does three jobs: +# +# * timeout, because the suite has no timeout anywhere and under a sanitizer a +# hang would block the whole run indefinitely; +# * capture stderr, because log_path only rescues the reports that go through +# sanitizer_common. GCC's UBSan prints its non-fatal "runtime error:" +# diagnostics straight to stderr no matter what log_path says, and the suite +# runs the CLI as "$* 1> /dev/null 2> /dev/null", so without this they are +# lost. The two channels are complementary, not duplicated: with log_path +# set, ASan writes only to its log file; +# * file both channels under the test they belong to. A whole-suite run is +# ~180 tests, and a flat directory of stderr. and ubsan. files +# cannot be traced back to the test that produced them. +# +# The shim is generated in two halves: an unquoted here-document for the values +# this script has to bake in, then a quoted one for the body, so that the body +# needs no escaping and reads like the shell script it is. +SHIM="$LOG_DIR/darktable-cli" +{ +cat <>"$log_dir/stderr-$pass.$$" +EOF +} > "$SHIM" +chmod +x "$SHIM" + +DARKTABLE_CLI="$SHIM" +export DARKTABLE_CLI + +# Belt and braces: darktable itself ignores this (see the shim), but other +# OpenMP users in the process, rawspeed included, do honour it. +if [ "$NO_OPENMP" -eq 1 ]; then + OMP_NUM_THREADS=1 + export OMP_NUM_THREADS +fi + +OPENCL_ARG="--disable-opencl" +[ "$WITH_OPENCL" -eq 1 ] && OPENCL_ARG="" + +cat <&1 | tee "$SUITE_OUT" +ELAPSED=$(( $(date +%s) - START )) + +# The suite's ./run always exits 0, so read its own tally instead. Note that a +# failing test here is not necessarily a sanitizer finding: the expected.png +# references were produced by a -O3 -ffast-math Release build, so a +# RelWithDebInfo build drifts on some tests for reasons unrelated to +# instrumentation. +SUITE_TOTAL=$(sed -n 's/^Total test *\([0-9][0-9]*\)$/\1/p' "$SUITE_OUT" | tail -1) +SUITE_ERRORS=$(sed -n 's/^Errors *\([0-9][0-9]*\)$/\1/p' "$SUITE_OUT" | tail -1) +SUITE_TOTAL=${SUITE_TOTAL:-0} +SUITE_ERRORS=${SUITE_ERRORS:-0} + +printf '\nSuite finished in %dm%02ds: %s of %s tests reported a failure\n\n' \ + $((ELAPSED / 60)) $((ELAPSED % 60)) "$SUITE_ERRORS" "$SUITE_TOTAL" + +# --------------------------------------------------------------------------- +# Aggregate the sanitizer reports +# --------------------------------------------------------------------------- + +# One stderr capture per CLI invocation; most of them are empty. Sweep the +# per-test directories too, then drop any directory left with nothing in it, so +# that what remains is exactly the set of tests that reported something. +find "$LOG_DIR" -name 'stderr-*' -size 0 -delete 2>/dev/null +find "$LOG_DIR" -mindepth 1 -type d -empty -delete 2>/dev/null + +SUMMARY="$LOG_DIR/summary.txt" + +"$DT_SRC_DIR/tools/sanitizer/aggregate-reports.py" "$LOG_DIR" \ + --build-dir "$BUILD_DIR" --output "$SUMMARY" +FINDINGS_RC=$? + +# 2 means a runtime died during start-up: the suite ran, but part of it was +# never instrumented, so the findings below understate the truth. +if [ "$FINDINGS_RC" -eq 2 ]; then + echo + echo "warning: a sanitizer failed to start during this run, so the findings" + echo " above cover less than the whole suite." +fi + +echo "full reports: $LOG_DIR" +echo "summary: $SUMMARY" +echo "suite output: $SUITE_OUT" + +# Non-zero if the sanitizers found anything, or if the suite itself failed. +if [ "$FINDINGS_RC" -ne 0 ] || [ "$SUITE_ERRORS" -ne 0 ]; then + exit 1 +fi +exit 0 diff --git a/tools/sanitizer/README.md b/tools/sanitizer/README.md new file mode 100644 index 00000000000..4e02e04876d --- /dev/null +++ b/tools/sanitizer/README.md @@ -0,0 +1,306 @@ +# Running darktable under runtime sanitizers + +## Quick start + +```bash +# 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" +``` + +The run ends with a deduplicated summary of every sanitizer finding and writes +the full reports plus a `summary.txt` under +`build-sanitize/sanitizer-logs//`. + +Reports are filed under the test that produced them, and tagged with the pass +they came from, so a whole-suite run stays traceable: + +``` +sanitizer-logs/20260904-103244/ +├── 0000-nop/ +│ ├── stderr-cpu.147106 captured stderr (GCC's UBSan reports here) +│ └── ubsan-cpu.147121 whatever the runtime's log_path caught +├── 0002-local-contrast/ +│ └── ... +├── darktable-cli the generated shim, for reproducing by hand +├── suite-output.txt +└── summary.txt +``` + +Tests that report nothing leave no directory behind, so what remains is exactly +the set that found something. The summary names the tests each finding turned up +in, which is most of the triage: one test behaving differently from the other +180 is a very different problem from a defect every test walks into. + +## Available sanitizers + +`--sanitize` takes a comma separated list. Slowdowns are relative to an +uninstrumented build of the same build type. + +| Value | What it finds | Cost | Notes | +|---|---|---|---| +| `address` | heap/stack/global overflows, use-after-free, double free | ~2x CPU, ~3x RSS | includes LeakSanitizer, but leak reporting is off by default (see below) | +| `undefined` | signed overflow, bad shifts, misaligned access, invalid casts, null deref | ~1.2x CPU | cheapest useful signal, good default for CI | +| `leak` | memory leaks at exit | ~native | standalone LSan, without ASan's slowdown | +| `thread` | data races, lock-order inversions | ~5-15x CPU, ~5-10x RSS | see the OpenMP caveat below | + +Mutually exclusive: `thread` with either `address` or `leak`. `address` already +includes `leak`, so asking for both is rejected. + +MemorySanitizer is deliberately **not** offered. It only produces usable results +when every dependency (glib, GTK, lcms2, exiv2, libjpeg, ...) is MSan +instrumented as well; against stock system libraries it reports nothing but +false positives. + +## Recommended invocations + +Baseline for the full 177-test suite is roughly 32-40 minutes with both the CPU +and the OpenCL pass. `run-integration-tests.sh` skips the OpenCL pass by +default, which roughly halves the work before the sanitizer overhead applies. + +```bash +# the default: memory errors + undefined behaviour, ~45-60 min for the suite +./build.sh --build-dir "$PWD/build-asan" --sanitize address,undefined \ + --build-type RelWithDebInfo --build-generator Ninja --disable-opencl +./tools/run-integration-tests.sh --build-dir "$PWD/build-asan" + +# cheap enough for every commit, ~20-25 min +./build.sh --build-dir "$PWD/build-ubsan" --sanitize undefined ... + +# leak hunting, roughly baseline speed +./build.sh --build-dir "$PWD/build-lsan" --sanitize leak ... + +# races: far too slow for the whole suite, run named tests +./build.sh --build-dir "$PWD/build-tsan" --sanitize thread ... +./tools/run-integration-tests.sh --build-dir "$PWD/build-tsan" 0035-multiple-modules +``` + +## Things worth knowing + +**Use `RelWithDebInfo`.** `Release` adds `-O3 -ffast-math -fno-finite-math-only`, +which lets the compiler assume no NaN/Inf and makes UBSan's floating point +checks unreliable. `Debug` defines `_DEBUG`, under which `dt_alloc_aligned()` +over-allocates by a cacheline and hands out an interior pointer, so ASan's +redzones no longer sit next to the user buffer and small over/underflows go +undetected. The build warns about both. + +**Judge the run by the report count, not by OK/FAILS.** The suite's +`expected.png` references came from a `-O3 -ffast-math` Release build. +RelWithDebInfo is `-O2` without fast-math, so some tests drift against the +Delta-E threshold for reasons that have nothing to do with sanitizers. Also, +when a sanitizer does end a test the suite just prints `FAILS: darktable-cli +errored` without any detail; the detail is in the log directory. + +**Use `run-integration-tests.sh`, not `src/tests/integration/run` directly.** +The suite invokes darktable-cli as `$* 1> /dev/null 2> /dev/null`, and +sanitizers report on stderr, so driving it by hand throws every finding away. +Two mechanisms rescue them, and both are needed: the `log_path=` settings in +the generated `sanitizer-env.sh` catch everything that reports through +sanitizer_common (ASan, LSan, TSan), and the driver's darktable-cli shim +captures stderr per invocation, because GCC's UBSan prints its non-fatal +`runtime error:` diagnostics directly to stderr no matter what `log_path` says. + +**A sanitizer that refuses to start is almost always ASLR.** If the runtime +dies before `main()` with + +``` +FATAL: ThreadSanitizer: unexpected memory mapping 0x793e64c72000-0x793e65100000 +``` + +(ASan words it as `Shadow memory range interleaves with an existing memory +mapping`), the kernel is handing out mappings with more entropy than the +runtime's fixed shadow layout can accommodate. Kernels default +`vm.mmap_rnd_bits` to 32; GCC's runtimes support at most 28. Either lower it + +```bash +sudo sysctl -w vm.mmap_rnd_bits=28 # persist via /etc/sysctl.d/ +``` + +or run with randomisation off, which needs no root: + +```bash +setarch -R build-tsan/bin/darktable-cli --version +``` + +`run-integration-tests.sh` preflights the binary once and falls back to +`setarch -R` by itself when it sees this, reporting the choice as `ASLR: off +(auto: ...)` in its banner. Force it either way with `--no-aslr` / `--keep-aslr`. +Without the preflight the symptom is misleading: every test fails in a fraction +of a second with `FAILS: darktable-cli errored`, which looks like a darktable +bug rather than an environment one. + +**Leak reporting is off under `address`.** darktable-cli exits without tearing +down its GTK/glib/lua state, so exit-time leak reports would fire on every +single test. Build with `--sanitize leak` when you actually want to hunt leaks. + +**OpenCL is off by default.** `--with-opencl` makes the suite render every test +a second time on the GPU, into `output-cl.png` beside the CPU's `output.png`, +and then diff the two against the per-test `cpugpu.maxpix` threshold where one +exists. So it doubles the number of instrumented darktable-cli invocations, on +top of whatever the sanitizer already costs. It also couples the two passes: +`run` sums both exit codes, so a failure in either one marks the whole test as +an error and skips the Delta-E comparison for both. + +The more important cost is signal quality. The ICD loader and the vendor driver +behind it are closed, uninstrumented binaries, but ASan still intercepts their +allocations, so their internal caches, worker pools and JIT buffers surface as +findings in code you neither own nor can fix. + +Which sanitizer actually pays for that depends on the build: + +* `address` builds run with `detect_leaks=0` (see above), so leaks are never + reported and `lsan.supp` is not consulted at all. What can still reach you is + driver-internal noise reported through ASan's interceptors. +* `leak` builds are where `lsan.supp` earns its keep, and where the GPU pass + will turn up allocations you have to add to it. + +To extend it, run the suite, find the `Direct leak` / `Indirect leak` entries in +the log directory, pick a frame that identifies the owning library, and add one +line per owner: + +``` +leak:libMyVendorOpenCL +``` + +The syntax is `leak:`, matched against function names and against +source and library paths anywhere in the leak's stack. Prefer the library soname +over a specific symbol: vendor symbols get renamed between driver releases, +paths generally do not. Keep each entry narrow enough to stay honest -- +`leak:libOpenCL.so` is a fair suppression, `leak:malloc` would bury darktable's +own leaks along with the driver's. + +The usual failure mode is a suppression that silently never matches, because a +typo just means the substring is not found. LSan can tell you which entries +fired, but the generated `sanitizer-env.sh` sets `print_suppressions=0`; change +it in `sanitizer-env.sh.in` and re-run CMake to regenerate, then check that each +new entry reports a non-zero count. + +The entries already in `lsan.supp` cover the ICD loader plus the AMD, Intel and +NVIDIA runtimes. A Mesa (rusticl) or POCL stack allocates through different +libraries and will need its own. + +Under `thread` the GPU pass is not worth attempting. The driver's own worker +threads carry no TSan annotations, so their synchronisation is invisible and +nearly every OpenCL call turns into a reported race. + +**ASan does not stop at the first finding.** The build passes +`-fsanitize-recover=address` and the environment sets `halt_on_error=0`, so a +recoverable error is reported and execution continues. Both halves are required: +without the compile flag every ASan finding is a hard abort no matter what +`halt_on_error` says. This matters because a single over-read during start-up +would otherwise end every test in the suite at the same place and hide +everything behind it. Errors ASan cannot recover from -- SIGSEGV, allocator +failures -- still end the process. A defect inside a loop reports once per +iteration, which is what the aggregator's deduplication is for. + +**TSan exits 0 on findings, on purpose.** Sanitizers default to `exitcode=66` +once they have reported anything, and the suite treats any non-zero exit as +`FAILS: darktable-cli errored` and never compares the images. A race does not +necessarily corrupt the export -- `output.png` is written normally -- so leaving +the default would discard the suite's own signal on every TSan test. The +generated `sanitizer-env.sh` therefore sets `exitcode=0` for TSan only. Nothing +is hidden: the reports still land in the log directory, and +`run-integration-tests.sh` still exits non-zero whenever the aggregator finds +anything. A genuine crash still exits non-zero by itself. ASan keeps the default, +because there `halt_on_error=1` means the run really was cut short. + +**TSan against a GCC build needs the suppressions here.** GCC's libgomp is not +built with TSan annotations, so TSan cannot see the happens-before edges that +OpenMP barriers establish and reports a race on essentially every parallel loop. +`tsan.supp` filters those. Clang with an annotated libomp (or archer) does not +need them. + +**Clang builds need libomp.** CMake's `FindOpenMP` tries `-fopenmp=libomp` +first, so a clang build needs the matching `libomp--dev` package +installed. GCC works out of the box. + +**`-Werror` is dropped for sanitizer builds.** Instrumentation perturbs the +optimizer and reliably produces fresh `-Wmaybe-uninitialized` / `-Wstringop-*` +diagnostics; with `-Werror -Wfatal-errors` those would abort the build before a +single sanitizer had a chance to run. The warnings themselves stay on. + +## Sanitizer inspired linters - catching bugs without running anything + +`tools/check-iop-pipe-data-sizes.py ` checks the invariant behind one defect +ASan found the hard way. A module that does not define `init_pipe()` gets `default_init_pipe()`, +which allocates `piece->data` as `calloc(1, self->params_size)`, by the size of the *params* struct. +The module then uses that buffer as its *data* struct, which is only sound while + +``` +sizeof(dt_iop__data_t) <= sizeof(dt_iop__params_t) +``` + +Nothing in the build enforces it, and the two sizes are never both visible to the compiler +in one place: `params_size` is a runtime field and the data struct is module-local, so it cannot +be a `static_assert` in shared code. +Both sizes are in the DWARF of the built plugins, which is what the script reads, via gdb. +It needs a build with debug info, any `RelWithDebInfo` tree, sanitizer or not. + +```bash +tools/check-iop-pipe-data-sizes.py build-sanitizers/ # exits 1 on a violation +VERBOSE=1 tools/check-iop-pipe-data-sizes.py build-sanitizers/ # list the skips +``` + +Modules that define their own `init_pipe()` are exempt, and those that read +`piece->data` as their params struct (or not at all) have nothing to compare. +The script takes the data type from how `piece->data` is actually used rather +than assuming the `dt_iop__data_t` convention, because several modules do +not follow it. + +Worth running after adding a field to any module's data struct: growing it past +the params struct is enough to start writing off the end of the allocation on +every pixelpipe run, and `src/iop/contrastntexture.c` did exactly that. + +`tools/check-iop-legacy-params.py [xmp-dir]` covers the other +invariant ASan found the hard way. A module's old parameter layouts survive as +`dt_iop__params_v_t` structs inside `legacy_params()`, and nothing checks +that they still match what those darktable versions actually wrote. When one +drifts, the conversion reads a struct's worth of bytes out of a shorter blob. +`src/iop/highlights.c` and `src/iop/denoiseprofile.c` both did this. + +Ground truth comes from the integration test XMPs, whose history entries span a +wide range of darktable versions, decoded through the same `gz`/hex encoding +`dt_exif_xmp_encode()` writes. Those sizes are compared against the struct sizes +in the plugins' DWARF: + +```bash +tools/check-iop-legacy-params.py build-sanitizers/ # 0 clean, 1 mismatch, 2 no data +``` + +A size mismatch is an error: both sides are measured. A version present in the +corpus with no matching struct in the debug info is only a warning, the +compiler need not emit a local type it had no use for, but it is worth +reading. It only sees versions the corpus happens to contain, so it is a +regression net rather than a proof. + +Both tools read DWARF through `tools/dwarf_types.py`. That uses `objdump` +rather than gdb because gdb resolves types through a lookup scope and cannot +reach the function-local typedefs the legacy conversions rely on, and rather +than pyelftools because binutils is already required to build darktable. + +## Files + +| Path | Purpose | +|---|---| +| `cmake/sanitizers.cmake` | flag assembly, validation, link check, env file generation | +| `DefineOptions.cmake` | the `DT_SANITIZE` / `DT_SANITIZE_EXTRA_CHECKS` cache entries | +| `tools/sanitizer/sanitizer-env.sh.in` | template for the generated `/bin/sanitizer-env.sh` | +| `tools/sanitizer/{ubsan,lsan,tsan}.supp` | suppression files | +| `tools/run-integration-tests.sh` | suite driver, timeout and stderr-capture shim | +| `tools/dwarf_types.py` | reads C type sizes out of a build's DWARF via `objdump`; shared by the two checkers below | +| `tools/check-iop-pipe-data-sizes.py` | static check that `default_init_pipe()` allocates enough for each module's data struct | +| `tools/check-iop-legacy-params.py` | static check that versioned legacy param structs match the history darktable actually wrote | +| `tools/sanitizer/aggregate-reports.py` | deduplicates the reports; also usable on its own against an old log dir. Exits 0 clean, 1 with findings, 2 when a runtime failed to start | + +`DT_SANITIZE_EXTRA_CHECKS=ON` adds clang's `-fsanitize=integer,implicit-conversion`. +It is off by default because darktable's pixel loops rely on wrapping arithmetic +and implicit narrowing, so it produces thousands of non-bugs. diff --git a/tools/sanitizer/aggregate-reports.py b/tools/sanitizer/aggregate-reports.py new file mode 100755 index 00000000000..05517c43baa --- /dev/null +++ b/tools/sanitizer/aggregate-reports.py @@ -0,0 +1,383 @@ +#!/usr/bin/env python3 +"""Aggregate and deduplicate sanitizer reports from a log directory. + +Reads the per-process files a sanitizer run leaves behind and collapses them +into a ranked list of distinct findings. Two kinds of file are picked up: + + . written by the sanitizers' log_path= option + stderr. captured by the darktable-cli shim in + tools/run-integration-tests.sh -- this is where GCC's UBSan + puts its non-fatal "runtime error:" diagnostics, which it + prints straight to stderr regardless of log_path + +Two reports are considered the same finding when their (address-normalized) +headline and their first three non-runtime stack frames match. + +Start-up failures are counted and reported separately. A runtime that dies +before main() instruments nothing, so "no findings" from such a run means "no +coverage", which is the opposite of a clean result. + +Exit status: 0 when the directory is clean, 1 when there are findings, 2 when a +sanitizer failed to start. Usable standalone on an old log dir: + + tools/sanitizer/aggregate-reports.py /sanitizer-logs/ +""" + +import argparse +import collections +import os +import re +import shutil +import subprocess +import sys +import textwrap + +# tools/run-integration-tests.sh files every report under a directory named +# after the test that produced it, and tags the name with the pass it came from, +# so a whole-suite run stays traceable: /0004-masks/ubsan-cpu.12345 +REPORT_FILE = re.compile(r"^(?:asan|ubsan|lsan|tsan|stderr)(?:-[a-z]+)?\.\d+$") + +# A sanitizer that cannot lay out its shadow memory dies here, before any of +# the program runs. Nearly always the kernel's ASLR entropy is higher than the +# runtime can cope with: kernels default vm.mmap_rnd_bits to 32 while GCC's +# runtimes expect at most 28. Matched before HEADERS, because the ASan spelling +# also satisfies the generic ERROR pattern below. +FATAL = re.compile( + r"^(?:==\d+==)?(?:FATAL|ERROR): (?P\w+Sanitizer): " + r"(?P(?:unexpected memory mapping" + r"|Shadow memory range interleaves" + r"|failed to allocate" + r"|unable to mmap)[^\n]*)" +) + +# One regex per report headline shape. The "====" prefix is an ASan-ism: +# ThreadSanitizer writes a bare "WARNING: ..." under a "====" separator line and +# carries the pid in a trailing "(pid=N)" instead, so the prefix is optional. +HEADERS = ( + re.compile(r"^(?:==\d+==)?ERROR: (?P\w+Sanitizer): (?P[^\n]*)"), + re.compile(r"^(?:==\d+==)?WARNING: (?PThreadSanitizer): (?P[^\n]*)"), + # LeakSanitizer emits one of these per distinct allocation site. + re.compile(r"^(?PDirect leak|Indirect leak) of (?P[^\n]*)"), + re.compile(r"^(?P[^\s:]+):(?P\d+):\d+: runtime error: (?P[^\n]*)"), +) + +# The trailing SUMMARY line restates the ERROR line above it, and the +# LeakSanitizer banner is followed by the "Direct/Indirect leak of ..." entries +# that carry the actual stack traces. Skipping both avoids double counting. +SKIP = re.compile( + r"^(SUMMARY: \w+Sanitizer:|(?:==\d+==)?ERROR: LeakSanitizer: detected memory leaks)" +) + +# Frame layouts differ between runtimes. clang and ASan print the address and an +# "in" separator: +# #1 0x55f4 in dt_dev_pixelpipe_process src/develop/pixelpipe_hb.c:2094 +# GCC's TSan drops both and appends the module the frame resolved to: +# #1 g_socket_send_message (libgio-2.0.so.0+0xa5c13) (BuildId: ...) +FRAME = re.compile( + r"^\s*#\d+ " + r"(?:0x[0-9a-f]+ in )?" + r"(?P\S+)" + r"(?: (?P\S+))?" + r"(?: \((?P[^()\s]+\+0x[0-9a-f]+)\))?" +) + +# Frames inside the sanitizer runtime itself say nothing about our bug. +NOISE = re.compile( + r"(libsanitizer|sanitizer_common|asan_|ubsan_|tsan_|lsan_|" + r"interception|__interceptor|libasan|libubsan|libtsan|liblsan)" +) + +FRAMES_PER_SIGNATURE = 3 + + +class Symbolizer: + """Resolve "libfoo.so+0x1234" frames the sanitizer left unsymbolized. + + TSan symbolizes frames in the main binary and its linked libraries but + leaves the dlopen'd iop plugins as module+offset, even though they are built + with debug info and llvm-symbolizer resolves them fine. Those frames are + where the interesting stacks are, so resolve them here instead. + + This runs after deduplication, on the frames actually about to be printed. + A full-suite ThreadSanitizer run is gigabytes of reports collapsing to a few + hundred findings, so resolving during parsing would symbolize the same + handful of addresses tens of thousands of times. Resolution is batched, one + symbolizer process per module. Without a build directory or an + llvm-symbolizer this is a no-op and frames stay as they were. + """ + + TOOL_NAMES = ["llvm-symbolizer"] + [ + "llvm-symbolizer-%d" % v for v in range(21, 13, -1)] + + # A frame rendered as bare module+offset, e.g. "libfoo.so+0x1234". + UNRESOLVED = re.compile(r"^([^()+\s]+\.so[^+\s]*)\+(0x[0-9a-f]+)$") + + def __init__(self, build_dir): + self.tool = next((shutil.which(n) for n in self.TOOL_NAMES + if shutil.which(n)), None) + # Distributions set DEBUGINFOD_URLS globally (/etc/debuginfod), and + # llvm-symbolizer then blocks on network lookups for every module whose + # build-id it cannot satisfy locally -- 45 seconds per module here, + # against 0.02 with it cleared. We only ever resolve locally built + # plugins, so there is nothing to fetch. + self.env = dict(os.environ, DEBUGINFOD_URLS="") + self.modules = {} + self.cache = {} + if build_dir and self.tool: + for root, dirs, names in os.walk(build_dir): + dirs[:] = [d for d in dirs if d != "CMakeFiles"] + for name in names: + if name.endswith(".so"): + self.modules.setdefault(name, os.path.join(root, name)) + + def enabled(self): + return bool(self.tool and self.modules) + + def prime(self, frames): + """Resolve every module+offset frame in this set, batched per module.""" + if not self.enabled(): + return + wanted = collections.defaultdict(set) + for frame in frames: + match = self.UNRESOLVED.match(frame) + if match and match.group(1) in self.modules: + wanted[match.group(1)].add(match.group(2)) + + for module, offsets in wanted.items(): + ordered = sorted(offsets) + try: + done = subprocess.run( + [self.tool, "--obj=%s" % self.modules[module]], + input="\n".join(ordered) + "\n", + capture_output=True, text=True, timeout=300, + env=self.env) + except (OSError, subprocess.SubprocessError): + continue + # Two lines per address -- symbol, then file:line -- blank separated. + blocks = done.stdout.split("\n\n") + for offset, block in zip(ordered, blocks): + lines = [l for l in block.splitlines() if l.strip()] + if not lines or lines[0].startswith("??"): + continue + text = lines[0] + if len(lines) > 1: + location = lines[1].split(" ")[0] + if location and not location.startswith("??"): + text += " " + location + self.cache[(module, offset)] = text + + def display(self, frame): + """The resolved form of a frame, or the frame unchanged.""" + match = self.UNRESOLVED.match(frame) + if match: + return self.cache.get((match.group(1), match.group(2)), frame) + return frame + + +def report_files(log_dir): + """Every per-process report below log_dir, including the per-test subdirs.""" + files = [] + for root, dirs, names in os.walk(log_dir): + dirs.sort() + files.extend(os.path.join(root, name) + for name in sorted(names) if REPORT_FILE.match(name)) + return files + + +def origin(log_dir, path): + """Which test a report belongs to, from the directory the shim filed it in.""" + relative = os.path.relpath(os.path.dirname(path), log_dir) + return "(run)" if relative == "." else relative + + +def headline(match): + groups = match.groupdict() + if groups.get("file"): + return "runtime error: %s (%s:%s)" % ( + groups["what"], + groups["file"], + groups["line"], + ) + return "%s: %s" % (groups.get("kind", "?"), groups.get("what", "").strip()) + + +def frame_text(match): + """Render one frame, falling back to the module when symbols are missing. + + GCC prints "" for both symbol and file when a frame was not + symbolized; the module and offset are then the only identifying part left, + and they are stable across runs because the offset is module-relative. + Symbolizer resolves those for display once the findings are deduplicated. + """ + parts = [ + part + for part in (match.group("sym"), match.group("loc")) + if part and part != "" + ] + return " ".join(parts) if parts else (match.group("mod") or "") + + +def collect_frames(lines, start): + """Return the first few meaningful frames after a headline, and where they end.""" + frames = [] + i = start + while i < len(lines) and len(frames) < FRAMES_PER_SIGNATURE: + match = FRAME.match(lines[i]) + if match: + text = frame_text(match) + if text and not NOISE.search(text): + frames.append(text) + elif not lines[i].strip() and frames: + break + i += 1 + return frames, i + + +def normalize(title): + """Strip the parts that differ between occurrences of one finding.""" + title = re.sub(r"\s*\(pid=\d+\)", "", title) + return re.sub(r"0x[0-9a-f]+", "0xADDR", title) + + +def signature(title, frames): + normalized = normalize(title) + if normalized.startswith(("Direct leak", "Indirect leak")): + # The byte and object counts vary run to run. + normalized = re.sub(r"\d+", "N", normalized) + return normalized, tuple(frames) + + +def parse(path): + """Yield ("fatal", title) or ("finding", (title, frames)) for one file.""" + try: + with open(path, errors="replace") as handle: + lines = handle.read().splitlines() + except OSError: + return + + i = 0 + while i < len(lines): + line = lines[i] + if SKIP.match(line): + i += 1 + continue + + fatal = FATAL.match(line) + if fatal: + yield "fatal", normalize(headline(fatal)) + i += 1 + continue + + match = next((m for m in (rx.match(line) for rx in HEADERS) if m), None) + if not match: + i += 1 + continue + + frames, i = collect_frames(lines, i + 1) + yield "finding", signature(headline(match), frames) + + +WIDTH = 79 + + +def describe(where, prefix): + """List every test a finding turned up in, wrapped onto continuation lines. + + Deliberately not truncated: which tests are affected, and which are not, is + the thing you are reading the summary for. break_on_hyphens stays off so + that a name like 0002-local-contrast is never split across two lines. + """ + return textwrap.wrap(", ".join(sorted(where)), + width=WIDTH, + initial_indent=prefix, + subsequent_indent=" " * len(prefix), + break_long_words=False, + break_on_hyphens=False) + + +STARTUP_HINT = """\ + A runtime that cannot map its shadow memory almost always means the kernel's + ASLR entropy is higher than it supports. Either lower it system-wide: + sudo sysctl -w vm.mmap_rnd_bits=28 + or run the binary with ASLR off: + setarch -R + tools/run-integration-tests.sh detects this and applies setarch itself.""" + + +def main(): + parser = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("log_dir", help="directory holding the sanitizer log files") + parser.add_argument("-o", "--output", help="also write the summary to this file") + parser.add_argument("--build-dir", + help="build tree whose plugins should be used to " + "symbolize module+offset frames") + args = parser.parse_args() + + files = report_files(args.log_dir) + + counts = collections.Counter() + fatals = collections.Counter() + # Which tests each finding was seen in. The same defect usually fires in + # many tests, and knowing whether it is one test or all of them is most of + # the triage. + seen_in = collections.defaultdict(set) + tests = set() + for path in files: + where = origin(args.log_dir, path) + for kind, item in parse(path): + if kind == "fatal": + fatals[item] += 1 + else: + counts[item] += 1 + seen_in[item].add(where) + tests.add(where) + + total = sum(counts.values()) + + # Now that the reports have collapsed to a handful of findings, resolve the + # plugin frames among the ones about to be printed. + symbolizer = Symbolizer(args.build_dir) + symbolizer.prime({frame for _, frames in counts for frame in frames}) + + lines = [ + "Sanitizer report summary", + "=" * 72, + "log dir : %s" % args.log_dir, + "report files : %d" % len(files), + "tests : %d" % len(tests), + "raw reports : %d" % total, + "unique : %d" % len(counts), + "", + ] + + if fatals: + lines.append( + "!! %d sanitizer start-up failure(s) -- this run produced NO coverage" + % sum(fatals.values()) + ) + for title, count in fatals.most_common(): + lines.append(" [%4dx] %s" % (count, title)) + lines.append(STARTUP_HINT) + lines.append("") + + for finding, count in counts.most_common(): + title, frames = finding + lines.append("[%4dx] %s" % (count, title)) + lines.extend(" %s" % symbolizer.display(frame) for frame in frames) + lines.extend(describe(seen_in[finding], " in: ")) + lines.append("") + + text = "\n".join(lines) + print(text) + if args.output: + with open(args.output, "w") as handle: + handle.write(text + "\n") + + if fatals: + return 2 + return 1 if total else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/sanitizer/lsan.supp b/tools/sanitizer/lsan.supp new file mode 100644 index 00000000000..a626d7bb653 --- /dev/null +++ b/tools/sanitizer/lsan.supp @@ -0,0 +1,27 @@ +# LeakSanitizer suppressions for darktable. +# +# Only used by -DDT_SANITIZE=leak builds (ASan builds run with detect_leaks=0, +# see sanitizer-env.sh.in). +# +# Syntax: leak: +# Add entries here as runs turn up allocations that are owned by a library and +# intentionally never released. + +# GLib/GObject intern their type system, quark and thread-default-context data +# for the lifetime of the process and never free it. +leak:g_type_register_static +leak:g_type_add_interface_static +leak:g_quark_init +leak:g_main_context_new +leak:g_private_set_alloc0 + +# The OpenCL ICD loader keeps per-platform state alive until process exit and +# vendor ICDs commonly leak outright. Not darktable's memory. +leak:libOpenCL.so +leak:libamdocl +leak:libigdrcl +leak:libnvidia + +# GTK/gdk-pixbuf module and icon theme caches are process-lifetime. +leak:gtk_init +leak:gdk_pixbuf_io_init diff --git a/tools/sanitizer/sanitizer-env.sh.in b/tools/sanitizer/sanitizer-env.sh.in new file mode 100644 index 00000000000..d47cddda88a --- /dev/null +++ b/tools/sanitizer/sanitizer-env.sh.in @@ -0,0 +1,128 @@ +# shellcheck shell=sh +# +# Sanitizer runtime environment for this darktable build. +# Generated by cmake/sanitizers.cmake -- do not edit, edit the .sh.in template. +# +# Source it, do not execute it: +# +# DT_SAN_LOGDIR=/some/dir . /bin/sanitizer-env.sh +# +# If DT_SAN_LOGDIR is unset it defaults to a fresh directory under $TMPDIR. +# +# The log_path settings matter because the integration test suite runs +# darktable-cli as "$* 1> /dev/null 2> /dev/null" (see src/tests/integration/run) +# and sanitizers report on stderr. They are not sufficient on their own: only +# the runtimes that report through sanitizer_common honour log_path, while +# GCC's UBSan prints its non-fatal "runtime error:" diagnostics directly to +# stderr regardless. tools/run-integration-tests.sh therefore also captures +# stderr per invocation. If you drive the suite yourself, redirect stderr +# somewhere you can read it afterwards. + +DT_SANITIZERS="@DT_SANITIZE_ACTIVE@" +DT_SAN_SUPPRESSION_DIR="@DT_SANITIZE_SUPPRESSION_DIR@" + +if [ -z "${DT_SAN_LOGDIR:-}" ]; then + DT_SAN_LOGDIR="${TMPDIR:-/tmp}/darktable-sanitizer-$(date +%Y%m%d-%H%M%S)-$$" +fi +mkdir -p "$DT_SAN_LOGDIR" + +export DT_SANITIZERS DT_SAN_LOGDIR + +# Distributions set DEBUGINFOD_URLS globally (see /etc/debuginfod). The +# symbolizer then blocks on network lookups for modules whose build-id it cannot +# satisfy locally, and the runtimes give up and print unsymbolized "" +# frames for the dlopen'd iop plugins -- exactly the frames worth reading. +# Everything we need is in the build tree. +DEBUGINFOD_URLS="" +export DEBUGINFOD_URLS + +# Readable stack traces. GCC's sanitizer runtimes use llvm-symbolizer too when +# they can find it, and fall back to a much slower addr2line path otherwise. +if [ -x "@DT_LLVM_SYMBOLIZER@" ]; then + ASAN_SYMBOLIZER_PATH="@DT_LLVM_SYMBOLIZER@" + TSAN_SYMBOLIZER_PATH="@DT_LLVM_SYMBOLIZER@" + UBSAN_SYMBOLIZER_PATH="@DT_LLVM_SYMBOLIZER@" + export ASAN_SYMBOLIZER_PATH TSAN_SYMBOLIZER_PATH UBSAN_SYMBOLIZER_PATH +fi + +# ASan and TSan inflate stack frames considerably. darktable already insists on +# 2 MiB thread stacks (WANTED_THREADS_STACK_SIZE, enforced in +# src/common/dtpthread.c); give the OpenMP worker threads more headroom. +OMP_STACKSIZE="${OMP_STACKSIZE:-8M}" +export OMP_STACKSIZE + +# Only export the options of the sanitizers actually built in. All four share +# sanitizer_common's log_path, so whichever set is parsed last decides where the +# reports land -- and which that is differs by runtime. A GCC address,undefined +# build wrote its ASan reports to ubsan., and a clang thread-only build +# wrote ThreadSanitizer reports to ubsan-., both of which send you +# looking in the wrong file. Setting only what applies makes the naming say what +# the report is. +_dt_san_active() +{ + case ",${DT_SANITIZERS}," in + *",$1,"*) return 0 ;; + esac + return 1 +} + +# detect_leaks=0 on purpose: darktable-cli exits without tearing down its +# GTK/glib/lua state, so exit-time leak reports would fire on every single run. +# Build with -DDT_SANITIZE=leak when you actually want to hunt leaks. +# +# halt_on_error=0 pairs with -fsanitize-recover=address from +# cmake/sanitizers.cmake: the process carries on after a recoverable finding, so +# one run collects everything instead of stopping at whichever error happens to +# come first. Errors ASan cannot recover from still end the process. Expect +# repeated reports of the same defect when it sits in a loop -- that is what +# aggregate-reports.py deduplicates. +if _dt_san_active address; then +ASAN_OPTIONS="log_path=$DT_SAN_LOGDIR/asan\ +:detect_leaks=0\ +:halt_on_error=0\ +:abort_on_error=0\ +:detect_odr_violation=0\ +:detect_stack_use_after_return=1\ +:strict_string_checks=1\ +:print_stats=0\ +:handle_segv=1\ +:handle_abort=1" +export ASAN_OPTIONS +fi + +# halt_on_error=0 pairs with -fsanitize-recover=all: one run collects every +# finding instead of stopping at the first. +if _dt_san_active undefined; then +UBSAN_OPTIONS="log_path=$DT_SAN_LOGDIR/ubsan\ +:print_stacktrace=1\ +:halt_on_error=0\ +:suppressions=$DT_SAN_SUPPRESSION_DIR/ubsan.supp" +export UBSAN_OPTIONS +fi + +if _dt_san_active leak || _dt_san_active address; then +LSAN_OPTIONS="log_path=$DT_SAN_LOGDIR/lsan\ +:suppressions=$DT_SAN_SUPPRESSION_DIR/lsan.supp\ +:print_suppressions=0" +export LSAN_OPTIONS +fi + +# exitcode=0 overrides the sanitizer default of 66. TSan applies that exit code +# once it has reported anything, and the integration suite treats any non-zero +# exit as "darktable-cli errored" and skips the image comparison entirely. Since +# a race does not necessarily corrupt the export -- output.png is written +# normally -- that would throw away the suite's actual signal on every single +# test. The findings are not lost by this: they are in the log, and +# run-integration-tests.sh still exits non-zero when the aggregator sees any. +# A genuine crash still exits non-zero on its own. +if _dt_san_active thread; then +TSAN_OPTIONS="log_path=$DT_SAN_LOGDIR/tsan\ +:suppressions=$DT_SAN_SUPPRESSION_DIR/tsan.supp\ +:history_size=7\ +:second_deadlock_stack=1\ +:halt_on_error=0\ +:exitcode=0" +export TSAN_OPTIONS +fi + +unset -f _dt_san_active diff --git a/tools/sanitizer/tsan.supp b/tools/sanitizer/tsan.supp new file mode 100644 index 00000000000..83aa20b38c5 --- /dev/null +++ b/tools/sanitizer/tsan.supp @@ -0,0 +1,20 @@ +# ThreadSanitizer suppressions for darktable. +# +# Syntax: race:, deadlock:, called_from_lib:, ... +# +# GCC's libgomp is not built with TSan annotations, so TSan cannot see the +# happens-before edges established by OpenMP barriers and reduction clauses. It +# therefore reports a race on essentially every parallel loop. These entries +# make TSan runs against a GCC build usable at all; they are NOT needed with +# clang + an annotated libomp/archer. +called_from_lib:libgomp.so +race:^GOMP_ +race:^gomp_ + +# Same story for the OpenCL ICD loader and vendor drivers. +called_from_lib:libOpenCL.so + +# GLib's own lock-free/atomic data structures confuse TSan when glib itself is +# not instrumented. +called_from_lib:libglib-2.0.so +called_from_lib:libgobject-2.0.so diff --git a/tools/sanitizer/ubsan.supp b/tools/sanitizer/ubsan.supp new file mode 100644 index 00000000000..24ff8f3650e --- /dev/null +++ b/tools/sanitizer/ubsan.supp @@ -0,0 +1,11 @@ +# UndefinedBehaviorSanitizer suppressions for darktable. +# +# Syntax: : +# e.g. alignment:src/external/LibRaw +# +# Keep this file small and justified. A suppression here hides a real UBSan +# check, so prefer fixing the code. Entries belong here when the undefined +# behaviour lives in third party code we do not control. + +# Start empty. Populate from the first full suite run, and record why each +# entry exists.