commit bafacade3dd182d3a6c85d7656fc15a6beee939f Author: Jordan Rose Date: Fri Mar 15 09:28:26 2013 [analyzer] Model trivial copy/move assignment operators with a bind as well. r175234 allowed the analyzer to model trivial copy/move constructors as an aggregate bind. This commit extends that to trivial assignment operators as well. Like the last commit, one of the motivating factors here is not warning when the right-hand object is partially-initialized; this can have legitimate uses. Unlike r175234, which modified ExprEngine, this change is implemented as a checker (CXXTrivialAssignmentChecker) using the old evalCall callback mechanism. This seems cleaner than adding more special cases into ExprEngine, and once we've designed a CallEvent-based interface for evalCall, we can migrate the constructor case to this as well. diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index b7df10e..facb53d 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -23,6 +23,7 @@ add_clang_library(clangStaticAnalyzerCheckers ChrootChecker.cpp ClangCheckers.cpp CommonBugCategories.cpp + CXXTrivialAssignment.cpp DeadStoresChecker.cpp DebugCheckers.cpp DereferenceChecker.cpp diff --git a/lib/StaticAnalyzer/Checkers/CXXTrivialAssignment.cpp b/lib/StaticAnalyzer/Checkers/CXXTrivialAssignment.cpp new file mode 100644 index 0000000..6172baa --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/CXXTrivialAssignment.cpp @@ -0,0 +1,78 @@ +//=== CXXTrivialAssignment.cpp --------------------------------- -*- C++ -*--=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Models C++ trivial assignment operations using compound binds rather than +// element-wise assignments. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class CXXTrivialAssignmentChecker : public Checker< eval::Call > { +public: + bool evalCall(const CallExpr *CE, CheckerContext &C) const; +}; +} + +bool CXXTrivialAssignmentChecker::evalCall(const CallExpr *CE, + CheckerContext &C) const { + const FunctionDecl *FD = C.getCalleeDecl(CE); + const CXXMethodDecl *MD = dyn_cast_or_null(FD); + if (!MD) + return false; + + if (!MD->isCopyAssignmentOperator() && + !MD->isMoveAssignmentOperator()) + return false; + if (!MD->isTrivial()) + return false; + + const Expr *LHS, *RHS; + if (const CXXOperatorCallExpr *OCE = dyn_cast(CE)) { + LHS = OCE->getArg(0); + RHS = OCE->getArg(1); + } else { + const CXXMemberCallExpr *MCE = cast(CE); + LHS = MCE->getImplicitObjectArgument(); + RHS = MCE->getArg(0); + } + + Optional LHSLoc = C.getSVal(LHS).getAs(); + if (!LHSLoc) + return true; + + ProgramStateRef State = C.getState(); + + // Make sure to handle cases where the RHS is a known location and where it is + // unknown. + SVal RHSVal = C.getSVal(RHS); + if (Optional RHSLoc = RHSVal.getAs()) + RHSVal = State->getSVal(*RHSLoc); + + // FIXME: Checkers shouldn't have to do this explicitly. + SubEngine *Eng = C.getStateManager().getOwningEngine(); + State = Eng->processPointerEscapedOnBind(State, *LHSLoc, RHSVal); + + State = State->bindLoc(*LHSLoc, RHSVal); + State = State->BindExpr(CE, C.getLocationContext(), *LHSLoc); + C.addTransition(State); + return true; +} + +void ento::registerTrivialAssignment(CheckerManager &mgr) { + mgr.registerChecker(); +} diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index bbcf9aa..813bc8d 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -84,6 +84,10 @@ def DynamicTypePropagation : Checker<"DynamicTypePropagation">, HelpText<"Generate dynamic type information">, DescFile<"DynamicTypePropagation.cpp">; +def TrivialAssignment : Checker<"TrivialAssignment">, + HelpText<"Model C++ trivial assignment operators">, + DescFile<"CXXTrivialAssignment.cpp">; + } // end "core" let ParentPackage = CoreAlpha in { diff --git a/test/Analysis/ctor-inlining.mm b/test/Analysis/ctor-inlining.mm index 1603ff3..fd8caf3 100644 --- a/test/Analysis/ctor-inlining.mm +++ b/test/Analysis/ctor-inlining.mm @@ -149,6 +149,19 @@ namespace PODUninitialized { : x(Other.x), y(Other.y) // expected-warning {{undefined}} { } + + NonPOD &operator=(const NonPOD &Other) + { + x = Other.x; + y = Other.y; // expected-warning {{undefined}} + return *this; + } + NonPOD &operator=(NonPOD &&Other) + { + x = Other.x; + y = Other.y; // expected-warning {{undefined}} + return *this; + } }; class NonPODWrapper { @@ -166,6 +179,19 @@ namespace PODUninitialized { : x(Other.x), y(Other.y) // expected-warning {{undefined}} { } + + Inner &operator=(const Inner &Other) + { + x = Other.x; // expected-warning {{undefined}} + y = Other.y; + return *this; + } + Inner &operator=(Inner &&Other) + { + x = Other.x; // expected-warning {{undefined}} + y = Other.y; + return *this; + } }; Inner p; @@ -216,4 +242,68 @@ namespace PODUninitialized { w.p.y = 1; NonPODWrapper w2 = move(w); } + + // Not strictly about constructors, but trivial assignment operators should + // essentially work the same way. + namespace AssignmentOperator { + void testPOD() { + POD p; + p.x = 1; + POD p2; + p2 = p; // no-warning + clang_analyzer_eval(p2.x == 1); // expected-warning{{TRUE}} + POD p3; + p3 = move(p); // no-warning + clang_analyzer_eval(p3.x == 1); // expected-warning{{TRUE}} + + PODWrapper w; + w.p.y = 1; + PODWrapper w2; + w2 = w; // no-warning + clang_analyzer_eval(w2.p.y == 1); // expected-warning{{TRUE}} + PODWrapper w3; + w3 = move(w); // no-warning + clang_analyzer_eval(w3.p.y == 1); // expected-warning{{TRUE}} + } + + void testReturnValue() { + POD p; + p.x = 1; + POD p2; + clang_analyzer_eval(&(p2 = p) == &p2); // expected-warning{{TRUE}} + + PODWrapper w; + w.p.y = 1; + PODWrapper w2; + clang_analyzer_eval(&(w2 = w) == &w2); // expected-warning{{TRUE}} + } + + void testNonPOD() { + NonPOD p; + p.x = 1; + NonPOD p2; + p2 = p; + } + + void testNonPODMove() { + NonPOD p; + p.x = 1; + NonPOD p2; + p2 = move(p); + } + + void testNonPODWrapper() { + NonPODWrapper w; + w.p.y = 1; + NonPODWrapper w2; + w2 = w; + } + + void testNonPODWrapperMove() { + NonPODWrapper w; + w.p.y = 1; + NonPODWrapper w2; + w2 = move(w); + } + } }