-
Notifications
You must be signed in to change notification settings - Fork 96
Add optional GCC static analyzer support to CMake and Conan builds #817
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
base: master
Are you sure you want to change the base?
Conversation
pnoltes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR :)
Some small remarks and a question.
Is it also possible to enable one of the CI builds (ubuntu, gcc) with the analyzer option?
And then suppress the warnings we have not yet fixed (-Wno-analyzer-double-fclose, etc).
This can also help in follow up task (enable the analyzer warnings, flag per flag).
conanfile.py
Outdated
| tc.cache_variables["BUILD_ERROR_INJECTOR_CURL"] = "ON" | ||
| tc.cache_variables["CELIX_ERR_BUFFER_SIZE"] = str(self.options.celix_err_buffer_size) | ||
| # tc.cache_variables["CMAKE_PROJECT_Celix_INCLUDE"] = os.path.join(self.build_folder, "conan_paths.cmake") | ||
| if self.options.enable_gcc_analyzer and self.settings.compiler == "gcc": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed, the "enable_gcc_analyzer" will be available in CMake as ENABLE_GCC_ANALYZER
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnoltes means the setting of CMAKE_C_FLAGS/CMAKE_CXX_FLAGS is not needed, which is already included in CMAKE. enable_gcc_analyzer is still needed to enable the corresponding CMake option (ENABLE_GCC_ANALYZER).
My understanding is that -fanalyzer gcc option does not affect binary output, and thus we should remove enable_gcc_analyzer from the package id computation, i.e. adding del self.info.options.enable_gcc_analyzer to the package_id method.
| set(CMAKE_C_FLAGS "-fanalyzer ${CMAKE_C_FLAGS}") | ||
| set(CMAKE_CXX_FLAGS "-fanalyzer ${CMAKE_CXX_FLAGS}") | ||
| else() | ||
| message(WARNING "ENABLE_GCC_ANALYZER is only supported with GCC compiler 10.0.0 OR higher.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| message(WARNING "ENABLE_GCC_ANALYZER is only supported with GCC compiler 10.0.0 OR higher.") | |
| message(WARNING "ENABLE_GCC_ANALYZER is only supported with GCC") |
I would not mention the version, because that is not what is tested in the if.
|
Hi @pnoltes, thanks for the review. i can surely enable the analyzer in the Ubuntu GCC CI build and suppress the warnings, but i have a question. By reviewing the docs of -fanalyzer see Options That Control Static Analysis, how do you suggest adding these flags to the build in efficient way like to make it easy to suppress every flag one after the other ? my approach is 07c3f50 if there is a better and more efficient approach i will be happy to learn and apply |
This PR adds optional support for GCC’s static analyzer (-fanalyzer) to the Celix build system.
What’s included
Introduces a new
CMakeoption:ENABLE_GCC_ANALYZER (OFF by default)
When enabled and using
GCC,-fanalyzeris added to:CMAKE_C_FLAGSCMAKE_CXX_FLAGSExtends conanfile.py with a corresponding option
(enable_gcc_analyzer)so the analyzer can also be enabled in Conan-based builds.What’s not included
No analyzer warnings are fixed in this PR.
The analyzer is not enabled in CI.
No behavior or runtime changes.
GCC’s static analyzer has been improving significantly in recent releases and can help detect issues such as possible null dereferences and resource leaks.
This PR lays the groundwork for experimenting with the analyzer locally and for potential future CI integration.
Analyzer warnings can be addressed incrementally in follow-up PRs once there is agreement on scope and expectations.