Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -464,6 +464,10 @@ HelpText<"Checks C++ copy and move assignment operators for self assignment">, Documentation; +def SmartPtrModeling: Checker<"SmartPtr">, + HelpText<"Model behavior of C++ smart pointers">, + Documentation; + def MoveChecker: Checker<"Move">, HelpText<"Find use-after-move bugs in C++">, CheckerOptions<[ 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 + SmartPtrModeling.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/SmartPtrModeling.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -0,0 +1,74 @@ +// SmartPtrModeling.cpp - Model behavior of C++ smart pointers - 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" + +using namespace clang; +using namespace ento; + +namespace { +class SmartPtrModeling : public Checker { + bool isNullAfterMoveMethod(const CXXInstanceCall *Call) const; + +public: + bool evalCall(const CallExpr *CE, CheckerContext &C) const; +}; +} // end of anonymous namespace + +bool SmartPtrModeling::isNullAfterMoveMethod( + const CXXInstanceCall *Call) const { + // TODO: Update CallDescription to support anonymous calls? + // TODO: Handle other methods, such as .get() or .release(). + // But once we do, we'd need a visitor to explain null dereferences + // that are found via such modeling. + const auto *CD = dyn_cast(Call->getDecl()); + return CD && CD->getConversionType()->isBooleanType(); +} + +bool SmartPtrModeling::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 || !isNullAfterMoveMethod(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::registerSmartPtrModeling(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterSmartPtrModeling(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,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection\ +// RUN: -analyzer-checker cplusplus.Move,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 + // TODO: Report a null dereference (instead). + *P.get() = 1; // expected-warning {{Method called on moved-from object 'P'}} +} 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,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,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,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,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,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,cplusplus.SmartPtr,debug.ExprInspection\ // RUN: -verify=expected,peaceful,aggressive // RUN: not %clang_analyze_cc1 -verify %s \ @@ -933,3 +933,17 @@ 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}} + + // TODO: Explain why (bool)P is false. + if (P) // peaceful-note{{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'}} + + // TODO: Warn on a null dereference here. + a->foo(); +}