From db4ebfbe0c17975274361041772d8d33f4112db5 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 17 Aug 2026 10:26:38 +0200 Subject: [PATCH 1/3] fix: address CodeChecker findings Add a local CodeChecker runner that generates and merges compilation databases for the supported configurations, then runs clangsa and clang-tidy while propagating the analyzer result. Fix actionable findings in native minidump and crash handling, JSON, logging, consent, and value serialization. Exclude tests for now and suppress intentional or deferred reports, with TODOs for issues that should be revisited. --- .codechecker-ignore | 2 + .codechecker-suppress | 12 +++ scripts/run-codechecker.py | 80 +++++++++++++++++++ .../native/minidump/sentry_minidump_linux.c | 4 +- src/backends/native/sentry_crash_daemon.c | 9 ++- src/sentry_core.c | 2 + src/sentry_json.c | 2 + src/sentry_logs.c | 4 +- src/sentry_value.c | 12 +++ 9 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 .codechecker-suppress create mode 100755 scripts/run-codechecker.py diff --git a/.codechecker-ignore b/.codechecker-ignore index 5c3ff49479..255497553a 100644 --- a/.codechecker-ignore +++ b/.codechecker-ignore @@ -2,3 +2,5 @@ -*/vendor/* -/usr/* -*/atomic_base.h +# TODO: re-enable tests after addressing CodeChecker reports +-*/tests/* diff --git a/.codechecker-suppress b/.codechecker-suppress new file mode 100644 index 0000000000..321835409e --- /dev/null +++ b/.codechecker-suppress @@ -0,0 +1,12 @@ +5f583f6b5592cb24d91c043a66efb2b5||sentry_backend_inproc.c||TODO: Fix nullable signal-context path +e424fa2c2faf433cf48256ac1e659dfa||sentry_http_transport_curl.c||Intentionally ignored curl debug data +47bb7e7d44d6e5cf20f6fc4b2860cd5b||sentry_logger.c||Logging API accepts a runtime format +fa583be73838173c02357431ab86c05b||sentry_logs.c||Logging API accepts a runtime format +ff9dc7d976d46411308e4fbc3b23c24a||sentry_logs.c||Logging API accepts a runtime format +ca6b03d250a728170be00fadb09900ad||sentry_logs.c||Logging API accepts a runtime format +bfafc180af44acc534a4da113b02baf6||sentry_alloc.c||Preserve calloc zero-size semantics +877ae33a5806c6afdc9f147ef735c016||sentry_backend_crashpad.cpp||Keep portable post-switch fallback +25537027bfbdb890e1f7e7e5f73272ef||sentry_backend_crashpad.cpp||Keep portable post-switch fallback +5462e2d08b25802a4baf335e54641c7c||sentry_backend_crashpad.cpp||Keep portable post-switch fallback +5c963678cf1863557bbc86151aabb4cb||sentry_crash_daemon.c||Logging function accepts a runtime format +8109abd7bbb04b81a4d9bde8e9ea3cb1||sentry_modulefinder_linux.c||TODO: Initialize module cache outside its mutex diff --git a/scripts/run-codechecker.py b/scripts/run-codechecker.py new file mode 100755 index 0000000000..d37ddd43f6 --- /dev/null +++ b/scripts/run-codechecker.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 + +import json +import os +import subprocess +from pathlib import Path + +BACKENDS = ("none", "inproc", "breakpad", "crashpad", "native") +PROJECT_DIR = Path(__file__).resolve().parent.parent +BUILD_ROOT = PROJECT_DIR / "build" / "codechecker" + + +def run(command, *, check=True): + print("+ {}".format(" ".join(map(str, command))), flush=True) + return subprocess.run(command, cwd=PROJECT_DIR, check=check) + + +def main(): + run(["CodeChecker", "version"]) + + compilation = [] + + for backend in BACKENDS: + build_dir = BUILD_ROOT / backend + run( + [ + "cmake", + "-S", + PROJECT_DIR, + "-B", + build_dir, + "-DCMAKE_BUILD_TYPE=Debug", + "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON", + "-DSENTRY_BUILD_EXAMPLES=OFF", + "-DSENTRY_BUILD_TESTS=OFF", + "-DSENTRY_BACKEND={}".format(backend), + ] + ) + run(["cmake", "--build", build_dir, "--target", "sentry", "--parallel"]) + + with (build_dir / "compile_commands.json").open() as commands: + compilation.extend(json.load(commands)) + + compilation_database = BUILD_ROOT / "compile_commands.json" + with compilation_database.open("w") as commands: + json.dump(compilation, commands) + + disabled_checkers = ( + "readability-magic-numbers", + "cppcoreguidelines-avoid-magic-numbers", + "readability-else-after-return", + "clang-diagnostic-reserved-identifier", + "clang-diagnostic-reserved-macro-identifier", + "cert-err33-c", + ) + result = run( + [ + "CodeChecker", + "check", + "--jobs", + str(os.cpu_count()), + "--analyzers", + "clangsa", + "clang-tidy", + *("--disable={}".format(checker) for checker in disabled_checkers), + "--print-steps", + "--ignore", + PROJECT_DIR / ".codechecker-ignore", + "--suppress", + PROJECT_DIR / ".codechecker-suppress", + "--logfile", + compilation_database, + ], + check=False, + ) + return result.returncode + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/backends/native/minidump/sentry_minidump_linux.c b/src/backends/native/minidump/sentry_minidump_linux.c index 2eb874d480..1730f8f003 100644 --- a/src/backends/native/minidump/sentry_minidump_linux.c +++ b/src/backends/native/minidump/sentry_minidump_linux.c @@ -719,7 +719,7 @@ write_thread_context( // Copy control/status words context.float_save.control_word = fpregs.cwd; context.float_save.status_word = fpregs.swd; - context.float_save.tag_word = fpregs.ftw; + context.float_save.tag_word = (uint8_t)fpregs.ftw; context.float_save.error_opcode = fpregs.fop; // On x86_64, FPU IP/DP are 64-bit. The FXSAVE format splits them // across offset (low 32) and selector (high 16) fields. @@ -2685,6 +2685,8 @@ write_linux_dso_debug_stream( case AT_BASE: at_base = auxv[i].a_un.a_val; break; + default: + break; } } sentry_free(auxv_buf); diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index d81c7937be..e2a0143a40 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -611,7 +611,7 @@ static bool is_valid_code_addr(uint64_t addr) { // Must be non-null and in typical code range - if (addr == 0 || addr < 0x1000) { + if (addr < 0x1000) { return false; } #if defined(__x86_64__) || defined(_M_AMD64) @@ -3160,13 +3160,14 @@ build_native_event(const sentry_crash_context_t *ctx, sentry_value_set_by_key(event, "level", sentry_value_new_string(level)); // Build exception - const char *signal_name = "UNKNOWN"; #if defined(SENTRY_PLATFORM_UNIX) int signal_number = ctx->platform.signum; - signal_name = get_signal_name(signal_number); + const char *signal_name = get_signal_name(signal_number); #elif defined(SENTRY_PLATFORM_WINDOWS) // Exception code is used directly below as unsigned - signal_name = "EXCEPTION"; + const char *signal_name = "EXCEPTION"; +#else + const char *signal_name = "UNKNOWN"; #endif sentry_value_t exc = sentry_value_new_object(); diff --git a/src/sentry_core.c b/src/sentry_core.c index 7a2eb68b04..b87e36c599 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -443,6 +443,8 @@ set_user_consent(sentry_user_consent_t new_val) case SENTRY_USER_CONSENT_UNKNOWN: sentry__path_remove(consent_path); break; + default: + break; } sentry__path_free(consent_path); } diff --git a/src/sentry_json.c b/src/sentry_json.c index 922880f0d5..f9e3352cc1 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -664,6 +664,8 @@ tokens_to_value(jsmntok_t *tokens, size_t token_count, const char *buf, } case JSMN_UNDEFINED: break; + default: + goto error; } #undef POP diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 4573abf7b4..9549b9f792 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -84,7 +84,7 @@ static val = va_arg(*args_copy, int); break; case PRINTF_LENGTH_CHAR: - val = (signed char)va_arg(*args_copy, int); + val = (int64_t)(signed char)va_arg(*args_copy, int); break; case PRINTF_LENGTH_SHORT: val = (short)va_arg(*args_copy, int); @@ -478,6 +478,8 @@ debug_print_log(sentry_level_t level, const char *log_body) case SENTRY_LEVEL_FATAL: SENTRY_FATALF("LOG: %s", log_body); break; + default: + break; } } diff --git a/src/sentry_value.c b/src/sentry_value.c index 52332357aa..2267643e91 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -333,6 +333,8 @@ thing_free(thing_t *thing) case THING_TYPE_STRING: sentry_free(thing->payload._ptr); break; + default: + break; } sentry_free(thing); } @@ -788,6 +790,8 @@ sentry_value_get_type(sentry_value_t value) return SENTRY_VALUE_TYPE_INT64; case THING_TYPE_UINT64: return SENTRY_VALUE_TYPE_UINT64; + default: + break; } UNREACHABLE("invalid thing type"); } else if ((value._bits & TAG_MASK) == TAG_CONST) { @@ -1212,6 +1216,8 @@ sentry_value_get_length(sentry_value_t value) return ((const list_t *)thing->payload._ptr)->len; case THING_TYPE_OBJECT: return ((const obj_t *)thing->payload._ptr)->len; + default: + break; } } return 0; @@ -1469,6 +1475,9 @@ sentry__jsonwriter_write_value(sentry_jsonwriter_t *jw, sentry_value_t value) sentry__jsonwriter_write_object_end(jw); break; } + default: + UNREACHABLE("invalid value type during JSON serialization"); + break; } } @@ -1541,6 +1550,9 @@ value_to_msgpack(mpack_writer_t *writer, sentry_value_t value) mpack_finish_map(writer); break; } + default: + UNREACHABLE("invalid value type during MessagePack serialization"); + break; } } From ea4a0556ecbe9e51069595ddd6b0814a1fce3be3 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 17 Aug 2026 12:33:48 +0200 Subject: [PATCH 2/3] ff --- src/backends/native/sentry_crash_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index e2a0143a40..787a627ac1 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -3167,7 +3167,7 @@ build_native_event(const sentry_crash_context_t *ctx, // Exception code is used directly below as unsigned const char *signal_name = "EXCEPTION"; #else - const char *signal_name = "UNKNOWN"; +# error Unsupported platform #endif sentry_value_t exc = sentry_value_new_object(); From 9e74ae76702db003d51ff087480637ac4dc454fe Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 17 Aug 2026 11:10:38 +0200 Subject: [PATCH 3/3] ci: fix CodeChecker analysis The tests/cmake.py integration runs CodeChecker after every CMake build. Since the test suite creates many temporary build configurations, it repeatedly analyzes the same source files and takes hours to finish. Install the latest CodeChecker 6.28.2 from PyPI instead of the broken Snap installation, and run CodeChecker in a dedicated job that builds each backend once, merges their compilation databases, and performs a single analysis. --- .github/workflows/ci.yml | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64de2126c4..7cd8466e73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,11 +140,11 @@ jobs: CXX: clang++-20 ERROR_ON_WARNINGS: 1 RUN_ANALYZER: kcov - - name: Linux (GCC 13.3.0 + code-checker + valgrind) + - name: Linux (GCC 13.3.0 + valgrind) CC: gcc-13 CXX: g++-13 os: ubuntu-24.04 - RUN_ANALYZER: code-checker,valgrind + RUN_ANALYZER: valgrind - name: Linux (GCC + musl + libunwind) os: ubuntu-latest container: ghcr.io/getsentry/sentry-native-alpine:3.24 @@ -353,10 +353,6 @@ jobs: echo "$HOME/.dotnet" >> $GITHUB_PATH echo "DOTNET_ROOT=$HOME/.dotnet" >> $GITHUB_ENV - - name: Installing CodeChecker - if: ${{ contains(env['RUN_ANALYZER'], 'code-checker') }} - run: sudo snap install codechecker --classic - - name: Expose llvm@15 PATH for Mac if: ${{ runner.os == 'macOS' }} run: echo $(brew --prefix llvm@15)/bin >> $GITHUB_PATH @@ -505,13 +501,38 @@ jobs: fail_ci_if_error: false verbose: true + codechecker: + name: CodeChecker + runs-on: ubuntu-24.04 + env: + CC: gcc-13 + CXX: g++-13 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: recursive + + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.12" + cache: "pip" + + - name: Install dependencies + run: | + sudo apt update + sudo apt install cmake clang clang-tidy gcc-13 g++-13 zlib1g-dev libcurl4-openssl-dev + python -m pip install codechecker==6.28.2 + + - name: Analyze + run: python scripts/run-codechecker.py + archive: name: Create Release Archive runs-on: ubuntu-latest - needs: [lint, test] + needs: [lint, test, codechecker] # only run this on pushes, combined with the CI triggers, this will only # run on master or the release branch - if: ${{ needs.test.result == 'success' && github.event_name == 'push' }} + if: ${{ needs.test.result == 'success' && needs.codechecker.result == 'success' && github.event_name == 'push' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: