Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look into static analysis options #180

Open
azonenberg opened this issue Sep 21, 2020 · 8 comments
Open

Look into static analysis options #180

azonenberg opened this issue Sep 21, 2020 · 8 comments
Assignees
Labels
build Build system and infrastructure
Milestone

Comments

@azonenberg
Copy link
Collaborator

No description provided.

@bvernoux
Copy link
Contributor

bvernoux commented Oct 1, 2020

A good start will be to always check the source code (especially new source code) with CppCheck 2.1 (or more) http://cppcheck.sourceforge.net/

@tarunik
Copy link
Collaborator

tarunik commented Oct 2, 2020

As long as the code compiles with clang (which may be worthwhile on its own for diagnostics/compiler-portability reasons), we should give the clang-analyzer a whirl as well.

@azonenberg
Copy link
Collaborator Author

azonenberg commented Oct 2, 2020

Let's start with cppcheck. @tarunik can you work on that?

My thought is to detect cppcheck and (if present) create a new build target "analysis" which is built by default as part of "make all", which runs cppcheck and displays all detected error messages. This should be optional and gracefully degrade if the user doesn't have cppcheck installed.

@tarunik
Copy link
Collaborator

tarunik commented Oct 2, 2020

Yeah -- it looks like CMake has built-in support for at least cppcheck and clang-tidy (albeit not the full clang-analyzer?): https://blog.kitware.com/static-checks-with-cmake-cdash-iwyu-clang-tidy-lwyu-cpplint-and-cppcheck/

@azonenberg
Copy link
Collaborator Author

Oh even better. Add cppcheck integration and send a PR when you're ready? We'll close this ticket when that's done, then think about other static analyzers in the future if we see a need for it.

tarunik added a commit to tarunik/scopehal-apps that referenced this issue Oct 9, 2020
This currently works with cppcheck on Linux using CMake's built-in support.
tarunik added a commit to tarunik/scopehal-apps that referenced this issue Oct 9, 2020
@tarunik
Copy link
Collaborator

tarunik commented Oct 9, 2020

Did you want static analysis mode to require all the static analyzers to be present? (so far, it'll be cppcheck and clang-analyzer, clang-tidy will likely go on the list as well)

@azonenberg
Copy link
Collaborator Author

Ideally it should detect each individually and enable as many as it find and knows how to use.

tarunik added a commit to tarunik/scopehal-apps that referenced this issue Oct 11, 2020
Note that this causes g++ to spit out a spurious warning about ignoring a linker input file since it's not linking anything.  This warning is utterly harmless and only appears in ANALYZE=true mode.
azonenberg added a commit that referenced this issue Oct 11, 2020
Add static analyzer support (CPPCheck and clang-analyzer, at the moment) for #180
@azonenberg azonenberg added the build Build system and infrastructure label Oct 11, 2020
@azonenberg
Copy link
Collaborator Author

Merged, tested, and fixed a bunch of findings.

On my machine, cppcheck fails silently (no errors displayed but nonzero exit code) on VICPSocketTransport.cpp.

@azonenberg azonenberg added this to the v0.1 milestone Apr 24, 2021
@azonenberg azonenberg assigned Johnsel and unassigned tarunik Sep 10, 2022
@azonenberg azonenberg modified the milestones: v0.1, v0.2 Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system and infrastructure
Projects
None yet
Development

No branches or pull requests

5 participants