llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis
Author: Yuan Suo (suoyuan666)
<details>
<summary>Changes</summary>
After completing the implementation of `buildOriginFlowChain`, we should
integrate it into the remaining diagnostics. I started with
`use-after-invalidation` because there is already an overload of
`buildOriginFlowChain` for `UseFact`.
While testing, I noticed the following diagnostics:
```txt
test.cpp:9:23: note: result of call to 'begin<std::vector<int>>'
aliases the storage of parameter 'v'
9 | auto it = std::find(std::begin(v), std::end(v), 3); //
expected-warning {{parameter 'v' is later invalidated}} \
| ^~~~~~~~~~~~~
test.cpp:9:13: note: result of call to
'find<__gnu_cxx::__normal_iterator<int *, std::vector<int>>,
int>' aliases the storage of parameter 'v'
9 | auto it = std::find(std::begin(v), std::end(v), 3); //
expected-warning {{parameter 'v' is later invalidated}} \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
I think these diagnostics could be made clearer, but I'd prefer to address that
in a separate PR.
---
Patch is 38.00 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/208891.diff
5 Files Affected:
- (modified)
clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+8-6)
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+7-3)
- (modified) clang/lib/Sema/SemaLifetimeSafety.h (+10-4)
- (modified) clang/test/Sema/LifetimeSafety/invalidations.cpp (+108-55)
- (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+5-3)
``````````diff
diff --git
a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 80a23f18baebd..f10153c52d70b 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -85,12 +85,14 @@ class LifetimeSafetySemaHelper {
// Reports when a reference/iterator is used after the container operation
// that invalidated it.
- virtual void reportUseAfterInvalidation(const Expr *IssueExpr,
- const Expr *UseExpr,
- const Expr *InvalidationExpr) {}
- virtual void reportUseAfterInvalidation(const ParmVarDecl *PVD,
- const Expr *UseExpr,
- const Expr *InvalidationExpr) {}
+ virtual void
+ reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr,
+ const Expr *InvalidationExpr,
+ llvm::ArrayRef<const Expr *> ExprChain) {}
+ virtual void
+ reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr,
+ const Expr *InvalidationExpr,
+ llvm::ArrayRef<const Expr *> ExprChain) {}
virtual void reportInvalidatedField(const Expr *IssueExpr,
const FieldDecl *Field,
const Expr *InvalidationExpr) {}
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index f72f7f80abbc0..5c90e72909a1a 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -263,12 +263,16 @@ class LifetimeChecker {
if (Warning.InvalidatedByExpr) {
if (IssueExpr)
// Use-after-invalidation of an object on stack.
- SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(),
- Warning.InvalidatedByExpr);
+ SemaHelper->reportUseAfterInvalidation(
+ IssueExpr, UF->getUseExpr(), Warning.InvalidatedByExpr,
+ getExprChain(
+ LoanPropagation.buildOriginFlowChain(UF, LID, Cfg)));
else if (InvalidatedPVD)
// Use-after-invalidation of a parameter.
SemaHelper->reportUseAfterInvalidation(
- InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr);
+ InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr,
+ getExprChain(
+ LoanPropagation.buildOriginFlowChain(UF, LID, Cfg)));
} else
// Scope-based expiry (use-after-scope).
diff --git a/clang/lib/Sema/SemaLifetimeSafety.h
b/clang/lib/Sema/SemaLifetimeSafety.h
index 48edf5f3f070f..c659a9d1c4da5 100644
--- a/clang/lib/Sema/SemaLifetimeSafety.h
+++ b/clang/lib/Sema/SemaLifetimeSafety.h
@@ -188,8 +188,10 @@ class LifetimeSafetySemaHelperImpl : public
LifetimeSafetySemaHelper {
<< DanglingGlobal->getEndLoc();
}
- void reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr,
- const Expr *InvalidationExpr) override {
+ void
+ reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr,
+ const Expr *InvalidationExpr,
+ llvm::ArrayRef<const Expr *> ExprChain) override {
auto WarnDiag = isa<CXXDeleteExpr>(InvalidationExpr)
? diag::warn_lifetime_safety_use_after_free
: diag::warn_lifetime_safety_invalidation;
@@ -197,11 +199,14 @@ class LifetimeSafetySemaHelperImpl : public
LifetimeSafetySemaHelper {
S.Diag(IssueExpr->getExprLoc(), WarnDiag)
<< InvalidatedSubject << IssueExpr->getSourceRange();
reportInvalidationSite(InvalidationExpr, InvalidatedSubject);
+ reportAliasingChain(ExprChain);
S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here)
<< UseExpr->getSourceRange();
}
- void reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr,
- const Expr *InvalidationExpr) override {
+ void
+ reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr,
+ const Expr *InvalidationExpr,
+ llvm::ArrayRef<const Expr *> ExprChain) override {
auto WarnDiag = isa<CXXDeleteExpr>(InvalidationExpr)
? diag::warn_lifetime_safety_use_after_free
@@ -211,6 +216,7 @@ class LifetimeSafetySemaHelperImpl : public
LifetimeSafetySemaHelper {
S.Diag(PVD->getSourceRange().getBegin(), WarnDiag)
<< InvalidatedSubject << PVD->getSourceRange();
reportInvalidationSite(InvalidationExpr, InvalidatedSubject);
+ reportAliasingChain(ExprChain);
S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here)
<< UseExpr->getSourceRange();
}
diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp
b/clang/test/Sema/LifetimeSafety/invalidations.cpp
index be1acc6bc7fbc..127e375bc023c 100644
--- a/clang/test/Sema/LifetimeSafety/invalidations.cpp
+++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp
@@ -7,7 +7,8 @@ bool Bool();
namespace SimpleResize {
void IteratorInvalidAfterResize(int new_size) {
std::vector<int> v;
- auto it = std::begin(v); // expected-warning {{local variable 'v' is later
invalidated}}
+ auto it = std::begin(v); // expected-warning {{local variable 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of local variable 'v'}}
v.resize(new_size); // expected-note {{local variable 'v' is
invalidated here}}
*it; // expected-note {{later used here}}
}
@@ -48,7 +49,8 @@ void PointerToContainerTest(std::vector<int>* v) {
namespace InvalidateBeforeSwap {
void InvalidateBeforeSwapIterators(std::vector<int> v1, std::vector<int> v2) {
- auto it1 = std::begin(v1); // expected-warning {{parameter 'v1' is later
invalidated}}
+ auto it1 = std::begin(v1); // expected-warning {{parameter 'v1' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v1'}}
auto it2 = std::begin(v2);
if (it1 == std::end(v1) || it2 == std::end(v2)) return;
*it1 = 0; // ok
@@ -62,7 +64,8 @@ void InvalidateBeforeSwapIterators(std::vector<int> v1,
std::vector<int> v2) {
}
void InvalidateBeforeSwapContainers(std::vector<int> v1, std::vector<int> v2) {
- auto it1 = std::begin(v1); // expected-warning {{parameter 'v1' is later
invalidated}}
+ auto it1 = std::begin(v1); // expected-warning {{parameter 'v1' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v1'}}
auto it2 = std::begin(v2);
if (it1 == std::end(v1) || it2 == std::end(v2)) return;
*it1 = 0; // ok
@@ -77,7 +80,8 @@ bool A();
bool B();
void SameConditionInvalidatesThenValidatesIterator() {
std::vector<int> container;
- auto it = container.begin(); // expected-warning {{local variable
'container' is later invalidated}}
+ auto it = container.begin(); // expected-warning {{local variable
'container' is later invalidated}} \
+ // expected-note {{result of call to 'begin'
aliases the storage of local variable 'container'}}
if (it == container.end()) return;
const bool a = A();
if (a) {
@@ -108,7 +112,9 @@ void MergeWithDifferentContainerValuesInvalidated() {
std::vector<int> v1, v2, v3;
auto it = std::find(v1.begin(), v1.end(), 10);
if (Bool()) {
- it = std::find(v2.begin(), v2.end(), 10); // expected-warning {{local
variable 'v2' is later invalidated}}
+ it = std::find(v2.begin(), v2.end(), 10); // expected-warning {{local
variable 'v2' is later invalidated}} \
+ // expected-note {{result of
call to 'find<__gnu_cxx::basic_iterator<int>, int>' aliases the storage of
local variable 'v2'}} \
+ // expected-note {{result of
call to 'begin' aliases the storage of local variable 'v2'}}
} else {
it = std::find(v3.begin(), v3.end(), 10);
}
@@ -119,7 +125,8 @@ void MergeWithDifferentContainerValuesInvalidated() {
namespace InvalidationInLoops {
void IteratorInvalidationInAForLoop(std::vector<int> v) {
- for (auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}}
+ for (auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
it != std::end(v);
++it) { // expected-note {{later used here}}
if (Bool()) {
@@ -129,7 +136,8 @@ void IteratorInvalidationInAForLoop(std::vector<int> v) {
}
void IteratorInvalidationInAWhileLoop(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}}
+ auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
while (it != std::end(v)) {
if (Bool()) {
v.erase(it); // expected-note {{parameter 'v' is invalidated here}}
@@ -155,7 +163,8 @@ void
NoIteratorInvalidationInAWhileLoopErase(std::unordered_map<int, int> mp) {
void IteratorInvalidationInAForeachLoop(std::vector<int> v) {
for (int& x : v) { // expected-warning {{parameter 'v' is later
invalidated}} \
- // expected-note {{later used here}}
+ // expected-note {{later used here}} \
+ // expected-note {{result of call to 'end' aliases the
storage of parameter 'v'}}
if (x % 2 == 0) {
v.erase(std::find(v.begin(), v.end(), 1)); // expected-note {{parameter
'v' is invalidated here}}
}
@@ -183,7 +192,9 @@ void IteratorCheckedAfterFind(std::vector<int> v) {
}
void IteratorCheckedAfterFindThenErased(std::vector<int> v) {
- auto it = std::find(std::begin(v), std::end(v), 3); // expected-warning
{{parameter 'v' is later invalidated}}
+ auto it = std::find(std::begin(v), std::end(v), 3); // expected-warning
{{parameter 'v' is later invalidated}} \
+ // expected-note
{{result of call to 'find<__gnu_cxx::basic_iterator<int>, int>' aliases the
storage of parameter 'v'}} \
+ // expected-note
{{result of call to 'begin<std::vector<int>>' aliases the storage of parameter
'v'}}
if (it != std::end(v)) {
v.erase(it); // expected-note {{parameter 'v' is invalidated here}}
}
@@ -201,7 +212,8 @@ void UseReturnedIteratorAfterInsert(std::vector<int> v) {
}
void UseInvalidIteratorAfterInsert(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}}
+ auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
v.insert(it, 10); // expected-note {{parameter 'v' is invalidated
here}}
if (it != std::end(v)) { // expected-note {{later used here}}
*it;
@@ -220,7 +232,8 @@ void IteratorValidAfterInsert(std::vector<int> v) {
}
void IteratorInvalidAfterInsert(std::vector<int> v, int value) {
- auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}}
+ auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
v.insert(it, 0); // expected-note {{parameter 'v' is invalidated
here}}
*it; // expected-note {{later used here}}
}
@@ -228,7 +241,8 @@ void IteratorInvalidAfterInsert(std::vector<int> v, int
value) {
namespace SimpleInvalidIterators {
void IteratorUsedAfterErase(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{parameter 'v' is
later invalidated}}
+ auto it = std::begin(v); // expected-warning {{parameter 'v' is
later invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
for (; it != std::end(v); ++it) { // expected-note {{later used here}}
if (*it > 3) {
v.erase(it); // expected-note {{parameter 'v' is
invalidated here}}
@@ -245,7 +259,8 @@ void IteratorUsedAfterPushBackParam(std::vector<int>& v) {
// expected-warning {
}
void IteratorUsedAfterPushBack(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}}
+ auto it = std::begin(v); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of parameter 'v'}}
if (it != std::end(v) && *it == 3) {
v.push_back(4); // expected-note {{parameter 'v' is invalidated here}}
}
@@ -254,59 +269,70 @@ void IteratorUsedAfterPushBack(std::vector<int> v) {
void IteratorUsedAfterPreIncrement() {
std::vector<int> v;
- auto it = v.begin(); // expected-warning {{local variable 'v' is later
invalidated}}
- auto next = ++it;
+ auto it = v.begin(); // expected-warning {{local variable 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'begin'
aliases the storage of local variable 'v'}}
+ auto next = ++it; // expected-note {{expression aliases the storage
of local variable 'v'}}
v.push_back(1); // expected-note {{local variable 'v' is
invalidated here}}
(void)*next; // expected-note {{later used here}}
}
void IteratorUsedAfterPostDecrement(std::vector<int> v) {
- auto it = v.rbegin(); // expected-warning {{parameter 'v' is later
invalidated}}
- auto prev = it--;
+ auto it = v.rbegin(); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'rbegin'
aliases the storage of parameter 'v'}}
+ auto prev = it--; // expected-note {{expression aliases the storage
of parameter 'v'}}
v.push_back(1); // expected-note {{parameter 'v' is invalidated
here}}
(void)*prev; // expected-note {{later used here}}
}
void IteratorUsedAfterAddition() {
std::vector<int> v;
- auto it = v.cbegin(); // expected-warning {{local variable 'v' is later
invalidated}}
- auto next = it + 5;
+ auto it = v.cbegin(); // expected-warning {{local variable 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'cbegin'
aliases the storage of local variable 'v'}}
+ auto next = it + 5; // expected-note {{expression aliases the storage
of local variable 'v'}}
v.push_back(1); // expected-note {{local variable 'v' is
invalidated here}}
(void)*next; // expected-note {{later used here}}
}
void IteratorUsedAfterReverseSubtraction(std::vector<int> v) {
- auto it = v.crbegin(); // expected-warning {{parameter 'v' is later
invalidated}}
- auto prev = 5 - it;
+ auto it = v.crbegin(); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'crbegin'
aliases the storage of parameter 'v'}}
+ auto prev = 5 - it; // expected-note {{local variable 'it' aliases the
storage of parameter 'v'}} \
+ // expected-note {{expression aliases the storage
of parameter 'v'}}
v.push_back(1); // expected-note {{parameter 'v' is invalidated
here}}
(void)*prev; // expected-note {{later used here}}
}
void IteratorUsedAfterAddAdd(std::vector<int> v) {
- auto it = v.cbegin(); // expected-warning {{parameter 'v' is later
invalidated}}
- auto next = (it + 5) + 5;
+ auto it = v.cbegin(); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'cbegin'
aliases the storage of parameter 'v'}}
+ auto next = (it + 5) + 5; // expected-note 2 {{expression aliases the
storage of parameter 'v'}}
v.push_back(1); // expected-note {{parameter 'v' is invalidated
here}}
(void)*next; // expected-note {{later used here}}
}
void IteratorUsedAfterMixedAddition() {
std::vector<int> v;
- auto it = v.cbegin(); // expected-warning {{local variable 'v' is
later invalidated}}
- auto next = 1 + it + 2 + 3;
+ auto it = v.cbegin(); // expected-warning {{local variable 'v' is
later invalidated}} \
+ // expected-note {{result of call to 'cbegin'
aliases the storage of local variable 'v'}}
+ auto next = 1 + it + 2 + 3; // expected-note 3 {{expression aliases the
storage of local variable 'v'}} \
+ // expected-note {{local variable 'it' aliases
the storage of local variable 'v'}}
v.push_back(1); // expected-note {{local variable 'v' is
invalidated here}}
(void)*next; // expected-note {{later used here}}
}
void IteratorUsedAfterPreIncrementAddAssign(std::vector<int> v) {
- auto it = v.begin(); // expected-warning {{parameter 'v' is later
invalidated}}
- it = ++it + 1 + 2;
+ auto it = v.begin(); // expected-warning {{parameter 'v' is later
invalidated}} \
+ // expected-note {{result of call to 'begin'
aliases the storage of parameter 'v'}}
+ it = ++it + 1 + 2; // expected-note 3 {{expression aliases the
storage of parameter 'v'}}
v.push_back(1); // expected-note {{parameter 'v' is
invalidated here}}
(void)*it; // expected-note {{later used here}}
}
void IteratorUsedAfterBeginAddAssign() {
std::vector<int> v;
- auto it = v.begin() + 1; // expected-warning {{local variable 'v' is
later invalidated}}
+ auto it = v.begin() + 1; // expected-warning {{local variable 'v' is
later invalidated}} \
+ // expected-note {{expression aliases the
storage of local variable 'v'}} \
+ // expected-note {{result of call to 'begin'
aliases the storage of local variable 'v'}}
v.push_back(1); // expected-note {{local variable 'v' is
invalidated here}}
(void)*it; // expected-note {{later used here}}
}
@@ -314,7 +340,9 @@ void IteratorUsedAfterBeginAddAssign() {
void IteratorUsedAfterStdBeginAddAssign() {
std::vector<int> v;
std::vector<int>::iterator it;
- it = std::begin(v) + 1; // expected-warning {{local variable 'v' is
later invalidated}}
+ it = std::begin(v) + 1; // expected-warning {{local variable 'v' is
later invalidated}} \
+ // expected-note {{expression aliases the
storage of local variable 'v'}} \
+ // expected-note {{result of call to
'begin<std::vector<int>>' aliases the storage of local variable 'v'}}
v.push_back(1); // expected-note {{local variable 'v' is
invalidated here}}
(void)*it; // expected-note {{later used here}}
}
@@ -324,13 +352,14 @@ namespace InvalidatingThroughContainerAliases {
void IteratorInvalidatedThroughLocalReferenceAlias() {
std::vector<int> vv;
std::vector<int> &v = vv;
- auto it = vv.begin(); // expected-warning {{local variable 'vv' is later
invalidated}}
+ auto it = vv.begin(); // expected-warning {{local variable 'vv' is later
invalidated}} \
+ // expected-note {{result of call to 'begin' aliases
the storage of local variable 'vv'}}
v.push_back(42); // expected-note {{local variable 'vv' is invalidate...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/208891
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
