Fixes the leak checking workflow. (#6205)

- It uses now the changed lines provided by the get_real_shas workflow,
  which prevents failing if PR gets out of sync with base branch.
- Fixes a bug in lsan_check.py which used the wrong line number as reference.
- Moves the whole leak detection workflow out of ci.yml to simplify it.
- Uploads the leak stack traces as artifact for local analysis.

.github/workflows/ci.yml

@@ -60,7 +60,6 @@ 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,
@@ -108,21 +107,6 @@ 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
-            lsan_options: log_path=/tmp/lsan_logs/log
-            continue-on-error: false
-            run_tests: true
-            enabled: ${{ (needs.changes.outputs.edited == 'true' && github.event_name == 'pull_request') }}
-            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
@@ -293,7 +277,6 @@ 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
@@ -337,7 +320,6 @@ 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)
@@ -359,12 +341,7 @@ 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 != '' && matrix.enabled
-        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: |

.github/workflows/leaks.yml

@@ -0,0 +1,127 @@
+name: New Leaks
+
+on:
+  push:
+    paths:
+      - '**.c'
+      - '**.h'
+      - '**.in'
+      - '**.inc'
+
+  pull_request:
+
+# Automatically cancel any previous workflow on new push.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }}
+  cancel-in-progress: true
+
+jobs:
+  real_pr_shas:
+    uses: rizinorg/rizin/.github/workflows/get_real_pr_shas.yml@dev
+
+  build-and-check-leaks:
+    needs:
+      - real_pr_shas
+    name: ${{ matrix.name }}
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: ${{ matrix.timeout }}
+    strategy:
+      fail-fast: false
+      matrix:
+        name:
+          [
+            linux-gcc-tests-lsan,
+          ]
+        include:
+          - name: linux-gcc-tests-lsan
+            os: ubuntu-24.04
+            build_system: meson
+            compiler: gcc
+            cflags: "-DRZ_ASSERT_STDOUT=1 -ftrivial-auto-var-init=pattern -funsigned-char"
+            meson_options: -Dbuildtype=debug -Db_sanitize=leak --werror
+            timeout: 120
+
+    env:
+      CC: ${{ matrix.compiler }}
+      CFLAGS: ${{ matrix.cflags }}
+      ASAN_OPTIONS: ${{ matrix.asan_options }}
+      LOG_PREFIX: leaks
+      LSAN_LOGS_DIR: /tmp/lsan_logs/
+
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          fetch-depth: 2
+
+      - name: Install python and other dependencies
+        run: sudo apt-get --assume-yes install python3-wheel python3-setuptools libcapstone4 libcapstone-dev
+
+      - name: Install meson and ninja
+        if: matrix.build_system == 'meson'
+        run: pip3 install --user meson ninja PyYAML
+        env:
+          PIP_BREAK_SYSTEM_PACKAGES: 1
+
+      - name: Checkout rzpipe
+        uses: actions/checkout@v6
+        with:
+          repository: rizinorg/rz-pipe
+          path: test/rz-pipe
+
+      - name: Install test dependencies
+        run: pip3 install --user "file://$GITHUB_WORKSPACE/test/rz-pipe#egg=rzpipe&subdirectory=python" requests
+        env:
+          PIP_BREAK_SYSTEM_PACKAGES: 1
+
+      - name: Install Linux test dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get --assume-yes install libc6 libc6-i386 libc6-dev debuginfod
+
+      - name: Install gdbserver dependency
+        run: sudo apt-get --assume-yes install gdbserver
+
+      - name: Build with Meson
+        if: matrix.build_system == 'meson'
+        run: |
+          export PATH=$(python3 -m site --user-base)/bin:${HOME}/.local/bin:${PATH}
+          meson setup --prefix=${HOME} ${{ matrix.meson_options }} build && ninja -C build
+
+      - name: Install with meson
+        if: matrix.build_system == 'meson'
+        run: |
+          # Install rizin
+          export PATH=${HOME}/bin:$(python3 -m site --user-base)/bin:${HOME}/.local/bin:${PATH}
+          ninja -C build install
+
+      - name: Checkout our Testsuite Binaries
+        uses: actions/checkout@v6
+        with:
+          repository: rizinorg/rizin-testbins
+          path: test/bins
+
+      - name: Collecting leaks from unit, integration, regression tests
+        run: |
+          export PATH=${HOME}/bin:$(python3 -m site --user-base)/bin:${HOME}/.local/bin:${PATH}
+          export LSAN_OPTIONS="log_path=$LSAN_LOGS_DIR/$LOG_PREFIX"
+
+          meson test -C build --suite unit -q || true
+          meson test -C build --suite integration -q || true
+          cd test
+          rz-test -q || true
+
+      - name: Archive logs
+        run: |
+          tar czvf lsan_reports.tgz $LSAN_LOGS_DIR
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: lsan_logs
+          path: lsan_reports.tgz
+
+      - name: Check for new leaks
+        env:
+          CHANGES: ${{ needs.real_pr_shas.outputs.CHANGES }}
+        run: |
+          echo "$CHANGES" > changes.diff
+          ./sys/lsan_check.py changes.diff $LSAN_LOGS_DIR/*

sys/lsan_check.py

@@ -3,59 +3,38 @@
 # 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:
+def get_changed_lines(diff: str) -> dict[str, list[range]]:
     """
-    Return dict:  file-path -> [(start_line, end_line), …]
+    Return dict:  filename -> [(start_line, end_line), …]
     representing *added/modified* line ranges in the current branch.
     """
-    changed: Dict[Path, List[Tuple[int, int]]] = {}
-
-    try:
-        subprocess.check_call(["git", "rev-parse", "--verify", f"{base_ref}"])
-        subprocess.check_call(["git", "rev-parse", "--verify", f"{head_ref}"])
-    except subprocess.CalledProcessError:
-        # References were malformed.
-        print(f"One or both references are invalid: {base_ref} and {head_ref}")
-        sys.exit(1)
-
-    # --unified=0  gives hunks like “@@ -L,C +L,C @@”  (we care about the + side)
-    cmd = ["git", "diff", "--unified=0", f"{base_ref}..{head_ref}"]
-    try:
-        out = subprocess.check_output(cmd, text=True)
-    except subprocess.CalledProcessError as e:
-        print(f"Random failure: stderr: {e.stderr}\nstdout: {e.stdout}\n")
-        sys.exit(1)
-    if not out:
-        return None
+    changed: dict[Path, list[tuple[int, int]]] = {}
 
     path = None
-    for line in out.splitlines():
+    for line in diff.splitlines():
         # New file header
-        if line.startswith("+++"):
+        if line.startswith("+++ b/"):
             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))
+        # Changed lines
+        m = re.match(r"@@ -\d+(,\d+)? \+(?P<line_b>\d+)(,(?P<n_b>\d+))? @@", line)
+        if m and path:
+            start = int(m.group("line_b"))
+            count = int(m.group("n_b")) if m.group("n_b") else 1
+            if count != 0:
+                changed[path].append(range(start, start + count))
     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]] = []
+    asan_output: str, changed: dict[str, list[range]]
+) -> 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]
@@ -70,8 +49,7 @@ def parse_asan_leaks(
             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]
+                line in start_end for start_end in changed[name]
             ):
                 leaks.append((name, line, trace))
                 break
@@ -79,21 +57,27 @@ def parse_asan_leaks(
 
 
 def main() -> None:
-    if len(sys.argv) < 4:
+    if len(sys.argv) < 3:
         print("Supply ASAN output via stdin or file argument")
-        print(f"{sys.argv[0]} <base_git_ref> <head_git_ref> [<file.log> ...]")
+        print(f"{sys.argv[0]} <some.diff> [<file.log> ...]")
         sys.exit(2)
-    base_ref = sys.argv[1]
-    head_ref = sys.argv[2]
+    diff_file = sys.argv[1]
+    with open(diff_file, "r", encoding="utf8") as f:
+        diff = f.read()
 
-    changed = get_changed_lines(base_ref, head_ref)
+    changed = get_changed_lines(diff)
     if not changed:
         print("No changed files")
         sys.exit(1)
+    print(changed)
 
     asan_text = ""
-    for i in range(3, len(sys.argv)):
-        asan_text += Path(sys.argv[i]).read_text(encoding="utf8")
+    for i in range(2, len(sys.argv)):
+        p = Path(sys.argv[i])
+        if p.is_dir():
+            print(f"Skip dir: {p}")
+            continue
+        asan_text += p.read_text(encoding="utf8")
     total_leaks, leaks = parse_asan_leaks(asan_text.strip(), changed)
 
     print("\nLEAK REPORT\n")
@@ -108,7 +92,7 @@ def main() -> None:
         for f, l, trace in leaks:
             print("-" * 32)
             print(f"\n{indent}{f}:{l}\n")
-            print(f"{indent}{trace.replace("\n", "\n" + indent)}\n")
+            print(f"{indent}{trace.replace('\n', '\n' + indent)}\n")
         sys.exit(1)
 
     print("No new leaks in changed lines")