Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -491,6 +491,10 @@ Dependencies<[IteratorModeling]>, Documentation; +def SmartPtrChecker: Checker<"SmartPtr">, + HelpText<"Checks use of C++ smart pointers">, + Documentation; + def UninitializedObjectChecker: Checker<"UninitializedObject">, HelpText<"Reports uninitialized fields after object construction">, Documentation; Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -84,6 +84,7 @@ ReturnUndefChecker.cpp RunLoopAutoreleaseLeakChecker.cpp SimpleStreamChecker.cpp + SmartPtrChecker.cpp StackAddrEscapeChecker.cpp StdLibraryFunctionsChecker.cpp StreamChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/Move.h =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/Move.h @@ -0,0 +1,30 @@ +//=== Move.h - Tracking moved-from objects. ------------------------*- 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 +// +//===----------------------------------------------------------------------===// +// +// Defines inter-checker API for the use-after-move checker. It allows +// dependent checkers to figure out if an object is in a moved-from state. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H + +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" + +namespace clang { +namespace ento { +namespace move { + +/// Returns true if the object is known to have been recently std::moved. +bool isMovedFrom(ProgramStateRef State, const MemRegion *Region); + +} // namespace move +} // namespace ento +} // namespace clang + +#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H Index: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -227,6 +227,18 @@ REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, RegionState) +// Define the inter-checker API. +namespace clang { +namespace ento { +namespace move { +bool isMovedFrom(ProgramStateRef State, const MemRegion *Region) { + const RegionState *RS = State->get(Region); + return RS && (RS->isMoved() || RS->isReported()); +} +} // namespace move +} // namespace ento +} // namespace clang + // If a region is removed all of the subregions needs to be removed too. static ProgramStateRef removeFromState(ProgramStateRef State, const MemRegion *Region) { Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp @@ -0,0 +1,84 @@ +// SmartPtrChecker.cpp - Check use of moved-from objects. - 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 +// +//===----------------------------------------------------------------------===// +// +// This file defines a checker that models various aspects of +// C++ smart pointer behavior. +// +//===----------------------------------------------------------------------===// + +#include "Move.h" + +#include "clang/AST/ExprCXX.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.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" +#include "llvm/ADT/StringSet.h" + +using namespace clang; +using namespace ento; + +namespace { +class SmartPtrChecker : public Checker { + std::vector NullAfterMoveMethods = { + {{ "unique_ptr", "get" }}, + {{ "unique_ptr", "release" }}, + {{ "shared_ptr", "get" }}, + {{ "shared_ptr", "release" }}, + {{ "weak_ptr", "get" }}, + {{ "weak_ptr", "release" }}, + }; + bool isNullAfterMoveMethod(const CXXInstanceCall *Call); + +public: + bool evalCall(const CallExpr *CE, CheckerContext &C) const; +}; +} // end of anonymous namespace + +bool SmartPtrChecker::isNullAfterMoveMethod(const CXXInstanceCall *Call) { + // TODO: Update CallDescription to support anonymous calls? + const auto *CD = dyn_cast(Call->getDecl()); + if (CD && CD->getConversionType()->isBooleanType()) + return true; + + return llvm::any_of( + NullAfterMoveMethods, + std::bind(&CallEvent::isCalled, Call, std::placeholders::_1)); +} + +bool SmartPtrChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { + CallEventRef<> CallRef = C.getStateManager().getCallEventManager().getCall( + CE, C.getState(), C.getLocationContext()); + const auto *Call = dyn_cast_or_null(CallRef); + if (!Call) + return false; + + ProgramStateRef State = C.getState(); + const MemRegion *ThisR = Call->getCXXThisVal().getAsRegion(); + + if (!move::isMovedFrom(State, ThisR)) { + // TODO: Model this case as well. At least, avoid invalidation of globals. + return false; + } + + // TODO: Add a note to bug reports describing this decision. + C.addTransition( + State->BindExpr(CE, C.getLocationContext(), + C.getSValBuilder().makeZeroVal(CE->getType()))); + return true; +} + +void ento::registerSmartPtrChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterSmartPtrChecker(const LangOptions &LO) { + return LO.CPlusPlus; +} Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -789,6 +789,7 @@ typename std::add_lvalue_reference::type operator*() const; T *operator->() const; + operator bool() const; }; } #endif Index: clang/test/Analysis/smart-ptr.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/smart-ptr.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection\ +// RUN: -analyzer-checker cplusplus.Move,alpha.cplusplus.SmartPtr\ +// RUN: -std=c++11 -verify %s + +#include "Inputs/system-header-simulator-cxx.h" + +void clang_analyzer_warnIfReached(); + +void derefAfterMove(std::unique_ptr P) { + std::unique_ptr Q = std::move(P); + if (Q) + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + *Q.get() = 1; // no-warning + if (P) + clang_analyzer_warnIfReached(); // no-warning + // FIXME: Implement suppress-on-sink so that use-after-move warning didn't + // show up here. It's redundant when there's already a null dereference. + *P.get() = 1; // expected-warning {{Method called on moved-from object 'P'}} + // expected-warning@-1 {{Dereference of null pointer}} +} Index: clang/test/Analysis/use-after-move.cpp =================================================================== --- clang/test/Analysis/use-after-move.cpp +++ clang/test/Analysis/use-after-move.cpp @@ -1,36 +1,36 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=unexplored_first_queue\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,peaceful,non-aggressive // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=dfs -DDFS\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,peaceful,non-aggressive // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=unexplored_first_queue\ // RUN: -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,non-aggressive // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=dfs -DDFS\ // RUN: -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,non-aggressive // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=unexplored_first_queue\ // RUN: -analyzer-config cplusplus.Move:WarnOn=All\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,peaceful,aggressive // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\ // RUN: -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\ // RUN: -analyzer-config exploration_strategy=dfs -DDFS\ // RUN: -analyzer-config cplusplus.Move:WarnOn=All\ -// RUN: -analyzer-checker debug.ExprInspection\ +// RUN: -analyzer-checker core,alpha.cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,peaceful,aggressive // RUN: not %clang_analyze_cc1 -verify %s \ @@ -933,3 +933,18 @@ P->foo(); // expected-warning{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} // expected-note@-1{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} } + +void getAfterMove(std::unique_ptr P) { + std::unique_ptr Q = std::move(P); // peaceful-note {{Object 'P' is moved}} + + if (P) // peaceful-note {{Taking false branch}} + // expected-note@-1 {{Taking false branch}} + clang_analyzer_warnIfReached(); // no-warning + + A *a = P.get(); // peaceful-warning {{Method called on moved-from object 'P'}} + // peaceful-note@-1 {{Method called on moved-from object 'P'}} + // FIXME: Explain why is this a null value. + // expected-note@-3 {{'a' initialized to a null pointer value}} + a->foo(); // expected-warning {{Called C++ object pointer is null}} + // expected-note@-1 {{Called C++ object pointer is null}} +}