[analyzer] [RetainCountChecker] Smart pointer support.

[george.karpenkov]

Summary

Do not escape values stored into known smart pointer fields

[NoQ]

Maybe put these new headers into Inputs where other system header simulators already belong?

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

533–535 ↗

(On Diff #182167)

I think it should be safe to unconditionally cast<> here. I doubt you'll ever try to apply this function to a memory space, even accidentally.

So i guess you can actually collapse all these casts into a single line of code.

1181 ↗

(On Diff #182167)

shouldEscapeRegion(MR) currently implies !isSmartPtrField(MR).

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

209 ↗

(On Diff #182167)

I suspect that this may crash when RD is an anonymous class. getNameAsString() should be safer.

[george.karpenkov]

Addressed comments. Not sure about Inputs: plenty of headers are in the root directory.

[NoQ]

Ok!

cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

enum class BehaviorSummary {
 
     // Function either returns zero, or the input parameter.
     IdentityOrZero
   };
 
   Optional<BehaviorSummary> canEval(const CallExpr *CE, const FunctionDecl *FD,
                                     bool &hasTrustedImplementationAnnotation);
 
+  /// \return Whether the type corresponds to a known smart pointer
+  /// implementation (that is, everything about it is inlineable).
+  static bool isKnownSmartPointer(QualType QT);
+
   bool isTrustedReferenceCountImplementation(const FunctionDecl *FD);
 
   const RetainSummary *getSummary(const CallEvent &Call,
                                   QualType ReceiverType=QualType());
 
   const RetainSummary *getFunctionSummary(const FunctionDecl *FD);
 
   const RetainSummary *getMethodSummary(Selector S, const ObjCInterfaceDecl *ID,

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ,
   if (SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol()) {
     if (RE.getKind() == RetEffect::NoRetHard)
       state = removeRefBinding(state, Sym);
   }
 
   C.addTransition(state);
 }
 
+static bool isSmartPtrField(const MemRegion *MR) {
+  const auto *TR = dyn_cast<TypedValueRegion>(
+    cast<SubRegion>(MR)->getSuperRegion());
+  return TR && RetainSummaryManager::isKnownSmartPointer(TR->getValueType());
+}
+
+
 /// A value escapes in these possible cases:
 ///
 /// - binding to something that is not a memory region.
 /// - 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, so be conservative here and let the symbol go.
+/// 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 (isSmartPtrField(R))
+    return false;
+
   const auto *VR = dyn_cast<VarRegion>(R);
+
   if (!R->hasStackStorage() || !VR)
     return true;
 
   const VarDecl *VD = VR->getDecl();
   if (!VD->hasAttr<CleanupAttr>())
     return false; // CleanupAttr attaches destructors, which cause escaping.
   return true;
 }

cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

 static bool isSubclass(const Decl *D,
                        StringRef ClassName) {
   using namespace ast_matchers;
   DeclarationMatcher SubclassM = cxxRecordDecl(isSameOrDerivedFrom(ClassName));
   return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
 static bool isOSObjectSubclass(const Decl *D) {
-  return isSubclass(D, "OSObject");
+  return isSubclass(D, "OSMetaClassBase");
 }
 
 static bool isOSObjectDynamicCast(StringRef S) {
   return S == "safeMetaCast";
 }
 
 static bool isOSIteratorSubclass(const Decl *D) {
   return isSubclass(D, "OSIterator");
if (!PT.isNull())
       if (CXXRecordDecl *RD = PT->getAsCXXRecordDecl())
         if (isOSObjectSubclass(RD))
           return true;
   }
 
   return false;
 }
 
+bool
+RetainSummaryManager::isKnownSmartPointer(QualType QT) {
+  QT = QT.getCanonicalType();
+  const auto *RD = QT->getAsCXXRecordDecl();
+  if (!RD)
+    return false;
+  const IdentifierInfo *II = RD->getIdentifier();
+  if (II && II->getName() == "smart_ptr")
+    if (const auto *ND = dyn_cast<NamespaceDecl>(RD->getDeclContext()))
+      if (ND->getNameAsString() == "os")
+        return true;
+  return false;
+}
+
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
                                             StringRef FName, QualType RetTy) {
   if (RetTy->isPointerType()) {
     const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
     if (PD && isOSObjectSubclass(PD)) {
       if (const IdentifierInfo *II = FD->getIdentifier()) {
         if (isOSObjectDynamicCast(II->getName()))

cfe/trunk/test/Analysis/os_object_base.h

+#ifndef _OS_BASE_H
+#define _OS_BASE_H
+
+#define OS_CONSUME __attribute__((os_consumed))
+#define OS_RETURNS_RETAINED __attribute__((os_returns_retained))
+#define OS_RETURNS_RETAINED_ON_ZERO __attribute__((os_returns_retained_on_zero))
+#define OS_RETURNS_RETAINED_ON_NONZERO __attribute__((os_returns_retained_on_non_zero))
+#define OS_RETURNS_NOT_RETAINED __attribute__((os_returns_not_retained))
+#define OS_CONSUMES_THIS __attribute__((os_consumes_this))
+
+#define OSTypeID(type)   (type::metaClass)
+
+#define OSDynamicCast(type, inst)   \
+    ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
+
+#define OSTypeAlloc(type)   ((type *) ((type::metaClass)->alloc()))
+
+using size_t = decltype(sizeof(int));
+
+struct OSMetaClass;
+
+struct OSMetaClassBase {
+  static OSMetaClassBase *safeMetaCast(const OSMetaClassBase *inst,
+                                       const OSMetaClass *meta);
+
+  virtual void retain() const;
+  virtual void release() const;
+  virtual void free();
+  virtual ~OSMetaClassBase(){};
+};
+
+struct OSObject : public OSMetaClassBase {
+  virtual ~OSObject(){}
+
+  unsigned int foo() { return 42; }
+
+  virtual OS_RETURNS_NOT_RETAINED OSObject *identity();
+
+  static OSObject *generateObject(int);
+
+  static OSObject *getObject();
+  static OSObject *GetObject();
+
+  static void * operator new(size_t size);
+
+  static const OSMetaClass * const metaClass;
+};
+
+struct OSMetaClass : public OSMetaClassBase {
+  virtual OSObject * alloc();
+  virtual ~OSMetaClass(){}
+};
+
+#endif /* _OS_BASE_H */

cfe/trunk/test/Analysis/os_smart_ptr.h

+#ifndef _OS_SMART_POINTER_H
+#define _OS_SMART_POINTER_H
+
+#include "os_object_base.h"
+
+namespace os {
+
+template<class T>
+struct smart_ptr {
+  smart_ptr() : pointer(nullptr) {}
+
+  explicit smart_ptr(T *&p) : pointer(p) {
+    if (pointer) {
+      _retain(pointer);
+    }
+  }
+
+  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() {
+    if (pointer) {
+      _release(pointer);
+    }
+  }
+
+  void reset() {
+    smart_ptr().swap(*this);
+  }
+
+  T *get() const {
+    return pointer;
+  }
+
+  T ** get_for_out_param() {
+    reset();
+    return &pointer;
+  }
+
+  T * operator->() const {
+    OSPTR_LOG("Dereference smart_ptr with %p\n", pointer);
+    return pointer;
+  }
+
+  explicit
+  operator bool() const {
+    return pointer != nullptr;
+  }
+
+  inline void
+  swap(smart_ptr &p) {
+    T *temp = pointer;
+    pointer = p.pointer;
+    p.pointer = temp;
+  }
+
+  static inline void
+  _retain(T *obj) {
+    obj->retain();
+  }
+
+  static inline void
+  _release(T *obj) {
+    obj->release();
+  }
+
+  static inline T *
+  _alloc() {
+    return new T;
+  }
+
+  T *pointer;
+};
+}
+
+#endif /* _OS_SMART_POINTER_H */

cfe/trunk/test/Analysis/osobject-retain-release.cpp

 // RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-output=text\
 // RUN:                    -analyzer-checker=core,osx -verify %s
 
-struct OSMetaClass;
-
-#define OS_CONSUME __attribute__((os_consumed))
-#define OS_RETURNS_RETAINED __attribute__((os_returns_retained))
-#define OS_RETURNS_RETAINED_ON_ZERO __attribute__((os_returns_retained_on_zero))
-#define OS_RETURNS_RETAINED_ON_NONZERO __attribute__((os_returns_retained_on_non_zero))
-#define OS_RETURNS_NOT_RETAINED __attribute__((os_returns_not_retained))
-#define OS_CONSUMES_THIS __attribute__((os_consumes_this))
-
-#define OSTypeID(type)   (type::metaClass)
-
-#define OSDynamicCast(type, inst)   \
-    ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
-
-using size_t = decltype(sizeof(int));
-
-struct OSObject {
-  virtual void retain();
-  virtual void release() {};
-  virtual void free();
-  virtual ~OSObject(){}
-
-  unsigned int foo() { return 42; }
-
-  virtual OS_RETURNS_NOT_RETAINED OSObject *identity();
-
-  static OSObject *generateObject(int);
-
-  static OSObject *getObject();
-  static OSObject *GetObject();
-
-  static void * operator new(size_t size);
-
-  static const OSMetaClass * const metaClass;
-};
+#include "os_object_base.h"
+#include "os_smart_ptr.h"
 
 struct OSIterator : public OSObject {
-
   static const OSMetaClass * const metaClass;
 };
 
 struct OSArray : public OSObject {
   unsigned int getCount();
 
   OSIterator * getIterator();
 
struct MyArray : public OSArray {
   OSObject *generateObject(OSObject *input) override;
 };
 
 struct OtherStruct {
   static void doNothingToArray(OSArray *array);
   OtherStruct(OSArray *arr);
 };
 
-struct OSMetaClassBase {
-  static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
-};
 
 void escape(void *);
 void escape_with_source(void *p) {}
 bool coin();
 
 typedef int kern_return_t;
 typedef kern_return_t IOReturn;
 typedef kern_return_t OSReturn;

 }
 
 OSObject *getObject();
 typedef bool (^Blk)(OSObject *);
 
 void test_escape_to_unknown_block(Blk blk) {
   blk(getObject()); // no-crash
 }
+
+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:72{{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{{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:77{{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@-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:72{{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{{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:77{{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@-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);
+  }
+  obj->release();
+}

cfe/trunk/test/Analysis/test-separate-retaincount.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.cocoa.RetainCount -DNO_CF_OBJECT -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.OSObjectRetainCount -DNO_OS_OBJECT -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-config "osx.cocoa.RetainCount:CheckOSObject=false" -DNO_OS_OBJECT -verify %s
 
+#include "os_object_base.h"
+
 typedef const void * CFTypeRef;
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
 
 #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
 extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
 
 using size_t = decltype(sizeof(int));
 
-struct OSObject {
-  virtual void retain();
-  virtual void release();
-
-  static void * operator new(size_t size);
-  virtual ~OSObject(){}
-};
-
 void cf_overrelease() {
   CFTypeRef cf = CFCreate();
   CFRelease(cf);
   CFRelease(cf);
 #ifndef NO_CF_OBJECT
   // expected-warning@-2{{Reference-counted object is used after it is released}}
 #endif
 }