diff options
author | Jordan Rose <jordan_rose@apple.com> | 2012-09-08 01:47:28 +0000 |
---|---|---|
committer | Jordan Rose <jordan_rose@apple.com> | 2012-09-08 01:47:28 +0000 |
commit | 82f2ad456a82da1b9cb7ddfc994c8f5fa44b59e6 (patch) | |
tree | 3b17266037a3fc4a8d92b72ec51cd910a6405311 | |
parent | a435e6989de2c668c5c512dda48e6f8756e0ba2c (diff) | |
download | clang-82f2ad456a82da1b9cb7ddfc994c8f5fa44b59e6.tar.gz |
[analyzer] ObjCSelfInitChecker should always clean up in postCall checks.
ObjCSelfInitChecker stashes information in the GDM to persist it across
function calls; it is stored in pre-call checks and retrieved post-call.
The post-call check is supposed to clear out the stored state, but was
failing to do so in cases where the call did not have a symbolic return
value.
This was actually causing the inappropriate cache-out from r163361.
Per discussion with Anna, we should never actually cache out when
assuming the receiver of an Objective-C message is non-nil, because
we guarded that node generation by checking that the state has changed.
Therefore, the only states that could reach this exact ExplodedNode are
ones that should have merged /before/ making this assumption.
r163361 has been reverted and the test case removed, since it won't
actually test anything interesting now.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163449 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp | 5 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/ExprEngineObjC.cpp | 8 | ||||
-rw-r--r-- | test/Analysis/retain-release-crashes.m | 62 |
3 files changed, 7 insertions, 68 deletions
diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index 2fb022928e..dc902b944f 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -140,7 +140,8 @@ static void addSelfFlag(ProgramStateRef state, SVal val, SelfFlagEnum flag, CheckerContext &C) { // We tag the symbol that the SVal wraps. if (SymbolRef sym = val.getAsSymbol()) - C.addTransition(state->set<SelfFlag>(sym, getSelfFlags(val, C) | flag)); + state = state->set<SelfFlag>(sym, getSelfFlags(val, state) | flag); + C.addTransition(state); } static bool hasSelfFlag(SVal val, SelfFlagEnum flag, CheckerContext &C) { @@ -310,7 +311,7 @@ void ObjCSelfInitChecker::checkPostCall(const CallEvent &CE, const Expr *CallExpr = CE.getOriginExpr(); if (CallExpr) addSelfFlag(state, state->getSVal(CallExpr, C.getLocationContext()), - prevFlags, C); + prevFlags, C); return; } } diff --git a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index abe18bf835..4f1a76e89e 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -192,8 +192,10 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, } // Generate a transition to non-Nil state. - if (notNilState != State) + if (notNilState != State) { Pred = Bldr.generateNode(currStmt, Pred, notNilState); + assert(Pred && "Should have cached out already!"); + } } } else { // Check for special class methods. @@ -245,9 +247,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, } } - // Evaluate the call if we haven't cached out. - if (Pred) - defaultEvalCall(Bldr, Pred, *UpdatedMsg); + defaultEvalCall(Bldr, Pred, *UpdatedMsg); } ExplodedNodeSet dstPostvisit; diff --git a/test/Analysis/retain-release-crashes.m b/test/Analysis/retain-release-crashes.m deleted file mode 100644 index 40171a39c9..0000000000 --- a/test/Analysis/retain-release-crashes.m +++ /dev/null @@ -1,62 +0,0 @@ -// 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 |