From: Artem Dergachev Date: Thu, 5 Oct 2017 08:43:32 +0000 (+0000) Subject: [analyzer] Fix leak false positives on stuff put in C++/ObjC initializer lists. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7728f3542e45915878ff7f98d7dd14bca4fe933e;p=clang [analyzer] Fix leak false positives on stuff put in C++/ObjC initializer lists. The analyzer now realizes that C++ std::initializer_list objects and Objective-C boxed structure/array/dictionary expressions can potentially maintain a reference to the objects that were put into them. This avoids false memory leak posivites and a few other issues. This is a conservative behavior; for now, we do not model what actually happens to the objects after being passed into such initializer lists. rdar://problem/32918288 Differential Revision: https://reviews.llvm.org/D35216 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314975 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index cd35ef095d..0126e03d60 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -827,6 +827,21 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE, } } +namespace { +class CollectReachableSymbolsCallback final : public SymbolVisitor { + InvalidatedSymbols Symbols; + +public: + explicit CollectReachableSymbolsCallback(ProgramStateRef State) {} + const InvalidatedSymbols &getSymbols() const { return Symbols; } + + bool VisitSymbol(SymbolRef Sym) override { + Symbols.insert(Sym); + return true; + } +}; +} // end anonymous namespace + void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet &DstTop) { PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), @@ -1103,8 +1118,29 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, SVal result = svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx, resultType, currBldrCtx->blockCount()); - ProgramStateRef state = N->getState()->BindExpr(Ex, LCtx, result); - Bldr2.generateNode(S, N, state); + ProgramStateRef State = N->getState()->BindExpr(Ex, LCtx, result); + + // Escape pointers passed into the list, unless it's an ObjC boxed + // expression which is not a boxable C structure. + if (!(isa(Ex) && + !cast(Ex)->getSubExpr() + ->getType()->isRecordType())) + for (auto Child : Ex->children()) { + assert(Child); + + SVal Val = State->getSVal(Child, LCtx); + + CollectReachableSymbolsCallback Scanner = + State->scanReachableSymbols( + Val); + const InvalidatedSymbols &EscapedSymbols = Scanner.getSymbols(); + + State = getCheckerManager().runCheckersForPointerEscape( + State, EscapedSymbols, + /*CallEvent*/ nullptr, PSK_EscapeOther, nullptr); + } + + Bldr2.generateNode(S, N, State); } getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); @@ -2237,21 +2273,6 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred, getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, *this); } -namespace { -class CollectReachableSymbolsCallback final : public SymbolVisitor { - InvalidatedSymbols Symbols; - -public: - CollectReachableSymbolsCallback(ProgramStateRef State) {} - const InvalidatedSymbols &getSymbols() const { return Symbols; } - - bool VisitSymbol(SymbolRef Sym) override { - Symbols.insert(Sym); - return true; - } -}; -} // end anonymous namespace - // A value escapes in three possible cases: // (1) We are binding to something that is not a memory region. // (2) We are binding to a MemrRegion that does not have stack storage. diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp index c73635686d..e9658a067c 100644 --- a/test/Analysis/initializer.cpp +++ b/test/Analysis/initializer.cpp @@ -1,7 +1,9 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s void clang_analyzer_eval(bool); +#include "Inputs/system-header-simulator-cxx.h" + class A { int x; public: @@ -204,3 +206,17 @@ struct C { const char(&f)[2]; }; } + +namespace CXX_initializer_lists { +struct C { + C(std::initializer_list list); +}; +void foo() { + C empty{}; // no-crash + + // Do not warn that 'x' leaks. It might have been deleted by + // the destructor of 'c'. + int *x = new int; + C c{x}; // no-warning +} +} diff --git a/test/Analysis/objc-boxing.m b/test/Analysis/objc-boxing.m index 963374b3ef..66f24ddf77 100644 --- a/test/Analysis/objc-boxing.m +++ b/test/Analysis/objc-boxing.m @@ -5,6 +5,16 @@ void clang_analyzer_eval(int); typedef signed char BOOL; typedef long NSInteger; typedef unsigned long NSUInteger; + +@protocol NSObject +@end +@interface NSObject {} +@end +@protocol NSCopying +@end +@protocol NSCoding +@end + @interface NSString @end @interface NSString (NSStringExtensionMethods) + (id)stringWithUTF8String:(const char *)nullTerminatedCString; @@ -28,7 +38,15 @@ typedef unsigned long NSUInteger; + (NSNumber *)numberWithUnsignedInteger:(NSUInteger)value ; @end +@interface NSValue : NSObject +- (void)getValue:(void *)value; ++ (NSValue *)valueWithBytes:(const void *)value + objCType:(const char *)type; +@end +typedef typeof(sizeof(int)) size_t; +extern void *malloc(size_t); +extern void free(void *); extern char *strdup(const char *str); id constant_string() { @@ -39,6 +57,23 @@ id dynamic_string() { return @(strdup("boxed dynamic string")); // expected-warning{{Potential memory leak}} } +typedef struct __attribute__((objc_boxable)) { + const char *str; +} BoxableStruct; + +id leak_within_boxed_struct() { + BoxableStruct bs; + bs.str = strdup("dynamic string"); // The duped string shall be owned by val. + NSValue *val = @(bs); // no-warning + return val; +} + +id leak_of_boxed_struct() { + BoxableStruct *bs = malloc(sizeof(BoxableStruct)); // The pointer stored in bs isn't owned by val. + NSValue *val = @(*bs); // expected-warning{{Potential leak of memory pointed to by 'bs'}} + return val; +} + id const_char_pointer(int *x) { if (x) return @(3); diff --git a/test/Analysis/objc-for.m b/test/Analysis/objc-for.m index 41709bee35..f1d0cf18d6 100644 --- a/test/Analysis/objc-for.m +++ b/test/Analysis/objc-for.m @@ -1,6 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.cocoa.Loops,debug.ExprInspection -verify %s void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(); #define nil ((id)0) @@ -20,11 +21,13 @@ typedef unsigned long NSUInteger; @interface NSArray : NSObject - (NSUInteger)count; - (NSEnumerator *)objectEnumerator; ++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count; @end @interface NSDictionary : NSObject - (NSUInteger)count; - (id)objectForKey:(id)key; ++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id /* */ [])keys count:(NSUInteger)count; @end @interface NSDictionary (SomeCategory) @@ -324,3 +327,19 @@ void protocolMethods(NSMutableArray *array) { for (id key in array) clang_analyzer_eval(0); // expected-warning{{FALSE}} } + +NSArray *globalArray; +NSDictionary *globalDictionary; +void boxedArrayEscape(NSMutableArray *array) { + if ([array count]) + return; + globalArray = @[array]; + for (id key in array) + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + + if ([array count]) + return; + globalDictionary = @{ @"array" : array }; + for (id key in array) + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} +}