baloghadamsoftware created this revision.
Herald added subscribers: mgorny, srhines.
A possible error is to write ``malloc(strlen(s+1))`` instead of
``malloc(strlen(s)+1)``. Unfortunately the former is also valid syntactically,
but allocates less memory by two bytes (if ``s`` is at least one character
long, undefined behavior otherwise) which may result in overflow cases. This
check detects such cases and also suggests the fix for them.
https://reviews.llvm.org/D39121
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp
clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp
Index: test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s misc-misplaced-operator-in-strlen-in-alloc %t
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+
+size_t strlen(const char*);
+
+void bad_malloc(char *name) {
+ char *new_name = (char*) malloc(strlen(name + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+ // CHECK-FIXES: {{^ char \*new_name = \(char\*\) malloc\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_calloc(char *name) {
+ char *new_names = (char*) calloc(2, strlen(name + 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Binary operator + 1 is inside 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: Binary operator + 1 is inside strlen
+ // CHECK-FIXES: {{^ char \*new_name = \(char\*\) realloc\(old_name, }}strlen(name) + 1{{\);$}}
+}
Index: docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - misc-misplaced-operator-in-strlen-in-alloc
+
+misc-misplaced-operator-in-strlen-in-alloc
+==========================================
+
+Finds cases a value is added to or subtracted from the string in the parameter
+of ``strlen()`` method instead of to the result and use its return value as an
+argument of a memory allocation function (``malloc()``, ``calloc()``,
+``realloc()``).
+
+Example code:
+
+.. code-block:: c
+
+ void bad_malloc(char *str) {
+ char *c = (char*) malloc(strlen(str + 1));
+ }
+
+
+The suggested fix is to add value 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);
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -7,9 +7,9 @@
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
+ android-cloexec-dup
android-cloexec-epoll-create
android-cloexec-epoll-create1
- android-cloexec-dup
android-cloexec-fopen
android-cloexec-inotify-init
android-cloexec-inotify-init1
@@ -38,7 +38,7 @@
cert-msc30-c (redirects to cert-msc50-cpp) <cert-msc30-c>
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) <cert-oop11-cpp>
- cppcoreguidelines-c-copy-assignment-signature
+ cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
@@ -76,8 +76,8 @@
hicpp-explicit-conversions (redirects to google-explicit-constructor) <hicpp-explicit-conversions>
hicpp-function-size (redirects to readability-function-size) <hicpp-function-size>
hicpp-invalid-access-moved (redirects to misc-use-after-move) <hicpp-invalid-access-moved>
- hicpp-move-const-arg (redirects to misc-move-const-arg) <hicpp-move-const-arg>
hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) <hicpp-member-init>
+ hicpp-move-const-arg (redirects to misc-move-const-arg) <hicpp-move-const-arg>
hicpp-named-parameter (redirects to readability-named-parameter) <hicpp-named-parameter>
hicpp-new-delete-operators (redirects to misc-new-delete-overloads) <hicpp-new-delete-operators>
hicpp-no-array-decay (redirects to cppcoreguidelines-pro-bounds-array-to-pointer-decay) <hicpp-no-array-decay>
@@ -95,7 +95,7 @@
hicpp-use-noexcept (redirects to modernize-use-noexcept) <hicpp-use-noexcept>
hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
- hicpp-vararg (redirects to cppcoreguidelines-pro-type-varg) <hicpp-vararg>
+ hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
llvm-header-guard
llvm-include-order
llvm-namespace-comment
@@ -115,6 +115,7 @@
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-misplaced-const
+ misc-misplaced-operator-in-strlen-in-alloc
misc-misplaced-widening-cast
misc-move-const-arg
misc-move-constructor-init
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,14 @@
Improvements to clang-tidy
--------------------------
+- New `misc-misplaced-operator-in-strlen-in-alloc
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.html>`_ check
+
+ Finds cases a value is added to or subtracted from the string in the parameter
+ of ``strlen()`` method instead of to the result and use its return value as an
+ argument of a memory allocation function (``malloc()``, ``calloc()``,
+ ``realloc()``). The check also suggests the appropriate fix.
+
- Renamed checks to use correct term "implicit conversion" instead of "implicit
cast" and modified messages and option names accordingly:
Index: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h
@@ -0,0 +1,35 @@
+//===--- 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_MISC_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-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 misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
Index: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -0,0 +1,68 @@
+//===--- 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 misc {
+
+void MisplacedOperatorInStrlenInAllocCheck::registerMatchers(
+ MatchFinder *Finder) {
+ const auto BadUse =
+ callExpr(
+ callee(functionDecl(hasName("strlen"))),
+ hasAnyArgument(ignoringParenImpCasts(
+ binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-")))
+ .bind("BinOp"))))
+ .bind("StrLen");
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("malloc"))),
+ hasArgument(0, anyOf(hasDescendant(BadUse), BadUse)))
+ .bind("Alloc"),
+ this);
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(anyOf(hasName("calloc"), hasName("realloc")))),
+ hasArgument(1, anyOf(hasDescendant(BadUse), BadUse)))
+ .bind("Alloc"),
+ this);
+}
+
+void MisplacedOperatorInStrlenInAllocCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ // FIXME: Add callback implementation.
+ const auto *Alloc = Result.Nodes.getNodeAs<CallExpr>("Alloc");
+ const auto *StrLen = Result.Nodes.getNodeAs<CallExpr>("StrLen");
+ const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("BinOp");
+
+ const std::string LHSText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(BinOp->getLHS()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ const std::string RHSText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(BinOp->getRHS()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+
+ auto Hint = FixItHint::CreateReplacement(
+ StrLen->getSourceRange(),
+ "strlen(" + LHSText + ")" +
+ ((BinOp->getOpcode() == BO_Add) ? " + " : " - ") + RHSText);
+
+ diag(Alloc->getLocStart(), "Binary operator %0 %1 is inside strlen")
+ << ((BinOp->getOpcode() == BO_Add) ? "+" : "-") << BinOp->getRHS()
+ << Hint;
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -25,6 +25,7 @@
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "MisplacedConstCheck.h"
+#include "MisplacedOperatorInStrlenInAllocCheck.h"
#include "MisplacedWideningCastCheck.h"
#include "MoveConstantArgumentCheck.h"
#include "MoveConstructorInitCheck.h"
@@ -72,6 +73,8 @@
CheckFactories.registerCheck<LambdaFunctionNameCheck>(
"misc-lambda-function-name");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+ CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
+ "misc-misplaced-operator-in-strlen-in-alloc");
CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
"misc-unconventional-assign-operator");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -6,6 +6,7 @@
ForwardingReferenceOverloadCheck.cpp
LambdaFunctionNameCheck.cpp
MisplacedConstCheck.cpp
+ MisplacedOperatorInStrlenInAllocCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
DanglingHandleCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
