ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ztamas edited the summary of this revision.
ztamas edited the summary of this revision.
ztamas edited the summary of this revision.
ztamas added a reviewer: aaron.ballman.
Added CertSTR34C option to filter out unrelated use cases.
The SEI cert catches explicit integer casts (two use cases),
while in the case of `signed char` \ `unsigned char`
comparison, we have an implicit conversion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79334
Files:
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cert-str34-c %t
+
+// Check whether alias is actually working.
+int SimpleVarDeclaration() {
+ signed char CCharacter = -5;
+ int NCharacter = CCharacter;
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [cert-str34-c]
+
+ return NCharacter;
+}
+
+// Check whether bugprone-signed-char-misuse.CertSTR34C option is set correctly.
+int SignedUnsignedCharEquality(signed char SCharacter) {
+ unsigned char USCharacter = 'a';
+ if (SCharacter == USCharacter) // no warning
+ return 1;
+ return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -320,6 +320,7 @@
`cert-oop11-cpp <cert-oop11-cpp.html>`_, `performance-move-constructor-init <performance-move-constructor-init.html>`_, "Yes"
`cert-oop54-cpp <cert-oop54-cpp.html>`_, `bugprone-unhandled-self-assignment <bugprone-unhandled-self-assignment.html>`_,
`cert-pos44-c <cert-pos44-c.html>`_, `bugprone-bad-signal-to-kill-thread <bugprone-bad-signal-to-kill-thread.html>`_,
+ `cert-str34-c <cert-str34-c.html>`_, `bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_,
`clang-analyzer-core.CallAndMessage <clang-analyzer-core.CallAndMessage.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
`clang-analyzer-core.DivideZero <clang-analyzer-core.DivideZero.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
`clang-analyzer-core.NonNullParamChecker <clang-analyzer-core.NonNullParamChecker.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-str34-c
+.. meta::
+ :http-equiv=refresh: 5;URL=bugprone-signed-char-misuse.html
+
+cert-str34-c
+==============
+
+The cert-str34-c check is an alias, please see
+`bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_
+for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,6 +3,9 @@
bugprone-signed-char-misuse
===========================
+`cert-str34-c` redirects here as an alias for this check. For the CERT alias,
+the `CertSTR34C` option is set to `1`.
+
Finds those ``signed char`` -> integer conversions which might indicate a
programming error. The basic problem with the ``signed char``, that it might
store the non-ASCII characters as negative values. This behavior can cause a
@@ -108,3 +111,9 @@
check. This is useful when a typedef introduces an integer alias like
``sal_Int8`` or ``int8_t``. In this case, human misinterpretation is not
an issue.
+
+.. option:: CertSTR34C
+
+ When non-zero, the check will warn only in the cases described by the
+ STR34-C SEI cert rule. This check covers more use cases, which can be
+ limited with this option.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -168,6 +168,11 @@
:doc:`bugprone-reserved-identifier
<clang-tidy/checks/bugprone-reserved-identifier>` was added.
+- New alias :doc:`cert-str34-c
+ <clang-tidy/checks/cert-str34-c>` to
+ :doc:`bugprone-signed-char-misuse
+ <clang-tidy/checks/bugprone-signed-char-misuse>` was added.
+
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../bugprone/BadSignalToKillThreadCheck.h"
#include "../bugprone/ReservedIdentifierCheck.h"
+#include "../bugprone/SignedCharMisuseCheck.h"
#include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h"
#include "../bugprone/UnhandledSelfAssignmentCheck.h"
#include "../google/UnnamedNamespaceInHeaderCheck.h"
@@ -108,6 +109,9 @@
// POS
CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>(
"cert-pos44-c");
+ // STR
+ CheckFactories.registerCheck<bugprone::SignedCharMisuseCheck>(
+ "cert-str34-c");
}
ClangTidyOptions getModuleOptions() override {
@@ -115,6 +119,7 @@
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0";
+ Opts["cert-str34-c.CertSTR34C"] = "1";
return Options;
}
};
Index: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
@@ -38,6 +38,7 @@
const std::string &CastBindName) const;
const std::string CharTypdefsToIgnoreList;
+ const bool CertSTR34C;
};
} // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
@@ -29,10 +29,12 @@
SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")) {}
+ CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")),
+ CertSTR34C(Options.get("CertSTR34C", false)) {}
void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
+ Options.store(Opts, "CertSTR34C", CertSTR34C);
}
// Create a matcher for char -> integer cast.
@@ -92,16 +94,18 @@
Finder->addMatcher(Declaration, this);
- // Catch signed char/unsigned char comparison.
- const auto CompareOperator =
- expr(binaryOperator(hasAnyOperatorName("==", "!="),
- anyOf(allOf(hasLHS(SignedCharCastExpr),
- hasRHS(UnSignedCharCastExpr)),
- allOf(hasLHS(UnSignedCharCastExpr),
- hasRHS(SignedCharCastExpr)))))
- .bind("comparison");
-
- Finder->addMatcher(CompareOperator, this);
+ if (!CertSTR34C) {
+ // Catch signed char/unsigned char comparison.
+ const auto CompareOperator =
+ expr(binaryOperator(hasAnyOperatorName("==", "!="),
+ anyOf(allOf(hasLHS(SignedCharCastExpr),
+ hasRHS(UnSignedCharCastExpr)),
+ allOf(hasLHS(UnSignedCharCastExpr),
+ hasRHS(SignedCharCastExpr)))))
+ .bind("comparison");
+
+ Finder->addMatcher(CompareOperator, this);
+ }
// Catch array subscripts with signed char -> integer conversion.
// Matcher for C arrays.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
