[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
dcoughlin added a comment. Thanks Malhar! Committed in r311063. Repository: rL LLVM https://reviews.llvm.org/D36441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
This revision was automatically updated to reflect the committed changes.
Closed by commit rL311063: [analyzer] Add support for reference counting of
parameters on the callee side (authored by dcoughlin).
Changed prior to commit:
https://reviews.llvm.org/D36441?vs=111420=111463#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36441
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release-inline.m
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 , SymbolRef sym);
+// Finds the location where a leak warning for 'sym' should be raised.
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
+// Produces description of a leak warning which is printed on the console.
+void createDescription(CheckerContext , bool GCEnabled, bool IncludeAllocationLine);
+
public:
CFRefLeakReport(CFRefBug , const LangOptions , bool GCEnabled,
const SummaryLogTy , ExplodedNode *n, SymbolRef sym,
@@ -2427,13 +2444,25 @@
return llvm::make_unique(L, os.str());
}
-CFRefLeakReport::CFRefLeakReport(CFRefBug , const LangOptions ,
- bool GCEnabled, const SummaryLogTy ,
- ExplodedNode *n, SymbolRef sym,
- CheckerContext ,
- bool IncludeAllocationLine)
- : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) {
+void CFRefLeakReport::deriveParamLocation(CheckerContext , 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 ,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 , 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 , const LangOptions ,
+ bool GCEnabled, const SummaryLogTy ,
+ ExplodedNode *n, SymbolRef sym,
+ CheckerContext ,
+ bool IncludeAllocationLine)
+ : CFRefReport(D, LOpts,
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
malhar1995 updated this revision to Diff 111420.
malhar1995 added a comment.
This patch 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).
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,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.
//===--===//
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
@@ -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 , SymbolRef sym);
+// Finds the location where a leak warning for 'sym' should be raised.
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
+// Produces description of a leak warning which is printed on the console.
+void createDescription(CheckerContext , bool GCEnabled, bool
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
dcoughlin added a comment. Thanks! This looks good to me, with the note about factoring out the duplicated logic addressed, Would you factor out that logic into a static function and update phabricator summary to be a commit message. Then I will commit! Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:3971 +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)); Can you factor out the logic that looks whether the type name starts with "isl_" to a static function that types a type and is called something 'isGeneralizedObjectRef"? This will avoid duplicating the logic twice here and will also give us a single place to update when we add an attribute indicating that a type is reference counted. Repository: rL LLVM https://reviews.llvm.org/D36441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
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 , SymbolRef sym);
+// Finds the location where a leak warning for 'sym' should be raised.
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
+// Produces description of a leak warning which is printed on the console.
+void createDescription(CheckerContext , bool GCEnabled, bool IncludeAllocationLine);
+
public:
CFRefLeakReport(CFRefBug , const LangOptions , bool GCEnabled,
const SummaryLogTy , ExplodedNode *n, SymbolRef sym,
@@ -2427,13 +2437,25 @@
return llvm::make_unique(L, os.str());
}
-CFRefLeakReport::CFRefLeakReport(CFRefBug , const LangOptions ,
- bool GCEnabled, const SummaryLogTy ,
- ExplodedNode *n, SymbolRef sym,
- CheckerContext ,
- bool IncludeAllocationLine)
- : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) {
+void CFRefLeakReport::deriveParamLocation(CheckerContext , 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);
+
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
dcoughlin added a comment.
Nice work! This looks good, with some nits and testing comments inline. Have
you run this on real code? What kind of false positives do you see?
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:1853
+
+void deriveParamLocation(CheckerContext , SymbolRef sym);
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
Let's dispense with tradition and add some doxygen comments describing what
these methods do.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2442
+ const DeclRegion *Region = dyn_cast(sym->getOriginRegion());
+ if (Region) {
LLVM style says to use `auto *` here rather than repeating the name of the type
twice.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2479
+ AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+ // assert(AllocStmt && "Cannot find allocation statement");
+
If you don't want this assertion, you should remove it rather than comment it
out.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2524
+deriveParamLocation(Ctx, sym);
+
+ createDescription(Ctx, GCEnabled, IncludeAllocationLine);
I'm worried that in the future it would be possible to reach createDescription
without the Location, UniqueingLocation, and UniqueingDecl being filled in. Can
you please add an assertion for this in createDescription().
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2683
+ DefaultBool PerformCalleeSideParameterChecking;
+
malhar1995 wrote:
> This might be used in the future in case callee side parameter checking is
> added for Core Foundation and Objective-C objects.
Let's not add this if it is not used in the code.. You should either use this
to control the checking for your generalized case or remove it.
Comment at: test/Analysis/retain-release-inline.m:305
+void
callee_side_parameter_checking_leak(__attribute__((annotate("rc_ownership_consumed")))
isl_basic_map *bmap) { // expected-warning {{Potential leak}}
+}
Please include the entire diagnostic text in the expected-warning{{}}. This
will guarantee that future changes to the checker don't change it unexpectedly.
Comment at: test/Analysis/retain-release-inline.m:306
+void
callee_side_parameter_checking_leak(__attribute__((annotate("rc_ownership_consumed")))
isl_basic_map *bmap) { // expected-warning {{Potential leak}}
+}
+
I'd like to see some more tests. For example, you should get a warning if you
set the parameter to something else; also if you return it.
Do you have a test checking for that you don't warn when a consumed parameter
is correctly released?
Repository:
rL LLVM
https://reviews.llvm.org/D36441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
dcoughlin added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:3960-3971
+ 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();
+if (hasRCAnnotation(Param, "rc_ownership_consumed"))
+ state = setRefBinding(state, Sym,
malhar1995 wrote:
> Getting function summary and checking for `ArgEffects` doesn't seem like an
> option to me as currently there is no way to differentiate between Core
> Foundation and Generalized objects just by looking at their ArgEffects.
>
> However, this loop results in `check-clang-analysis` failure in
> `retain-release.m` on lines `1140` and `2237`.
You can differentiate based on the Type of the parameter and use ArgEffects to
determine what the initial ref binding should be. I'd like there to be a single
point of truth about what it means for a parameter to annotated.
Repository:
rL LLVM
https://reviews.llvm.org/D36441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
malhar1995 added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2521-2523
+ deriveAllocLocation(Ctx, sym);
+ if (!AllocBinding)
+deriveParamLocation(Ctx, sym);
I'm not sure what difference it will make if I change the ordering of the
invocations of `deriveAllocLocation` and 'deriveParamLocation`.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:2683
+ DefaultBool PerformCalleeSideParameterChecking;
+
This might be used in the future in case callee side parameter checking is
added for Core Foundation and Objective-C objects.
Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:3960-3971
+ 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();
+if (hasRCAnnotation(Param, "rc_ownership_consumed"))
+ state = setRefBinding(state, Sym,
Getting function summary and checking for `ArgEffects` doesn't seem like an
option to me as currently there is no way to differentiate between Core
Foundation and Generalized objects just by looking at their ArgEffects.
However, this loop results in `check-clang-analysis` failure in
`retain-release.m` on lines `1140` and `2237`.
Repository:
rL LLVM
https://reviews.llvm.org/D36441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36441: Add Support for Reference Counting of Parameters on the Callee Side in RetainCountChecker
malhar1995 created this revision.
Herald added a subscriber: eraman.
Current RetainCountChecker performs reference counting of function
arguments/parameters only on the caller side and not on the callee side.
This patch aims to add support for reference counting on the callee-side for
objects of 'Generalized' ObjKind.
This patch is still a work in progress.
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}}
+}
+
// 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,10 @@
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}}
+}
+
//===--===//
// Test returning retained and not-retained values.
//===--===//
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1848,6 +1848,12 @@
class CFRefLeakReport : public CFRefReport {
const MemRegion* AllocBinding;
+const Stmt *AllocStmt;
+
+void deriveParamLocation(CheckerContext , SymbolRef sym);
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
+void createDescription(CheckerContext , bool GCEnabled, bool IncludeAllocationLine);
+
public:
CFRefLeakReport(CFRefBug , const LangOptions , bool GCEnabled,
const SummaryLogTy , ExplodedNode *n, SymbolRef sym,
@@ -2427,13 +2433,25 @@
return llvm::make_unique(L, os.str());
}
-CFRefLeakReport::CFRefLeakReport(CFRefBug , const LangOptions ,
- bool GCEnabled, const SummaryLogTy ,
- ExplodedNode *n, SymbolRef sym,
- CheckerContext ,
- bool IncludeAllocationLine)
- : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) {
+void CFRefLeakReport::deriveParamLocation(CheckerContext , SymbolRef sym) {
+ const SourceManager& SMgr = Ctx.getSourceManager();
+
+ if (!sym->getOriginRegion())
+return;
+ const DeclRegion *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 ,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 +2475,13 @@
// 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);
+ // assert(AllocStmt && "Cannot find allocation statement");
+
+ if (!AllocStmt) {
+AllocBinding = nullptr;
+return;
+ }
PathDiagnosticLocation AllocLocation =
PathDiagnosticLocation::createBegin(AllocStmt, SMgr,
@@ -2469,8 +2492,9 @@
// 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 , bool GCEnabled, bool IncludeAllocationLine) {
Description.clear();
llvm::raw_string_ostream os(Description);
os
