On Sep 6, 2012, at 4:44 PM, Jordan Rose <[email protected]> wrote:
> Author: jrose > Date: Thu Sep 6 18:44:36 2012 > New Revision: 163361 > > URL: http://llvm.org/viewvc/llvm-project?rev=163361&view=rev > Log: > [analyzer] Don't crash if we cache out while evaluating an ObjC message. > > A bizarre series of coincidences led us to generate a previously-seen > node in the middle of processing an Objective-C message, where we assume > the receiver is non-nil. Should we cash out? / Why are we cashing out? > We were assuming that such an assumption would > never "cache out" like this, and blithely went on using a null ExplodedNode > as the predecessor for the next step in evaluation. > > Although the test case committed here is complicated, this could in theory > happen in other ways as well, so the correct fix is just to test if the > non-nil assumption results in an ExplodedNode we've seen before. > > <rdar://problem/12243648> > > Added: > cfe/trunk/test/Analysis/retain-release-crashes.m > Modified: > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp?rev=163361&r1=163360&r2=163361&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp Thu Sep 6 18:44:36 > 2012 > @@ -245,8 +245,9 @@ > } > } > > - // Evaluate the call. > - defaultEvalCall(Bldr, Pred, *UpdatedMsg); > + // Evaluate the call if we haven't cached out. > + if (Pred) > + defaultEvalCall(Bldr, Pred, *UpdatedMsg); > } > > ExplodedNodeSet dstPostvisit; > > Added: cfe/trunk/test/Analysis/retain-release-crashes.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-crashes.m?rev=163361&view=auto > ============================================================================== > --- cfe/trunk/test/Analysis/retain-release-crashes.m (added) > +++ cfe/trunk/test/Analysis/retain-release-crashes.m Thu Sep 6 18:44:36 2012 > @@ -0,0 +1,62 @@ > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze > -analyzer-checker=core,osx.cocoa.RetainCount,osx.cocoa.SelfInit,debug.ExprInspection > -verify -w %s > + > +// This file contains crash regression tests; please do not remove any > checkers > +// from the RUN line because they may have been necessary to produce the > crash. > +// (Adding checkers should be fine.) > + > +void clang_analyzer_eval(int); > + > +@interface NSObject > +- (id)init; > +@end > + > +@interface Foo : NSObject > +@end > + > +void touch(id, SEL); > +id getObject(); > +int getInt(); > + > + > +@implementation Foo > +// Bizarre crash related to the ExprEngine reaching a previously-seen > +// ExplodedNode /during/ the processing of a message. Removing any > +// parts of this test case seem not to trigger the crash any longer. > +// <rdar://problem/12243648> > +- (id)init { > + // Step 0: properly call the superclass's initializer > + self = [super init]; > + if (!self) return self; > + > + // Step 1: Perturb the state with a new conjured symbol. > + int value = getInt(); > + > + // Step 2: Loop. Some loops seem to trigger this, some don't. > + // The original used a for-in loop. > + while (--value) { > + // Step 3: Make it impossible to retain-count 'self' by calling > + // a function that takes a "callback" (in this case, a selector). > + // Note that this does not trigger the crash if you use a message! > + touch(self, @selector(hi)); > + } > + > + // Step 4: Use 'self', so that we know it's non-nil. > + [self bar]; > + > + // Step 5: Once again, make it impossible to retain-count 'self'... > + // ...while letting ObjCSelfInitChecker mark this as an interesting > + // message, since 'self' is an argument... > + // ...but this time do it in such a way that we'll also assume that > + // 'other' is non-nil. Once we've made the latter assumption, we > + // should cache out. > + id other = getObject(); > + [other use:self withSelector:@selector(hi)]; > + > + // Step 6: Check that we did, in fact, keep the assumptions about 'self' > + // and 'other' being non-nil. > + clang_analyzer_eval(other != 0); // expected-warning{{TRUE}} > + clang_analyzer_eval(self != 0); // expected-warning{{TRUE}} > + > + return self; > +} > +@end > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
