diff --git a/clang/lib/Checker/StackAddrLeakChecker.cpp b/clang/lib/Checker/StackAddrLeakChecker.cpp index 34a06fb500e5..f4a9db62df4b 100644 --- a/clang/lib/Checker/StackAddrLeakChecker.cpp +++ b/clang/lib/Checker/StackAddrLeakChecker.cpp @@ -35,12 +35,58 @@ 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()); } + +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 << "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 << "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 << "stack-allocated block declared on line " + << SM.getInstantiationLineNumber(L); + } + else if (const VarRegion *VR = dyn_cast(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 @@ void StackAddrLeakChecker::EmitStackError(CheckerContext &C, const MemRegion *R, // 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(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(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(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(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 @@ 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"); - - 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(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/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c index f8cadbe281be..342b3b1430a2 100644 --- a/clang/test/Analysis/stack-addr-ps.c +++ b/clang/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/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c index 847ed7da5b17..39808ed873c8 100644 --- a/clang/test/Analysis/stackaddrleak.c +++ b/clang/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}} +}