-
Notifications
You must be signed in to change notification settings - Fork 383
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
Gcc analyzer native support #4030
Gcc analyzer native support #4030
Conversation
84a6396
to
472611b
Compare
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.
analyzer/.coverage might have been committed accidentally.
@@ -3,7 +3,8 @@ | |||
"analyzers": { | |||
"clangsa": "clang", | |||
"clang-tidy": "clang-tidy", | |||
"cppcheck": "cppcheck" | |||
"cppcheck": "cppcheck", | |||
"gcc": "g++" |
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 an interesting question, how we could support both gcc
and g++
. Can this gcc analyzer analyze both .c and .cpp build actions?
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.
The same thing we do with clang. As I understand, these are functionally the same:
clang++ == clang -x cpp
clang == clang++ -x c
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.
I am not ready with the review. I try to install gcc13 and then
continue testing...
472611b
to
fdd6a77
Compare
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.
Nice job!
In general looks good.
- Please fix the binary exploration stuff. that's prohibitive even for the release-candidate
- some error and warning messages look too aggessive
- we will need to categorize the checkers into profiles and severities
- gcc static analyzer is said to be not working very well on c++ code at least in gcc 12. Should we recommend introduce a gcc analyzer option to only analyze c files? https://developers.redhat.com/articles/2022/04/12/state-static-analysis-gcc-12-compiler#
otherwise how the user gets rid off false warnings in C++ files. - also add the gcc analyzer support to this doc https://github.com/Ericsson/codechecker/blob/master/docs/supported_code_analyzers.md
I'm afraid you tested an outdated version :^) I'll see to the other comments ASAP. |
Looking at the current version of this PR, messages look right. Do you agree? |
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.
- Please change the used gcc binary to gcc instead of g++
- please fix the error message for the gcc analyzer options
Then this is good to go for RC.
Config options for gcc. | ||
""" | ||
# TODO | ||
return [] |
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.
To return an empty list is valid here I think.
Now, I am getting this error:
workspace/test-projects/xerces-c]$ CodeChecker analyzers --analyzer-config gcc
[ERROR 2023-10-16 12:00] - Failed to get analyzer configuration options for 'gcc' analyzer! Please try to upgrade your analyzer version to use this feature.
However not supporting any option is not an error.
CodeChecker should write something like this instead:
No analyzer options supported for gcc.
f1ae95b
to
058ecfe
Compare
I intend to fix that with the alternative solution for #4041 (where we will have at least one analyzer config for each analyzer). |
058ecfe
to
a6b442e
Compare
a6b442e
to
0619997
Compare
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.
LGTM. Please follow-up todos in later patches.
WIP. Depends on #4011.