From: Ted Kremenek Date: Wed, 9 Dec 2009 23:29:55 +0000 (+0000) Subject: Fix null dereference in OSAtomicChecker and special case SymbolicRegions. We still... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6bcd5a04db4eb9d51e7f92a4edc418737a5aeefd;p=clang Fix null dereference in OSAtomicChecker and special case SymbolicRegions. We still aren't handling them correctly; I've added to failing test cases to test/Analysis/NSString-failed-cases.m that should pass and then be merged in to test/Analysis/NSString.m. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@90993 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/OSAtomicChecker.cpp b/lib/Analysis/OSAtomicChecker.cpp index 03e9e38206..5a89345883 100644 --- a/lib/Analysis/OSAtomicChecker.cpp +++ b/lib/Analysis/OSAtomicChecker.cpp @@ -98,11 +98,20 @@ bool OSAtomicChecker::EvalOSAtomicCompareAndSwap(CheckerContext &C, ExplodedNodeSet Tmp; SVal location = state->getSVal(theValueExpr); // Here we should use the value type of the region as the load type. - const MemRegion *R = location.getAsRegion()->StripCasts(); QualType LoadTy; - if (R) { - LoadTy = cast(R)->getValueType(Ctx); - location = loc::MemRegionVal(R); + if (const MemRegion *R = location.getAsRegion()) { + // We must be careful, as SymbolicRegions aren't typed. + const MemRegion *strippedR = R->StripCasts(); + // FIXME: This isn't quite the right solution. One test case in 'test/Analysis/NSString.m' + // is giving the wrong result. + const TypedRegion *typedR = + isa(strippedR) ? cast(R) : + dyn_cast(strippedR); + + if (typedR) { + LoadTy = typedR->getValueType(Ctx); + location = loc::MemRegionVal(typedR); + } } Engine.EvalLoad(Tmp, const_cast(theValueExpr), C.getPredecessor(), state, location, OSAtomicLoadTag, LoadTy); diff --git a/test/Analysis/NSString-failed-cases.m b/test/Analysis/NSString-failed-cases.m new file mode 100644 index 0000000000..d44f17bf46 --- /dev/null +++ b/test/Analysis/NSString-failed-cases.m @@ -0,0 +1,115 @@ +// RUN: clang-cc -triple i386-apple-darwin10 -analyze -analyzer-experimental-internal-checks -checker-cfref -analyzer-store=region -analyzer-constraints=basic -verify %s +// RUN: clang-cc -triple i386-apple-darwin10 -analyze -analyzer-experimental-internal-checks -checker-cfref -analyzer-store=region -analyzer-constraints=range -verify %s +// RUN: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -analyzer-experimental-internal-checks -checker-cfref -analyzer-store=region -analyzer-constraints=basic -verify %s +// RUN: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -analyzer-experimental-internal-checks -checker-cfref -analyzer-store=region -analyzer-constraints=range -verify %s +// XFAIL: * + +//===----------------------------------------------------------------------===// +// The following code is reduced using delta-debugging from +// Foundation.h (Mac OS X). +// +// It includes the basic definitions for the test cases below. +// Not directly including Foundation.h directly makes this test case +// both svelte and portable to non-Mac platforms. +//===----------------------------------------------------------------------===// + +#ifdef TEST_64 +typedef long long int64_t; +_Bool OSAtomicCompareAndSwap64Barrier( int64_t __oldValue, int64_t __newValue, volatile int64_t *__theValue ); +#define COMPARE_SWAP_BARRIER OSAtomicCompareAndSwap64Barrier +typedef int64_t intptr_t; +#else +typedef int int32_t; +_Bool OSAtomicCompareAndSwap32Barrier( int32_t __oldValue, int32_t __newValue, volatile int32_t *__theValue ); +#define COMPARE_SWAP_BARRIER OSAtomicCompareAndSwap32Barrier +typedef int32_t intptr_t; +#endif + +typedef const void * CFTypeRef; +typedef const struct __CFString * CFStringRef; +typedef const struct __CFAllocator * CFAllocatorRef; +extern const CFAllocatorRef kCFAllocatorDefault; +extern CFTypeRef CFRetain(CFTypeRef cf); +void CFRelease(CFTypeRef cf); +typedef const struct __CFDictionary * CFDictionaryRef; +const void *CFDictionaryGetValue(CFDictionaryRef theDict, const void *key); +extern CFStringRef CFStringCreateWithFormat(CFAllocatorRef alloc, CFDictionaryRef formatOptions, CFStringRef format, ...); +typedef signed char BOOL; +typedef int NSInteger; +typedef unsigned int NSUInteger; +@class NSString, Protocol; +extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2))); +typedef NSInteger NSComparisonResult; +typedef struct _NSZone NSZone; +@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator; +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (oneway void)release; +- (id)retain; +- (id)autorelease; +@end +@protocol NSCopying +- (id)copyWithZone:(NSZone *)zone; +@end +@protocol NSMutableCopying +- (id)mutableCopyWithZone:(NSZone *)zone; +@end +@protocol NSCoding +- (void)encodeWithCoder:(NSCoder *)aCoder; +@end +@interface NSObject {} +- (id)init; ++ (id)alloc; +@end +extern id NSAllocateObject(Class aClass, NSUInteger extraBytes, NSZone *zone); +typedef struct {} NSFastEnumerationState; +@protocol NSFastEnumeration +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len; +@end +@class NSString; +typedef struct _NSRange {} NSRange; +@interface NSArray : NSObject +- (NSUInteger)count; +@end +@interface NSMutableArray : NSArray +- (void)addObject:(id)anObject; +- (id)initWithCapacity:(NSUInteger)numItems; +@end +typedef unsigned short unichar; +@class NSData, NSArray, NSDictionary, NSCharacterSet, NSData, NSURL, NSError, NSLocale; +typedef NSUInteger NSStringCompareOptions; +@interface NSString : NSObject - (NSUInteger)length; +- (NSComparisonResult)compare:(NSString *)string; +- (NSComparisonResult)compare:(NSString *)string options:(NSStringCompareOptions)mask; +- (NSComparisonResult)compare:(NSString *)string options:(NSStringCompareOptions)mask range:(NSRange)compareRange; +- (NSComparisonResult)compare:(NSString *)string options:(NSStringCompareOptions)mask range:(NSRange)compareRange locale:(id)locale; +- (NSComparisonResult)caseInsensitiveCompare:(NSString *)string; +- (NSArray *)componentsSeparatedByCharactersInSet:(NSCharacterSet *)separator; ++ (id)stringWithFormat:(NSString *)format, ... __attribute__((format(__NSString__, 1, 2))); +@end +@interface NSSimpleCString : NSString {} @end +@interface NSConstantString : NSSimpleCString @end +extern void *_NSConstantStringClassReference; + +//===----------------------------------------------------------------------===// +// Test cases. These should all be merged into NSString.m once these tests +// stop reporting leaks. +//===----------------------------------------------------------------------===// + +// FIXME: THIS TEST CASE INCORRECTLY REPORTS A LEAK. +void testOSCompareAndSwapXXBarrier_parameter(NSString **old) { + NSString *s = [[NSString alloc] init]; // no-warning + if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) old)) + [s release]; + else + [*old release]; +} + +// FIXME: THIS TEST CASE INCORRECTLY REPORTS A LEAK. +void testOSCompareAndSwapXXBarrier_parameter_no_direct_release(NSString **old) { + NSString *s = [[NSString alloc] init]; // no-warning + if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) old)) + return; + else + [*old release]; +} diff --git a/test/Analysis/NSString.m b/test/Analysis/NSString.m index 295cac4275..7ce9769a2c 100644 --- a/test/Analysis/NSString.m +++ b/test/Analysis/NSString.m @@ -291,7 +291,7 @@ void testOSCompareAndSwap() { [old release]; } -void testOSCompareAndSwapXXBarrier() { +void testOSCompareAndSwapXXBarrier_local() { NSString *old = 0; NSString *s = [[NSString alloc] init]; // no-warning if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) &old)) @@ -300,7 +300,7 @@ void testOSCompareAndSwapXXBarrier() { [old release]; } -void testOSCompareAndSwapXXBarrier_positive() { +void testOSCompareAndSwapXXBarrier_local_no_direct_release() { NSString *old = 0; NSString *s = [[NSString alloc] init]; // no-warning if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) &old)) @@ -315,7 +315,7 @@ int testOSCompareAndSwapXXBarrier_id(Class myclass, id xclass) { return 0; } -void test_objc_atomicCompareAndSwap() { +void test_objc_atomicCompareAndSwap_local() { NSString *old = 0; NSString *s = [[NSString alloc] init]; // no-warning if (!objc_atomicCompareAndSwapPtr(0, s, &old)) @@ -324,7 +324,7 @@ void test_objc_atomicCompareAndSwap() { [old release]; } -void test_objc_atomicCompareAndSwap_positive() { +void test_objc_atomicCompareAndSwap_local_no_direct_release() { NSString *old = 0; NSString *s = [[NSString alloc] init]; // no-warning if (!objc_atomicCompareAndSwapPtr(0, s, &old)) @@ -333,6 +333,22 @@ void test_objc_atomicCompareAndSwap_positive() { [old release]; } +void test_objc_atomicCompareAndSwap_parameter(NSString **old) { + NSString *s = [[NSString alloc] init]; // no-warning + if (!objc_atomicCompareAndSwapPtr(0, s, old)) + [s release]; + else + [*old release]; +} + +void test_objc_atomicCompareAndSwap_parameter_no_direct_release(NSString **old) { + NSString *s = [[NSString alloc] init]; // expected-warning{{leak}} + if (!objc_atomicCompareAndSwapPtr(0, s, old)) + return; + else + [*old release]; +} + // Test stringWithFormat () void test_stringWithFormat() {