diff options
| -rw-r--r-- | clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 4 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/Move.h | 30 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp | 12 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp | 74 | ||||
| -rw-r--r-- | clang/test/Analysis/Inputs/system-header-simulator-cxx.h | 1 | ||||
| -rw-r--r-- | clang/test/Analysis/smart-ptr.cpp | 18 | ||||
| -rw-r--r-- | clang/test/Analysis/use-after-move.cpp | 26 |
8 files changed, 160 insertions, 6 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 2d02aa0cc2b..0b16a2a4292 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -464,6 +464,10 @@ def CXXSelfAssignmentChecker : Checker<"SelfAssignment">, HelpText<"Checks C++ copy and move assignment operators for self assignment">, Documentation<NotDocumented>; +def SmartPtrModeling: Checker<"SmartPtr">, + HelpText<"Model behavior of C++ smart pointers">, + Documentation<NotDocumented>; + def MoveChecker: Checker<"Move">, HelpText<"Find use-after-move bugs in C++">, CheckerOptions<[ diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 6c112460780..f8201f33c48 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -84,6 +84,7 @@ add_clang_library(clangStaticAnalyzerCheckers ReturnUndefChecker.cpp RunLoopAutoreleaseLeakChecker.cpp SimpleStreamChecker.cpp + SmartPtrModeling.cpp StackAddrEscapeChecker.cpp StdLibraryFunctionsChecker.cpp StreamChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/Move.h b/clang/lib/StaticAnalyzer/Checkers/Move.h new file mode 100644 index 00000000000..10644a8fcb3 --- /dev/null +++ b/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 diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index 545f310a51b..891a350ff23 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -227,6 +227,18 @@ private: 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<TrackedRegionMap>(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) { diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp new file mode 100644 index 00000000000..451a6c8a906 --- /dev/null +++ b/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<eval::Call> { + 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<CXXConversionDecl>(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<CXXInstanceCall>(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<SmartPtrModeling>(); +} + +bool ento::shouldRegisterSmartPtrModeling(const LangOptions &LO) { + return LO.CPlusPlus; +} diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index cca9d7486ea..3b3ac83b427 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -789,6 +789,7 @@ namespace std { typename std::add_lvalue_reference<T>::type operator*() const; T *operator->() const; + operator bool() const; }; } #endif diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp new file mode 100644 index 00000000000..3f1782480b4 --- /dev/null +++ b/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<int> P) { + std::unique_ptr<int> 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'}} +} diff --git a/clang/test/Analysis/use-after-move.cpp b/clang/test/Analysis/use-after-move.cpp index 29e62f8c28e..5e4179b1f13 100644 --- a/clang/test/Analysis/use-after-move.cpp +++ b/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 @@ void localUniquePtrWithArrow(std::unique_ptr<A> P) { 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<A> P) { + std::unique_ptr<A> 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(); +} |

