[clang-tools-extra] fedd526 - [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.
Tamás Zolnai via cfe-commits
cfe-commits at lists.llvm.org
Wed May 6 03:36:32 PDT 2020
Author: Tamás Zolnai
Date: 2020-05-06T12:36:01+02:00
New Revision: fedd52682ec70fd13b08eeac99ee0954292af9da
URL: https://github.com/llvm/llvm-project/commit/fedd52682ec70fd13b08eeac99ee0954292af9da
DIFF: https://github.com/llvm/llvm-project/commit/fedd52682ec70fd13b08eeac99ee0954292af9da.diff
LOG: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.
Summary:
Added `DiagnoseSignedUnsignedCharComparisons` 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 implicit
conversions.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79334
Added:
clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst
clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp
Modified:
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/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
index 66f00e35c7e7..3f72e5d516c5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
@@ -29,10 +29,14 @@ static Matcher<TypedefDecl> hasAnyListedName(const std::string &Names) {
SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")) {}
+ CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")),
+ DiagnoseSignedUnsignedCharComparisons(
+ Options.get("DiagnoseSignedUnsignedCharComparisons", true)) {}
void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
+ Options.store(Opts, "DiagnoseSignedUnsignedCharComparisons",
+ DiagnoseSignedUnsignedCharComparisons);
}
// Create a matcher for char -> integer cast.
@@ -92,16 +96,18 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
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 (DiagnoseSignedUnsignedCharComparisons) {
+ // 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.
diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
index 3fa0c9c0a088..84d3bbbf4e76 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
@@ -38,6 +38,7 @@ class SignedCharMisuseCheck : public ClangTidyCheck {
const std::string &CastBindName) const;
const std::string CharTypdefsToIgnoreList;
+ const bool DiagnoseSignedUnsignedCharComparisons;
};
} // namespace bugprone
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 6459dcf5627d..6592d2247b56 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/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 @@ class CERTModule : public ClangTidyModule {
// POS
CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>(
"cert-pos44-c");
+ // STR
+ CheckFactories.registerCheck<bugprone::SignedCharMisuseCheck>(
+ "cert-str34-c");
}
ClangTidyOptions getModuleOptions() override {
@@ -115,6 +119,7 @@ class CERTModule : public ClangTidyModule {
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0";
+ Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "0";
return Options;
}
};
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 97ef4271d615..9e4fa62b7ab6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -168,6 +168,11 @@ New check aliases
: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
^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
index c2bd2df062b1..2894495995a0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ b/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 `DiagnoseSignedUnsignedCharComparisons` option is set to `0`.
+
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,8 @@ so both arguments will have the same type.
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:: DiagnoseSignedUnsignedCharComparisons
+
+ When nonzero, the check will warn on ``signed char``/``unsigned char`` comparisons,
+ otherwise these comparisons are ignored. By default, this option is set to ``1``.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst
new file mode 100644
index 000000000000..f28311b98a1c
--- /dev/null
+++ b/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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c3d047f30292..b4d09d526726 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -320,6 +320,7 @@ Clang-Tidy Checks
`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>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp
new file mode 100644
index 000000000000..2257066210e1
--- /dev/null
+++ b/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.DiagnoseSignedUnsignedCharComparisons option is set correctly.
+int SignedUnsignedCharEquality(signed char SCharacter) {
+ unsigned char USCharacter = 'a';
+ if (SCharacter == USCharacter) // no warning
+ return 1;
+ return 0;
+}
More information about the cfe-commits
mailing list