xazax.hun created this revision.
xazax.hun added reviewers: gribozavr, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs.
We should never track if a gsl::Pointer is created from an unannotated type.
For example the code below is definitely buggy:
reference_wrapper<int> f() {
int i;
return i; // Invalid!
}
While the code below can be safe:
some_iterator f() {
MyUnannotatedSpan local = ...;
return std::begin(local); // this is fine
}
Note that, this problem will be solved once we have function annotations, as
each constructor can be annotated what it actually does.
This patch also fixes a false positive from the use of address of operator.
All in all this patch reduces the number of warnings emitted but all it fixes
all the false positives we know so far. The added tests with the TODO will be
useful once we start to add function annotations and great for documenting the
currently known false negatives.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66806
Files:
clang/lib/Sema/SemaInit.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
@@ -7,13 +7,21 @@
struct [[gsl::Pointer(int)]] MyIntPointer {
MyIntPointer(int *p = nullptr);
// Conversion operator and constructor conversion will result in two
- // different ASTs. The former is tested with another owner and
+ // different ASTs. The former is tested with another owner and
// pointer type.
MyIntPointer(const MyIntOwner &);
int &operator*();
MyIntOwner toOwner();
};
+struct MySpecialIntPointer : MyIntPointer {
+};
+
+// We did see examples in the wild when a derived class changes
+// the ownership model. So we have a test for it.
+struct [[gsl::Owner(int)]] MyOwnerIntPointer : MyIntPointer {
+};
+
struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
MyLongPointerFromConversion(long *p = nullptr);
long &operator*();
@@ -56,21 +64,21 @@
};
void dangligGslPtrFromTemporary() {
- MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+ MyIntPointer p = Y{}.a; // TODO
(void)p;
}
struct DanglingGslPtrField {
- MyIntPointer p; // expected-note 2{{pointer member declared here}}
+ MyIntPointer p; // expected-note {{pointer member declared here}}
MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
- DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
+ DanglingGslPtrField(int i) : p(&i) {} // TODO
DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
};
MyIntPointer danglingGslPtrFromLocal() {
int j;
- return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}}
+ return &j; // TODO
}
MyIntPointer returningLocalPointer() {
@@ -124,6 +132,7 @@
struct basic_iterator {
basic_iterator operator++();
T& operator*() const;
+ T* operator->() const;
};
template<typename T>
@@ -141,6 +150,12 @@
template <typename C>
auto data(const C &c) -> decltype(c.data());
+template <typename C>
+auto begin(C &c) -> decltype(c.begin());
+
+template<typename T, int N>
+T *begin(T (&array)[N]);
+
template <typename T>
struct vector {
typedef __gnu_cxx::basic_iterator<T> iterator;
@@ -158,6 +173,8 @@
template<typename T>
struct basic_string {
+ basic_string();
+ basic_string(const T *);
const T *c_str() const;
operator basic_string_view<T> () const;
};
@@ -188,8 +205,23 @@
template<typename T>
T any_cast(const any& operand);
+
+template<typename T>
+struct reference_wrapper {
+ template<typename U>
+ reference_wrapper(U &&);
+};
+
+template<typename T>
+reference_wrapper<T> ref(T& t) noexcept;
}
+struct Unannotated {
+ typedef std::vector<int>::iterator iterator;
+ iterator begin();
+ operator iterator() const;
+};
+
void modelIterators() {
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;
@@ -298,3 +330,123 @@
const std::vector<int> &v = getVec();
const int *val = v.data(); // Ok, it is lifetime extended.
}
+
+void handleTernaryOperator(bool cond) {
+ std::basic_string<char> def;
+ std::basic_string_view<char> v = cond ? def : ""; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+std::reference_wrapper<int> danglingPtrFromNonOwnerLocal() {
+ int i = 5;
+ return i; // TODO
+}
+
+std::reference_wrapper<int> danglingPtrFromNonOwnerLocal2() {
+ int i = 5;
+ return std::ref(i); // TODO
+}
+
+std::reference_wrapper<int> danglingPtrFromNonOwnerLocal3() {
+ int i = 5;
+ return std::reference_wrapper<int>(i); // TODO
+}
+
+std::reference_wrapper<Unannotated> danglingPtrFromNonOwnerLocal4() {
+ Unannotated i;
+ return std::reference_wrapper<Unannotated>(i); // TODO
+}
+
+std::reference_wrapper<Unannotated> danglingPtrFromNonOwnerLocal5() {
+ Unannotated i;
+ return std::ref(i); // TODO
+}
+
+int *returnPtrToLocalArray() {
+ int a[5];
+ return std::begin(a); // TODO
+}
+
+struct ptr_wrapper {
+ std::vector<int>::iterator member;
+};
+
+ptr_wrapper getPtrWrapper();
+
+std::vector<int>::iterator returnPtrFromWrapper() {
+ ptr_wrapper local = getPtrWrapper();
+ return local.member;
+}
+
+std::vector<int>::iterator returnPtrFromWrapperThroughRef() {
+ ptr_wrapper local = getPtrWrapper();
+ ptr_wrapper &local2 = local;
+ return local2.member;
+}
+
+std::vector<int>::iterator returnPtrFromWrapperThroughRef2() {
+ ptr_wrapper local = getPtrWrapper();
+ std::vector<int>::iterator &local2 = local.member;
+ return local2;
+}
+
+void checkPtrMemberFromAggregate() {
+ std::vector<int>::iterator local = getPtrWrapper().member; // OK.
+}
+
+std::vector<int>::iterator doNotInterferWithUnannotated() {
+ Unannotated value;
+ // Conservative choice for now. Probably not ok, but we do not warn.
+ return std::begin(value);
+}
+
+std::vector<int>::iterator doNotInterferWithUnannotated2() {
+ Unannotated value;
+ return value;
+}
+
+std::vector<int>::iterator supportDerefAddrofChain(int a, std::vector<int>::iterator value) {
+ switch (a) {
+ default:
+ return value;
+ case 1:
+ return *&value;
+ case 2:
+ return *&*&value;
+ case 3:
+ return *&*&*&value;
+ }
+}
+
+int &supportDerefAddrofChain2(int a, std::vector<int>::iterator value) {
+ switch (a) {
+ default:
+ return *value;
+ case 1:
+ return **&value;
+ case 2:
+ return **&*&value;
+ case 3:
+ return **&*&*&value;
+ }
+}
+
+int *supportDerefAddrofChain3(int a, std::vector<int>::iterator value) {
+ switch (a) {
+ default:
+ return &*value;
+ case 1:
+ return &*&*value;
+ case 2:
+ return &*&**&value;
+ case 3:
+ return &*&**&*&value;
+ }
+}
+
+MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) {
+ return ptr;
+}
+
+MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) {
+ return ptr; // expected-warning {{address of stack memory associated with parameter 'ptr' returned}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6645,6 +6645,10 @@
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
LocalVisitor Visit) {
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
+ // We are not interested in the temporary base objects of gsl Pointers:
+ // Temp().ptr; // Here ptr might not dangle.
+ if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
+ return;
Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
if (Arg->isGLValue())
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
@@ -7165,6 +7169,8 @@
for (auto It = Path.rbegin(), End = Path.rend(); It != End; ++It) {
if (It->Kind == IndirectLocalPathEntry::VarInit)
continue;
+ if (It->Kind == IndirectLocalPathEntry::AddressOf)
+ continue;
return It->Kind == IndirectLocalPathEntry::GslPointerInit;
}
return false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
