Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -462,6 +462,7 @@ ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; } friend class RetainSummaryManager; + friend class RetainCountChecker; }; } // end anonymous namespace @@ -1319,6 +1320,13 @@ 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 @@ 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 @@ 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 @@ // 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 @@ // 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 @@ 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 @@ : public Checker< check::Bind, check::DeadSymbols, check::EndAnalysis, + check::BeginFunction, check::EndFunction, check::PostStmt, check::PostStmt, @@ -2682,6 +2732,7 @@ 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 @@ 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(); Index: cfe/trunk/test/Analysis/retain-release-inline.m =================================================================== --- cfe/trunk/test/Analysis/retain-release-inline.m +++ cfe/trunk/test/Analysis/retain-release-inline.m @@ -302,6 +302,9 @@ __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 @@ 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. //===----------------------------------------------------------------------===//