[cfe-commits] r163361 - in /cfe/trunk: lib/StaticAnalyzer/Core/ExprEngineObjC.cpp test/Analysis/retain-release-crashes.m
Anna Zaks
ganna at apple.com
Thu Sep 6 16:58:37 PDT 2012
On Sep 6, 2012, at 4:44 PM, Jordan Rose <jordan_rose at apple.com> 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);
> +
> + at interface NSObject
> +- (id)init;
> + at end
> +
> + at interface Foo : NSObject
> + at end
> +
> +void touch(id, SEL);
> +id getObject();
> +int getInt();
> +
> +
> + at 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;
> +}
> + at end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list