[analyzer] Handle std::make_unique

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

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 64c61a3..87a49cf 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -238,6 +238,14 @@ public:
                                                 const LocationContext *LCtx,
                                                 unsigned Count);
 
+  /// Conjure a symbol representing heap allocated memory region.
+  ///
+  /// Note, now, the expression *doesn't* need to represent a location.
+  /// But the type need to!
+  DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
+                                                const LocationContext *LCtx,
+                                                QualType type, unsigned Count);
+
   DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
       SymbolRef parentSymbol, const TypedValueRegion *region);
 

diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 2793980..253606b 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -38,6 +38,7 @@ using namespace clang;
 using namespace ento;
 
 namespace {
+
 class SmartPtrModeling
     : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges,
                      check::LiveSymbols> {
@@ -88,6 +89,9 @@ private:
       {{"swap", 1}, &SmartPtrModeling::handleSwapMethod},
       {{"get"}, &SmartPtrModeling::handleGet}};
   const CallDescription StdSwapCall{{"std", "swap"}, 2};
+  const CallDescription StdMakeUniqueCall{{"std", "make_unique"}};
+  const CallDescription StdMakeUniqueForOverwriteCall{
+      {"std", "make_unique_for_overwrite"}};
 };
 } // end of anonymous namespace
 
@@ -187,12 +191,27 @@ static QualType getInnerPointerType(CheckerContext C, const CXXRecordDecl *RD) {
     return {};
 
   auto TemplateArgs = TSD->getTemplateArgs().asArray();
-  if (TemplateArgs.size() == 0)
+  if (TemplateArgs.empty())
     return {};
   auto InnerValueType = TemplateArgs[0].getAsType();
   return C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
 }
 
+// This is for use with standalone-functions like std::make_unique,
+// std::make_unique_for_overwrite, etc. It reads the template parameter and
+// returns the pointer type corresponding to it,
+static QualType getPointerTypeFromTemplateArg(const CallEvent &Call,
+                                              CheckerContext &C) {
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+  if (!FD || !FD->isFunctionTemplateSpecialization())
+    return {};
+  const auto &TemplateArgs = FD->getTemplateSpecializationArgs()->asArray();
+  if (TemplateArgs.size() == 0)
+    return {};
+  auto ValueType = TemplateArgs[0].getAsType();
+  return C.getASTContext().getPointerType(ValueType.getCanonicalType());
+}
+
 // Helper method to get the inner pointer type of specialized smart pointer
 // Returns empty type if not found valid inner pointer type.
 static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
@@ -248,6 +267,7 @@ bool isStdOstreamOperatorCall(const CallEvent &Call) {
 
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
+
   ProgramStateRef State = C.getState();
 
   // If any one of the arg is a unique_ptr, then
@@ -271,6 +291,50 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
     return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
   }
 
+  if (Call.isCalled(StdMakeUniqueCall) ||
+      Call.isCalled(StdMakeUniqueForOverwriteCall)) {
+    if (!ModelSmartPtrDereference)
+      return false;
+    
+    const Optional<SVal> ThisRegionOpt = Call.getReturnValueUnderConstruction();
+    if (!ThisRegionOpt)
+      return false;
+
+    const auto PtrVal = C.getSValBuilder().getConjuredHeapSymbolVal(
+        Call.getOriginExpr(), C.getLocationContext(),
+        getPointerTypeFromTemplateArg(Call, C), C.blockCount());
+
+    const MemRegion *ThisRegion = ThisRegionOpt->getAsRegion();
+    State = State->set<TrackedRegionMap>(ThisRegion, PtrVal);
+    State = State->assume(PtrVal, true);
+
+    // TODO: ExprEngine should do this for us.
+    // For a bit more context:
+    // 1) Why do we need this? Since we are modelling a "function"
+    // that returns a constructed object we need to store this information in
+    // the program state.
+    //
+    // 2) Why does this work?
+    // `updateObjectsUnderConstruction` does exactly as it sounds.
+    //
+    // 3) How should it look like when moved to the Engine?
+    // It would be nice if we can just
+    // pretend we don't need to know about this - ie, completely automatic work.
+    // However, realistically speaking, I think we would need to "signal" the
+    // ExprEngine evalCall handler that we are constructing an object with this
+    // function call (constructors obviously construct, hence can be
+    // automatically deduced).
+    auto &Engine = State->getStateManager().getOwningEngine();
+    State = Engine.updateObjectsUnderConstruction(
+        *ThisRegionOpt, nullptr, State, C.getLocationContext(),
+        Call.getConstructionContext(), {});
+
+    // We don't leave a note here since it is guaranteed the
+    // unique_ptr from this call is non-null (hence is safe to de-reference).
+    C.addTransition(State);
+    return true;
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
     return false;
 

diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index a5e99ce..b459b5a 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@ SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
                                       const LocationContext *LCtx,
                                       unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-    return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+                                      const LocationContext *LCtx,
+                                      QualType type, unsigned VisitCount) {
+  assert(Loc::isLocType(type));
+  assert(SymbolManager::canSymbolicate(type));
+  if (type->isNullPtrType())
+    return makeZeroVal(type);
 
-  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount);
+  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount);
   return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));
 }
 

diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index bef03db..ff64c5b 100644 (file)
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1033,6 +1033,16 @@ bool operator>=(nullptr_t x, const unique_ptr<T> &y);
 template <typename T>
 bool operator<=(nullptr_t x, const unique_ptr<T> &y);
 
+template <class T, class... Args>
+unique_ptr<T> make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template <class T>
+unique_ptr<T> make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 

diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp
index 299fed8..136b635 100644 (file)
--- a/clang/test/Analysis/smart-ptr-text-output.cpp
+++ b/clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -310,3 +317,61 @@ void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr<A> P) {
     // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique<A>();
+  if (!P) {   // expected-note {{Taking false branch}}
+    P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+    // expected-note@-1 {{Taking true branch}}
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+    // expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite<A>();
+  if (!P) {   // expected-note {{Taking false branch}}
+    P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+    // expected-note@-1 {{Taking true branch}}
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+    // expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+    auto P = std::make_unique<G>(&x);
+    // FIXME: There should not be a state split here, it should take the true path.
+    clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+    // expected-warning@-1 {{FALSE}}
+    // expected-note@-2 {{Assuming the condition is true}}
+    // expected-note@-3 {{Assuming the condition is false}}
+    // expected-note@-4 {{TRUE}}
+    // expected-note@-5 {{FALSE}}
+    // expected-note@-6 {{Assuming the condition is false}}
+  }
+  // FIXME: Should be fixed when unique_ptr desctructors are
+  // properly modelled. This includes modelling the call to
+  // the destructor of the inner pointer type.
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}