[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
diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
index 2fb0229..dc902b9 100644
--- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
@@ -140,7 +140,8 @@
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 @@
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 abe18bf..4f1a76e 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -192,8 +192,10 @@
}
// 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 @@
}
}
- // 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 40171a3..0000000
--- 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
