From d0a0574a7c1ef3f4d7c2a6b3f51ce6efe4c96225 Mon Sep 17 00:00:00 2001 From: Max Lv Date: Wed, 15 Jul 2026 12:01:10 +0800 Subject: [PATCH] Add code quality tooling: clang-tidy, sanitizers, coverage - Export compile_commands.json for tooling - .clang-tidy: clang-analyzer + bugprone + cert checks scoped to src/, with stylistic/false-positive-prone checks disabled - ENABLE_SANITIZERS CMake option (ASan + UBSan) - ENABLE_COVERAGE CMake option with an lcov/genhtml 'coverage' target - CI: asan job (ctest + stress test under ASan/UBSan), clang-tidy job (warning-count ratchet, baseline 29 with pinned clang-tidy-18), coverage job (lcov summary + HTML report artifact) Co-Authored-By: Claude Fable 5 --- .clang-tidy | 36 ++++++++++++ .github/workflows/tests.yml | 106 ++++++++++++++++++++++++++++++++++++ .gitignore | 1 + CLAUDE.md | 10 ++++ CMakeLists.txt | 43 +++++++++++++++ 5 files changed, 196 insertions(+) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..08f92a894 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,36 @@ +# clang-tidy configuration for shadowsocks-libev project sources (src/ only). +# Bundled submodules (libcork/, libipset/, libbloom/) are third-party code and +# are excluded by the file regex used in CI: +# run-clang-tidy -p build '/src/[^/]+\.c$' +# +# The check set is intentionally conservative: the full clang static analyzer +# plus the bugprone/cert checks that are actionable for this codebase. +# Checks disabled below are either stylistic churn (macro-parentheses, +# reserved-identifier: the codebase uses _prefixed names and vendored +# uthash.h/generated config.h trip them ~600 times) or produce false +# positives with the libev callback style (casting-through-void, +# multi-level-implicit-pointer-conversion). +Checks: > + clang-analyzer-*, + bugprone-*, + cert-*, + -bugprone-assignment-in-if-condition, + -bugprone-branch-clone, + -bugprone-casting-through-void, + -bugprone-easily-swappable-parameters, + -bugprone-inc-dec-in-conditions, + -bugprone-implicit-widening-of-multiplication-result, + -bugprone-macro-parentheses, + -bugprone-multi-level-implicit-pointer-conversion, + -bugprone-narrowing-conversions, + -bugprone-reserved-identifier, + -bugprone-signed-char-misuse, + -bugprone-switch-missing-default-case, + -cert-dcl37-c, + -cert-dcl51-cpp, + -cert-err33-c, + -cert-str34-c, + -clang-analyzer-security.insecureAPI.strcpy, + -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling +WarningsAsErrors: '' +HeaderFilterRegex: '.*/src/[^/]+\.h$' diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d88e6cafc..694b16f29 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -55,3 +55,109 @@ jobs: sudo apt-get install -y qemu-system-x86 bash tests/test_redir_qemu.sh build/shared/bin/ timeout-minutes: 8 + + asan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + submodules: recursive + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y libpcre2-dev libmbedtls-dev libsodium-dev libev-dev libc-ares-dev + + - name: Build with ASan + UBSan + run: | + mkdir -p build && cd build + cmake .. -DENABLE_SANITIZERS=ON + make -j"$(nproc)" + + - name: CTest (ASan + UBSan) + run: ctest --test-dir build -LE memcheck --output-on-failure --no-tests=error + + - name: Stress test (ASan + UBSan) + run: python3 tests/stress_test.py --bin build/shared/bin/ --size 10 + + clang-tidy: + runs-on: ubuntu-latest + env: + # Ratchet: number of pre-existing clang-tidy warnings (see .clang-tidy), + # counted with clang-tidy-18 (pinned below so runner upgrades don't move + # the number). CI fails if the count goes UP; lower this value as + # findings get fixed. + MAX_WARNINGS: 29 + steps: + - uses: actions/checkout@v6 + with: + submodules: recursive + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y clang-tidy-18 clang-tools-18 libpcre2-dev libmbedtls-dev libsodium-dev libev-dev libc-ares-dev + + - name: Configure (compile_commands.json) + run: cmake -S . -B build + + - name: Run clang-tidy on project sources + run: | + run-clang-tidy-18 -quiet -p build '/src/[^/]+\.c$' 2>/dev/null | tee tidy.log || true + grep 'warning:' tidy.log | sort -u > warnings.txt || true + count="$(wc -l < warnings.txt)" + echo "clang-tidy warnings: $count (max allowed: $MAX_WARNINGS)" + { + echo "### clang-tidy: $count warnings (budget: $MAX_WARNINGS)" + echo '```' + cat warnings.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + if ! grep -q . warnings.txt && grep -qE 'error:' tidy.log; then + echo "clang-tidy produced errors:"; grep -E 'error:' tidy.log | sort -u + exit 1 + fi + if [ "$count" -gt "$MAX_WARNINGS" ]; then + echo "FAIL: warning count $count exceeds budget $MAX_WARNINGS." + echo "Fix the new warnings (or, if a check is misfiring, adjust .clang-tidy)." + exit 1 + fi + + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + submodules: recursive + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y lcov libpcre2-dev libmbedtls-dev libsodium-dev libev-dev libc-ares-dev + + - name: Build with coverage instrumentation + run: | + mkdir -p build && cd build + cmake .. -DENABLE_COVERAGE=ON + make -j"$(nproc)" + + - name: Run tests + run: | + ctest --test-dir build -LE memcheck --output-on-failure --no-tests=error + python3 tests/stress_test.py --bin build/shared/bin/ --size 10 + + - name: Generate coverage report + run: | + cmake --build build --target coverage + { + echo '### Coverage (src/, unit + stress tests)' + echo '```' + lcov --list build/coverage.info --ignore-errors unused,empty + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload HTML report + uses: actions/upload-artifact@v4 + with: + name: coverage-html + path: build/coverage-html/ diff --git a/.gitignore b/.gitignore index c8741ed34..385ad90f7 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ # Ignore files generated by configure build/ +build-*/ .deps/ /Makefile src/Makefile diff --git a/CLAUDE.md b/CLAUDE.md index d51578659..01b2e9095 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,6 +69,16 @@ python tests/test.py --bin build/bin/ -c tests/aes-gcm.json Uses **uncrustify** with the config at `.uncrustify.cfg`. Key settings: 4-space indent, no tabs, 120-column width, K&R brace style (braces on same line). +## Code Quality Tooling + +- **clang-tidy**: config in `.clang-tidy` (clang-analyzer + bugprone + cert checks, scoped to `src/`). The build exports `compile_commands.json` automatically. Run locally: + ```bash + run-clang-tidy -quiet -p build '/src/[^/]+\.c$' + ``` + On macOS with Homebrew LLVM, add `-extra-arg="-isysroot$(xcrun --show-sdk-path)"`. CI (`clang-tidy` job in `tests.yml`) enforces a warning-count ratchet via `MAX_WARNINGS` — lower it when fixing findings; never raise it without justification. +- **Sanitizers**: `cmake .. -DENABLE_SANITIZERS=ON` builds with ASan + UBSan. CI runs ctest and the stress test under sanitizers on every PR. +- **Coverage**: `cmake .. -DENABLE_COVERAGE=ON`, run tests, then `make coverage` (needs lcov). HTML report lands in `build/coverage-html/`. CI uploads it as the `coverage-html` artifact. + ## Architecture ### Binaries (all in `src/`) diff --git a/CMakeLists.txt b/CMakeLists.txt index 27091adf4..b8a108ae7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,12 +10,32 @@ project(${PROJECT_NAME} VERSION ${PROJECT_VERSION}) include(GNUInstallDirs) +# Compilation database for clang-tidy and other tooling +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + # Compiler flags matching autotools # Note: -Werror is applied per-target in src/CMakeLists.txt to avoid # breaking bundled submodules (libcork, libipset, libbloom) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 -D_GNU_SOURCE") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -O2 -Wall -Wno-deprecated-declarations -fno-strict-aliasing") +option(ENABLE_SANITIZERS "Build with AddressSanitizer and UndefinedBehaviorSanitizer" OFF) +if (ENABLE_SANITIZERS) + set(SANITIZER_FLAGS "-fsanitize=address,undefined -fno-sanitize-recover=all -fno-omit-frame-pointer") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZER_FLAGS}") + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${SANITIZER_FLAGS}") + message(STATUS "Sanitizers enabled (ASan + UBSan)") +endif () + +option(ENABLE_COVERAGE "Build with coverage instrumentation (gcov)" OFF) +if (ENABLE_COVERAGE) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O0 --coverage") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --coverage") + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --coverage") + message(STATUS "Coverage instrumentation enabled") +endif () + set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin) @@ -185,6 +205,29 @@ if(BUILD_TESTING) add_subdirectory(tests) endif() +# Coverage report (requires ENABLE_COVERAGE=ON, lcov and genhtml) +# Usage: make coverage (after running ctest / integration tests) +if (ENABLE_COVERAGE) + find_program(LCOV_EXECUTABLE lcov) + find_program(GENHTML_EXECUTABLE genhtml) + if (LCOV_EXECUTABLE AND GENHTML_EXECUTABLE) + add_custom_target(coverage + COMMAND ${LCOV_EXECUTABLE} --capture --directory ${PROJECT_BINARY_DIR} + --output-file ${PROJECT_BINARY_DIR}/coverage.info + --include ${PROJECT_SOURCE_DIR}/src/* + --ignore-errors mismatch,unused,empty,gcov + COMMAND ${LCOV_EXECUTABLE} --list ${PROJECT_BINARY_DIR}/coverage.info + COMMAND ${GENHTML_EXECUTABLE} ${PROJECT_BINARY_DIR}/coverage.info + --output-directory ${PROJECT_BINARY_DIR}/coverage-html + COMMAND ${CMAKE_COMMAND} -E echo "Coverage report: ${PROJECT_BINARY_DIR}/coverage-html/index.html" + WORKING_DIRECTORY ${PROJECT_BINARY_DIR} + VERBATIM + ) + else () + message(WARNING "lcov/genhtml not found - 'coverage' target unavailable") + endif () +endif () + # Install ss-nat on Linux if(LINUX) install(PROGRAMS src/ss-nat DESTINATION ${CMAKE_INSTALL_BINDIR})