From b715a7cef11664c1c47cfc3dcc503aadc58b6cac Mon Sep 17 00:00:00 2001 From: Ted Kremenek <kremenek@apple.com> Date: Sat, 12 Feb 2011 03:03:54 +0000 Subject: [PATCH] Weaken the ObjCSelfInitChecker to only warn when one calls an 'init' method within an 'init' method. This is a temporary stop gap to avoid false positives while we investigate how to make it smarter. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@125427 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/GRStateTrait.h | 13 +++++++++ .../Checkers/ObjCSelfInitChecker.cpp | 29 ++++++++++++++----- test/Analysis/self-init.m | 12 ++++---- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h b/include/clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h index 914625a2856..411441f8fe3 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h @@ -145,6 +145,19 @@ namespace ento { delete (typename data_type::Factory*) Ctx; } }; + + // Partial specialization for bool. + template <> struct GRStatePartialTrait<bool> { + typedef bool data_type; + + static inline data_type MakeData(void* const* p) { + return (bool) (uintptr_t) p; + } + static inline void *MakeVoidPtr(data_type d) { + return (void*) (uintptr_t) d; + } + }; + } // end GR namespace } // end clang namespace diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index f3292b4856e..9737683656b 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -109,6 +109,7 @@ public: } // end anonymous namespace typedef llvm::ImmutableMap<SymbolRef, unsigned> SelfFlag; +namespace { struct CalledInit {}; } namespace clang { namespace ento { @@ -119,6 +120,10 @@ namespace ento { return &index; } }; + template <> + struct GRStateTrait<CalledInit> : public GRStatePartialTrait<bool> { + static void *GDMIndex() { static int index = 0; return &index; } + }; } } @@ -133,8 +138,8 @@ static SelfFlagEnum getSelfFlags(SVal val, CheckerContext &C) { return getSelfFlags(val, C.getState()); } -static void addSelfFlag(SVal val, SelfFlagEnum flag, CheckerContext &C) { - const GRState *state = C.getState(); +static void addSelfFlag(const GRState *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)); @@ -161,9 +166,13 @@ static void checkForInvalidSelf(const Expr *E, CheckerContext &C, const char *errorStr) { if (!E) return; + + if (!C.getState()->get<CalledInit>()) + return; + if (!isInvalidSelf(E, C)) return; - + // Generate an error node. ExplodedNode *N = C.generateSink(); if (!N) @@ -188,8 +197,14 @@ void ObjCSelfInitChecker::postVisitObjCMessage(CheckerContext &C, if (isInitMessage(msg)) { // Tag the return value as the result of an initializer. const GRState *state = C.getState(); + + // FIXME this really should be context sensitive, where we record + // the current stack frame (for IPA). Also, we need to clean this + // value out when we return from this method. + state = state->set<CalledInit>(true); + SVal V = state->getSVal(msg.getOriginExpr()); - addSelfFlag(V, SelfFlag_InitRes, C); + addSelfFlag(state, V, SelfFlag_InitRes, C); return; } @@ -262,10 +277,10 @@ void ObjCSelfInitChecker::PostVisitGenericCall(CheckerContext &C, I = CE->arg_begin(), E = CE->arg_end(); I != E; ++I) { SVal argV = state->getSVal(*I); if (isSelfVar(argV, C)) { - addSelfFlag(state->getSVal(cast<Loc>(argV)), preCallSelfFlags, C); + addSelfFlag(state, state->getSVal(cast<Loc>(argV)), preCallSelfFlags, C); return; } else if (hasSelfFlag(argV, SelfFlag_Self, C)) { - addSelfFlag(state->getSVal(CE), preCallSelfFlags, C); + addSelfFlag(state, state->getSVal(CE), preCallSelfFlags, C); return; } } @@ -277,7 +292,7 @@ void ObjCSelfInitChecker::visitLocation(CheckerContext &C, const Stmt *S, // value is the object that 'self' points to. const GRState *state = C.getState(); if (isSelfVar(location, C)) - addSelfFlag(state->getSVal(cast<Loc>(location)), SelfFlag_Self, C); + addSelfFlag(state, state->getSVal(cast<Loc>(location)), SelfFlag_Self, C); } // FIXME: A callback should disable checkers at the start of functions. diff --git a/test/Analysis/self-init.m b/test/Analysis/self-init.m index b4c6f29e387..b7d41cd02a6 100644 --- a/test/Analysis/self-init.m +++ b/test/Analysis/self-init.m @@ -102,7 +102,7 @@ static id _commonInit(MyObj *self) { } -(id)init6 { - [NSBundle loadNibNamed:@"Window" owner:myivar]; // expected-warning {{Instance variable used}} + [NSBundle loadNibNamed:@"Window" owner:myivar]; // no-warning return [self initWithSomething:0]; } @@ -121,17 +121,17 @@ static id _commonInit(MyObj *self) { } -(id)init9 { - [self doSomething]; - return self; // expected-warning {{Returning 'self'}} + [self doSomething]; + return self; // no-warning } -(id)init10 { - myivar = 0; // expected-warning {{Instance variable used}} - return self; + myivar = 0; // no-warning + return self; } -(id)init11 { - return self; // expected-warning {{Returning 'self'}} + return self; // no-warning } -(id)init12 { -- GitLab