Tweak stack address checker to report multiple cases where globals may reference stack memory.
Also refactor the diagnostics so that we print out the kind of stack memory returned.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@106210 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Checker/StackAddrLeakChecker.cpp b/lib/Checker/StackAddrLeakChecker.cpp
index 34a06fb..f4a9db6 100644
--- a/lib/Checker/StackAddrLeakChecker.cpp
+++ b/lib/Checker/StackAddrLeakChecker.cpp
@@ -35,12 +35,58 @@
void EvalEndPath(GREndPathNodeBuilder &B, void *tag, GRExprEngine &Eng);
private:
void EmitStackError(CheckerContext &C, const MemRegion *R, const Expr *RetE);
+ SourceRange GenName(llvm::raw_ostream &os, const MemRegion *R,
+ SourceManager &SM);
};
}
void clang::RegisterStackAddrLeakChecker(GRExprEngine &Eng) {
Eng.registerCheck(new StackAddrLeakChecker());
}
+
+SourceRange StackAddrLeakChecker::GenName(llvm::raw_ostream &os,
+ const MemRegion *R,
+ SourceManager &SM) {
+ // Get the base region, stripping away fields and elements.
+ R = R->getBaseRegion();
+ SourceRange range;
+ os << "Address of ";
+
+ // Check if the region is a compound literal.
+ if (const CompoundLiteralRegion* CR = dyn_cast<CompoundLiteralRegion>(R)) {
+ const CompoundLiteralExpr* CL = CR->getLiteralExpr();
+ os << "stack memory associated with a compound literal "
+ "declared on line "
+ << SM.getInstantiationLineNumber(CL->getLocStart())
+ << " returned to caller";
+ range = CL->getSourceRange();
+ }
+ else if (const AllocaRegion* AR = dyn_cast<AllocaRegion>(R)) {
+ const Expr* ARE = AR->getExpr();
+ SourceLocation L = ARE->getLocStart();
+ range = ARE->getSourceRange();
+ os << "stack memory allocated by call to alloca() on line "
+ << SM.getInstantiationLineNumber(L);
+ }
+ else if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) {
+ const BlockDecl *BD = BR->getCodeRegion()->getDecl();
+ SourceLocation L = BD->getLocStart();
+ range = BD->getSourceRange();
+ os << "stack-allocated block declared on line "
+ << SM.getInstantiationLineNumber(L);
+ }
+ else if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
+ os << "stack memory associated with local variable '"
+ << VR->getString() << '\'';
+ range = VR->getDecl()->getSourceRange();
+ }
+ else {
+ assert(false && "Invalid region in ReturnStackAddressChecker.");
+ }
+
+ return range;
+}
+
void StackAddrLeakChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
const Expr *RetE) {
ExplodedNode *N = C.GenerateSink();
@@ -54,46 +100,8 @@
// Generate a report for this bug.
llvm::SmallString<512> buf;
llvm::raw_svector_ostream os(buf);
- SourceRange range;
-
- // Get the base region, stripping away fields and elements.
- R = R->getBaseRegion();
-
- // Check if the region is a compound literal.
- if (const CompoundLiteralRegion* CR = dyn_cast<CompoundLiteralRegion>(R)) {
- const CompoundLiteralExpr* CL = CR->getLiteralExpr();
- os << "Address of stack memory associated with a compound literal "
- "declared on line "
- << C.getSourceManager().getInstantiationLineNumber(CL->getLocStart())
- << " returned to caller";
- range = CL->getSourceRange();
- }
- else if (const AllocaRegion* AR = dyn_cast<AllocaRegion>(R)) {
- const Expr* ARE = AR->getExpr();
- SourceLocation L = ARE->getLocStart();
- range = ARE->getSourceRange();
- os << "Address of stack memory allocated by call to alloca() on line "
- << C.getSourceManager().getInstantiationLineNumber(L)
- << " returned to caller";
- }
- else if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) {
- const BlockDecl *BD = BR->getCodeRegion()->getDecl();
- SourceLocation L = BD->getLocStart();
- range = BD->getSourceRange();
- os << "Address of stack-allocated block declared on line "
- << C.getSourceManager().getInstantiationLineNumber(L)
- << " returned to caller";
- }
- else if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
- os << "Address of stack memory associated with local variable '"
- << VR->getString() << "' returned";
- range = VR->getDecl()->getSourceRange();
- }
- else {
- assert(false && "Invalid region in ReturnStackAddressChecker.");
- return;
- }
-
+ SourceRange range = GenName(os, R, C.getSourceManager());
+ os << " returned to caller";
RangedBugReport *report = new RangedBugReport(*BT_returnstack, os.str(), N);
report->addRange(RetE->getSourceRange());
if (range.isValid())
@@ -132,10 +140,10 @@
private:
const StackFrameContext *CurSFC;
public:
- const MemRegion *src, *dst;
+ llvm::SmallVector<std::pair<const MemRegion*, const MemRegion*>, 10> V;
CallBack(const LocationContext *LCtx)
- : CurSFC(LCtx->getCurrentStackFrame()), src(0), dst(0) {}
+ : CurSFC(LCtx->getCurrentStackFrame()) {}
bool HandleBinding(StoreManager &SMgr, Store store,
const MemRegion *region, SVal val) {
@@ -151,11 +159,8 @@
dyn_cast<StackSpaceRegion>(vR->getMemorySpace())) {
// If the global variable holds a location in the current stack frame,
// record the binding to emit a warning.
- if (SSR->getStackFrame() == CurSFC) {
- src = region;
- dst = vR;
- return false;
- }
+ if (SSR->getStackFrame() == CurSFC)
+ V.push_back(std::make_pair(region, vR));
}
return true;
@@ -164,24 +169,36 @@
CallBack cb(B.getPredecessor()->getLocationContext());
state->getStateManager().getStoreManager().iterBindings(state->getStore(),cb);
-
- // Did we find any globals referencing stack memory?
- if (!cb.src)
+
+ if (cb.V.empty())
return;
// Generate an error node.
ExplodedNode *N = B.generateNode(state, tag, B.getPredecessor());
if (!N)
return;
-
- if (!BT_stackleak)
- BT_stackleak = new BuiltinBug("Stack address stored into global variable",
- "Stack address was saved into a global variable. "
- "is dangerous because the address will become invalid "
- "after returning from the function");
-
- BugReport *R =
- new BugReport(*BT_stackleak, BT_stackleak->getDescription(), N);
- Eng.getBugReporter().EmitReport(R);
+ if (!BT_stackleak)
+ BT_stackleak =
+ new BuiltinBug("Stack address stored into global variable",
+ "Stack address was saved into a global variable. "
+ "This is dangerous because the address will become "
+ "invalid after returning from the function");
+
+ for (unsigned i = 0, e = cb.V.size(); i != e; ++i) {
+ // Generate a report for this bug.
+ llvm::SmallString<512> buf;
+ llvm::raw_svector_ostream os(buf);
+ SourceRange range = GenName(os, cb.V[i].second,
+ Eng.getContext().getSourceManager());
+ os << " is still referred to by the global variable '";
+ const VarRegion *VR = cast<VarRegion>(cb.V[i].first->getBaseRegion());
+ os << VR->getDecl()->getNameAsString()
+ << "' upon returning to the caller. This will be a dangling reference";
+ RangedBugReport *report = new RangedBugReport(*BT_stackleak, os.str(), N);
+ if (range.isValid())
+ report->addRange(range);
+
+ Eng.getBugReporter().EmitReport(report);
+ }
}