From: Ted Kremenek Date: Thu, 17 Jun 2010 04:21:37 +0000 (+0000) Subject: Tweak stack address checker to report multiple cases where globals may reference... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a8166156a6414ddd6a68514dc4f48e95d2259977;p=clang 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 34a06fb500..f4a9db62df 100644 --- a/lib/Checker/StackAddrLeakChecker.cpp +++ b/lib/Checker/StackAddrLeakChecker.cpp @@ -35,65 +35,73 @@ public: 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()); } -void StackAddrLeakChecker::EmitStackError(CheckerContext &C, const MemRegion *R, - const Expr *RetE) { - ExplodedNode *N = C.GenerateSink(); - - if (!N) - return; - if (!BT_returnstack) - BT_returnstack=new BuiltinBug("Return of address to stack-allocated memory"); - - // 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. +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(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"; + 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(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"; + os << "stack memory allocated by call to alloca() on line " + << SM.getInstantiationLineNumber(L); } else if (const BlockDataRegion *BR = dyn_cast(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"; + os << "stack-allocated block declared on line " + << SM.getInstantiationLineNumber(L); } else if (const VarRegion *VR = dyn_cast(R)) { - os << "Address of stack memory associated with local variable '" - << VR->getString() << "' returned"; + 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(); + + if (!N) return; - } + if (!BT_returnstack) + BT_returnstack=new BuiltinBug("Return of address to stack-allocated memory"); + + // Generate a report for this bug. + llvm::SmallString<512> buf; + llvm::raw_svector_ostream os(buf); + 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 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag, private: const StackFrameContext *CurSFC; public: - const MemRegion *src, *dst; + llvm::SmallVector, 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 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag, dyn_cast(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 @@ void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag, 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"); + 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"); - BugReport *R = - new BugReport(*BT_stackleak, BT_stackleak->getDescription(), N); - - Eng.getBugReporter().EmitReport(R); + 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(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); + } } diff --git a/test/Analysis/stack-addr-ps.c b/test/Analysis/stack-addr-ps.c index f8cadbe281..342b3b1430 100644 --- a/test/Analysis/stack-addr-ps.c +++ b/test/Analysis/stack-addr-ps.c @@ -3,11 +3,11 @@ int* f1() { int x = 0; - return &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned.}} expected-warning{{address of stack memory associated with local variable 'x' returned}} + return &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned}} expected-warning{{address of stack memory associated with local variable 'x' returned}} } int* f2(int y) { - return &y; // expected-warning{{Address of stack memory associated with local variable 'y' returned.}} expected-warning{{address of stack memory associated with local variable 'y' returned}} + return &y; // expected-warning{{Address of stack memory associated with local variable 'y' returned}} expected-warning{{address of stack memory associated with local variable 'y' returned}} } int* f3(int x, int *y) { @@ -16,7 +16,7 @@ int* f3(int x, int *y) { if (x) y = &w; - return y; // expected-warning{{Address of stack memory associated with local variable 'w' returned.}} + return y; // expected-warning{{Address of stack memory associated with local variable 'w' returned to caller}} } void* compound_literal(int x, int y) { diff --git a/test/Analysis/stackaddrleak.c b/test/Analysis/stackaddrleak.c index 847ed7da5b..39808ed873 100644 --- a/test/Analysis/stackaddrleak.c +++ b/test/Analysis/stackaddrleak.c @@ -4,7 +4,7 @@ char const *p; void f0() { char const str[] = "This will change"; - p = str; // expected-warning {{Stack address was saved into a global variable.}} + p = str; // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}} } void f1() { @@ -14,7 +14,7 @@ void f1() { } void f2() { - p = (const char *) __builtin_alloca(12); // expected-warning {{Stack address was saved into a global variable.}} + p = (const char *) __builtin_alloca(12); // expected-warning{{Address of stack memory allocated by call to alloca() on line 17 is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}} } // PR 7383 - previosly the stack address checker would crash on this example @@ -24,3 +24,11 @@ static int pr7383(__const char *__) return 0; } extern __const char *__const pr7383_list[]; + +// Test that we catch multiple returns via globals when analyzing a function. +void test_multi_return() { + static int *a, *b; + int x; + a = &x; + b = &x; // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'a' upon returning}} expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'b' upon returning}} +}