Charusso marked 5 inline comments as done.
Charusso added a comment.
Thanks for the feedback! Given that it will remain an `alpha` checker for a
long time (~1 year), no one really should use it.
================
Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""""""""""""""""""""""""""
----------------
Szelethus wrote:
> balazske wrote:
> > There are already more checkers that can check for CERT related problems
> > but not specially made for these. These checkers do not reside in this new
> > `cert` group. And generally a checker does not check for specifically a
> > CERT rule, instead for more of them or other things too, or more checkers
> > can detect a single rule. (And the user can think that only these CERT
> > rules are checkable that exist in this package, that is not true.) So I do
> > not like the introduction of this new `cert` package. (The documentation of
> > existing checkers lists if the checker is designed for a CERT rule.)
> I disagree to some extent. I think it would be great to have a `cert` package
> that houses all checkers for each of the rules with the addition of checker
> aliases. Clang-tidy has something similar as well!
I designed the checker only for the rule STR31-C that is why I have picked
package `cert`. Clang-Tidy could aliasing checks. For example the check could
be in the `bugprone` category aliased to `cert` and both could trigger the
analysis.
> the user can think that only these CERT rules are checkable
We only need to move `security.FloatLoopCounter` under the `cert` package. What
else SEI CERT rules are finished off other than package `cert`? It is not my
fault if someone could not package well or could not catch most of the issues
of a SEI CERT rule or could not reach the SEI CERT group to note the fact the
Analyzer catch a very tiny part of a rule. However, this patch package well and
could catch most of the STR31-C rule.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h:22
+extern const char *const CXXObjectLifecycle;
+extern const char *const SecurityError;
+} // namespace categories
----------------
balazske wrote:
> Are there already not other checkers that find security related bugs (the
> taint checker?)? Why do these not use a `SecurityError`? It is not bad to
> have a `SecurityError` but maybe there is a reason why was it not there
> already. If these categories are exclusive it is hard to find out what
> problem (probably already existing bug type in other checkers) belongs to
> what category (it can be for this checker `UnixAPI` or `MemoryError` too?).
We have only three pure security checkers: `security.insecureAPI`,
`security.FloatLoopCounter` (FLP30-C, FLP30-CPP) and
`alpha.security.cert.pos.34c`. The non-alpha checkers are very old, but Ted
definitely made that category:
```
const char *bugType = "Floating point variable used as loop counter";
BR.EmitBasicReport(bugType, "Security", os.str().c_str(),
FS->getLocStart(), ranges.data(), ranges.size());
```
-
https://github.com/llvm/llvm-project/commit/9c49762776b4d2cb16911dec0c873c90a6508968
Every other security-ish checker does not catch the CERT rule's examples. May
if the checker evolves I would pick MemoryError, but it will not evolve soon.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70411/new/
https://reviews.llvm.org/D70411
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
