Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -30,6 +30,7 @@ #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" +#include "MisplacedPointerArithmeticInAllocCheck.h" #include "MisplacedWideningCastCheck.h" #include "MoveForwardingReferenceCheck.h" #include "MultipleStatementMacroCheck.h" @@ -105,6 +106,8 @@ "bugprone-macro-repeated-side-effects"); CheckFactories.registerCheck( "bugprone-misplaced-operator-in-strlen-in-alloc"); + CheckFactories.registerCheck( + "bugprone-misplaced-pointer-arithmetic-in-alloc"); CheckFactories.registerCheck( "bugprone-misplaced-widening-cast"); CheckFactories.registerCheck( Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -22,6 +22,7 @@ MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MisplacedOperatorInStrlenInAllocCheck.cpp + MisplacedPointerArithmeticInAllocCheck.cpp MisplacedWideningCastCheck.cpp MoveForwardingReferenceCheck.cpp MultipleStatementMacroCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h @@ -0,0 +1,36 @@ +//===--- MisplacedPointerArithmeticInAllocCheck.h - clang-tidy---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_ALLOC_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_ALLOC_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds cases where an integer is added to or subracted from the result of a +/// memory allocation function instead of its argument. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-operator-in-alloc.html +class MisplacedPointerArithmeticInAllocCheck : public ClangTidyCheck { +public: + MisplacedPointerArithmeticInAllocCheck(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_ALLOC_H Index: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp @@ -0,0 +1,132 @@ +//===--- MisplacedPointerArithmeticInAllocCheck.cpp - clang-tidy-----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "MisplacedPointerArithmeticInAllocCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +static FixItHint composeHint(const BinaryOperator *Outer, const Expr *Inner, + char ParenChar, const SourceManager &SM, + const LangOptions &LO) { + const StringRef LeftText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Outer->getLHS()->getSourceRange()), + SM, LO); + const StringRef RightText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Outer->getRHS()->getSourceRange()), + SM, LO); + const StringRef InnerText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Inner->getSourceRange()), SM, LO); + const StringRef AheadOfClosingParen = + InnerText.substr(0, InnerText.rfind(ParenChar)); + + StringRef LeftAhead, LeftBehind; + std::tie(LeftAhead, LeftBehind) = LeftText.split(InnerText); + + return FixItHint::CreateReplacement( + Outer->getSourceRange(), + (LeftAhead + AheadOfClosingParen + + ((Outer->getOpcode() == BO_Add) ? " + " : " - ") + RightText + + std::string(1, ParenChar) + LeftBehind) + .str()); +} + +void MisplacedPointerArithmeticInAllocCheck::registerMatchers( + MatchFinder *Finder) { + const auto AllocFunc = functionDecl( + anyOf(hasName("::malloc"), hasName("std::malloc"), hasName("::alloca"), + hasName("::calloc"), hasName("std::calloc"), hasName("::realloc"), + hasName("std::realloc"))); + + const auto AllocFuncPtr = + varDecl(hasType(isConstQualified()), + hasInitializer(ignoringParenImpCasts( + declRefExpr(hasDeclaration(AllocFunc))))); + + const auto AdditiveOperator = + binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-"))); + + const auto IntExpr = expr(hasType(isInteger())); + + const auto AllocCall = + callExpr(callee(decl(anyOf(AllocFunc, AllocFuncPtr)))).bind("AllocCall"); + + Finder->addMatcher( + binaryOperator( + AdditiveOperator, + hasLHS(anyOf(AllocCall, castExpr(hasSourceExpression(AllocCall)))), + hasRHS(IntExpr)) + .bind("PtrArith"), + this); + + const auto CXXConstructorWithSingleIntArg = + cxxConstructExpr(argumentCountIs(1), hasArgument(0, IntExpr)); + + const auto New = + cxxNewExpr(unless(isArray()), + has(CXXConstructorWithSingleIntArg)).bind("New"); + + Finder->addMatcher( + binaryOperator( + AdditiveOperator, + hasLHS(anyOf(New, castExpr(New))), + hasRHS(IntExpr)) + .bind("PtrArith"), + this); + + const auto ArrayNew = cxxNewExpr(isArray()).bind("ArrayNew"); + + Finder->addMatcher( + binaryOperator( + AdditiveOperator, + hasLHS(anyOf(ArrayNew, castExpr(ArrayNew))), + hasRHS(IntExpr)) + .bind("PtrArith"), + this); +} + +void MisplacedPointerArithmeticInAllocCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *PtrArith = Result.Nodes.getNodeAs("PtrArith"); + std::string CallName; + + FixItHint Hint; + if (const auto *AllocCall = + Result.Nodes.getNodeAs("AllocCall")) { + Hint = composeHint(PtrArith, AllocCall, ')', *Result.SourceManager, + getLangOpts()); + const NamedDecl *Func = AllocCall->getDirectCallee(); + if (!Func) { + Func = cast(AllocCall->getCalleeDecl()); + } + CallName = Func->getName().str(); + } else if (const auto *New = Result.Nodes.getNodeAs("New")) { + Hint = composeHint(PtrArith, New, ')', *Result.SourceManager, + getLangOpts()); + CallName = "operator new"; + } else { + const auto *ArrayNew = Result.Nodes.getNodeAs("ArrayNew"); + Hint = composeHint(PtrArith, ArrayNew, ']', *Result.SourceManager, + getLangOpts()); + CallName = "operator new[]"; + } + + diag(PtrArith->getBeginLoc(), + "arithmetic operation is applied to the result of %0() instead of its " + "size-like argument") + << CallName << Hint; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -137,6 +137,13 @@ Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites them to use ``Register`` +- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc + ` check. + + Finds cases where an integer expression is added to or subtracted from the + result of a memory allocation function (``malloc()``, ``calloc()``, + ``realloc()``, ``alloca()``) instead of its argument. + - New :doc:`objc-missing-hash ` check. Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst @@ -0,0 +1,25 @@ +.. title:: clang-tidy - bugprone-misplaced-pointer-arithmetic-in-alloc + +bugprone-misplaced-pointer-artithmetic-in-alloc +=============================================== + +Finds cases where an integer expression is added to or subtracted from the +result of a memory allocation function (``malloc()``, ``calloc()``, +``realloc()``, ``alloca()``) instead of its argument. The check detects error +cases even if one of these functions is called by a constant function pointer. + +Example code: + +.. code-block:: c + + void bad_malloc(int n) { + char *p = (char*) malloc(n) + 10; + } + + +The suggested fix is to add the integer expression to the argument of +``malloc`` and not to its result. In the example above the fix would be + +.. code-block:: c + + char *p = (char*) malloc(n + 10); Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -57,6 +57,7 @@ bugprone-macro-parentheses bugprone-macro-repeated-side-effects bugprone-misplaced-operator-in-strlen-in-alloc + bugprone-misplaced-pointer-arithmetic-in-alloc bugprone-misplaced-widening-cast bugprone-move-forwarding-reference bugprone-multiple-statement-macro Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-pointer-arithmetic-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); + +void bad_malloc(int n) { + char *p = (char *)malloc(n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = \(char \*\)malloc\(n}} + 10{{\);$}} + + p = (char *)malloc(n) - 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument + // CHECK-FIXES: {{^ p = \(char \*\)malloc\(n}} - 10{{\);$}} + + p = (char *)malloc(n) + n; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument + // CHECK-FIXES: {{^ p = \(char \*\)malloc\(n}} + n{{\);$}} + + p = (char *)malloc(n) - (n + 10); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument + // CHECK-FIXES: {{^ p = \(char \*\)malloc\(n}} - (n + 10){{\);$}} + + p = (char *)malloc(n) - n + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument + // CHECK-FIXES: {{^ p = \(char \*\)malloc\(n}} - n{{\) \+ 10;$}} + // FIXME: should be {{^ p = \(char \*\)malloc\(n}} - n + 10{{\);$}} +} + +void bad_alloca(int n) { + char *p = (char *)alloca(n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of alloca() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = \(char \*\)alloca\(n}} + 10{{\);$}} +} + +void bad_realloc(char *s, int n) { + char *p = (char *)realloc(s, n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of realloc() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = \(char \*\)realloc\(s, n}} + 10{{\);$}} +} + +void bad_calloc(int n, int m) { + char *p = (char *)calloc(m, n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of calloc() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = \(char \*\)calloc\(m, n}} + 10{{\);$}} +} + +void (*(*const alloc_ptr)(size_t)) = malloc; + +void bad_indirect_alloc(int n) { + char *p = (char *)alloc_ptr(n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of alloc_ptr() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = \(char \*\)alloc_ptr\(n}} + 10{{\);$}} +} Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.cpp @@ -0,0 +1,53 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-pointer-arithmetic-in-alloc %t + +class C { + int num; +public: + explicit C(int n) : num(n) {} +}; + +void bad_new(int n, int m) { + C *p = new C(n) + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument + // CHECK-FIXES: {{^ C \*p = new C\(n}} + 10{{\);$}} + + p = new C(n) - 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument + // CHECK-FIXES: {{^ p = new C\(n}} - 10{{\);$}} + + p = new C(n) + m; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument + // CHECK-FIXES: {{^ p = new C\(n}} + m{{\);$}} + + p = new C(n) - (m + 10); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument + // CHECK-FIXES: {{^ p = new C\(n}} - (m + 10){{\);$}} + + p = new C(n) - m + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument + // CHECK-FIXES: {{^ p = new C\(n}} - m{{\) \+ 10;$}} + // FIXME: Should be {{^ p = new C\(n}} - m + 10{{\);$}} +} + +void bad_new_array(int n, int m) { + char *p = new char[n] + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument + // CHECK-FIXES: {{^ char \*p = new char\[n}} + 10{{\];$}} + + p = new char[n] - 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument + // CHECK-FIXES: {{^ p = new char\[n}} - 10{{\];$}} + + p = new char[n] + m; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument + // CHECK-FIXES: {{^ p = new char\[n}} + m{{\];$}} + + p = new char[n] - (m + 10); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument + // CHECK-FIXES: {{^ p = new char\[n}} - (m + 10){{\];$}} + + p = new char[n] - m + 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument + // CHECK-FIXES: {{^ p = new char\[n}} - m{{\] \+ 10;$}} + // FIXME: should be {{^ p = new char\[n}} - m + 10{{\];$}} +}