[clang-tools-extra] r318907 - [clang-tidy] Misplaced Operator in Strlen in Alloc

Adam Balogh via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 23 04:33:12 PST 2017


Author: baloghadamsoftware
Date: Thu Nov 23 04:33:12 2017
New Revision: 318907

URL: http://llvm.org/viewvc/llvm-project?rev=318907&view=rev
Log:
[clang-tidy] Misplaced Operator in Strlen in Alloc

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.

Fix for r318906, forgot to add new files.


Added:
    clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
    clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
    clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
    clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp

Added: clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp?rev=318907&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp Thu Nov 23 04:33:12 2017
@@ -0,0 +1,92 @@
+//===--- 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);
+}
+
+void MisplacedOperatorInStrlenInAllocCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  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 StringRef StrLenText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(StrLen->getSourceRange()),
+      *Result.SourceManager, getLangOpts());
+  const StringRef StrLenBegin = StrLenText.substr(0, StrLenText.find('(') + 1);
+  const StringRef Arg0Text = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(StrLen->getArg(0)->getSourceRange()),
+      *Result.SourceManager, getLangOpts());
+  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") << StrLen->getDirectCallee()->getName() << Hint;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h?rev=318907&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h Thu Nov 23 04:33:12 2017
@@ -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 argument to a function
+/// in the ``strlen()`` family instead of the result and value is used as an
+/// argument to 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

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst?rev=318907&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst Thu Nov 23 04:33:12 2017
@@ -0,0 +1,37 @@
+.. 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 argument to ``strlen()``,
+``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()``, and ``wcsnlen_s()``
+instead of the result and the value is used as an argument to a memory
+allocation function (``malloc()``, ``calloc()``, ``realloc()``, ``alloca()``).
+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.
+
+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);
+
+
+Example for silencing the diagnostic:
+
+.. code-block:: c
+
+    void bad_malloc(char *str) {
+      char *c = (char*) malloc(strlen((str + 1)));
+    }

Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c?rev=318907&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c Thu Nov 23 04:33:12 2017
@@ -0,0 +1,77 @@
+// 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
+}

Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp?rev=318907&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp Thu Nov 23 04:33:12 2017
@@ -0,0 +1,33 @@
+// 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 std
+
+namespace non_std {
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+size_t strlen(const char *);
+} // namespace non_std
+
+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));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // 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));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // Ignore functions of the strlen family in custom namespaces
+}




More information about the cfe-commits mailing list