llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Yuan Suo (suoyuan666)
<details>
<summary>Changes</summary>
After introducing `buildOriginFlowChain` to use-after-scope diagnostics, it
should support multi-block analysis. This also allows it to be reused by other
diagnostics.
In some loops, `UseFact` may appear before `OriginFlowFact`:
```cpp
void for_loop_use_before_loop_body(MyObj safe) {
MyObj* p = &safe;
for (int i = 0; i < 1; ++i) {
(void)*p;
MyObj s;
p = &s;
}
(void)*p;
}
```
So, I remove the `StartPoint` parameter to find the `OriginID` correctly.
---
Full diff: https://github.com/llvm/llvm-project/pull/204592.diff
9 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+1)
- (modified)
clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h (+5-2)
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+4-2)
- (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+10-5)
- (modified) clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp (+75-24)
- (modified) clang/lib/Sema/SemaLifetimeSafety.h (+2-2)
- (modified) clang/test/Sema/LifetimeSafety/safety-c.c (+3-1)
- (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+23-10)
- (modified) clang/unittests/Analysis/LifetimeSafetyTest.cpp (+2-1)
``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index 88b509e1b94df..e0ddfe95b00b2 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -374,6 +374,7 @@ class FactManager {
/// Retrieves all the facts in the block containing Program Point P.
/// \note This is intended for testing only.
llvm::ArrayRef<const Fact *> getBlockContaining(ProgramPoint P) const;
+ std::optional<size_t> getBlockID(ProgramPoint P) const;
unsigned getNumFacts() const { return NextFactID.Value; }
diff --git
a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h
index 724c6eee7d3c2..42d7df0838f35 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h
@@ -16,6 +16,7 @@
#define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOAN_PROPAGATION_H
#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h"
+#include "clang/Analysis/Analyses/PostOrderCFGView.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "llvm/ADT/ImmutableMap.h"
@@ -46,10 +47,12 @@ class LoanPropagationAnalysis {
/// where the loan was originally issued.
llvm::SmallVector<OriginID>
buildOriginFlowChain(ProgramPoint StartPoint, const OriginID StartOID,
- const LoanID TargetLoan) const;
+ const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const;
llvm::SmallVector<OriginID>
- buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan) const;
+ buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const;
private:
class Impl;
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index d41d6f43f837b..2fe52585087e4 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -68,6 +68,7 @@ class LifetimeChecker {
LifetimeSafetySemaHelper *SemaHelper;
ASTContext &AST;
const Decl *FD;
+ const PostOrderCFGView *POV;
static SourceLocation
GetFactLoc(llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> F)
{
@@ -90,7 +91,8 @@ class LifetimeChecker {
LifetimeSafetySemaHelper *SemaHelper)
: LoanPropagation(LoanPropagation), MovedLoans(MovedLoans),
LiveOrigins(LiveOrigins), FactMgr(FM), SemaHelper(SemaHelper),
- AST(ADC.getASTContext()), FD(ADC.getDecl()) {
+ AST(ADC.getASTContext()), FD(ADC.getDecl()),
+ POV(ADC.getAnalysis<PostOrderCFGView>()) {
for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
for (const Fact *F : FactMgr.getFacts(B))
if (const auto *EF = F->getAs<ExpireFact>())
@@ -268,7 +270,7 @@ class LifetimeChecker {
// Scope-based expiry (use-after-scope).
SemaHelper->reportUseAfterScope(
IssueExpr, UF->getUseExpr(), MovedExpr, ExpiryLoc,
- getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID)));
+ getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID,
POV)));
} else if (const auto *OEF =
CausingFact.dyn_cast<const OriginEscapesFact *>()) {
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index 3d7fbcdacc830..823f1f686f904 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -145,12 +145,17 @@ void FactManager::dump(const CFG &Cfg,
AnalysisDeclContext &AC) const {
llvm::ArrayRef<const Fact *>
FactManager::getBlockContaining(ProgramPoint P) const {
- for (const auto &BlockToFactsVec : BlockToFacts) {
- for (const Fact *F : BlockToFactsVec)
- if (F == P)
- return BlockToFactsVec;
- }
+ std::optional<size_t> BlockIndex = getBlockID(P);
+ if (BlockIndex)
+ return BlockToFacts[BlockIndex.value()];
return {};
}
+std::optional<size_t> FactManager::getBlockID(ProgramPoint P) const {
+ for (size_t i = 0; i < BlockToFacts.size(); ++i)
+ for (const Fact *F : BlockToFacts[i])
+ if (F == P)
+ return i;
+ return std::nullopt;
+}
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
index a67b1b3c0f826..8e2d9a6577427 100644
--- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
@@ -130,6 +130,11 @@ struct Lattice {
}
};
+struct BuildOriginFlowChainResult {
+ const llvm::SmallVector<OriginID> OriginFlowChain;
+ const bool Complete;
+};
+
class AnalysisImpl
: public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Forward> {
public:
@@ -204,22 +209,72 @@ class AnalysisImpl
llvm::SmallVector<OriginID>
buildOriginFlowChain(ProgramPoint StartPoint, const OriginID StartOID,
- const LoanID TargetLoan) const {
+ const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const {
assert(getLoans(StartOID, StartPoint).contains(TargetLoan) &&
"TargetLoan must be present in the StartOID at the StartPoint");
+ std::optional<OriginID> FinalOID;
+ llvm::DenseMap<OriginID, OriginID> VistedOriginIDs;
+
+ const auto OriginFlowChainFilter = [&VistedOriginIDs](OriginID FinalOID) {
+ llvm::SmallVector<OriginID> OriginFlowChain;
+ while (true) {
+ OriginFlowChain.push_back(FinalOID);
+ const auto NextOriginID = VistedOriginIDs.find(FinalOID);
+ if (NextOriginID == VistedOriginIDs.end())
+ break;
+ FinalOID = NextOriginID->second;
+ }
+ return OriginFlowChain;
+ };
+
+ const auto InsertVistedOriginIDs =
+ [&VistedOriginIDs, &FinalOID](llvm::ArrayRef<OriginID> OriginFlowChain,
+ OriginID &StartOID) {
+ if (!VistedOriginIDs.empty())
+ VistedOriginIDs.insert({OriginFlowChain[0], StartOID});
+
+ for (size_t i = 0; i < OriginFlowChain.size() - 1; ++i)
+ VistedOriginIDs.insert(
+ {OriginFlowChain[i + 1], OriginFlowChain[i]});
+
+ StartOID = OriginFlowChain.back();
+ FinalOID = StartOID;
+ };
+
+ std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint);
+ assert(BlockID.has_value());
+ const auto StartIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block)
{
+ return Block->getBlockID() == BlockID;
+ });
+
+ OriginID CurrOID = StartOID;
+ for (const CFGBlock *B :
+ llvm::reverse(llvm::make_range(POV->begin(), StartIt + 1))) {
+ BuildOriginFlowChainResult BuildResult =
+ buildOriginFlowChain(B, CurrOID, TargetLoan);
+ if (!BuildResult.OriginFlowChain.empty())
+ InsertVistedOriginIDs(BuildResult.OriginFlowChain, CurrOID);
+ if (BuildResult.Complete)
+ return OriginFlowChainFilter(*FinalOID);
+ }
+
+ llvm_unreachable(
+ "buildOriginFlowChain should return at BuildResult.Complete");
+ }
+
+ BuildOriginFlowChainResult
+ buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID,
+ const LoanID TargetLoan) const {
OriginID CurrOID = StartOID;
llvm::SmallVector<OriginID> OriginFlowChain;
- llvm::ArrayRef<const Fact *> Facts =
FactMgr.getBlockContaining(StartPoint);
- const auto *StartIt = llvm::find(Facts, StartPoint);
- assert(StartIt != Facts.end());
- for (const Fact *F :
- llvm::reverse(llvm::make_range(Facts.begin(), StartIt))) {
+ for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) {
if (const auto *IF = F->getAs<IssueFact>())
if (IF->getLoanID() == TargetLoan) {
assert(IF->getOriginID() == CurrOID);
- return OriginFlowChain;
+ return {OriginFlowChain, true};
}
const auto *OFF = F->getAs<OriginFlowFact>();
@@ -235,20 +290,17 @@ class AnalysisImpl
CurrOID = SrcOriginID;
}
- // FIXME: Ideally, this return is unreachable and should be an assert
- // because we expect to always finish at an IssueFact. But since current
- // traversal is limited to a single CFG block, multi-block OriginFlowChain
- // construction might miss the IssueFact. We should add llvm_unreachable
- // here once multi-block support is implemented.
- return {};
+ return {OriginFlowChain, false};
}
llvm::SmallVector<OriginID>
- buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan) const {
+ buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const {
for (const OriginList *Cur = UF->getUsedOrigins(); Cur;
Cur = Cur->peelOuterOrigin())
if (getLoans(Cur->getOuterOriginID(), UF).contains(TargetLoan))
- return buildOriginFlowChain(UF, Cur->getOuterOriginID(), TargetLoan);
+ return buildOriginFlowChain(UF, Cur->getOuterOriginID(), TargetLoan,
+ POV);
return {};
}
@@ -303,16 +355,15 @@ LoanSet LoanPropagationAnalysis::getLoans(OriginID OID,
ProgramPoint P) const {
return PImpl->getLoans(OID, P);
}
-llvm::SmallVector<OriginID>
-LoanPropagationAnalysis::buildOriginFlowChain(ProgramPoint StartPoint,
- const OriginID StartOID,
- const LoanID TargetLoan) const {
- return PImpl->buildOriginFlowChain(StartPoint, StartOID, TargetLoan);
+llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain(
+ ProgramPoint StartPoint, const OriginID StartOID, const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const {
+ return PImpl->buildOriginFlowChain(StartPoint, StartOID, TargetLoan, POV);
}
-llvm::SmallVector<OriginID>
-LoanPropagationAnalysis::buildOriginFlowChain(const UseFact *UF,
- const LoanID TargetLoan) const {
- return PImpl->buildOriginFlowChain(UF, TargetLoan);
+llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain(
+ const UseFact *UF, const LoanID TargetLoan,
+ const PostOrderCFGView *POV) const {
+ return PImpl->buildOriginFlowChain(UF, TargetLoan, POV);
}
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Sema/SemaLifetimeSafety.h
b/clang/lib/Sema/SemaLifetimeSafety.h
index a8bde363e3397..f4649019ceb1d 100644
--- a/clang/lib/Sema/SemaLifetimeSafety.h
+++ b/clang/lib/Sema/SemaLifetimeSafety.h
@@ -550,10 +550,10 @@ class LifetimeSafetySemaHelperImpl : public
LifetimeSafetySemaHelper {
if (OriginExprChain.empty())
return;
- const Expr *LastExpr = OriginExprChain.back();
+ const Expr *LastExpr = OriginExprChain.front();
std::string IssueStr = getDiagSubjectDescription(LastExpr);
- for (const Expr *CurrExpr : reverse(OriginExprChain.drop_back())) {
+ for (const Expr *CurrExpr : OriginExprChain.drop_front()) {
if (!shouldShowInAliasChain(CurrExpr, LastExpr))
continue;
S.Diag(CurrExpr->getBeginLoc(),
diff --git a/clang/test/Sema/LifetimeSafety/safety-c.c
b/clang/test/Sema/LifetimeSafety/safety-c.c
index 95c8cf7bb00c7..c87a9b4768f2c 100644
--- a/clang/test/Sema/LifetimeSafety/safety-c.c
+++ b/clang/test/Sema/LifetimeSafety/safety-c.c
@@ -93,7 +93,9 @@ void conditional_operator_lifetimebound(int cond) {
int *p;
{
int a, b;
- p = identity(cond ? &a // expected-warning {{local variable 'a' does
not live long enough}}
+ p = identity(cond ? &a // expected-warning {{local variable 'a' does
not live long enough}} \
+ // expected-note {{result of call to 'identity'
aliases the storage of local variable 'a'}} \
+ // expected-note {{result of call to 'identity'
aliases the storage of local variable 'b'}}
: &b); // expected-warning {{local variable 'b' does
not live long enough}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp
b/clang/test/Sema/LifetimeSafety/safety.cpp
index 6fc275b51a9d0..51b042afa8760 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -236,7 +236,7 @@ void overrides_potential(bool cond) {
{
MyObj s;
q = &s; // expected-warning {{does not live long enough}}
- p = q;
+ p = q; // expected-note {{local variable 'q' aliases the storage of
local variable 's'}}
} // expected-note {{destroyed here}}
if (cond) {
@@ -448,7 +448,7 @@ void loan_from_previous_iteration(MyObj safe, bool
condition) {
p = &x; // expected-warning {{does not live long enough}}
if (condition)
- q = p;
+ q = p; // expected-note {{local variable 'p' aliases the storage of
local variable 'x'}}
(void)*p;
(void)*q; // expected-note {{later used here}}
} // expected-note {{destroyed here}}
@@ -845,7 +845,8 @@ void lifetimebound_multiple_args_potential(bool cond) {
MyObj obj1;
if (cond) {
MyObj obj2;
- v = Choose(true,
+ v = Choose(true, // expected-note {{result of call to
'Choose' aliases the storage of local variable 'obj1'}} \
+ // expected-note {{result of call to
'Choose' aliases the storage of local variable 'obj2'}}
obj1, // expected-warning {{local variable 'obj1'
does not live long enough}}
obj2); // expected-warning {{local variable 'obj2'
does not live long enough}}
} // expected-note {{destroyed here}}
@@ -939,7 +940,7 @@ void lifetimebound_partial_safety(bool cond) {
if (cond) {
MyObj temp_obj;
- v = Choose(true,
+ v = Choose(true, // expected-note {{result of call to 'Choose'
aliases the storage of local variable 'temp_obj'}}
safe_obj,
temp_obj); // expected-warning {{local variable 'temp_obj' does
not live long enough}}
} // expected-note {{destroyed here}}
@@ -1222,7 +1223,9 @@ void conditional_operator_lifetimebound(bool cond) {
MyObj* p;
{
MyObj a, b;
- p = Identity(cond ? &a // expected-warning {{local variable 'a' does
not live long enough}}
+ p = Identity(cond ? &a // expected-warning {{local variable 'a' does
not live long enough}} \
+ // expected-note {{result of call to 'Identity'
aliases the storage of local variable 'a'}} \
+ // expected-note {{result of call to 'Identity'
aliases the storage of local variable 'b'}}
: &b); // expected-warning {{local variable 'b' does
not live long enough}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
@@ -1232,8 +1235,11 @@ void conditional_operator_lifetimebound_nested(bool
cond) {
MyObj* p;
{
MyObj a, b;
- p = Identity(cond ? Identity(&a) // expected-warning {{local variable
'a' does not live long enough}}
- : Identity(&b)); // expected-warning {{local variable
'b' does not live long enough}}
+ p = Identity(cond ? Identity(&a) // expected-warning {{local variable
'a' does not live long enough}} \
+ // expected-note 2 {{result of call to
'Identity' aliases the storage of local variable 'a'}} \
+ // expected-note {{result of call to
'Identity' aliases the storage of local variable 'b'}}
+ : Identity(&b)); // expected-warning {{local variable
'b' does not live long enough}} \
+ // expected-note {{result of call to
'Identity' aliases the storage of local variable 'b'}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
}
@@ -1242,9 +1248,15 @@ void conditional_operator_lifetimebound_nested_deep(bool
cond) {
MyObj* p;
{
MyObj a, b, c, d;
- p = Identity(cond ? Identity(cond ? &a // expected-warning {{local
variable 'a' does not live long enough}}
+ p = Identity(cond ? Identity(cond ? &a // expected-warning {{local
variable 'a' does not live long enough}} \
+ // expected-note 2 {{result of
call to 'Identity' aliases the storage of local variable 'a'}} \
+ // expected-note 2 {{result of
call to 'Identity' aliases the storage of local variable 'b'}} \
+ // expected-note {{result of
call to 'Identity' aliases the storage of local variable 'c'}} \
+ // expected-note {{result of
call to 'Identity' aliases the storage of local variable 'd'}}
: &b) // expected-warning {{local
variable 'b' does not live long enough}}
- : Identity(cond ? &c // expected-warning {{local
variable 'c' does not live long enough}}
+ : Identity(cond ? &c // expected-warning {{local
variable 'c' does not live long enough}} \
+ // expected-note {{result of
call to 'Identity' aliases the storage of local variable 'c'}} \
+ // expected-note {{result of
call to 'Identity' aliases the storage of local variable 'd'}}
: &d)); // expected-warning {{local
variable 'd' does not live long enough}}
} // expected-note 4 {{destroyed here}}
(void)*p; // expected-note 4 {{later used here}}
@@ -1428,7 +1440,8 @@ void range_based_for_use_after_scope() {
View v;
{
MyObjStorage s;
- for (const MyObj &o : s) { // expected-warning {{local variable 's' does
not live long enough}}
+ for (const MyObj &o : s) { // expected-warning {{local variable 's' does
not live long enough}} \
+ // expected-note {{local variable '__range2'
aliases the storage of local variable 's'}}
v = o;
}
} // expected-note {{destroyed here}}
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index 78b7449958140..8d5ed9c88dcff 100644
--- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
+++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
@@ -215,7 +215,8 @@ class LifetimeTestHelper {
for (LoanID LID : EndLoanIDs) {
llvm::SmallVector<OriginID> OriginFlowChain =
Runner.getAnalysis().getLoanPropagation().buildOriginFlowChain(
- getProgramPoint(Annotation), *StartOriginID, LID);
+ getProgramPoint(Annotation), *StartOriginID, LID,
+ Runner.getAnalysisContext().getAnalysis<PostOrderCFGView>());
if (!OriginFlowChain.empty())
return OriginFlowChain;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/204592
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
