r180234 - [analyzer] Fix a crash in RetainCountChecker - we should not rely on CallEnter::getCallExpr to return non-NULL

[Jordan Rose]

Not my reduction; the originator's (James W. Walker). Glad it helped, though.


On Apr 24, 2013, at 17:41 , Anna Zaks <ganna at apple.com> wrote:

> Author: zaks
> Date: Wed Apr 24 19:41:32 2013
> New Revision: 180234
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=180234&view=rev
> Log:
> [analyzer] Fix a crash in RetainCountChecker - we should not rely on CallEnter::getCallExpr to return non-NULL
> 
> We get a CallEnter with a null expression, when processing a destructor. All other users of
> CallEnter::getCallExpr work fine with null as return value.
> 
> (Addresses PR15832, Thanks to Jordan for reducing the test case!)
> 
> Modified:
>    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
>    cfe/trunk/test/Analysis/retain-release.mm
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=180234&r1=180233&r2=180234&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Wed Apr 24 19:41:32 2013
> @@ -2192,7 +2192,7 @@ GetAllocationSite(ProgramStateManager& S
>     if (!InitMethodContext)
>       if (Optional<CallEnter> CEP = N->getLocation().getAs<CallEnter>()) {
>         const Stmt *CE = CEP->getCallExpr();
> -        if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(CE)) {
> +        if (const ObjCMessageExpr *ME = dyn_cast_or_null<ObjCMessageExpr>(CE)) {
>           const Stmt *RecExpr = ME->getInstanceReceiver();
>           if (RecExpr) {
>             SVal RecV = St->getSVal(RecExpr, NContext);
> 
> Modified: cfe/trunk/test/Analysis/retain-release.mm
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.mm?rev=180234&r1=180233&r2=180234&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/retain-release.mm (original)
> +++ cfe/trunk/test/Analysis/retain-release.mm Wed Apr 24 19:41:32 2013
> @@ -83,6 +83,7 @@ typedef UInt32 CFStringEncoding;
> enum {
> kCFStringEncodingMacRoman = 0,     kCFStringEncodingWindowsLatin1 = 0x0500,     kCFStringEncodingISOLatin1 = 0x0201,     kCFStringEncodingNextStepLatin = 0x0B01,     kCFStringEncodingASCII = 0x0600,     kCFStringEncodingUnicode = 0x0100,     kCFStringEncodingUTF8 = 0x08000100,     kCFStringEncodingNonLossyASCII = 0x0BFF      ,     kCFStringEncodingUTF16 = 0x0100,     kCFStringEncodingUTF16BE = 0x10000100,     kCFStringEncodingUTF16LE = 0x14000100,      kCFStringEncodingUTF32 = 0x0c000100,     kCFStringEncodingUTF32BE = 0x18000100,     kCFStringEncodingUTF32LE = 0x1c000100  };
> extern CFStringRef CFStringCreateWithCString(CFAllocatorRef alloc, const char *cStr, CFStringEncoding encoding);
> +extern CFStringRef CFStringCreateCopy(CFAllocatorRef alloc, CFStringRef theString);
> typedef double CFTimeInterval;
> typedef CFTimeInterval CFAbsoluteTime;
> extern CFAbsoluteTime CFAbsoluteTimeGetCurrent(void);
> @@ -269,7 +270,6 @@ extern void CGContextDrawLinearGradient(
>     CGGradientRef gradient, CGPoint startPoint, CGPoint endPoint,
>     CGGradientDrawingOptions options);
> extern CGColorSpaceRef CGColorSpaceCreateDeviceRGB(void);
> -
> //===----------------------------------------------------------------------===//
> // Test cases.
> //===----------------------------------------------------------------------===//
> @@ -408,3 +408,56 @@ void testCallback() {
> }
> @end
> 
> +//===----------------------------------------------------------------------===//
> +// Don't crash on getting a null expression from CallEnter corresponding to a
> +// destructor.
> +//===----------------------------------------------------------------------===//
> +
> +template <typename X>
> +class Holder {
> +public:
> +	Holder() throw();
> +	~Holder() throw() {}
> +	X* get() const throw();
> +	void reset(X* p) throw();
> +private:
> +	X* ptr_;
> +};
> +
> +template<typename X>
> +inline
> +Holder<X>::Holder() throw()
> +: ptr_(0){}
> +
> +template <typename X>
> +inline
> +X* Holder<X>::get() const throw() {
> +	return ptr_;
> +}
> +
> +template <typename X>
> +inline
> +void Holder<X>::reset(X* p) throw() {
> +	if (ptr_ != p) {
> +		if (ptr_ != 0) {
> +			::CFRelease( ptr_ );
> +		}
> +		ptr_ = p;
> +	}
> +}
> +
> +class radar13722286 {
> +public:
> +  radar13722286() {}
> +private:
> +	void			PrepareBitmap();
> +	Holder<const struct __CFString>	mStr;
> +};
> +
> +void	radar13722286::PrepareBitmap() {
> +	if (mStr.get() != 0) {
> +		Holder<const struct __CFString> str1;
> +		mStr.reset( CFStringCreateCopy( 0, str1.get() ) ); //expected-warning {{Potential leak of an object}}
> +	}
> +}
> +
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits