RedDocMD updated this revision to Diff 351628.
RedDocMD added a comment.

Fixed up technique used to find inner pointer type, put TODO's


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===================================================================
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,3 +1,8 @@
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// 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\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
@@ -313,3 +318,27 @@
     // 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>(); // expected-note {{std::unique_ptr 'P' constructed by std::make_unique_for_overwrite is null}}
+  *P;                                           // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+#endif
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
@@ -978,6 +978,17 @@
 void swap(unique_ptr<T> &x, unique_ptr<T> &y) noexcept {
   x.swap(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
 
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -35,6 +35,7 @@
 using namespace ento;
 
 namespace {
+
 class SmartPtrModeling
     : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges,
                      check::LiveSymbols> {
@@ -76,6 +77,9 @@
       {{"release"}, &SmartPtrModeling::handleRelease},
       {{"swap", 1}, &SmartPtrModeling::handleSwap},
       {{"get"}, &SmartPtrModeling::handleGet}};
+  const CallDescription StdMakeUniqueCall{{"std", "make_unique"}};
+  const CallDescription StdMakeUniqueForOverwriteCall{
+      {"std", "make_unique_for_overwrite"}};
 };
 } // end of anonymous namespace
 
@@ -135,6 +139,21 @@
   return State;
 }
 
+// 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) {
@@ -177,7 +196,60 @@
 
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
+
   ProgramStateRef State = C.getState();
+
+  if (Call.isCalled(StdMakeUniqueCall)) {
+    const Optional<SVal> ThisRegionOpt = Call.getReturnValueUnderConstruction();
+    if (!ThisRegionOpt)
+      return false;
+
+    const auto PtrVal = C.getSValBuilder().conjureSymbolVal(
+        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.
+    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 (Call.isCalled(StdMakeUniqueForOverwriteCall)) {
+    const Optional<SVal> ThisRegionOpt = Call.getReturnValueUnderConstruction();
+    if (!ThisRegionOpt)
+      return false;
+    const MemRegion *ThisRegion = ThisRegionOpt->getAsRegion();
+    const auto NullVal = C.getSValBuilder().makeNull();
+    State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
+
+    // TODO: ExprEngine should do this for us.
+    auto &Engine = State->getStateManager().getOwningEngine();
+    State = Engine.updateObjectsUnderConstruction(
+        *ThisRegionOpt, nullptr, State, C.getLocationContext(),
+        Call.getConstructionContext(), {});
+
+    C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+                                                     llvm::raw_ostream &OS) {
+      if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+          !BR.isInteresting(ThisRegion))
+        return;
+      OS << "std::unique_ptr";
+      checkAndPrettyPrintRegion(OS, ThisRegion);
+      OS << " constructed by std::make_unique_for_overwrite is null";
+    }));
+    return true;
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
     return false;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to