Repository navigation
task: coverity - #3070
task: coverity#3070jharlow-intel wants to merge 7 commits into
Conversation
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/3070/index.html |
|
Array API standard conformance tests for dpnp=0.21.0dev16=np2py314h8d9cdd5_13 ran successfully. |
|
coverage: 82.64% (+0.03%) from 82.613% — task/coverity into master |
Unpinned `cython` resolves to whatever PyPI has that Monday, and a Cython upgrade rewrites the generated C, which shuffles Coverity CIDs and loses their triage. Pin 3.3.0, the version conda-forge currently resolves for the regular builds, matching the mkl_fft and mkl-service scans. Also align the upload-artifact pin with master (v7.0.2).
| cov-configure --gcc | ||
| cov-configure --template --comptype clangcc --compiler icx | ||
| cov-configure --template --comptype clangcxx --compiler icpx |
There was a problem hiding this comment.
This is the same setup as dpctl's coverity workflow, and there it captures less than half of the units: run 39184872884 reports Emitted 18 C/C++ compilation units (46%) successfully.
Every failed unit in its cov-int/build-log.txt stops inside the SYCL headers:
sycl/sampler.hpp:unknown type name '__ocl_sampler_t'sycl/detail/image_ocl_types.hpp:unknown type name '__ocl_image2d_ro_t'sycl/detail/is_device_copyable.hpp:'T' does not refer to a value
So the generic clang template parses <sycl/sycl.hpp> without SYCL mode. dpctl's Scan project still shows "No builds were successfully analyzed yet". In dpnp, 225 of 256 .cpp/.pyx sources use SYCL directly, so the capture here would be much lower than 46%.
mkl_umath reaches 100% by preferring the native Intel compiler type from cov-configure --list-compiler-types (intel_oneapi_icx, then intel_icx:linux). Could we try that for icpx too? It's only proven for plain C there, so a dispatch run would need to confirm it handles the SYCL TUs.
There was a problem hiding this comment.
Confirmed, and it is worse than 46% suggests: per source, the clang templates capture only 9 of dpctl's 30. Tried your suggestion on dpctl, whose workflow can be dispatched from a branch (IntelPython/dpctl#2401).
intel_icx:linux (the Scan build tool does not ship intel_oneapi_icx yet) captures all 30. It emits each SYCL source twice, a host pass and a spir64 device pass. The device pass never parses: Coverity runs it with an x86_64 target, so the device-only headers fail on __fp16, and with -fnative-half-type passed through it crashes on the __spirv_ocl_* builtins instead. The host pass is what gets analyzed, but the failed device passes keep the Emitted (P%) total at 66% for dpctl, and it would be about 50% here.
So the 85% gate could never pass. In b01a398 it checks per source instead: every workspace source outside CMakeFiles/ needs a completed emit. On dpctl that passes 30/30 (run 40340192472) and fails the master log at 9/30. b01a398 also skips the upload on non-master refs, so a branch dispatch cannot replace master's results on the dashboard. dpnp can only be dispatched after merge, so its first run will be the real check.
| if ! grep -qE "Emitted [1-9][0-9]* .*compilation unit" cov-build.log; then | ||
| echo "::error::Coverity captured 0 compilation units — the C++ build did not run under cov-build." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This only rejects 0 units, so a run that captures e.g. 3 of ~250 units still passes and uses up a scan. Could we match C/C++ explicitly and fail on a low capture rate? Something like:
pct=$(grep -oP 'Emitted [0-9]+ C/C\+\+ compilation units? \(\K[0-9]+(?=%\))' cov-build.log | tail -1)
if [ -z "${pct}" ] || [ "${pct}" -lt 85 ]; then
echo "::error::Coverity captured only ${pct:-0}% of C/C++ compilation units."
exit 1
fiThere was a problem hiding this comment.
Done in 0bd09e7 with your regex and 85%. Added || true on the pipeline: under pipefail a no-match grep in pct=$(...) makes bash -e exit before the ::error:: prints. Checked against the dpctl log (fails at 46%) plus 100%, 84% and no-Emitted-line cases. The step summary is now written before the gate so it shows on failing runs too.
There was a problem hiding this comment.
Update: b01a398 replaces the percentage gate with a per-source check. With the Intel compiler type the percentage counts SYCL device passes that never parse, so 85% is unreachable. Details in the L137 thread.
| # Force a clean rebuild so icpx genuinely runs and Coverity can capture | ||
| # it. A stale build tree would skip compilation and yield "No files | ||
| # were emitted", which the upload rejects. | ||
| rm -rf _skbuild |
There was a problem hiding this comment.
Nit: _skbuild doesn't exist on a fresh checkout, so this is a no-op and the stale-tree comment above doesn't apply. Could drop it, or use build_locally.py --clean if an explicit clean is wanted.
| curl --no-progress-meter --fail-with-body \ | ||
| --retry 5 --retry-connrefused --retry-delay 5 \ | ||
| --form token="${COVERITY_SCAN_TOKEN}" \ | ||
| --form email="${COVERITY_SCAN_EMAIL}" \ | ||
| --form file=@cov-int.tgz \ | ||
| --form version="${GITHUB_SHA}" \ | ||
| --form description="GitHub Actions ${GITHUB_REF_NAME} (run ${GITHUB_RUN_ID})" \ | ||
| --form project="${COVERITY_PROJECT}" \ | ||
| https://scan.coverity.com/builds |
There was a problem hiding this comment.
Nit: if Scan rejects this direct POST because the tarball is too large, the fallback is the upload flow from the project's "Submit build" page:
POST /projects/<id>/builds/initreturns an uploadurland abuild_id.PUTthe tarball to thaturl.PUT /projects/<id>/builds/<build_id>/enqueue.
No change needed until the first run shows the size. Just flagging it.
There was a problem hiding this comment.
Thanks. The step already logs the tarball size, so the first run will show whether we need the init/PUT/enqueue flow.
The generic clangcc/clangcxx templates parse the SYCL headers without SYCL mode, so most SYCL translation units fail to emit (dpctl's scan with the same setup captures 46%). Register icx/icpx with Coverity's Intel oneAPI compiler type instead, as mkl_umath does, and fail the job when fewer than 85% of C/C++ units are captured so a partial capture does not use up a scan. Drop the _skbuild removal, which is a no-op on a fresh checkout.
The native Intel compiler type emits each SYCL source as a host pass and a spir64 device pass. The Scan build tool cannot parse the device pass, so the overall emit percentage tops out near 50% even when every source is captured. Require every project source to have a completed emit instead, and skip the upload on feature-branch dispatches so they can validate capture without replacing master's results on the dashboard.
https://scan.coverity.com/projects/intelpython-dpnp?tab=overview
Pretty much the same as the others