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

[cfg] Update labels file for updated checkers #4383

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gamesh411
Copy link
Collaborator

Since the last release some ClangSA checkers were moved to different packages and some new checkers were introduced. This change updates the label configuration of CodeChecker to reflect this.

I just eyeballed the severities and the profiles, so it's open for suggestions.

Notes:
alpha.security.taint.TaintPropagation does not have a documentation link, so I just put the GenericTaintChecker link as doc-url. Alternatively, we could use doc entry: https://clang.llvm.org/docs/analyzer/user-docs/TaintAnalysisConfiguration.html

With this patch, all the taint-related checkers share the same severity and profiles as the now-removed alpha.security.taint.TaintPropagation.

I put BlockInCriticalSection in the default profile, as I have deemed it not too noisy, and the results were meaningful IMO.

Since the last release some ClangSA checkers were moved to different
packages and some new checkers were introduced. This change updates the
label configuration of CodeChecker to reflect this.
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates.
Some checkers might go to the default profile. Please the noisiness.

@@ -643,6 +663,10 @@
"profile:sensitive",
"severity:MEDIUM"
],
"bugprone-tagged-union-member-count": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe could go into the default profile?

@@ -487,6 +503,10 @@
"sei-cert:dcl51-cpp",
"severity:LOW"
],
"bugprone-return-const-ref-from-parameter": [
"doc_url:https://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html",
"severity:MEDIUM"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default profile?

@@ -788,6 +812,13 @@
"profile:sensitive",
"severity:MEDIUM"
],
"cert-arr39-c": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this checker is an alias of the bugprone-sizeof-expression.
To avoid duplicate findings, we just add the main checker to the profiles and the guidelines. So please move these to the bugprone-sizeof-expression.
"guideline:sei-cert",
"profile:security",
"sei-cert:arr39-c",

@@ -796,6 +827,13 @@
"doc_url:https://clang.llvm.org/extra/clang-tidy/checks/cert/con54-cpp.html",
"severity:MEDIUM"
],
"cert-ctr56-cpp": [
"doc_url:https://clang.llvm.org/extra/clang-tidy/checks/cert/ctr56-cpp.html",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just an alias checker please move teh guideline and sei-cert profile correspondence to
checker
bugprone-pointer-arithmetic-on-polymorphic-object

@@ -194,6 +198,10 @@
"sei-cert:pos44-c",
"severity:MEDIUM"
],
"bugprone-bitwise-pointer-cast": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please consider adding all bugprone chekcers to the default profile if they are not too noisy on the open source projects.

@@ -278,7 +273,7 @@
"profile:sensitive",
"severity:HIGH"
],
"alpha.unix.SimpleStream": [
"unix.SimpleStream": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe to profile:default depending how noisy it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants