|
| 1 | +# Triaging Coverity Scan findings |
| 2 | + |
| 3 | +Static analysis runs on [Coverity Scan](https://scan.coverity.com) via |
| 4 | +[`.github/workflows/coverity.yml`](../.github/workflows/coverity.yml) (weekly + on |
| 5 | +demand). Analysis runs on Black Duck's servers; triage is done in the Scan web UI. |
| 6 | + |
| 7 | +Every finding to date is a false positive in **generated** code — the |
| 8 | +Cython-generated `_pydfti.c` — not in code we maintain. This guide records the |
| 9 | +verified findings and how to keep triage from resetting. |
| 10 | + |
| 11 | +## Where findings come from |
| 12 | + |
| 13 | +`cov-build` captures two C translation units (the `.h` files under |
| 14 | +`mkl_fft/src/` are `#include`d into them, not compiled on their own): |
| 15 | + |
| 16 | +- **Template-generated** `mklfft.c` (from `mkl_fft/src/mklfft.c.src` via |
| 17 | + `_vendored/process_src_template.py`, wired up in `meson.build`). This is *not* |
| 18 | + Cython — it is our oneMKL DFTI descriptor/compute logic, just type-specialized |
| 19 | + for float32/float64/complex64/complex128. It pulls in the hand-written helpers |
| 20 | + in `mkl_fft/src/multi_iter.h` and `mkl_fft/src/mklfft.h`. A real bug can surface |
| 21 | + here, so treat it like hand-written code, **not** boilerplate — review every |
| 22 | + finding. |
| 23 | +- **Cython-generated** `_pydfti.c` (from `mkl_fft/_pydfti.pyx`): `__Pyx_*` / |
| 24 | + `__pyx_pw_*` / `__pyx_tp_*` helpers and wrappers are boilerplate — findings here |
| 25 | + are ~always false positives. `__pyx_pf_*` functions are the C translation of our |
| 26 | + `.pyx` bodies; a real `.pyx` bug could surface there, so keep them in scope |
| 27 | + (Coverity can't see Python-level invariants — see the `_allocate_result` |
| 28 | + OVERRUN fixed in [gh-364](https://github.com/IntelPython/mkl_fft/pull/364)). |
| 29 | + |
| 30 | +## Keeping triage durable: the Cython pin |
| 31 | + |
| 32 | +A Cython *version* bump regenerates `_pydfti.c` wholesale, which churns the |
| 33 | +Coverity CIDs and silently drops their triage — the same boilerplate then returns |
| 34 | +under new CIDs. So **Cython is pinned in `coverity.yml`** (not `pyproject.toml`, |
| 35 | +so shipped wheels are unaffected). The pin works only because the build runs with |
| 36 | +`--no-build-isolation`; bumping it means re-triaging the boilerplate. |
| 37 | + |
| 38 | +## Reducing the noise: a Project Component |
| 39 | + |
| 40 | +**Project Settings → Components** buckets defects by a path regex. Define one to |
| 41 | +group (not hide) the Cython unit so it can be filtered out of view — path-based, |
| 42 | +so it survives regeneration: |
| 43 | + |
| 44 | +- **Name:** `Generated-code` **Path regex:** `.*_pydfti\.c` |
| 45 | + |
| 46 | +Do **not** group `mklfft.c` (or the `mkl_fft/src/*.h` helpers) — that is our DFTI |
| 47 | +logic, not boilerplate. Group only — do **not** mark it *ignored*, as that also |
| 48 | +drops the `__pyx_pf_*` bodies (see [declined](#evaluated-and-declined)). |
| 49 | + |
| 50 | +## Review checklist |
| 51 | + |
| 52 | +Don't blanket-ignore the generated file — prioritise instead: |
| 53 | + |
| 54 | +1. **Findings in `mklfft.c` and `mkl_fft/src/*.h`** — review every one; this is |
| 55 | + our type-specialized oneMKL DFTI code and its inlined iterator/cache helpers, |
| 56 | + not boilerplate. |
| 57 | +2. **High/Medium findings in `__pyx_pf_*`** — verify against `_pydfti.pyx`; if it's |
| 58 | + a Python-level invariant Coverity can't see, mark `False Positive` with a |
| 59 | + reason. If it's a genuine defect, fix the `.pyx` (as with the `_allocate_result` |
| 60 | + OVERRUN). |
| 61 | +3. **Known false-positive families below** — carry the recorded disposition; |
| 62 | + match on **checker + mechanism**, not CID (CIDs reset on a Cython bump or an |
| 63 | + engine upgrade). |
| 64 | + |
| 65 | +## Known false-positive families |
| 66 | + |
| 67 | +Match on **checker + mechanism**, not CID (CIDs reset on a Cython bump or engine |
| 68 | +upgrade). Helper names below are from Cython 3.3.0 and vary between versions. |
| 69 | +All Minor severity, no runtime or security impact. |
| 70 | + |
| 71 | +| Family | Checker | Why it's a false positive | |
| 72 | +| --- | --- | --- | |
| 73 | +| `__pyx_tp_traverse_..._scope_struct_...` (Cython `tp_traverse` slot for a genexpr/closure scope, e.g. from `_get_element_strides`) | DEADCODE | Cython emits a uniform base-type traversal preamble `e = __Pyx_call_type_traverse(o, 1, v, a); if (e) return e;`. The synthesized scope object derives from `object`, whose traverse contributes nothing, so the helper returns 0 and the early-return is dead. The preamble *is* needed for scopes that derive from a GC type. | |
| 74 | + |
| 75 | +The DEADCODE family marks **Intentional**, with disposition **Ignore**. |
| 76 | + |
| 77 | +## Evaluated and declined |
| 78 | + |
| 79 | +- **Modeling files** correct the behavior of *called* functions; our FPs are |
| 80 | + intraprocedural (dead branches, compile-time-constant guards, `#if`), which |
| 81 | + models can't reach. |
| 82 | +- **Dropping the generated unit** (hard exclude), e.g. after `cov-build`: |
| 83 | + ```bash |
| 84 | + cov-manage-emit --dir cov-int --tu-pattern "file('.*_pydfti\\.c')" delete |
| 85 | + ``` |
| 86 | + Also drops the `__pyx_pf_*` bodies, so it's disabled in favour of the checklist. |
0 commit comments