[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368446: More warnings regarding gsl::Pointer and gsl::Owner
attributes (authored by xazax, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65120?vs=214228&id=214380#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
Files:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/inner-pointer.cpp
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
Index: cfe/trunk/lib/Sema/SemaInit.cpp
===
--- cfe/trunk/lib/Sema/SemaInit.cpp
+++ cfe/trunk/lib/Sema/SemaInit.cpp
@@ -6564,6 +6564,25 @@
return false;
}
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+ if (!isRecordWithAttr(Callee->getThisObjectType()) &&
+ !isRecordWithAttr(Callee->getThisObjectType()))
+return false;
+ if (!isRecordWithAttr(Callee->getReturnType()) &&
+ !Callee->getReturnType()->isPointerType())
+return false;
+ return llvm::StringSwitch(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) {
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6577,10 +6596,9 @@
};
if (auto *MCE = dyn_cast(Call)) {
-const FunctionDecl *Callee = MCE->getDirectCallee();
-if (auto *Conv = dyn_cast_or_null(Callee))
- if (isRecordWithAttr(Conv->getConversionType()))
-VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+const auto *MD = cast_or_null(MCE->getDirectCallee());
+if (MD && shouldTrackImplicitObjectArg(MD))
+ VisitPointerArg(MD, MCE->getImplicitObjectArgument());
return;
}
Index: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ cfe/trunk/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}}
}
@@ -124,12 +119,52 @@
global2 = MyLongOwnerWithConversion{}; // TODO ?
}
-struct IntVector {
- int *begin();
- int *end();
+namespace std {
+template
+struct basic_iterator {};
+
+template
+struct vector {
+ typedef basic_iterator iterator;
+ iterator begin();
+ T *data();
+};
+
+template
+struct basic_string {
+ const T *c_str() const;
+};
+
+template
+struct unique_ptr {
+ T *get() const;
};
+}
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector::iterator modelIteratorReturn() {
+ return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
+}
Index: cfe/trunk/test/Analysis/inner-pointer.cpp
===
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun marked 4 inline comments as done.
xazax.hun added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+ return llvm::StringSwitch(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
gribozavr wrote:
> xazax.hun wrote:
> > gribozavr wrote:
> > > Should we also check that `Callee->getParent` is an owner?
> > >
> > > Otherwise the check would complain about `begin()` on, for example, a
> > > `std::string_view`.
> > This is intentional. We only warn if the initialization chain can be
> > tracked back to a temporary (or local in some cases) owner.
> > So we warn for the following code:
> > ```
> > const char *trackThroughMultiplePointer() {
> > return std::basic_string_view(std::basic_string()).begin();
> > // expected-warning {{returning address of local temporary object}}
> > }
> > ```
> Makes sense, but then we should still check that `Callee->getParent` is an
> owner or a pointer.
Good point, done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun updated this revision to Diff 214228.
xazax.hun added a comment.
- Fix review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
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}}
}
@@ -124,12 +119,52 @@
global2 = MyLongOwnerWithConversion{}; // TODO ?
}
-struct IntVector {
- int *begin();
- int *end();
+namespace std {
+template
+struct basic_iterator {};
+
+template
+struct vector {
+ typedef basic_iterator iterator;
+ iterator begin();
+ T *data();
+};
+
+template
+struct basic_string {
+ const T *c_str() const;
+};
+
+template
+struct unique_ptr {
+ T *get() const;
};
+}
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector::iterator modelIteratorReturn() {
+ return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
+}
Index: clang/test/Analysis/inner-pointer.cpp
===
--- clang/test/Analysis/inner-pointer.cpp
+++ clang/test/Analysis/inner-pointer.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
+// RUN: -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
// RUN: %s -analyzer-output=text -verify
#include "Inputs/system-header-simulator-cxx.h"
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6564,6 +6564,25 @@
return false;
}
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+ if (!isRecordWithAttr(Callee->getThisObjectType()) &&
+ !isRecordWithAttr(Callee->getThisObjectType()))
+return false;
+ if (!isRecordWithAttr(Callee->getReturnType()) &&
+ !Callee->getReturnType()->isPointerType())
+return false;
+ return llvm::StringSwitch(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) {
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6577,10 +6596,9 @@
};
if (auto *MCE = dyn_cast(Call)) {
-const FunctionDecl *Callee = MCE->getDirectCallee();
-if (auto *Conv = dyn_cast_or_null(Callee))
- if (isRecordWithAttr(Conv->getConversionType()))
-VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+const auto *MD = cast_or_null(MCE->getDirectCallee()
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
gribozavr added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+ return llvm::StringSwitch(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
xazax.hun wrote:
> gribozavr wrote:
> > Should we also check that `Callee->getParent` is an owner?
> >
> > Otherwise the check would complain about `begin()` on, for example, a
> > `std::string_view`.
> This is intentional. We only warn if the initialization chain can be tracked
> back to a temporary (or local in some cases) owner.
> So we warn for the following code:
> ```
> const char *trackThroughMultiplePointer() {
> return std::basic_string_view(std::basic_string()).begin(); //
> expected-warning {{returning address of local temporary object}}
> }
> ```
Makes sense, but then we should still check that `Callee->getParent` is an
owner or a pointer.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun updated this revision to Diff 212206.
xazax.hun marked 3 inline comments as done.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Add new line to end of file.
- Rebase, calls no longer need special handling.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
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}}
}
@@ -124,12 +119,52 @@
global2 = MyLongOwnerWithConversion{}; // TODO ?
}
-struct IntVector {
- int *begin();
- int *end();
+namespace std {
+template
+struct basic_iterator {};
+
+template
+struct vector {
+ typedef basic_iterator iterator;
+ iterator begin();
+ T *data();
+};
+
+template
+struct basic_string {
+ const T *c_str() const;
+};
+
+template
+struct unique_ptr {
+ T *get() const;
};
+}
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector::iterator modelIteratorReturn() {
+ return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
+}
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
@@ -6559,6 +6559,22 @@
return false;
}
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+ if (!isRecordWithAttr(Callee->getReturnType()) &&
+ !Callee->getReturnType()->isPointerType())
+return false;
+ return llvm::StringSwitch(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) {
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6572,15 +6588,14 @@
};
if (auto *MCE = dyn_cast(Call)) {
-const FunctionDecl *Callee = MCE->getDirectCallee();
-if (auto *Conv = dyn_cast_or_null(Callee))
- if (isRecordWithAttr(Conv->getConversionType()))
-VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+const auto *MD = cast_or_null(MCE->getDirectCa
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:6563
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
gribozavr wrote:
> This looks like an ad-hoc rule. Is there a way to express it with attributes
> instead?
It is indeed an ad-hoc rule. The point is that we do know that `std` methods
behave this way and function attributes will be able to express the same in the
future. But we are still experimenting with what is the best spelling,
representation etc for those function attributes. So given the value of these
checks I do not want to wait until function annotations are upstreamed since we
can achieve the same with relatively little code. I run these checks on ~300
open source projects and not a single false positive was found in the latest
run.
Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+ return llvm::StringSwitch(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
gribozavr wrote:
> Should we also check that `Callee->getParent` is an owner?
>
> Otherwise the check would complain about `begin()` on, for example, a
> `std::string_view`.
This is intentional. We only warn if the initialization chain can be tracked
back to a temporary (or local in some cases) owner.
So we warn for the following code:
```
const char *trackThroughMultiplePointer() {
return std::basic_string_view(std::basic_string()).begin(); //
expected-warning {{returning address of local temporary object}}
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
gribozavr added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:6563
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
This looks like an ad-hoc rule. Is there a way to express it with attributes
instead?
Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+ return llvm::StringSwitch(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
Should we also check that `Callee->getParent` is an owner?
Otherwise the check would complain about `begin()` on, for example, a
`std::string_view`.
Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:170
+ return std::unique_ptr().get(); // expected-warning {{returning address
of local temporary object}}
+}
Please add a newline.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun updated this revision to Diff 211581.
xazax.hun added a comment.
- Remove unused parameter.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
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}}
}
@@ -124,12 +119,52 @@
global2 = MyLongOwnerWithConversion{}; // TODO ?
}
-struct IntVector {
- int *begin();
- int *end();
+namespace std {
+template
+struct basic_iterator {};
+
+template
+struct vector {
+ typedef basic_iterator iterator;
+ iterator begin();
+ T *data();
+};
+
+template
+struct basic_string {
+ const T *c_str() const;
};
+template
+struct unique_ptr {
+ T *get() const;
+};
+}
+
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector::iterator modelIteratorReturn() {
+ return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr().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,38 @@
return false;
}
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+ if (!isRecordWithAttr(Callee->getReturnType()) &&
+ !Callee->getReturnType()->isPointerType())
+return false;
+ return llvm::StringSwitch(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(Call)) {
+Callee = CE->getDirectCallee();
+if (!Callee)
+ return;
+QualType RetTy = Callee->getReturnType();
+if (isRecordWithAttr(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({IndirectLo
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:6564
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
+ const CXXMemberCallExpr *MCE) {
+ if (auto *Conv = dyn_cast_or_null(Callee))
The second param is unused, will fix in the next revision after the first round
of reviews.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65120/new/
https://reviews.llvm.org/D65120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/inner-pointer.cpp:2 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \ -// RUN: %s -analyzer-output=text -verify I had to disable the compiler warnings for this test file as some of the warnings overlapped with the warnings from the clang static analyzer (which is not bad IMO!). I think this is cleaner than adding `expected-warining` lines that are unrelated to the analyzer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65120/new/ https://reviews.llvm.org/D65120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes
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
+struct basic_iterator {};
+
+template
+struct vector {
+ typedef basic_iterator iterator;
+ iterator begin();
+ T *data();
+};
+
+template
+struct basic_string {
+ const T *c_str() const;
};
+template
+struct unique_ptr {
+ T *get() const;
+};
+}
+
void modelIterators() {
- int *it = IntVector{}.begin(); // TODO ?
+ std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
(void)it;
}
+
+std::vector::iterator modelIteratorReturn() {
+ return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+ std::basic_string s;
+ return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+ return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+ return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+ return std::unique_ptr().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(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+ return true;
+ if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+ if (!isRecordWithAttr(Callee->getReturnType()) &&
+ !Callee->g
