Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tidy/bugprone/BugproneTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "IntegerDivisionCheck.h" +#include "MisplacedOperatorInStrlenInAllocCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "UndefinedMemoryManipulationCheck.h" @@ -23,6 +24,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "bugprone-integer-division"); + CheckFactories.registerCheck( + "bugprone-misplaced-operator-in-strlen-in-alloc"); CheckFactories.registerCheck( "bugprone-suspicious-memset-usage"); CheckFactories.registerCheck( Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyBugproneModule BugproneTidyModule.cpp IntegerDivisionCheck.cpp + MisplacedOperatorInStrlenInAllocCheck.cpp SuspiciousMemsetUsageCheck.cpp UndefinedMemoryManipulationCheck.cpp Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h =================================================================== --- /dev/null +++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h @@ -0,0 +1,37 @@ +//===--- MisplacedOperatorInStrlenInAllocCheck.h - clang-tidy----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds cases where ``1`` is added to the string in the parameter of a +/// function in the ``strlen()`` family instead of to the result and use its +/// return value as an argument of a memory allocation function. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.html +class MisplacedOperatorInStrlenInAllocCheck : public ClangTidyCheck { +public: + MisplacedOperatorInStrlenInAllocCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp @@ -0,0 +1,100 @@ +//===--- MisplacedOperatorInStrlenInAllocCheck.cpp - clang-tidy------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "MisplacedOperatorInStrlenInAllocCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void MisplacedOperatorInStrlenInAllocCheck::registerMatchers( + MatchFinder *Finder) { + const auto StrLenFunc = functionDecl(anyOf( + hasName("::strlen"), hasName("::std::strlen"), hasName("::strnlen"), + hasName("::std::strnlen"), hasName("::strnlen_s"), + hasName("::std::strnlen_s"), hasName("::wcslen"), + hasName("::std::wcslen"), hasName("::wcsnlen"), hasName("::std::wcsnlen"), + hasName("::wcsnlen_s"), hasName("std::wcsnlen_s"))); + + const auto BadUse = + callExpr(callee(StrLenFunc), + hasAnyArgument(ignoringImpCasts( + binaryOperator(allOf(hasOperatorName("+"), + hasRHS(ignoringParenImpCasts( + integerLiteral(equals(1)))))) + .bind("BinOp")))) + .bind("StrLen"); + + const auto BadArg = anyOf( + allOf(hasDescendant(BadUse), + unless(binaryOperator(allOf( + hasOperatorName("+"), hasLHS(BadUse), + hasRHS(ignoringParenImpCasts(integerLiteral(equals(1)))))))), + BadUse); + + const auto Alloc0Func = + functionDecl(anyOf(hasName("::malloc"), hasName("std::malloc"), + hasName("::alloca"), hasName("std::alloca"))); + const auto Alloc1Func = + functionDecl(anyOf(hasName("::calloc"), hasName("std::calloc"), + hasName("::realloc"), hasName("std::realloc"))); + Finder->addMatcher( + callExpr(callee(Alloc0Func), hasArgument(0, BadArg)).bind("Alloc"), this); + Finder->addMatcher( + callExpr(callee(Alloc1Func), hasArgument(1, BadArg)).bind("Alloc"), this); + Finder->addMatcher( + cxxNewExpr(isArray(), hasArraySize(BadArg)).bind("Alloc"), this); +} + +void MisplacedOperatorInStrlenInAllocCheck::check( + const MatchFinder::MatchResult &Result) { + const Expr *Alloc = Result.Nodes.getNodeAs("Alloc"); + if (!Alloc) + Alloc = Result.Nodes.getNodeAs("Alloc"); + assert(Alloc && "Matched node bound by 'Alloc' shoud be either 'CallExpr'" + " or `CXXNewExpr`"); + + const auto *StrLen = Result.Nodes.getNodeAs("StrLen"); + const auto *BinOp = Result.Nodes.getNodeAs("BinOp"); + + const StringRef StrLenText = Lexer::getSourceText( + CharSourceRange::getTokenRange(StrLen->getSourceRange()), + *Result.SourceManager, getLangOpts()); + const StringRef Arg0Text = Lexer::getSourceText( + CharSourceRange::getTokenRange(StrLen->getArg(0)->getSourceRange()), + *Result.SourceManager, getLangOpts()); + const StringRef StrLenBegin = StrLenText.substr(0, StrLenText.find(Arg0Text)); + const StringRef StrLenEnd = StrLenText.substr( + StrLenText.find(Arg0Text) + Arg0Text.size(), StrLenText.size()); + + const StringRef LHSText = Lexer::getSourceText( + CharSourceRange::getTokenRange(BinOp->getLHS()->getSourceRange()), + *Result.SourceManager, getLangOpts()); + const StringRef RHSText = Lexer::getSourceText( + CharSourceRange::getTokenRange(BinOp->getRHS()->getSourceRange()), + *Result.SourceManager, getLangOpts()); + + auto Hint = FixItHint::CreateReplacement( + StrLen->getSourceRange(), + (StrLenBegin + LHSText + StrLenEnd + " + " + RHSText).str()); + + diag(Alloc->getLocStart(), + "Addition operator is applied to the argument of " + "%0 instead of its result; surround the addition subexpression with " + "parentheses to silence this warning") + << StrLen->getDirectCallee()->getName() << Hint; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,15 @@ Improvements to clang-tidy -------------------------- +- New `bugprone-misplaced-operator-in-strlen-in-alloc + `_ check + + Finds cases where ``1`` is added to the string in the parameter of + ``strlen()``, ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and + ``wcsnlen_s()`` functions instead of to the result and use its return value as + an argument of a memory allocation function (``malloc()``, ``calloc()``, + ``realloc()``, ``alloca()``) or the ``new[]`` operator in `C++`. + - Renamed checks to use correct term "implicit conversion" instead of "implicit cast" and modified messages and option names accordingly: Index: docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst @@ -0,0 +1,56 @@ +.. title:: clang-tidy - bugprone-misplaced-operator-in-strlen-in-alloc + +bugprone-misplaced-operator-in-strlen-in-alloc +============================================== + +Finds cases where ``1`` is added to the string in the parameter of ``strlen()``, +``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and ``wcsnlen_s()`` +functions instead of to the result and use its return value as an argument of a +memory allocation function (``malloc()``, ``calloc()``, ``realloc()``, +``alloca()``) or the ``new[]`` operator in `C++`. Cases where ``1`` is added +both to the parameter and the result of the ``strlen()``-like function are +ignored, as are cases where the whole addition is surrounded by extra +parentheses. + +`C` example code: + +.. code-block:: c + + void bad_malloc(char *str) { + char *c = (char*) malloc(strlen(str + 1)); + } + + +The suggested fix is to add ``1`` to the return value of ``strlen()`` and not +to its argument. In the example above the fix would be + +.. code-block:: c + + char *c = (char*) malloc(strlen(str) + 1); + + +`C++` example code: + +.. code-block:: c++ + + void bad_new(char *str) { + char *c = new char[strlen(str + 1)]; + } + + +As in the `C` code with the ``malloc()`` function, the suggested fix is to +add ``1`` to the return value of ``strlen()`` and not to its argument. In the +example above the fix would be + +.. code-block:: c++ + + char *c = new char[strlen(str) + 1]; + + +Example for silencing the bug report: + +.. code-block:: c + + void bad_malloc(char *str) { + char *c = (char*) malloc(strlen((str + 1))); + } Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -18,6 +18,7 @@ android-cloexec-socket boost-use-to-string bugprone-integer-division + bugprone-misplaced-operator-in-strlen-in-alloc bugprone-suspicious-memset-usage bugprone-undefined-memory-manipulation cert-dcl03-c (redirects to misc-static-assert) Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c @@ -0,0 +1,78 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); +void *alloca(size_t); +void *calloc(size_t, size_t); +void *realloc(void *, size_t); + +size_t strlen(const char*); +size_t strnlen(const char*, size_t); +size_t strnlen_s(const char*, size_t); + +typedef unsigned wchar_t; + +size_t wcslen(const wchar_t*); +size_t wcsnlen(const wchar_t*, size_t); +size_t wcsnlen_s(const wchar_t*, size_t); + +void bad_malloc(char *name) { + char *new_name = (char*) malloc(strlen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_name = \(char\*\) malloc\(}}strlen(name) + 1{{\);$}} + new_name = (char*) malloc(strnlen(name + 1, 10)); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Addition operator is applied to the argument of strnlen + // CHECK-FIXES: {{^ new_name = \(char\*\) malloc\(}}strnlen(name, 10) + 1{{\);$}} + new_name = (char*) malloc(strnlen_s(name + 1, 10)); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Addition operator is applied to the argument of strnlen_s + // CHECK-FIXES: {{^ new_name = \(char\*\) malloc\(}}strnlen_s(name, 10) + 1{{\);$}} +} + +void bad_malloc_wide(wchar_t *name) { + wchar_t *new_name = (wchar_t*) malloc(wcslen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Addition operator is applied to the argument of wcslen + // CHECK-FIXES: {{^ wchar_t \*new_name = \(wchar_t\*\) malloc\(}}wcslen(name) + 1{{\);$}} + new_name = (wchar_t*) malloc(wcsnlen(name + 1, 10)); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: Addition operator is applied to the argument of wcsnlen + // CHECK-FIXES: {{^ new_name = \(wchar_t\*\) malloc\(}}wcsnlen(name, 10) + 1{{\);$}} + new_name = (wchar_t*) malloc(wcsnlen_s(name + 1, 10)); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: Addition operator is applied to the argument of wcsnlen_s + // CHECK-FIXES: {{^ new_name = \(wchar_t\*\) malloc\(}}wcsnlen_s(name, 10) + 1{{\);$}} +} + +void bad_alloca(char *name) { + char *new_name = (char*) alloca(strlen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_name = \(char\*\) alloca\(}}strlen(name) + 1{{\);$}} +} + +void bad_calloc(char *name) { + char *new_names = (char*) calloc(2, strlen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_names = \(char\*\) calloc\(2, }}strlen(name) + 1{{\);$}} +} + +void bad_realloc(char * old_name, char *name) { + char *new_name = (char*) realloc(old_name, strlen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_name = \(char\*\) realloc\(old_name, }}strlen(name) + 1{{\);$}} +} + +void intentional1(char *name) { + char *new_name = (char*) malloc(strlen(name + 1) + 1); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // We have + 1 outside as well so we assume this is intentional +} + +void intentional2(char *name) { + char *new_name = (char*) malloc(strlen(name + 2)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // Only give warning for + 1, not + 2 +} + +void intentional3(char *name) { + char *new_name = (char*) malloc(strlen((name + 1))); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // If expression is in extra parentheses, consider it as intentional +} + Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t + +namespace std { +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +size_t strlen(const char*); +} + +namespace non_std { +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +size_t strlen(const char*); +} + +void bad_std_malloc_std_strlen(char *name) { + char *new_name = (char*) std::malloc(std::strlen(name + 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_name = \(char\*\) std::malloc\(}}std::strlen(name) + 1{{\);$}} +} + +void ignore_non_std_malloc_std_strlen(char *name) { + char *new_name = (char*) non_std::malloc(std::strlen(name + 1)); + // Ignore functions of the malloc family in custom namespaces +} + +void ignore_std_malloc_non_std_strlen(char *name) { + char *new_name = (char*) std::malloc(non_std::strlen(name + 1)); + // Ignore functions of the strlen family in custom namespaces +} + +void bad_new_strlen(char *name) { + char *new_name = new char[std::strlen(name + 1)]; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ char \*new_name = new char\[}}std::strlen(name) + 1{{\];$}} +} + +void good_new_strlen(char *name) { + char *new_name = new char[std::strlen(name) + 1]; + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:20: warning: Addition operator is applied to the argument of strlen +} + +class C { + char c; +public: + static void *operator new[](std::size_t count) { + return ::operator new(count); + } +}; + +void bad_custom_new_strlen(char *name) { + C *new_name = new C[std::strlen(name + 1)]; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Addition operator is applied to the argument of strlen + // CHECK-FIXES: {{^ C \*new_name = new C\[}}std::strlen(name) + 1{{\];$}} +} +