Skip to content

GHA/linux: replace scan-build with clang-tidy#20751

Closed
vszakats wants to merge 2 commits intocurl:masterfrom
vszakats:scan-build-to-clang-tidy
Closed

GHA/linux: replace scan-build with clang-tidy#20751
vszakats wants to merge 2 commits intocurl:masterfrom
vszakats:scan-build-to-clang-tidy

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Feb 27, 2026 •
edited
Loading

scan-build is a (Perl) wrapper around clang's built-in --analyze
option. Which look similar or identical to clang-tidy checkers under
the clang-analyzer-* namespace:
https://clang.llvm.org/docs/ClangStaticAnalyzer.html

Unless somebody has other information, it appears redundant to run
scan-build in parallel with clang-tidy in CI, now that the latter is
working reliably and with good performance for all curl components.

Another scan-build issue is the lack of a markup to suppress false
positives. It ignores NOLINT, yet finds the same false positives as
clang-tidy. This happens with scan-build v20+. v18 is silent, but it's
a blocker to upgrade to a newer version.

scan-build may still be a useful when combined with autotools, where
clang-tidy support is incomplete, slow (no parallelism), and uses
a distinct make target, which does not build binaries in the same pass.
But, scan-build also lacks extra checkers that are now enabled for
clang-tidy.

The clang-tidy job is also 30-40s faster than the one it replaced.

Also:

  • drop scan-build job configured the same way as a clang-tidy one.
    CI time saved: 6m30s
  • bump to clang-20 (from 18) in the replacement job.
  • build tests in the replacement job.
    To verify a cmake command-line reconstruction issue only hit in this
    job in CI.
    CI time cost: 1m40s
  • replacement job caught a minor, new, issue.
    Ref: b2076d3 vquic: fix unused variable warning reported by clang-tidy #20752
  • drop unused scan-build logic.

Bug: #20732 (comment)
Ref: #20732 (comment)

Before: https://github.com/curl/curl/actions/runs/22466648624/job/65074048881
After: https://github.com/curl/curl/actions/runs/22467420456/job/65076579641?pr=20751

@github-actions github-actions bot added the CI Continuous Integration label Feb 27, 2026
@vszakats vszakats mentioned this pull request Feb 27, 2026
Closed
2 tasks
vszakats added a commit to vszakats/curl that referenced this pull request Feb 27, 2026
@vszakats
vquic: fix unused variable warning reported by clang-tidy
495d70e 
Silencing (seen on Linux H3 v20 job):
```
lib/vquic/vquic.c:398:37: error: variable 'calls' set but not used [clang-diagnostic-unused-but-set-variable]
  398 |   size_t total_nread = 0, pkts = 0, calls = 0;
      |                                     ^
```

Cherry-picked from curl#20751
@vszakats vszakats mentioned this pull request Feb 27, 2026
Closed
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Feb 27, 2026 •
edited
Loading

Never a dull compiler run:

Error while processing /home/runner/work/curl/curl/bld/tests/libtest/lib1521.c.
/usr/include/openssl/macros.h:147:4: error: "OPENSSL_API_COMPAT expresses an impossible API compatibility level" [clang-diagnostic-error]
Found compiler error(s).
  147 | #  error "OPENSSL_API_COMPAT expresses an impossible API compatibility level"
      |    ^
FAILED: [code=1] tests/libtest/CMakeFiles/libtests-clang-tidy

https://github.com/curl/curl/actions/runs/22468472670/job/65079885471?pr=20751

edit: it looks like yet another compiler command-line reconstruction issue,
and mixed up order and/or -I/-isystem, resulting in including the wrong
OpenSSL header, from /usr/include.

vszakats added a commit that referenced this pull request Feb 27, 2026
@vszakats
vquic: fix unused variable warning reported by clang-tidy
b2076d3 
Silencing (seen in new GHA/Linux H3 v20 job):
```
lib/vquic/vquic.c:398:37: error: variable 'calls' set but not used [clang-diagnostic-unused-but-set-variable]
  398 |   size_t total_nread = 0, pkts = 0, calls = 0;
      |                                     ^
```

Cherry-picked from #20751

Closes #20752
@vszakats vszakats force-pushed the scan-build-to-clang-tidy branch from 54b781a to 73c7aca Compare February 27, 2026 01:47
@testclutch

This comment was marked as outdated.

Sign in to view
@vszakats vszakats marked this pull request as draft February 27, 2026 02:56
@vszakats vszakats force-pushed the scan-build-to-clang-tidy branch from 3e76fcc to 6db7070 Compare February 27, 2026 10:54
vszakats added a commit to vszakats/curl that referenced this pull request Feb 27, 2026
@vszakats
cmake: fix system include directory position for clang-tidy in tests
bafd9a4 
To avoid a system include masking a custom directory, and e.g. picking
up system OpenSSL headers from `/usr/include` on Linux, instead of the
correct ones from a custom header directory, move system include
directories to the back of the header path list. Also to match what
CMake seems to be doing for the C compiler command-lines it generates.

