Index: clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -685,6 +685,10 @@ Optional 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, Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -529,6 +529,13 @@ C.addTransition(state); } +static bool isSmartPtrField(const MemRegion *MR) { + const auto *TR = dyn_cast( + cast(MR)->getSuperRegion()); + return TR && RetainSummaryManager::isKnownSmartPointer(TR->getValueType()); +} + + /// A value escapes in these possible cases: /// /// - binding to something that is not a memory region. @@ -536,10 +543,15 @@ /// - 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(R); + if (!R->hasStackStorage() || !VR) return true; Index: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -146,7 +146,7 @@ } static bool isOSObjectSubclass(const Decl *D) { - return isSubclass(D, "OSObject"); + return isSubclass(D, "OSMetaClassBase"); } static bool isOSObjectDynamicCast(StringRef S) { @@ -199,6 +199,20 @@ 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(RD->getDeclContext())) + if (ND->getNameAsString() == "os") + return true; + return false; +} + const RetainSummary * RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD, StringRef FName, QualType RetTy) { Index: clang/test/Analysis/os_object_base.h =================================================================== --- /dev/null +++ clang/test/Analysis/os_object_base.h @@ -0,0 +1,54 @@ +#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 */ Index: clang/test/Analysis/os_smart_ptr.h =================================================================== --- /dev/null +++ clang/test/Analysis/os_smart_ptr.h @@ -0,0 +1,89 @@ +#ifndef _OS_SMART_POINTER_H +#define _OS_SMART_POINTER_H + +#include "os_object_base.h" + +namespace os { + +template +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 */ Index: clang/test/Analysis/osobject-retain-release.cpp =================================================================== --- clang/test/Analysis/osobject-retain-release.cpp +++ clang/test/Analysis/osobject-retain-release.cpp @@ -1,44 +1,10 @@ // 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; }; @@ -88,9 +54,6 @@ OtherStruct(OSArray *arr); }; -struct OSMetaClassBase { - static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta); -}; void escape(void *); void escape_with_source(void *p) {} @@ -616,3 +579,51 @@ void test_escape_to_unknown_block(Blk blk) { blk(getObject()); // no-crash } + +using OSObjectPtr = os::smart_ptr; + +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'}} + // expected-note@-1{{Returning from constructor for 'smart_ptr'}} + // 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'}} + // expected-note@-1{{Returning from constructor for 'smart_ptr'}} + // 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(); +} Index: clang/test/Analysis/test-separate-retaincount.cpp =================================================================== --- clang/test/Analysis/test-separate-retaincount.cpp +++ clang/test/Analysis/test-separate-retaincount.cpp @@ -2,6 +2,8 @@ // 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); @@ -11,14 +13,6 @@ 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);