steakhal updated this revision to Diff 365133.
steakhal marked 2 inline comments as done.
steakhal added a comment.
Removed the extra note about the temporary expression.
---
In D107078#2933682 <https://reviews.llvm.org/D107078#2933682>, @NoQ wrote:
> In D107078#2927899 <https://reviews.llvm.org/D107078#2927899>, @steakhal
> wrote:
>
>> I did not fix the extra blue 'bubble' note issue, and I think that is out of
>> the scope of this patch.
>
> This is an on-by-default checker. We should not knowingly regress it, even if
> temporarily.
I'm not exactly sure what would I regress, but to be on the safe side I won't
emit any extra notes this time.
WDYT about the current form?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107078/new/
https://reviews.llvm.org/D107078
Files:
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/test/Analysis/copy-elision.cpp
clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
clang/test/Analysis/loop-block-counts.c
clang/test/Analysis/stack-addr-ps.cpp
Index: clang/test/Analysis/stack-addr-ps.cpp
===================================================================
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -137,3 +137,28 @@
}
}
+void write_stack_address_to(char **q) {
+ char local;
+ *q = &local;
+ // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'local' is still referred to by the stack variable 'p' upon \
+returning to the caller}}
+}
+
+void test_stack() {
+ char *p;
+ write_stack_address_to(&p);
+}
+
+struct C {
+ ~C() {} // non-trivial class
+};
+
+C make1() {
+ C c;
+ return c; // no-warning
+}
+
+void test_copy_elision() {
+ C c1 = make1();
+}
Index: clang/test/Analysis/loop-block-counts.c
===================================================================
--- clang/test/Analysis/loop-block-counts.c
+++ clang/test/Analysis/loop-block-counts.c
@@ -5,6 +5,9 @@
void callee(void **p) {
int x;
*p = &x;
+ // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'x' is still referred to by the stack variable 'arr' upon \
+returning to the caller}}
}
void loop() {
Index: clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===================================================================
--- clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -71,6 +71,9 @@
void *allocaPtr;
int dontGetFilteredByNonPedanticMode = 0;
+ // expected-warning-re@+3 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller. This will be a dangling reference}}
UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) {
// All good!
}
@@ -86,6 +89,9 @@
TypedAllocaTest1() // expected-warning{{1 uninitialized field}}
: allocaPtr(static_cast<int *>(__builtin_alloca(sizeof(int)))) {}
+ // expected-warning-re@-2 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller. This will be a dangling reference}}
};
void fTypedAllocaTest1() {
@@ -96,6 +102,9 @@
int *allocaPtr;
int dontGetFilteredByNonPedanticMode = 0;
+ // expected-warning-re@+5 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller. This will be a dangling reference}}
TypedAllocaTest2()
: allocaPtr(static_cast<int *>(__builtin_alloca(sizeof(int)))) {
*allocaPtr = 55555;
@@ -341,6 +350,9 @@
void *&&vptrrref; // expected-note {{here}}
public:
+ // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller. This will be a dangling reference}}
VoidPointerRRefTest1(void *vptr, char) : vptrrref(static_cast<void *&&>(vptr)) { // expected-warning {{binding reference member 'vptrrref' to stack allocated parameter 'vptr'}}
// All good!
}
@@ -355,6 +367,9 @@
void **&&vpptrrref; // expected-note {{here}}
public:
+ // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller. This will be a dangling reference}}
VoidPointerRRefTest2(void **vptr, char) : vpptrrref(static_cast<void **&&>(vptr)) { // expected-warning {{binding reference member 'vpptrrref' to stack allocated parameter 'vptr'}}
// All good!
}
@@ -369,6 +384,9 @@
void *&vptrrref; // expected-note {{here}}
public:
+ // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller. This will be a dangling reference}}
VoidPointerLRefTest(void *vptr, char) : vptrrref(static_cast<void *&>(vptr)) { // expected-warning {{binding reference member 'vptrrref' to stack allocated parameter 'vptr'}}
// All good!
}
Index: clang/test/Analysis/copy-elision.cpp
===================================================================
--- clang/test/Analysis/copy-elision.cpp
+++ clang/test/Analysis/copy-elision.cpp
@@ -1,7 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 \
+// RUN: -analyzer-config eagerly-assume=false -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 \
+// RUN: -analyzer-config eagerly-assume=false -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 \
+// RUN: -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG \
+// RUN: -analyzer-config eagerly-assume=false -verify=expected,no-elide %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 \
+// RUN: -analyzer-config elide-constructors=false \
+// RUN: -analyzer-config eagerly-assume=false -verify %s
// Copy elision always occurs in C++17, otherwise it's under
// an on-by-default flag.
@@ -149,12 +155,21 @@
ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
return ClassWithoutDestructor(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithoutDestructor' is still \
+referred to by the stack variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) {
return make1(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithoutDestructor' is still \
+referred to by the stack variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
return make2(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithoutDestructor' is still \
+referred to by the stack variable 'v' upon returning to the caller}}
}
void testMultipleReturns() {
@@ -176,6 +191,9 @@
void consume(ClassWithoutDestructor c) {
c.push();
+ // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'c' is still referred to by the stack variable 'v' upon returning \
+to the caller}}
}
void testArgumentConstructorWithoutDestructor() {
@@ -246,6 +264,9 @@
ClassWithDestructor c;
TestCtorInitializer(AddressVector<ClassWithDestructor> &v)
: c(ClassWithDestructor(v)) {}
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithDestructor' is still referred \
+to by the stack variable 'v' upon returning to the caller}}
};
void testCtorInitializer() {
@@ -279,12 +300,21 @@
ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
return ClassWithDestructor(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithDestructor' is still referred \
+to by the stack variable 'v' upon returning to the caller}}
}
ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
return make1(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithDestructor' is still referred \
+to by the stack variable 'v' upon returning to the caller}}
}
ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
return make2(v);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::ClassWithDestructor' is still referred \
+to by the stack variable 'v' upon returning to the caller}}
}
void testMultipleReturnsWithDestructors() {
@@ -328,6 +358,9 @@
void consume(ClassWithDestructor c) {
c.push();
+ // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'c' is still referred to by the stack variable 'v' upon returning \
+to the caller}}
}
void testArgumentConstructorWithDestructor() {
@@ -364,4 +397,24 @@
#endif
}
+struct Foo {
+ Foo(Foo **q) {
+ *q = this;
+ }
+};
+
+Foo make1(Foo **r) {
+ return Foo(r);
+ // no-elide-warning@-1 {{Address of stack memory associated with temporary \
+object of type 'address_vector_tests::Foo' is still referred to by the stack \
+variable 'z' upon returning to the caller}}
+}
+
+void test_copy_elision() {
+ Foo *z;
+ // If the copy elided, 'z' points to 'tmp', otherwise it's a dangling pointer.
+ Foo tmp = make1(&z);
+ (void)tmp;
+}
+
} // namespace address_vector_tests
Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -11,9 +11,9 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/AST/ExprCXX.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -303,21 +303,53 @@
class CallBack : public StoreManager::BindingsHandler {
private:
CheckerContext &Ctx;
- const StackFrameContext *CurSFC;
+ const StackFrameContext *PoppedFrame;
+
+ /// Look for stack variables referring to popped stack variables.
+ /// Returns true only if it found some dangling stack variables
+ /// referred by an other stack variable from different stack frame.
+ bool checkForDanglingStackVariable(const MemRegion *Referrer,
+ const MemRegion *Referred) {
+ const auto *ReferrerMemSpace =
+ Referrer->getMemorySpace()->getAs<StackSpaceRegion>();
+ const auto *ReferredMemSpace =
+ Referred->getMemorySpace()->getAs<StackSpaceRegion>();
+
+ if (!ReferrerMemSpace || !ReferredMemSpace)
+ return false;
+
+ const auto *ReferrerFrame = ReferrerMemSpace->getStackFrame();
+ const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+
+ if (ReferrerMemSpace && ReferredMemSpace) {
+ if (ReferredFrame == PoppedFrame &&
+ ReferrerFrame->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
+ }
+ }
+ return false;
+ }
public:
SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V;
- CallBack(CheckerContext &CC) : Ctx(CC), CurSFC(CC.getStackFrame()) {}
+ CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {}
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
+ const MemRegion *VR = Val.getAsRegion();
+ if (!VR)
+ return true;
+
+ if (checkForDanglingStackVariable(Region, VR))
+ return true;
+ // Check the globals for the same.
if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace()))
return true;
- const MemRegion *VR = Val.getAsRegion();
- if (VR && isa<StackSpaceRegion>(VR->getMemorySpace()) &&
- !isArcManagedBlock(VR, Ctx) && !isNotInCurrentFrame(VR, Ctx))
+ if (VR && VR->hasStackStorage() && !isArcManagedBlock(VR, Ctx) &&
+ !isNotInCurrentFrame(VR, Ctx))
V.emplace_back(Region, VR);
return true;
}
@@ -344,19 +376,41 @@
"invalid after returning from the function");
for (const auto &P : Cb.V) {
+ const MemRegion *Referrer = P.first;
+ const MemRegion *Referred = P.second;
+
// Generate a report for this bug.
+ const StringRef CommonSuffix =
+ "upon returning to the caller. This will be a dangling reference";
SmallString<128> Buf;
llvm::raw_svector_ostream Out(Buf);
- SourceRange Range = genName(Out, P.second, Ctx.getASTContext());
- Out << " is still referred to by the ";
- if (isa<StaticGlobalSpaceRegion>(P.first->getMemorySpace()))
- Out << "static";
- else
- Out << "global";
- Out << " variable '";
- const VarRegion *VR = cast<VarRegion>(P.first->getBaseRegion());
- Out << *VR->getDecl()
- << "' upon returning to the caller. This will be a dangling reference";
+ const SourceRange Range = genName(Out, Referred, Ctx.getASTContext());
+
+ if (isa<CXXTempObjectRegion>(Referrer)) {
+ Out << " is still referred to by a temporary object on the stack "
+ << CommonSuffix;
+ auto Report =
+ std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N);
+ Ctx.emitReport(std::move(Report));
+ return;
+ }
+
+ const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
+ if (isa<StaticGlobalSpaceRegion>(Space))
+ return "static";
+ if (isa<GlobalsSpaceRegion>(Space))
+ return "global";
+ assert(isa<StackSpaceRegion>(Space));
+ return "stack";
+ }(Referrer->getMemorySpace());
+
+ // This cast supposed to succeed.
+ const VarRegion *ReferrerVar = cast<VarRegion>(Referrer->getBaseRegion());
+ const std::string ReferrerVarName =
+ ReferrerVar->getDecl()->getDeclName().getAsString();
+
+ Out << " is still referred to by the " << ReferrerMemorySpace
+ << " variable '" << ReferrerVarName << "' " << CommonSuffix;
auto Report =
std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N);
if (Range.isValid())
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
