Commit 852bf52c authored by Balázs Kéri's avatar Balázs Kéri
Browse files

[clang-tidy] Add check bugprone-multiple-new-in-one-expression. 

Reviewed By: donat.nagy
Fixed test failures with previous commit.

Differential Revision: https://reviews.llvm.org/D138777
parent 5362a0d8

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

+3 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ 
#include "MisplacedPointerArithmeticInAllocCheck.h"

#include "MisplacedWideningCastCheck.h"

#include "MoveForwardingReferenceCheck.h"

#include "MultipleNewInOneExpressionCheck.h"

#include "MultipleStatementMacroCheck.h"

#include "NoEscapeCheck.h"

#include "NonZeroEnumToBoolConversionCheck.h"
@@ -133,6 +134,8 @@ public: 
        "bugprone-misplaced-widening-cast");

    CheckFactories.registerCheck<MoveForwardingReferenceCheck>(

        "bugprone-move-forwarding-reference");

    CheckFactories.registerCheck<MultipleNewInOneExpressionCheck>(

        "bugprone-multiple-new-in-one-expression");

    CheckFactories.registerCheck<MultipleStatementMacroCheck>(

        "bugprone-multiple-statement-macro");

    CheckFactories.registerCheck<RedundantBranchConditionCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

+1 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ add_clang_library(clangTidyBugproneModule 
  MisplacedPointerArithmeticInAllocCheck.cpp

  MisplacedWideningCastCheck.cpp

  MoveForwardingReferenceCheck.cpp

  MultipleNewInOneExpressionCheck.cpp

  MultipleStatementMacroCheck.cpp

  NoEscapeCheck.cpp

  NonZeroEnumToBoolConversionCheck.cpp

clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp

0 → 100644
+162 −0
Original line number Diff line number Diff line 
//===--- MultipleNewInOneExpressionCheck.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 "MultipleNewInOneExpressionCheck.h"

#include "clang/AST/ASTContext.h"

#include "clang/ASTMatchers/ASTMatchFinder.h"

#include "clang/Lex/Lexer.h"



using namespace clang::ast_matchers;

using namespace clang;



namespace {



// Determine if the result of an expression is "stored" in some way.

// It is true if the value is stored into a variable or used as initialization

// or passed to a function or constructor.

// For this use case compound assignments are not counted as a "store" (the 'E'

// expression should have pointer type).

bool isExprValueStored(const Expr *E, ASTContext &C) {

  E = E->IgnoreParenCasts();

  // Get first non-paren, non-cast parent.

  ParentMapContext &PMap = C.getParentMapContext();

  DynTypedNodeList P = PMap.getParents(*E);

  if (P.size() != 1)

    return false;

  const Expr *ParentE;

  while ((ParentE = P[0].get<Expr>()) && ParentE->IgnoreParenCasts() == E) {

    P = PMap.getParents(P[0]);

    if (P.size() != 1)

      return false;

  }



  if (const auto *ParentVarD = P[0].get<VarDecl>())

    return ParentVarD->getInit()->IgnoreParenCasts() == E;



  if (!ParentE)

    return false;



  if (const auto *BinOp = dyn_cast<BinaryOperator>(ParentE))

    return BinOp->getOpcode() == BO_Assign &&

           BinOp->getRHS()->IgnoreParenCasts() == E;



  return isa<CallExpr, CXXConstructExpr>(ParentE);

}



} // namespace



namespace clang {

namespace tidy {

namespace bugprone {



AST_MATCHER_P(CXXTryStmt, hasHandlerFor,

              ast_matchers::internal::Matcher<QualType>, InnerMatcher) {

  for (unsigned NH = Node.getNumHandlers(), I = 0; I < NH; ++I) {

    const CXXCatchStmt *CatchS = Node.getHandler(I);

    // Check for generic catch handler (match anything).

    if (CatchS->getCaughtType().isNull())

      return true;

    ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);

    if (InnerMatcher.matches(CatchS->getCaughtType(), Finder, &Result)) {

      *Builder = std::move(Result);

      return true;

    }

  }

  return false;

}



AST_MATCHER(CXXNewExpr, mayThrow) {

  FunctionDecl *OperatorNew = Node.getOperatorNew();

  if (!OperatorNew)

    return false;

  return !OperatorNew->getType()->castAs<FunctionProtoType>()->isNothrow();

}



void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) {

  auto BadAllocType =

      recordType(hasDeclaration(cxxRecordDecl(hasName("::std::bad_alloc"))));

  auto ExceptionType =

      recordType(hasDeclaration(cxxRecordDecl(hasName("::std::exception"))));

  auto BadAllocReferenceType = referenceType(pointee(BadAllocType));

  auto ExceptionReferenceType = referenceType(pointee(ExceptionType));



  auto CatchBadAllocType =

      qualType(hasCanonicalType(anyOf(BadAllocType, BadAllocReferenceType,

                                      ExceptionType, ExceptionReferenceType)));

  auto BadAllocCatchingTryBlock = cxxTryStmt(hasHandlerFor(CatchBadAllocType));



  auto NewExprMayThrow = cxxNewExpr(mayThrow());

  auto HasNewExpr1 = expr(anyOf(NewExprMayThrow.bind("new1"),

                                hasDescendant(NewExprMayThrow.bind("new1"))));

  auto HasNewExpr2 = expr(anyOf(NewExprMayThrow.bind("new2"),

                                hasDescendant(NewExprMayThrow.bind("new2"))));



  Finder->addMatcher(

      callExpr(

          hasAnyArgument(

              expr(HasNewExpr1).bind("arg1")),

          hasAnyArgument(

              expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),

          hasAncestor(BadAllocCatchingTryBlock)),

      this);

  Finder->addMatcher(

      cxxConstructExpr(

          hasAnyArgument(

              expr(HasNewExpr1).bind("arg1")),

          hasAnyArgument(

              expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),

          unless(isListInitialization()),

          hasAncestor(BadAllocCatchingTryBlock)),

      this);

  Finder->addMatcher(binaryOperator(hasLHS(HasNewExpr1), hasRHS(HasNewExpr2),

                                    unless(hasAnyOperatorName("&&", "||", ",")),

                                    hasAncestor(BadAllocCatchingTryBlock)),

                     this);

  Finder->addMatcher(

      cxxNewExpr(mayThrow(),

                 hasDescendant(NewExprMayThrow.bind("new2_in_new1")),

                 hasAncestor(BadAllocCatchingTryBlock))

          .bind("new1"),

      this);

}



void MultipleNewInOneExpressionCheck::check(

    const MatchFinder::MatchResult &Result) {

  const auto *NewExpr1 = Result.Nodes.getNodeAs<CXXNewExpr>("new1");

  const auto *NewExpr2 = Result.Nodes.getNodeAs<CXXNewExpr>("new2");

  const auto *NewExpr2InNewExpr1 =

      Result.Nodes.getNodeAs<CXXNewExpr>("new2_in_new1");

  if (!NewExpr2)

    NewExpr2 = NewExpr2InNewExpr1;

  assert(NewExpr1 && NewExpr2 && "Bound nodes not found.");



  // No warning if both allocations are not stored.

  // The value may be intentionally not stored (no deallocations needed or

  // self-destructing object).

  if (!isExprValueStored(NewExpr1, *Result.Context) &&

      !isExprValueStored(NewExpr2, *Result.Context))

    return;



  // In C++17 sequencing of a 'new' inside constructor arguments of another

  // 'new' is fixed. Still a leak can happen if the returned value from the

  // first 'new' is not saved (yet) and the second fails.

  if (getLangOpts().CPlusPlus17 && NewExpr2InNewExpr1)

    diag(NewExpr1->getBeginLoc(),

         "memory allocation may leak if an other allocation is sequenced after "

         "it and throws an exception")

        << NewExpr1->getSourceRange() << NewExpr2->getSourceRange();

  else

    diag(NewExpr1->getBeginLoc(),

         "memory allocation may leak if an other allocation is sequenced after "

         "it and throws an exception; order of these allocations is undefined")

        << NewExpr1->getSourceRange() << NewExpr2->getSourceRange();

}



} // namespace bugprone

} // namespace tidy

} // namespace clang

clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.h

0 → 100644
+35 −0
Original line number Diff line number Diff line 
//===--- MultipleNewInOneExpressionCheck.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_MULTIPLENEWINONEEXPRESSIONCHECK_H

#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MULTIPLENEWINONEEXPRESSIONCHECK_H



#include "../ClangTidyCheck.h"



namespace clang {

namespace tidy {

namespace bugprone {



/// For the user-facing documentation see:

/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/multiple-new-in-one-expression.html

class MultipleNewInOneExpressionCheck : public ClangTidyCheck {

public:

  MultipleNewInOneExpressionCheck(StringRef Name, ClangTidyContext *Context)

      : ClangTidyCheck(Name, Context) {}

  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {

    return LangOpts.CPlusPlus;

  }

  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_MULTIPLENEWINONEEXPRESSIONCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

+6 −0
Original line number Diff line number Diff line
@@ -106,6 +106,12 @@ Improvements to clang-tidy 
New checks

^^^^^^^^^^



- New :doc:`bugprone-multiple-new-in-one-expression

  <clang-tidy/checks/bugprone/multiple-new-in-one-expression>` check.



  Finds multiple ``new`` operator calls in a single expression, where the allocated

  memory by the first ``new`` may leak if the second allocation fails and throws exception.



- New :doc:`bugprone-non-zero-enum-to-bool-conversion

  <clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion>` check.