diff options
Diffstat (limited to 'clang/lib')
5 files changed, 125 insertions, 3 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 62ccc3cb497..3510c510c75 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -22,6 +22,7 @@ add_clang_library(clangStaticAnalyzerCheckers CheckerDocumentation.cpp ChrootChecker.cpp ClangCheckers.cpp + CXXSelfAssignmentChecker.cpp DeadStoresChecker.cpp DebugCheckers.cpp DereferenceChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp new file mode 100644 index 00000000000..7631322d255 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp @@ -0,0 +1,62 @@ +//=== CXXSelfAssignmentChecker.cpp -----------------------------*- C++ -*--===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines CXXSelfAssignmentChecker, which tests all custom defined +// copy and move assignment operators for the case of self assignment, thus +// where the parameter refers to the same location where the this pointer +// points to. The checker itself does not do any checks at all, but it +// causes the analyzer to check every copy and move assignment operator twice: +// once for when 'this' aliases with the parameter and once for when it may not. +// It is the task of the other enabled checkers to find the bugs in these two +// different cases. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { + +class CXXSelfAssignmentChecker : public Checker<check::BeginFunction> { +public: + CXXSelfAssignmentChecker(); + void checkBeginFunction(CheckerContext &C) const; +}; +} + +CXXSelfAssignmentChecker::CXXSelfAssignmentChecker() {} + +void CXXSelfAssignmentChecker::checkBeginFunction(CheckerContext &C) const { + if (!C.inTopFrame()) + return; + const auto *LCtx = C.getLocationContext(); + const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl()); + if (!MD) + return; + if (!MD->isCopyAssignmentOperator() && !MD->isMoveAssignmentOperator()) + return; + auto &State = C.getState(); + auto &SVB = C.getSValBuilder(); + auto ThisVal = + State->getSVal(SVB.getCXXThis(MD, LCtx->getCurrentStackFrame())); + auto Param = SVB.makeLoc(State->getRegion(MD->getParamDecl(0), LCtx)); + auto ParamVal = State->getSVal(Param); + ProgramStateRef SelfAssignState = State->bindLoc(Param, ThisVal); + C.addTransition(SelfAssignState); + ProgramStateRef NonSelfAssignState = State->bindLoc(Param, ParamVal); + C.addTransition(NonSelfAssignState); +} + +void ento::registerCXXSelfAssignmentChecker(CheckerManager &Mgr) { + Mgr.registerChecker<CXXSelfAssignmentChecker>(); +} diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index 488126b0088..e04aa395d49 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -3104,6 +3104,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, R->addVisitor(llvm::make_unique<NilReceiverBRVisitor>()); R->addVisitor(llvm::make_unique<ConditionBRVisitor>()); R->addVisitor(llvm::make_unique<LikelyFalsePositiveSuppressionBRVisitor>()); + R->addVisitor(llvm::make_unique<CXXSelfAssignmentBRVisitor>()); BugReport::VisitorList visitors; unsigned origReportConfigToken, finalReportConfigToken; diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 0e505463bb5..3b72244a52c 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1693,3 +1693,56 @@ UndefOrNullArgVisitor::VisitNode(const ExplodedNode *N, } return nullptr; } + +PathDiagnosticPiece * +CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, BugReport &BR) { + if (Satisfied) + return nullptr; + + auto Edge = Succ->getLocation().getAs<BlockEdge>(); + if (!Edge.hasValue()) + return nullptr; + + auto Tag = Edge->getTag(); + if (!Tag) + return nullptr; + + if (Tag->getTagDescription() != "cplusplus.SelfAssignment") + return nullptr; + + Satisfied = true; + + const auto *Met = + dyn_cast<CXXMethodDecl>(Succ->getCodeDecl().getAsFunction()); + assert(Met && "Not a C++ method."); + assert((Met->isCopyAssignmentOperator() || Met->isMoveAssignmentOperator()) && + "Not a copy/move assignment operator."); + + const auto *LCtx = Edge->getLocationContext(); + + const auto &State = Succ->getState(); + auto &SVB = State->getStateManager().getSValBuilder(); + + const auto Param = + State->getSVal(State->getRegion(Met->getParamDecl(0), LCtx)); + const auto This = + State->getSVal(SVB.getCXXThis(Met, LCtx->getCurrentStackFrame())); + + auto L = PathDiagnosticLocation::create(Met, BRC.getSourceManager()); + + if (!L.isValid() || !L.asLocation().isValid()) + return nullptr; + + SmallString<256> Buf; + llvm::raw_svector_ostream Out(Buf); + + Out << "Assuming " << Met->getParamDecl(0)->getName() << + ((Param == This) ? " == " : " != ") << "*this"; + + auto *Piece = new PathDiagnosticEventPiece(L, Out.str()); + Piece->addRange(Met->getSourceRange()); + + return Piece; +} diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index 3d51062da35..3020ab54d0b 100644 --- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -426,6 +426,13 @@ static bool shouldSkipFunction(const Decl *D, // Count naming convention errors more aggressively. if (isa<ObjCMethodDecl>(D)) return false; + // We also want to reanalyze all C++ copy and move assignment operators to + // separately check the two cases where 'this' aliases with the parameter and + // where it may not. (cplusplus.SelfAssignmentChecker) + if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) { + if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) + return false; + } // Otherwise, if we visited the function before, do not reanalyze it. return Visited.count(D); @@ -437,9 +444,7 @@ AnalysisConsumer::getInliningModeForFunction(const Decl *D, // We want to reanalyze all ObjC methods as top level to report Retain // Count naming convention errors more aggressively. But we should tune down // inlining when reanalyzing an already inlined function. - if (Visited.count(D)) { - assert(isa<ObjCMethodDecl>(D) && - "We are only reanalyzing ObjCMethods."); + if (Visited.count(D) && isa<ObjCMethodDecl>(D)) { const ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(D); if (ObjCM->getMethodFamily() != OMF_init) return ExprEngine::Inline_Minimal; |
