[analyzer] PR41269: Add a bit of C++ smart pointer modeling.

Implement cplusplus.SmartPtrModeling, a new checker that doesn't
emit any warnings but models methods of smart pointers more precisely.

For now the only thing it does is make `(bool) P` return false when `P`
is a freshly moved pointer. This addresses a false positive in the
use-after-move-checker.

Differential Revision: https://reviews.llvm.org/D60796

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358944 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 2d02aa0cc2bad10b338d2d5e64904e65ade0b46c..0b16a2a429239d45792ce66dd4b3adf2875b02c7 100644 (file)
--- a/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/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/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 6c1124607804de17112057edaf23492bca51bad6..f8201f33c48eff511c6d83c9a6d56e4c645866f6 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/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/lib/StaticAnalyzer/Checkers/Move.h b/lib/StaticAnalyzer/Checkers/Move.h
new file mode 100644 (file)
index 0000000..10644a8
--- /dev/null
+++ b/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/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index 545f310a51bf0d9d1bf72f01bb4ae721462d50b2..891a350ff23ae37c28762d1ba0f6d745e881e9b8 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/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/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
new file mode 100644 (file)
index 0000000..451a6c8
--- /dev/null
+++ b/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/test/Analysis/Inputs/system-header-simulator-cxx.h b/test/Analysis/Inputs/system-header-simulator-cxx.h
index cca9d7486ea8565cee522d9327ba574a03e9082e..3b3ac83b42721d867cddf3bdf2b32833de951452 100644 (file)
--- a/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/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/test/Analysis/smart-ptr.cpp b/test/Analysis/smart-ptr.cpp
new file mode 100644 (file)
index 0000000..3f17824
--- /dev/null
+++ b/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/test/Analysis/use-after-move.cpp b/test/Analysis/use-after-move.cpp
index 29e62f8c28eaa85e393e8e6cf290e9439ea1b269..5e4179b1f13f97b22d5d994d1724f53314847c95 100644 (file)
--- a/test/Analysis/use-after-move.cpp
+++ b/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();
+}