malhar1995 updated this revision to Diff 110514.
malhar1995 marked 7 inline comments as done.
malhar1995 added a comment.
Addressed comments.
P.S: I'm yet to test this callee-side parameter checking functionality on the
actual ISL codebase. I'll do that ASAP.
Repository:
rL LLVM
https://reviews.llvm.org/D36441
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
test/Analysis/retain-release-inline.m
Index: test/Analysis/retain-release-inline.m
===================================================================
--- test/Analysis/retain-release-inline.m
+++ 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,26 @@
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;
+}
+
//===----------------------------------------------------------------------===//
// Test returning retained and not-retained values.
//===----------------------------------------------------------------------===//
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -462,6 +462,7 @@
ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; }
friend class RetainSummaryManager;
+ friend class RetainCountChecker;
};
} // end anonymous namespace
@@ -1848,6 +1849,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 +2437,25 @@
return llvm::make_unique<PathDiagnosticEventPiece>(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<DeclRegion>(sym->getOriginRegion());
+ if (Region) {
+ const Decl *PDecl = Region->getDecl();
+ if (PDecl && isa<ParmVarDecl>(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 +2479,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 +2495,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 +2513,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<CFRefLeakReportVisitor>(sym, GCEnabled, Log));
}
@@ -2498,6 +2540,7 @@
: public Checker< check::Bind,
check::DeadSymbols,
check::EndAnalysis,
+ check::BeginFunction,
check::EndFunction,
check::PostStmt<BlockExpr>,
check::PostStmt<CastExpr>,
@@ -2682,6 +2725,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 +3947,36 @@
return N;
}
+void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const {
+ if (!Ctx.inTopFrame())
+ return;
+
+ const LocationContext *LCtx = Ctx.getLocationContext();
+ const FunctionDecl *FD = dyn_cast<FunctionDecl>(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 && Ty.getAsString().substr(0, 4) == "isl_")
+ state = setRefBinding(state, Sym, RefVal::makeOwned(RetEffect::ObjKind::Generalized, Ty));
+ else if (Ty.getAsString().substr(0, 4) == "isl_")
+ 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<RefBindings>();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
