[analyzer] Add modeling for unque_ptr::get()

[vrnithinkumar]

Summary

Modeling to return tracked inner pointer for get() method

[NoQ]

Excellent, thanks!

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

362–363

You'll have to actively handle this case, sooner or later. Consider the following test cases that won't work until you do:

void foo(std::unique_ptr<A> p) {
  A *x = p.get();
  A *y = p.get();
  clang_analyzer_eval(x == y); // expected-warning{{TRUE}}
  if (!x) {
    y->foo(); // expected-warning{{Called C++ object pointer is null}}
  }
}

[xazax.hun]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

362–363

You mean the case where we do not have an inner pointer registered in the state yet, right?

I believe we might also have to handle similar cases for operator bool() as well.

[vrnithinkumar]

• Using conjureSymbolVal in case of missing inner pointer value

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

362–363

Added the above test case.
Using conjureSymbolVal in case of missing inner pointer value

362–363

void foo(std::unique_ptr<A> P) {
  A *X = P.get();
  if (!X) {
    P->foo(); // expected-warning {{Dereference of null smart pointer 'Pl' [alpha.cplusplus.SmartPtr]}}
  }
}

I was trying to add the above use case. Since we are using conjureSymbolVal in case of missing inner pointer value.

But still the inner pointer value is constrained to [0, 0] in false branch, InnerPointVal->isZeroConstant() returning false.
Also I tried State->isNull(*InnerPointVal).isConstrainedTrue(); This is also not working.
How should we check whether the conjureSymbolVal for inner pointer value is constrained to [0, 0]?

[NoQ]

This patch looks correct to me at a glance. I think we should land it as is and debug/improve later.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

362–363

How should we check whether the conjureSymbolVal for inner pointer value is constrained to [0, 0]?

View exploded graphs. That's literally the only reasonable answer to every such question. In particular, it shows you constraints for all symbols at every moment of time, and given that you implemented printState() it also shows you inner pointer values that you keep track of at every moment of time.

Check if it's still the same symbol. Check that the symbol lives long enough - or does it get forgotten about in the middle? - if so you might need to get your checkLiveSymbols callback right.

Please let us know if you still can't seem to debug it on your own.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

 // 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
 //
 //===----------------------------------------------------------------------===//
 //
checkRegionChanges(ProgramStateRef State,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const LocationContext *LCtx, const CallEvent *Call) const;
 
 private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+  void handleGet(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
       void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
       {{"reset"}, &SmartPtrModeling::handleReset},
       {{"release"}, &SmartPtrModeling::handleRelease},
-      {{"swap", 1}, &SmartPtrModeling::handleSwap}};
+      {{"swap", 1}, &SmartPtrModeling::handleSwap},
+      {{"get"}, &SmartPtrModeling::handleGet}};
 };
 } // end of anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
 
 // Define the inter-checker API.
 namespace clang {
 namespace ento {
C.addTransition(
         BR.markInteresting(ArgRegion);
         OS << "Swapped null smart pointer ";
         ArgRegion->printPretty(OS);
         OS << " with smart pointer ";
         ThisRegion->printPretty(OS);
       }));
 }
 
+void SmartPtrModeling::handleGet(const CallEvent &Call,
+                                 CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
+  if (!IC)
+    return;
+
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
+    return;
+
+  const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
+  if (!InnerPointVal)
+    return;
+
+  State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+                          *InnerPointVal);
+  //TODO: Add NoteTag, for how the raw pointer got using 'get' method.
+  C.addTransition(State);
+}
+
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
   Checker->ModelSmartPtrDereference =
       Mgr.getAnalyzerOptions().getCheckerBooleanOption(
           Checker, "ModelSmartPtrDereference");
 }
 
 bool ento::shouldRegisterSmartPtrModeling(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.CPlusPlus;
 }

clang/test/Analysis/smart-ptr-text-output.cpp

 
 void noNoteTagsForNonMatchingBugType() {
   std::unique_ptr<A> P; // No note.
   std::unique_ptr<A> P1; // No note.
   P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr<A> P; // FIXME: add note "Default constructed smart pointer 'P' is null"
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+  // expected-note@-1 {{Called C++ object pointer is null}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr<A> P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr<A> P) {
+  P.get()->foo(); // No warning.
+}

clang/test/Analysis/smart-ptr.cpp

void derefOnSwappedValidPtr() {
   std::unique_ptr<A> PValid(new A());
   P.swap(PValid);
   (*P).foo(); // No warning.
   PValid->foo(); // No warning.
   std::swap(P, PValid);
   P->foo(); // No warning.
   PValid->foo(); // No warning.
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr<A> P;
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr<A> P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr<A> P) {
+  P.get()->foo(); // No warning.
+}