#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
+#include "../utils/Matchers.h"
using namespace clang::ast_matchers;
namespace tidy {
namespace misc {
-AST_MATCHER(BinaryOperator, isComparisonOperator) {
- return Node.isComparisonOp();
-}
-
static const char KnownStringCompareFunctions[] = "__builtin_memcmp;"
"__builtin_strcasecmp;"
"__builtin_strcmp;"
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
WarnOnImplicitComparison(Options.get("WarnOnImplicitComparison", 1)),
- WarnOnLogicalNotComparison(Options.get("WarnOnLogicalNotComparison", 1)),
+ WarnOnLogicalNotComparison(Options.get("WarnOnLogicalNotComparison", 0)),
StringCompareLikeFunctions(
Options.get("StringCompareLikeFunctions", "")) {}
void SuspiciousStringCompareCheck::registerMatchers(MatchFinder *Finder) {
// Match relational operators.
const auto ComparisonUnaryOperator = unaryOperator(hasOperatorName("!"));
- const auto ComparisonBinaryOperator = binaryOperator(isComparisonOperator());
+ const auto ComparisonBinaryOperator =
+ binaryOperator(matchers::isComparisonOperator());
const auto ComparisonOperator =
expr(anyOf(ComparisonUnaryOperator, ComparisonBinaryOperator));
std::vector<std::string> FunctionNames;
ParseFunctionNames(KnownStringCompareFunctions, &FunctionNames);
ParseFunctionNames(StringCompareLikeFunctions, &FunctionNames);
+
+ // Match a call to a string compare functions.
const auto FunctionCompareDecl =
functionDecl(hasAnyName(std::vector<StringRef>(FunctionNames.begin(),
FunctionNames.end())))
.bind("decl");
-
- // Match a call to a string compare functions.
- const auto StringCompareCallExpr =
+ const auto DirectStringCompareCallExpr =
callExpr(hasDeclaration(FunctionCompareDecl)).bind("call");
+ const auto MacroStringCompareCallExpr =
+ conditionalOperator(
+ anyOf(hasTrueExpression(ignoringParenImpCasts(DirectStringCompareCallExpr)),
+ hasFalseExpression(ignoringParenImpCasts(DirectStringCompareCallExpr))));
+ // The implicit cast is not present in C.
+ const auto StringCompareCallExpr = ignoringParenImpCasts(
+ anyOf(DirectStringCompareCallExpr, MacroStringCompareCallExpr));
if (WarnOnImplicitComparison) {
- // Detect suspicious calls to string compare (missing comparator) [only C]:
+ // Detect suspicious calls to string compare:
// 'if (strcmp())' -> 'if (strcmp() != 0)'
Finder->addMatcher(
stmt(anyOf(ifStmt(hasCondition(StringCompareCallExpr)),
whileStmt(hasCondition(StringCompareCallExpr)),
doStmt(hasCondition(StringCompareCallExpr)),
- forStmt(hasCondition(StringCompareCallExpr))))
- .bind("missing-comparison"),
- this);
-
- Finder->addMatcher(expr(StringCompareCallExpr,
- unless(hasParent(ComparisonOperator)),
- unless(hasParent(implicitCastExpr())))
- .bind("missing-comparison"),
- this);
-
- // Detect suspicious calls to string compare with implicit comparison:
- // 'if (strcmp())' -> 'if (strcmp() != 0)'
- // 'if (!strcmp())' is considered valid (see WarnOnLogicalNotComparison)
- Finder->addMatcher(
- implicitCastExpr(hasType(isInteger()),
- hasSourceExpression(StringCompareCallExpr),
- unless(hasParent(ComparisonOperator)))
+ forStmt(hasCondition(StringCompareCallExpr)),
+ binaryOperator(
+ anyOf(hasOperatorName("&&"), hasOperatorName("||")),
+ hasEitherOperand(StringCompareCallExpr))))
.bind("missing-comparison"),
this);
-
- // Detect suspicious cast to an inconsistant type.
- Finder->addMatcher(
- implicitCastExpr(unless(hasType(isInteger())),
- hasSourceExpression(StringCompareCallExpr))
- .bind("invalid-conversion"),
- this);
}
if (WarnOnLogicalNotComparison) {
this);
}
- // Detect suspicious calls to string compare functions: 'strcmp() == -1'.
+ // Detect suspicious cast to an inconsistant type (i.e. not integer type).
+ Finder->addMatcher(
+ implicitCastExpr(unless(hasType(isInteger())),
+ hasSourceExpression(StringCompareCallExpr))
+ .bind("invalid-conversion"),
+ this);
+
+ // Detect suspicious operator with string compare function as operand.
+ Finder->addMatcher(
+ binaryOperator(
+ unless(anyOf(matchers::isComparisonOperator(), hasOperatorName("&&"),
+ hasOperatorName("||"), hasOperatorName("="))),
+ hasEitherOperand(StringCompareCallExpr))
+ .bind("suspicious-operator"),
+ this);
+
+ // Detect comparison to invalid constant: 'strcmp() == -1'.
const auto InvalidLiteral = ignoringParenImpCasts(
anyOf(integerLiteral(unless(equals(0))),
unaryOperator(hasOperatorName("-"),
has(integerLiteral(unless(equals(0))))),
characterLiteral(), cxxBoolLiteral()));
- Finder->addMatcher(binaryOperator(isComparisonOperator(),
+ Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
hasEitherOperand(StringCompareCallExpr),
hasEitherOperand(InvalidLiteral))
.bind("invalid-comparison"),
<< Decl;
}
+ if (const auto* BinOp = Result.Nodes.getNodeAs<BinaryOperator>("suspicious-operator")) {
+ diag(Call->getLocStart(), "results of function %0 used by operator '%1'")
+ << Decl << BinOp->getOpcodeStr();
+ }
+
if (Result.Nodes.getNodeAs<Stmt>("invalid-conversion")) {
diag(Call->getLocStart(), "function %0 has suspicious implicit cast")
<< Decl;
static const unsigned char V[] = "xyz";
static const wchar_t W[] = L"abc";
+int strlen(const char *);
+
int memcmp(const void *, const void *, size);
int wmemcmp(const wchar_t *, const wchar_t *, size);
int memicmp(const void *, const void *, size);
return 1;
}
+
+int strcmp_wrapper1(const char* a, const char* b) {
+ return strcmp(a, b);
+}
+
+int strcmp_wrapper2(const char* a, const char* b) {
+ return (a && b) ? strcmp(a, b) : 0;
+}
+
+#define macro_strncmp(s1, s2, n) \
+ (__extension__ (__builtin_constant_p (n) \
+ && ((__builtin_constant_p (s1) \
+ && strlen (s1) < ((size) (n))) \
+ || (__builtin_constant_p (s2) \
+ && strlen (s2) < ((size) (n)))) \
+ ? strcmp (s1, s2) : strncmp (s1, s2, n)))
+
+int strncmp_macro(const char* a, const char* b) {
+ if (macro_strncmp(a, b, 4))
+ return 0;
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is called without explicitly comparing result
+
+ if (macro_strncmp(a, b, 4) == 2)
+ return 0;
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' is compared to a suspicious constant
+
+ if (macro_strncmp(a, b, 4) <= .0)
+ return 0;
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: function 'strcmp' has suspicious implicit cast
+
+ if (macro_strncmp(a, b, 4) + 0)
+ return 0;
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: results of function 'strcmp' used by operator '+'
+
+ return 1;
+}