[analyzer] [RetainCountChecker] Escape values stored into smart pointers if they have non-stack storage

[george.karpenkov]

Summary

The previous exception for smart pointers was too general

[NoQ]

clang/test/Analysis/osobject-retain-release.cpp

669

I suspect that this object B does not necessarily has local storage here. Due to copy elision it may end up pretty much anywhere. The smart pointer is on the stack though. When exactly is the escape supposed to happen here? Do we have a more simple test? :)

Also does this need to be a method?

[NoQ]

*adds an internal tracking backreference*

rdar://problem/47323216

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

 /// - binding to a memregion that does not have stack storage
 /// - binding to a variable that has a destructor attached using CleanupAttr
 ///
 /// We do not currently model what happens when a symbol is
 /// assigned to a struct field, unless it is a known smart pointer
 /// implementation, about which we know that it is inlined.
 /// FIXME: This could definitely be improved upon.
 static bool shouldEscapeRegion(const MemRegion *R) {
+  if (!R->hasStackStorage())
+    return true;
+
   if (isSmartPtrField(R))
     return false;
 
   const auto *VR = dyn_cast<VarRegion>(R);
 
-  if (!R->hasStackStorage() || !VR)
+  if (!VR)
     return true;
 
   const VarDecl *VD = VR->getDecl();
   if (!VD->hasAttr<CleanupAttr>())
     return false; // CleanupAttr attaches destructors, which cause escaping.
   return true;
 }
 

clang/test/Analysis/os_smart_ptr.h

 #ifndef _OS_SMART_POINTER_H
 #define _OS_SMART_POINTER_H
 
 #include "os_object_base.h"
 
 namespace os {
 
+static struct no_retain_t {} no_retain;
+
 template<class T>
 struct smart_ptr {
   smart_ptr() : pointer(nullptr) {}
 
   explicit smart_ptr(T *&p) : pointer(p) {
     if (pointer) {
       _retain(pointer);
     }
   }
 
+  explicit smart_ptr(T *&p, no_retain_t) : pointer(p) {}
+
   smart_ptr(smart_ptr const &rhs) : pointer(rhs.pointer) {
     if (pointer) {
       _retain(pointer);
     }
   }
 
   smart_ptr & operator=(T *&rhs) {
     smart_ptr(rhs).swap(*this);
     return *this;
   }
 
   smart_ptr & operator=(smart_ptr &rhs) {
     smart_ptr(rhs).swap(*this);
     return *this;
   }
 
+  smart_ptr &
+  operator=(smart_ptr<T> const &rhs) {
+    smart_ptr(rhs.get()).swap(*this);
+    return *this;
+  }
+
   ~smart_ptr() {
     if (pointer) {
       _release(pointer);
     }
   }
 
   void reset() {
     smart_ptr().swap(*this);

clang/test/Analysis/osobject-retain-release.cpp

 
 using OSObjectPtr = os::smart_ptr<OSObject>;
 
 void test_smart_ptr_uaf() {
   OSObject *obj = new OSObject; // expected-note{{Operator 'new' returns an OSObject of type 'OSObject' with a +1 retain count}}
   {
     OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr<OSObject>'}}
    // expected-note@-1{{Returning from constructor for 'smart_ptr<OSObject>'}}
-    // expected-note@os_smart_ptr.h:13{{Taking true branch}}
-    // expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}}
-    // expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}}
-    // expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}}
+    // expected-note@os_smart_ptr.h:15{{Taking true branch}}
+    // expected-note@os_smart_ptr.h:16{{Calling 'smart_ptr::_retain'}}
+    // expected-note@os_smart_ptr.h:81{{Reference count incremented. The object now has a +2 retain count}}
+    // expected-note@os_smart_ptr.h:16{{Returning from 'smart_ptr::_retain'}}
   } // expected-note{{Calling '~smart_ptr'}}
-  // expected-note@os_smart_ptr.h:35{{Taking true branch}}
-  // expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}}
-  // expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}}
-  // expected-note@os_smart_ptr.h:36{{Returning from 'smart_ptr::_release'}}
+  // expected-note@os_smart_ptr.h:45{{Taking true branch}}
+  // expected-note@os_smart_ptr.h:46{{Calling 'smart_ptr::_release'}}
+  // expected-note@os_smart_ptr.h:86{{Reference count decremented. The object now has a +1 retain count}}
+  // expected-note@os_smart_ptr.h:46{{Returning from 'smart_ptr::_release'}}
  // expected-note@-5{{Returning from '~smart_ptr'}}
   obj->release(); // expected-note{{Object released}}
   obj->release(); // expected-warning{{Reference-counted object is used after it is released}}
 // expected-note@-1{{Reference-counted object is used after it is released}}
 }
 
 void test_smart_ptr_leak() {
   OSObject *obj = new OSObject; // expected-note{{Operator 'new' returns an OSObject of type 'OSObject' with a +1 retain count}}
   {
     OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr<OSObject>'}}
    // expected-note@-1{{Returning from constructor for 'smart_ptr<OSObject>'}}
-    // expected-note@os_smart_ptr.h:13{{Taking true branch}}
-    // expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}}
-    // expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}}
-    // expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}}
+    // expected-note@os_smart_ptr.h:15{{Taking true branch}}
+    // expected-note@os_smart_ptr.h:16{{Calling 'smart_ptr::_retain'}}
+    // expected-note@os_smart_ptr.h:81{{Reference count incremented. The object now has a +2 retain count}}
+    // expected-note@os_smart_ptr.h:16{{Returning from 'smart_ptr::_retain'}}
   } // expected-note{{Calling '~smart_ptr'}}
-  // expected-note@os_smart_ptr.h:35{{Taking true branch}}
-  // expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}}
-  // expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}}
-  // expected-note@os_smart_ptr.h:36{{Returning from 'smart_ptr::_release'}}
+  // expected-note@os_smart_ptr.h:45{{Taking true branch}}
+  // expected-note@os_smart_ptr.h:46{{Calling 'smart_ptr::_release'}}
+  // expected-note@os_smart_ptr.h:86{{Reference count decremented. The object now has a +1 retain count}}
+  // expected-note@os_smart_ptr.h:46{{Returning from 'smart_ptr::_release'}}
  // expected-note@-5{{Returning from '~smart_ptr'}}
 } // expected-warning{{Potential leak of an object stored into 'obj'}}
 // expected-note@-1{{Object leaked: object allocated and stored into 'obj' is not referenced later in this execution path and has a retain count of +1}}
 
 void test_smart_ptr_no_leak() {
   OSObject *obj = new OSObject;
   {
     OSObjectPtr p(obj);

 
 void test_tagged_retain_no_uaf() {
   OSObject *obj = new OSObject;
   obj->taggedRetain();
   obj->release();
   obj->release();
 }
 
+struct B {
+  OSObjectPtr p;
+  B(OSObjectPtr p) : p(p) {}
+
+  B test_escape_smart_ptr_into_field() {
+    OSObject *obj = new OSObject;
+    return B(OSObjectPtr(obj, os::no_retain));
+  }
+};
+