From 940b1d85d3f786c90e845429a60ffd0be736984e Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 10 Apr 2008 23:44:06 +0000 Subject: [PATCH] Fixed some logic errors in the CF ref count checker; we now can detect simple use-after-release errors. Added test case. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@49509 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/CFRefCount.cpp | 64 +++++++++++++++++++++++------------ lib/Analysis/GRExprEngine.cpp | 2 ++ test/Analysis-Apple/CFDate.c | 15 ++++++++ 3 files changed, 59 insertions(+), 22 deletions(-) create mode 100644 test/Analysis-Apple/CFDate.c diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp index d9de2cf2b5..575eb4c291 100644 --- a/lib/Analysis/CFRefCount.cpp +++ b/lib/Analysis/CFRefCount.cpp @@ -48,7 +48,8 @@ namespace { class RetEffect { public: - enum Kind { Alias = 0x0, OwnedSymbol = 0x1, NotOwnedSymbol = 0x2 }; + enum Kind { NoRet = 0x0, Alias = 0x1, OwnedSymbol = 0x2, + NotOwnedSymbol = 0x3 }; private: unsigned Data; @@ -69,6 +70,8 @@ public: static RetEffect MakeNotOwned() { return RetEffect(NotOwnedSymbol, 0); } + static RetEffect MakeNoRet() { return RetEffect(NoRet, 0); } + operator Kind() const { return getKind(); } void Profile(llvm::FoldingSetNodeID& ID) const { ID.AddInteger(Data); } @@ -133,6 +136,10 @@ class CFRefSummaryManager { CFRefSummary* getPersistentSummary(ArgEffects* AE, RetEffect RE); + CFRefSummary* getDoNothingSummary(unsigned Args); + void FillDoNothing(unsigned Args); + + public: CFRefSummaryManager(ASTContext& ctx) : Ctx(ctx) {} ~CFRefSummaryManager(); @@ -158,8 +165,6 @@ CFRefSummaryManager::~CFRefSummaryManager() { ArgEffects* CFRefSummaryManager::getArgEffects() { - assert (!ScratchArgs.empty()); - llvm::FoldingSetNodeID profile; profile.Add(ScratchArgs); void* InsertPos; @@ -314,20 +319,22 @@ CFRefSummary* CFRefSummaryManager::getCannedCFSummary(FunctionTypeProto* FT, // CFRetain: the return type should also be "CFTypeRef". if (RetTy.getTypePtr() != ArgT) return NULL; + + // The function's interface checks out. Generate a canned summary. + assert (ScratchArgs.empty()); + ScratchArgs.push_back(IncRef); + return getPersistentSummary(getArgEffects(), RetEffect::MakeAlias(0)); } else { // CFRelease: the return type should be void. if (RetTy != Ctx.VoidTy) return NULL; - } - // The function's interface checks out. Generate a canned summary. - - assert (ScratchArgs.empty()); - ScratchArgs.push_back(isRetain ? IncRef : DecRef); - - return getPersistentSummary(getArgEffects(), RetEffect::MakeAlias(0)); + assert (ScratchArgs.empty()); + ScratchArgs.push_back(DecRef); + return getPersistentSummary(getArgEffects(), RetEffect::MakeNoRet()); + } } static bool isCFRefType(QualType T) { @@ -354,21 +361,28 @@ static bool isCFRefType(QualType T) { return true; } +void CFRefSummaryManager::FillDoNothing(unsigned Args) { + for (unsigned i = 0; i != Args; ++i) + ScratchArgs.push_back(DoNothing); +} + +CFRefSummary* CFRefSummaryManager::getDoNothingSummary(unsigned Args) { + FillDoNothing(Args); + return getPersistentSummary(getArgEffects(), RetEffect::MakeNoRet()); +} CFRefSummary* CFRefSummaryManager::getCFSummaryCreateRule(FunctionTypeProto* FT) { if (!isCFRefType(FT->getResultType())) - return NULL; + return getDoNothingSummary(FT->getNumArgs()); assert (ScratchArgs.empty()); // FIXME: Add special-cases for functions that retain/release. For now // just handle the default case. - for (unsigned i = 0, n = FT->getNumArgs(); i != n; ++i) - ScratchArgs.push_back(DoNothing); - + FillDoNothing(FT->getNumArgs()); return getPersistentSummary(getArgEffects(), RetEffect::MakeOwned()); } @@ -376,16 +390,14 @@ CFRefSummary* CFRefSummaryManager::getCFSummaryGetRule(FunctionTypeProto* FT) { if (!isCFRefType(FT->getResultType())) - return NULL; + return getDoNothingSummary(FT->getNumArgs()); assert (ScratchArgs.empty()); // FIXME: Add special-cases for functions that retain/release. For now // just handle the default case. - for (unsigned i = 0, n = FT->getNumArgs(); i != n; ++i) - ScratchArgs.push_back(DoNothing); - + FillDoNothing(FT->getNumArgs()); return getPersistentSummary(getArgEffects(), RetEffect::MakeNotOwned()); } @@ -711,11 +723,11 @@ void CFRefCount::EvalCall(ExplodedNodeSet& Dst, if (hasError) { St = StateMgr.getPersistentState(StVals); - GRExprEngine::NodeTy* N = Builder.generateNode(CE, St, Pred); + + Builder.BuildSinks = true; + GRExprEngine::NodeTy* N = Builder.MakeNode(Dst, CE, Pred, St); if (N) { - N->markAsSink(); - switch (hasError) { default: assert(false); case RefVal::ErrorUseAfterRelease: @@ -741,6 +753,9 @@ void CFRefCount::EvalCall(ExplodedNodeSet& Dst, default: assert (false && "Unhandled RetEffect."); break; + case RetEffect::NoRet: + break; + case RetEffect::Alias: { unsigned idx = RE.getValue(); assert (idx < CE->getNumArgs()); @@ -810,7 +825,8 @@ CFRefCount::RefBindings CFRefCount::Update(RefBindings B, SymbolID sym, assert(false); case RefVal::Owned: - V = RefVal::makeOwned(V.getCount()+1); break; + V = RefVal::makeOwned(V.getCount()+1); + break; case RefVal::NotOwned: V = RefVal::makeNotOwned(V.getCount()+1); @@ -822,6 +838,8 @@ CFRefCount::RefBindings CFRefCount::Update(RefBindings B, SymbolID sym, break; } + break; + case DecRef: switch (V.getKind()) { default: @@ -851,6 +869,8 @@ CFRefCount::RefBindings CFRefCount::Update(RefBindings B, SymbolID sym, hasError = V.getKind(); break; } + + break; } return RefBFactory.Add(B, sym, V); diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 6d4217774d..3211da2918 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -678,6 +678,8 @@ void GRExprEngine::VisitCall(CallExpr* CE, NodeTy* Pred, unsigned size = Dst.size(); + SaveAndRestore OldSink(Builder->BuildSinks); + EvalCall(Dst, CE, cast(L), *DI); if (!Builder->BuildSinks && Dst.size() == size) diff --git a/test/Analysis-Apple/CFDate.c b/test/Analysis-Apple/CFDate.c new file mode 100644 index 0000000000..ea15994e4d --- /dev/null +++ b/test/Analysis-Apple/CFDate.c @@ -0,0 +1,15 @@ +// RUN: clang -checker-cfref -verify %s + +#include + +CFAbsoluteTime f1() { + CFAbsoluteTime t = CFAbsoluteTimeGetCurrent(); + CFDateRef date = CFDateCreate(NULL, t); + CFRetain(date); + CFRelease(date); + t = CFDateGetAbsoluteTime(date); + CFRelease(date); + t = CFDateGetAbsoluteTime(date); // expected-warning{{Reference-counted object is used after it is released.}} + return t; +} + -- 2.40.0