From a179e171dcf6c7cc11a5c2dc4f607bc81d7bb7e2 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Thu, 17 Aug 2017 04:19:07 +0000 Subject: [PATCH] [analyzer] Add support for reference counting of parameters on the callee side This commit adds the functionality of performing reference counting on the callee side for Integer Set Library (ISL) to Clang Static Analyzer's RetainCountChecker. Reference counting on the callee side can be extensively used to perform debugging within a function (For example: Finding leaks on error paths). Patch by Malhar Thakkar! Differential Revision: https://reviews.llvm.org/D36441 llvm-svn: 311063 --- .../Checkers/RetainCountChecker.cpp | 99 +++++++++++++++++-- clang/test/Analysis/retain-release-inline.m | 34 +++++++ 2 files changed, 124 insertions(+), 9 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index b2cd34a1e38e..c348ecf4fd8b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -462,6 +462,7 @@ private: ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; } friend class RetainSummaryManager; + friend class RetainCountChecker; }; } // end anonymous namespace @@ -1319,6 +1320,13 @@ static bool isTrustedReferenceCountImplementation(const FunctionDecl *FD) { return hasRCAnnotation(FD, "rc_ownership_trusted_implementation"); } +static bool isGeneralizedObjectRef(QualType Ty) { + if (Ty.getAsString().substr(0, 4) == "isl_") + return true; + else + return false; +} + //===----------------------------------------------------------------------===// // Summary creation for Selectors. //===----------------------------------------------------------------------===// @@ -1848,6 +1856,15 @@ namespace { class CFRefLeakReport : public CFRefReport { const MemRegion* AllocBinding; + const Stmt *AllocStmt; + + // Finds the function declaration where a leak warning for the parameter 'sym' should be raised. + void deriveParamLocation(CheckerContext &Ctx, SymbolRef sym); + // Finds the location where a leak warning for 'sym' should be raised. + void deriveAllocLocation(CheckerContext &Ctx, SymbolRef sym); + // Produces description of a leak warning which is printed on the console. + void createDescription(CheckerContext &Ctx, bool GCEnabled, bool IncludeAllocationLine); + public: CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, bool GCEnabled, const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym, @@ -2427,13 +2444,25 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, return llvm::make_unique(L, os.str()); } -CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, - bool GCEnabled, const SummaryLogTy &Log, - ExplodedNode *n, SymbolRef sym, - CheckerContext &Ctx, - bool IncludeAllocationLine) - : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) { +void CFRefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) { + const SourceManager& SMgr = Ctx.getSourceManager(); + if (!sym->getOriginRegion()) + return; + + auto *Region = dyn_cast(sym->getOriginRegion()); + if (Region) { + const Decl *PDecl = Region->getDecl(); + if (PDecl && isa(PDecl)) { + PathDiagnosticLocation ParamLocation = PathDiagnosticLocation::create(PDecl, SMgr); + Location = ParamLocation; + UniqueingLocation = ParamLocation; + UniqueingDecl = Ctx.getLocationContext()->getDecl(); + } + } +} + +void CFRefLeakReport::deriveAllocLocation(CheckerContext &Ctx,SymbolRef sym) { // Most bug reports are cached at the location where they occurred. // With leaks, we want to unique them by the location where they were // allocated, and only report a single path. To do this, we need to find @@ -2457,8 +2486,12 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, // FIXME: This will crash the analyzer if an allocation comes from an // implicit call (ex: a destructor call). // (Currently there are no such allocations in Cocoa, though.) - const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode); - assert(AllocStmt && "Cannot find allocation statement"); + AllocStmt = PathDiagnosticLocation::getStmt(AllocNode); + + if (!AllocStmt) { + AllocBinding = nullptr; + return; + } PathDiagnosticLocation AllocLocation = PathDiagnosticLocation::createBegin(AllocStmt, SMgr, @@ -2469,8 +2502,10 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, // leaks should be uniqued on the allocation site. UniqueingLocation = AllocLocation; UniqueingDecl = AllocNode->getLocationContext()->getDecl(); +} - // Fill in the description of the bug. +void CFRefLeakReport::createDescription(CheckerContext &Ctx, bool GCEnabled, bool IncludeAllocationLine) { + assert(Location.isValid() && UniqueingDecl && UniqueingLocation.isValid()); Description.clear(); llvm::raw_string_ostream os(Description); os << "Potential leak "; @@ -2485,6 +2520,20 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, os << " (allocated on line " << SL.getSpellingLineNumber() << ")"; } } +} + +CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, + bool GCEnabled, const SummaryLogTy &Log, + ExplodedNode *n, SymbolRef sym, + CheckerContext &Ctx, + bool IncludeAllocationLine) + : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) { + + deriveAllocLocation(Ctx, sym); + if (!AllocBinding) + deriveParamLocation(Ctx, sym); + + createDescription(Ctx, GCEnabled, IncludeAllocationLine); addVisitor(llvm::make_unique(sym, GCEnabled, Log)); } @@ -2498,6 +2547,7 @@ class RetainCountChecker : public Checker< check::Bind, check::DeadSymbols, check::EndAnalysis, + check::BeginFunction, check::EndFunction, check::PostStmt, check::PostStmt, @@ -2682,6 +2732,7 @@ public: SymbolRef Sym, ProgramStateRef state) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + void checkBeginFunction(CheckerContext &C) const; void checkEndFunction(CheckerContext &C) const; ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym, @@ -3903,6 +3954,36 @@ RetainCountChecker::processLeaks(ProgramStateRef state, return N; } +void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const { + if (!Ctx.inTopFrame()) + return; + + const LocationContext *LCtx = Ctx.getLocationContext(); + const FunctionDecl *FD = dyn_cast(LCtx->getDecl()); + + if (!FD || isTrustedReferenceCountImplementation(FD)) + return; + + ProgramStateRef state = Ctx.getState(); + + const RetainSummary *FunctionSummary = getSummaryManager(Ctx).getFunctionSummary(FD); + ArgEffects CalleeSideArgEffects = FunctionSummary->getArgEffects(); + + for (unsigned idx = 0, e = FD->getNumParams(); idx != e; ++idx) { + const ParmVarDecl *Param = FD->getParamDecl(idx); + SymbolRef Sym = state->getSVal(state->getRegion(Param, LCtx)).getAsSymbol(); + + QualType Ty = Param->getType(); + const ArgEffect *AE = CalleeSideArgEffects.lookup(idx); + if (AE && *AE == DecRef && isGeneralizedObjectRef(Ty)) + state = setRefBinding(state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty)); + else if (isGeneralizedObjectRef(Ty)) + state = setRefBinding(state, Sym, RefVal::makeNotOwned(RetEffect::ObjKind::Generalized, Ty)); + } + + Ctx.addTransition(state); +} + void RetainCountChecker::checkEndFunction(CheckerContext &Ctx) const { ProgramStateRef state = Ctx.getState(); RefBindingsTy B = state->get(); diff --git a/clang/test/Analysis/retain-release-inline.m b/clang/test/Analysis/retain-release-inline.m index 15332a0144b0..4fe6bca4a44a 100644 --- a/clang/test/Analysis/retain-release-inline.m +++ b/clang/test/Analysis/retain-release-inline.m @@ -302,6 +302,9 @@ void test_neg() { __attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *isl_basic_map_cow(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap); void free(void *); +void callee_side_parameter_checking_leak(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap) { // expected-warning {{Potential leak of an object}} +} + // As 'isl_basic_map_free' is annotated with 'rc_ownership_trusted_implementation', RetainCountChecker trusts its // implementation and doesn't analyze its body. If the annotation 'rc_ownership_trusted_implementation' is removed, // a leak warning is raised by RetainCountChecker as the analyzer is unable to detect a decrement in the reference @@ -348,6 +351,37 @@ void test_leak_with_trusted_implementation_annotate_attribute(__attribute__((ann isl_basic_map_free(bmap); } +void callee_side_parameter_checking_incorrect_rc_decrement(isl_basic_map *bmap) { + isl_basic_map_free(bmap); // expected-warning {{Incorrect decrement of the reference count}} +} + +__attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *callee_side_parameter_checking_return_notowned_object(isl_basic_map *bmap) { + return bmap; // expected-warning {{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}} +} + +__attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *callee_side_parameter_checking_assign_consumed_parameter_leak_return(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap1, __attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap2) { // expected-warning {{Potential leak of an object}} + bmap1 = bmap2; + isl_basic_map_free(bmap2); + return bmap1; +} + +__attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *callee_side_parameter_checking_assign_consumed_parameter_leak(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap1, __attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap2) { // expected-warning {{Potential leak of an object}} + bmap1 = bmap2; + isl_basic_map_free(bmap1); + return bmap2; +} + +__attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *error_path_leak(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap1, __attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap2) { // expected-warning {{Potential leak of an object}} + bmap1 = isl_basic_map_cow(bmap1); + if (!bmap1 || !bmap2) + goto error; + + isl_basic_map_free(bmap2); + return bmap1; +error: + return isl_basic_map_free(bmap1); +} + //===----------------------------------------------------------------------===// // Test returning retained and not-retained values. //===----------------------------------------------------------------------===//