[Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[vrnithinkumar]

Summary

• Adding the test case after checkLiveSymbols implementation checked in

[NoQ]

Nice catch, thx!

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

106–107

The intended ultimate test is !State->assume(X) and it's also much easier to write. I strongly suspect that we should remove these other APIs; they're only confusing.

[Szelethus]

Congrats on your GSoC! Unless I missed it, it seems like you haven't posted your final report on cfe-dev, even though its an amazing looking document with a lot of examples and explanations. I think it would be great to spread the message, its a work to be proud of!

[vrnithinkumar]

In D87043#2294531, @Szelethus wrote:

Congrats on your GSoC! Unless I missed it, it seems like you haven't posted your final report on cfe-dev, even though its an amazing looking document with a lot of examples and explanations. I think it would be great to spread the message, its a work to be proud of!

I am sorry, I took a break and I forgot about sharing the final report.
Thanks for the reminder.

[vrnithinkumar]

• Addressing review comment

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

106–107

Sorry for addressing the pending comment so late. :(
I was busy with my postponed exams.

[NoQ]

Looks great, thank you! I think you can commit it :)

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

if (RecordDecl->getDeclName().isIdentifier()) {
     StringRef Name = RecordDecl->getName();
     return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
   }
   return false;
 }
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal &&
+         !State->assume(InnerPointVal->castAs<DefinedOrUnknownSVal>(), true);
 }
 } // namespace smartptr
 } // namespace ento
 } // namespace clang
 
 // If a region is removed all of the subregions need to be removed too.
 static TrackedRegionMapTy
 removeTrackedSubregions(TrackedRegionMapTy RegionMap,

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

struct S {
   int callingFooWithUnknownPointer(std::unique_ptr<int> PUnknown) {
     P.swap(PUnknown);
     foo(); // No note on Calling 'S::foo'
     P.reset(new int(0)); // expected-note {{Assigning 0}}
     return 1 / *(P.get()); // expected-warning {{Division by zero [core.DivideZero]}}
     // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr<A> P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+    // 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'}}
+  }
+}

clang/test/Analysis/smart-ptr.cpp

void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr<A> P) {
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 std::unique_ptr<A> &&returnRValRefOfUniquePtr();
 
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr<A> P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
   std::unique_ptr<A> PToMove(new A());
   std::unique_ptr<A> P(std::move(PToMove));
   P->foo(); // No warning.
 }
 
void derefUnknownPtrMovedToConstruct(std::unique_ptr<A> PToMove) {
   std::unique_ptr<A> P(std::move(PToMove));
   PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
 }
 
 std::unique_ptr<A> &&functionReturnsRValueRef();
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr<A> P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
   std::unique_ptr<A> P;
   if (P)
     P->foo(); // No warning.
   else
     P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}

 
 int derefConditionOnUnKnownPtr(int *q) {
   std::unique_ptr<int> P(q);
   if (P)
     return *P; // No warning.
   else
     return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr<A> P) {
+  A *RP = P.get();
+  if (!RP) {
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  }
+}