xazax.hun created this revision.
xazax.hun added reviewers: gribozavr, rsmith, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, Szelethus, dkrupp,
rnkovacs.
This patch extends the warnings for additional cases:
1. When the temporary is the result of a function call.
2. When we call some well-known methods on a temporary object.
The latter is using a hard coded list of those well-known functions. Later
revisions might replace string matching with checking function annotations from
the lifetime papers. There are two reasons to hard code the list this way for
now:
1. The spelling and semantics of function annotations might change in the near
future after incorporating some implementation related experience so we are not
rushing adding those annotations to clang just yet (as opposed to class
annotations which are considered quite stable now.)
2. Checking for those well known functions is a huge usability boost for the
check.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65120
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/Analysis/inner-pointer.cpp
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===================================================================
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -2,7 +2,6 @@
struct [[gsl::Owner(int)]] MyIntOwner {
MyIntOwner();
int &operator*();
- int *c_str() const;
};
struct [[gsl::Pointer(int)]] MyIntPointer {
@@ -52,16 +51,6 @@
return t.releaseAsRawPointer(); // ok
}
-int *danglingRawPtrFromLocal() {
- MyIntOwner t;
- return t.c_str(); // TODO
-}
-
-int *danglingRawPtrFromTemp() {
- MyIntPointer p;
- return p.toOwner().c_str(); // TODO
-}
-
struct Y {
int a[4];
};
@@ -103,6 +92,12 @@
return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
}
+MyIntOwner makeTempOwner();
+
+MyIntPointer danglingGslPtrFromTemporary2() {
+ return makeTempOwner(); // expected-warning {{returning address of local temporary object}}
+}
+
MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
}
@@ -119,12 +114,52 @@
global2 = MyLongOwnerWithConversion{}; // TODO ?
}
-struct IntVector {
- int *begin();
- int *end();
+namespace std {
+template <typename T>
+struct basic_iterator {};
+
+template <typename T>
+struct vector {
+ typedef basic_iterator<T> iterator;
+ iterator begin();
+ T *data();
+};
+
+template<typename T>
+struct basic_string {
+ const T *c_str() const;
};
+template<typename T>
+struct unique_ptr {
+ T *get() const;
+};
+}
+
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector<int>::iterator it = std::vector<int>().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector<int>::iterator modelIteratorReturn() {
+ return std::vector<int>().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string<char> s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string<char>().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr<int> getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr<int>().get(); // expected-warning {{returning address of local temporary object}}
+}
\ No newline at end of file
Index: clang/test/Analysis/inner-pointer.cpp
===================================================================
--- clang/test/Analysis/inner-pointer.cpp
+++ clang/test/Analysis/inner-pointer.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
-// RUN: %s -analyzer-output=text -verify
+// RUN: %s -analyzer-output=text -Wno-dangling -Wno-dangling-field \
+// RUN: -Wno-return-stack-address -verify
#include "Inputs/system-header-simulator-cxx.h"
namespace std {
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6560,8 +6560,39 @@
return false;
}
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
+ const CXXMemberCallExpr *MCE) {
+ if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
+ if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+ return false;
+ if (!isRecordWithAttr<PointerAttr>(Callee->getReturnType()) &&
+ !Callee->getReturnType()->isPointerType())
+ return false;
+ return llvm::StringSwitch<bool>(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
+ .Cases("end", "rend", "cend", "crend", true)
+ .Cases("c_str", "data", "get", true)
+ .Default(false);
+}
+
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
LocalVisitor Visit) {
+ const FunctionDecl *Callee;
+ if (auto *CE = dyn_cast<CallExpr>(Call)) {
+ Callee = CE->getDirectCallee();
+ if (!Callee)
+ return;
+ QualType RetTy = Callee->getReturnType();
+ if (isRecordWithAttr<OwnerAttr>(RetTy)) {
+ Path.push_back({IndirectLocalPathEntry::GslOwnerTemporaryInit, Call,
+ RetTy->getAsCXXRecordDecl()});
+ Visit(Path, Call, RK_ReferenceBinding);
+ Path.pop_back();
+ }
+ }
+
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
if (Arg->isGLValue())
@@ -6573,22 +6604,21 @@
};
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
- const FunctionDecl *Callee = MCE->getDirectCallee();
- if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
- if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
- VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+ const auto *MD = cast<CXXMethodDecl>(Callee);
+ if (shouldTrackImplicitObjectArg(MD, MCE))
+ VisitPointerArg(MD, MCE->getImplicitObjectArgument());
return;
}
if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
- const auto* Ctor = CCE->getConstructor();
+ const auto *Ctor = CCE->getConstructor();
const CXXRecordDecl *RD = Ctor->getParent()->getCanonicalDecl();
if (RD->hasAttr<OwnerAttr>() && isa<CXXTemporaryObjectExpr>(Call)) {
Path.push_back({IndirectLocalPathEntry::GslOwnerTemporaryInit, Call, RD});
Visit(Path, Call, RK_ReferenceBinding);
Path.pop_back();
} else {
- if (RD->hasAttr<PointerAttr>() && CCE->getNumArgs() > 0)
+ if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0]);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
