From 22099333e62ef0a7b1bea28a9242595c2826d5ff Mon Sep 17 00:00:00 2001 From: Rot127 <45763064+Rot127@users.noreply.github.com> Date: Thu, 4 Dec 2025 11:02:01 +0000 Subject: [PATCH] Add leak check in CI (#5553) * Remove no longer supported option. suggestion_mode was removed with 4.0: https://pylint.readthedocs.io/en/latest/whatsnew/4/4.0/index.html5 * Add script to detect leaks in PRs. The script fails if any of the changed lines appear in a leak stack trace produced by LSAN. --- .github/workflows/ci.yml | 28 +++++++++++ .pylintrc | 4 -- sys/lsan_check.py | 105 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 4 deletions(-) create mode 100755 sys/lsan_check.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0dd1b7eab..b1324e5d3a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,6 +60,7 @@ jobs: linux-meson-gcc-tests, macos-meson-clang-tests, linux-gcc-tests-asan, + linux-gcc-tests-lsan, linux-clang-tests-asan, linux-gcc-tests-codecov, capstone-v4, @@ -107,6 +108,22 @@ jobs: enabled: ${{ needs.changes.outputs.edited == 'true' }} timeout: 120 allow_failure: false + - name: linux-gcc-tests-lsan + os: ubuntu-24.04 + build_system: meson + compiler: gcc + cflags: "-DASAN=1 -DRZ_ASSERT_STDOUT=1 -ftrivial-auto-var-init=pattern -funsigned-char" + meson_options: -Dbuildtype=debug -Db_sanitize=leak --werror + asan: true + enable: ${{ github.event_name == 'pull_request' }} + lsan_options: log_path=/tmp/lsan_logs/log + continue-on-error: false + run_tests: true + enabled: ${{ needs.changes.outputs.edited == 'true' }} + timeout: 120 + # The existing leaks in Rizin will make the tests fail otherwise + # before it runs the script to check for new leaks. + allow_failure: true - name: linux-gcc-tests-portable os: ubuntu-24.04 build_system: meson @@ -277,6 +294,7 @@ jobs: env: ASAN: ${{ matrix.asan }} ASAN_OPTIONS: ${{ matrix.asan_options }} + LSAN_OPTIONS: ${{ matrix.lsan_options }} CC: ${{ matrix.compiler }} - name: Checkout our Testsuite Binaries if: matrix.enabled @@ -288,10 +306,13 @@ jobs: if: matrix.run_tests && matrix.enabled && (github.event_name != 'pull_request' || contains(github.event.pull_request.head.ref, 'fuzz')) uses: actions/checkout@v6 with: + # Required for comparing the current branch to dev. + fetch-depth: 2 repository: rizinorg/rizin-fuzztargets path: test/fuzz/targets - name: Run integration tests and rz-test if: matrix.run_tests && matrix.enabled + continue-on-error: ${{ matrix.allow_failure }} run: | # Running the test suite export PATH=${HOME}/bin:$(python3 -m site --user-base)/bin:${HOME}/.local/bin:${PATH} @@ -317,9 +338,11 @@ jobs: env: ASAN: ${{ matrix.asan }} ASAN_OPTIONS: ${{ matrix.asan_options }} + LSAN_OPTIONS: ${{ matrix.lsan_options }} CC: ${{ matrix.compiler }} - name: Run fuzz tests if: matrix.run_tests && matrix.enabled && (github.event_name != 'pull_request' || contains(github.event.pull_request.head.ref, 'fuzz') || matrix.coverage) + continue-on-error: ${{ matrix.allow_failure }} run: | export PATH=${HOME}/bin:${HOME}/.local/bin:${PATH} export LD_LIBRARY_PATH=${HOME}/lib/$(uname -m)-linux-gnu:${HOME}/lib:${HOME}/lib64:${LD_LIBRARY_PATH} @@ -337,7 +360,12 @@ jobs: env: ASAN: ${{ matrix.asan }} ASAN_OPTIONS: ${{ matrix.asan_options }} + LSAN_OPTIONS: ${{ matrix.lsan_options }} CC: ${{ matrix.compiler }} + - name: Check for new leaks + if: matrix.lsan_options != '' + run: | + ./sys/lsan_check.py ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} /tmp/lsan_logs/* - name: Generate coverage data if: matrix.coverage && matrix.enabled run: | diff --git a/.pylintrc b/.pylintrc index c7016526ac..b5d669f609 100644 --- a/.pylintrc +++ b/.pylintrc @@ -36,10 +36,6 @@ load-plugins= # Pickle collected data for later comparisons. persistent=yes -# When enabled, pylint would attempt to guess common misconfiguration and emit -# user-friendly hints instead of false-positive error messages. -suggestion-mode=yes - # Allow loading of arbitrary C extensions. Extensions are imported into the # active Python interpreter and may run arbitrary code. unsafe-load-any-extension=no diff --git a/sys/lsan_check.py b/sys/lsan_check.py new file mode 100755 index 0000000000..76a59bf6b3 --- /dev/null +++ b/sys/lsan_check.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2025 Rot127 +# SPDX-License-Identifier: LGPL-3.0-only + +import re +import subprocess +import sys +from pathlib import Path +from typing import Dict, List, Tuple + + +def get_changed_lines( + base_ref: str, head_ref: str +) -> Dict[str, List[Tuple[int, int]]] | None: + """ + Return dict: file-path -> [(start_line, end_line), …] + representing *added/modified* line ranges in the current branch. + """ + changed: Dict[Path, List[Tuple[int, int]]] = {} + + # --unified=0 gives hunks like “@@ -L,C +L,C @@” (we care about the + side) + cmd = ["git", "diff", "--unified=0", f"{base_ref}..{head_ref}"] + out = subprocess.check_output(cmd, text=True, stderr=subprocess.DEVNULL) + if not out: + return None + + path = None + for line in out.splitlines(): + # New file header + if line.startswith("+++"): + path = Path(line.strip("+++ b/")).name + changed[path] = [] + continue + # Hunk header + m = re.match(r"@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@", line) + if m and path is not None: + start = int(m.group(1)) + count = int(m.group(2)) if m.group(2) else 1 + changed[path].append((start, start + count - 1)) + return changed + + +def parse_asan_leaks( + asan_output: str, changed: Dict[str, List[Tuple[int, int]]] +) -> Tuple[int, List[Tuple[Path, int, str]]]: + leaks: List[Tuple[Path, int]] = [] + leak_traces = re.split(r"\n\n", asan_output) + # Remove empty lines + leak_traces = [t for t in leak_traces if t] + total_leaks = len([x for x in leak_traces if " leak " in x]) + + # Split into individual leak lines and search. + leak_re = re.compile( + r"^\s*#\d+\s+0x[0-9a-fA-F]+\s+in\s+\w+\s+([^\n(]+):(\d+)", re.MULTILINE + ) + for trace in leak_traces: + for match in leak_re.finditer(trace): + name = Path(match.group(1)).name + line = int(match.group(2)) + if name in changed and any( + line in range(start_end[0], start_end[1] + 1) + for start_end in changed[name] + ): + leaks.append((name, line, trace)) + break + return (total_leaks, leaks) + + +def main() -> None: + if len(sys.argv) < 4: + print("Supply ASAN output via stdin or file argument") + print(f"{sys.argv[0]} [ ...]") + sys.exit(2) + base_ref = sys.argv[1] + head_ref = sys.argv[2] + + changed = get_changed_lines(base_ref, head_ref) + if not changed: + print("No changed files") + sys.exit(1) + + asan_text = "" + for i in range(3, len(sys.argv)): + asan_text += Path(sys.argv[i]).read_text(encoding="utf8") + total_leaks, leaks = parse_asan_leaks(asan_text.strip(), changed) + + print("\nLEAK REPORT\n") + print(f"Total leaks: {total_leaks}") + print(f"New leaks: {len(leaks)}\n") + + indent = " " + if leaks: + print("Memory leaks detected in changed lines:") + for f, l, trace in leaks: + print("-" * 32) + print(f"\n{indent}{f}:{l}\n") + print(f"{indent}{trace.replace("\n", "\n" + indent)}\n") + sys.exit(1) + + print("No new leaks in changed lines") + sys.exit(0) + + +if __name__ == "__main__": + main()