CMake seems to use `-I`, while for these invocations we stick with
`-isystem` just in case.

This area remains fragile and likely not the final issue.

Fixing (seen in GHA/linux H3 c-ares):
```
Error while processing bld/tests/libtest/lib1521.c.
/usr/include/openssl/macros.h:147:4: error: "OPENSSL_API_COMPAT expresses an impossible API compatibility level" [clang-diagnostic-error]
Found compiler error(s).
  147 | #  error "OPENSSL_API_COMPAT expresses an impossible API compatibility level"
      |    ^
FAILED: [code=1] tests/libtest/CMakeFiles/libtests-clang-tidy
```
Ref: https://github.com/curl/curl/actions/runs/22468472670/job/65079885471?pr=20751

Bug: curl#20751 (comment)
Cherry-picked from curl#20751
@vszakats vszakats mentioned this pull request Feb 27, 2026
Closed
@vszakats vszakats force-pushed the scan-build-to-clang-tidy branch from e382892 to 95adb1f Compare February 27, 2026 12:27
vszakats added a commit that referenced this pull request Feb 27, 2026
@vszakats
cmake: fix system include directory position for clang-tidy in tests
d9386a2 
To avoid a system include masking a custom directory, and e.g. picking
up system OpenSSL headers from `/usr/include` on Linux, instead of the
correct ones from a custom header directory, move system include
directories to the back of the header path list. Also to match what
CMake seems to be doing for the C compiler command-lines it generates.

CMake seems to use `-I`, while for these invocations we stick with
`-isystem` just in case.

This area remains fragile and likely not the final issue.

Fixing (seen in GHA/linux H3 c-ares):
```
Error while processing bld/tests/libtest/lib1521.c.
/usr/include/openssl/macros.h:147:4: error: "OPENSSL_API_COMPAT expresses an impossible API compatibility level" [clang-diagnostic-error]
Found compiler error(s).
  147 | #  error "OPENSSL_API_COMPAT expresses an impossible API compatibility level"
      |    ^
FAILED: [code=1] tests/libtest/CMakeFiles/libtests-clang-tidy
```
Ref: https://github.com/curl/curl/actions/runs/22468472670/job/65079885471?pr=20751

Bug: #20751 (comment)
Cherry-picked from #20751

Closes #20759
@vszakats
GHA/linux: replace scan-build with clang-tidy
35e6c9c 
[WIP]

scan-build is a Perl wrapper over clang's `--analyze` option, which
in turn uses the same checks as clang-tidy:
https://clang.llvm.org/docs/ClangStaticAnalyzer.html

`scan-build` is a (Perl) wrapper around clang's built-in `--analyze`
option. Which look similar or identical to clang-tidy checkers under
the `clang-analyzer-*` namespace:
https://clang.llvm.org/docs/ClangStaticAnalyzer.html

Unless somebody has other information, it appears redundant to run
scan-build in parallel with clang-tidy in CI, now that the latter is
working reliably and with good performance for all curl components.

Another scan-build issue is the lack of a markup to suppress false
positives. It ignores `NOLINT`, yet find the same false positives as
clang-tidy. This happens with scan-build v20+. (v19 is silent.)

scan-build may still be a useful option in conjunction with autotools,
where clang-tidy support is incomplete, slow (due to no parallelism),
and uses a distinct make target, which does not build targets at the
same time. But, scan-built lacks extra checkers that are now enabled for
clang-tidy.

Also:
- drop duplicate job.
- drop unused scan-build logic.

Bug: curl#20732 (comment)
Ref: curl#20732 (comment)
@vszakats
run it on tests [TEST]
c93e2f9
@vszakats vszakats force-pushed the scan-build-to-clang-tidy branch from 95adb1f to c93e2f9 Compare February 27, 2026 13:00
@vszakats vszakats marked this pull request as ready for review February 27, 2026 13:00
@vszakats vszakats closed this in ce4db9c Feb 27, 2026
@vszakats vszakats deleted the scan-build-to-clang-tidy branch February 27, 2026 13:09
@vszakats vszakats mentioned this pull request Mar 1, 2026
Closed
1 task
vszakats added a commit that referenced this pull request Mar 2, 2026
@vszakats
clang-tidy: enable clang-analyzer-* checks explicitly, fix fallout
7a77884 
v22.1.0 disabled them by default.

Fix fallout:
- http: check NULL to silence false positives in `HD_VAL()`.

Ref: https://releases.llvm.org/22.1.0/tools/clang/tools/extra/docs/ReleaseNotes.html#improvements-to-clang-tidy

Follow-up to da6fbb1 #20779
Follow-up to ce4db9c #20751

Closes #20778
@vszakats vszakats mentioned this pull request Mar 8, 2026
Closed
vszakats added a commit that referenced this pull request Mar 9, 2026
@vszakats
rand: drop scan-build silencer
716dc8c 
scan-build has been dropped in favor of clang-tidy and this false
positive no longer triggers with it.

Follow-up to ce4db9c #20751
Follow-up to 02f207a

Closes #20860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

CI Continuous Integration

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vszakats @testclutch