-
Notifications
You must be signed in to change notification settings - Fork 15k
[LifetimeSafety] Introduce a liveness-based lifetime policy #159991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
23c0690 to
071c85e
Compare
076766a to
1c079e3
Compare
071c85e to
5cfb70c
Compare
1c079e3 to
031309f
Compare
7d3188c to
c859c7e
Compare
c859c7e to
fc4c118
Compare
031309f to
4e63e10
Compare
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Utkarsh Saxena (usx95) ChangesThis PR replaces the forward
The Fixes: #156959 More details describing the design flaw in using Patch is 50.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159991.diff 4 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
index 512cb76cd6349..2cc3fb3d69eb4 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
@@ -55,6 +55,7 @@ class Fact;
class FactManager;
class LoanPropagationAnalysis;
class ExpiredLoansAnalysis;
+class LiveOriginAnalysis;
struct LifetimeFactory;
/// A generic, type-safe wrapper for an ID, distinguished by its `Tag` type.
@@ -89,6 +90,7 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, OriginID ID) {
// TODO(opt): Consider using a bitset to represent the set of loans.
using LoanSet = llvm::ImmutableSet<LoanID>;
using OriginSet = llvm::ImmutableSet<OriginID>;
+using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
/// A `ProgramPoint` identifies a location in the CFG by pointing to a specific
/// `Fact`. identified by a lifetime-related event (`Fact`).
@@ -110,8 +112,9 @@ class LifetimeSafetyAnalysis {
/// Returns the set of loans an origin holds at a specific program point.
LoanSet getLoansAtPoint(OriginID OID, ProgramPoint PP) const;
- /// Returns the set of loans that have expired at a specific program point.
- std::vector<LoanID> getExpiredLoansAtPoint(ProgramPoint PP) const;
+ /// TODO:Document.
+ std::vector<std::pair<OriginID, Confidence>>
+ getLiveOriginsAtPoint(ProgramPoint PP) const;
/// Finds the OriginID for a given declaration.
/// Returns a null optional if not found.
@@ -138,7 +141,7 @@ class LifetimeSafetyAnalysis {
std::unique_ptr<LifetimeFactory> Factory;
std::unique_ptr<FactManager> FactMgr;
std::unique_ptr<LoanPropagationAnalysis> LoanPropagation;
- std::unique_ptr<ExpiredLoansAnalysis> ExpiredLoans;
+ std::unique_ptr<LiveOriginAnalysis> LiveOrigins;
};
} // namespace internal
} // namespace clang::lifetimes
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index a90aa72797e2f..7faf97da60427 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -21,6 +21,7 @@
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
#include <cstdint>
#include <memory>
@@ -901,19 +902,26 @@ class DataflowAnalysis {
llvm::SmallBitVector Visited(Cfg.getNumBlockIDs() + 1);
while (const CFGBlock *B = W.dequeue()) {
- Lattice StateIn = getInState(B);
+ Lattice StateIn = *getInState(B);
Lattice StateOut = transferBlock(B, StateIn);
OutStates[B] = StateOut;
- Visited.set(B->getBlockID());
for (const CFGBlock *AdjacentB : isForward() ? B->succs() : B->preds()) {
if (!AdjacentB)
continue;
- Lattice OldInState = getInState(AdjacentB);
- Lattice NewInState = D.join(OldInState, StateOut);
+ Lattice OldInState;
+ bool SawFirstTime = false;
+ Lattice NewInState;
+ if (const Lattice *In = getInState(AdjacentB)) {
+ OldInState = *In;
+ NewInState = D.join(OldInState, StateOut);
+ } else {
+ OldInState = D.getInitialState();
+ SawFirstTime = true;
+ NewInState = StateOut;
+ }
// Enqueue the adjacent block if its in-state has changed or if we have
// never visited it.
- if (!Visited.test(AdjacentB->getBlockID()) ||
- NewInState != OldInState) {
+ if (SawFirstTime || NewInState != OldInState) {
InStates[AdjacentB] = NewInState;
W.enqueueBlock(AdjacentB);
}
@@ -924,7 +932,12 @@ class DataflowAnalysis {
protected:
Lattice getState(ProgramPoint P) const { return PerPointStates.lookup(P); }
- Lattice getInState(const CFGBlock *B) const { return InStates.lookup(B); }
+ const Lattice *getInState(const CFGBlock *B) const {
+ auto It = InStates.find(B);
+ if (It != InStates.end())
+ return &It->second;
+ return nullptr;
+ }
Lattice getOutState(const CFGBlock *B) const { return OutStates.lookup(B); }
@@ -1023,22 +1036,26 @@ static bool isSubsetOf(const llvm::ImmutableSet<T> &A,
// instead of the current AVL-tree-based ImmutableMap.
template <typename K, typename V, typename Joiner>
static llvm::ImmutableMap<K, V>
-join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
+join(const llvm::ImmutableMap<K, V> &A, const llvm::ImmutableMap<K, V> &B,
typename llvm::ImmutableMap<K, V>::Factory &F, Joiner JoinValues) {
if (A.getHeight() < B.getHeight())
- std::swap(A, B);
+ return join(B, A, F, JoinValues);
// For each element in B, join it with the corresponding element in A
// (or with an empty value if it doesn't exist in A).
+ llvm::ImmutableMap<K, V> Res = A;
for (const auto &Entry : B) {
const K &Key = Entry.first;
const V &ValB = Entry.second;
- if (const V *ValA = A.lookup(Key))
- A = F.add(A, Key, JoinValues(*ValA, ValB));
- else
- A = F.add(A, Key, ValB);
+ Res = F.add(Res, Key, JoinValues(A.lookup(Key), &ValB));
}
- return A;
+ for (const auto &Entry : A) {
+ const K &Key = Entry.first;
+ const V &ValA = Entry.second;
+ if (!B.contains(Key))
+ Res = F.add(Res, Key, JoinValues(&ValA, nullptr));
+ }
+ return Res;
}
} // namespace utils
@@ -1046,19 +1063,6 @@ join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
// Loan Propagation Analysis
// ========================================================================= //
-using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
-using ExpiredLoanMap = llvm::ImmutableMap<LoanID, const ExpireFact *>;
-
-/// An object to hold the factories for immutable collections, ensuring
-/// that all created states share the same underlying memory management.
-struct LifetimeFactory {
- llvm::BumpPtrAllocator Allocator;
- OriginLoanMap::Factory OriginMapFactory{Allocator, /*canonicalize=*/false};
- LoanSet::Factory LoanSetFactory{Allocator, /*canonicalize=*/false};
- ExpiredLoanMap::Factory ExpiredLoanMapFactory{Allocator,
- /*canonicalize=*/false};
-};
-
/// Represents the dataflow lattice for loan propagation.
///
/// This lattice tracks which loans each origin may hold at a given program
@@ -1102,10 +1106,10 @@ class LoanPropagationAnalysis
public:
LoanPropagationAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &LFactory)
- : DataflowAnalysis(C, AC, F),
- OriginLoanMapFactory(LFactory.OriginMapFactory),
- LoanSetFactory(LFactory.LoanSetFactory) {}
+ OriginLoanMap::Factory &OriginLoanMapFactory,
+ LoanSet::Factory &LoanSetFactory)
+ : DataflowAnalysis(C, AC, F), OriginLoanMapFactory(OriginLoanMapFactory),
+ LoanSetFactory(LoanSetFactory) {}
using Base::transfer;
@@ -1118,8 +1122,13 @@ class LoanPropagationAnalysis
Lattice join(Lattice A, Lattice B) {
OriginLoanMap JoinedOrigins =
utils::join(A.Origins, B.Origins, OriginLoanMapFactory,
- [&](LoanSet S1, LoanSet S2) {
- return utils::join(S1, S2, LoanSetFactory);
+ [&](const LoanSet *S1, const LoanSet *S2) {
+ assert((S1 || S2) && "unexpectedly merging 2 empty sets");
+ if (!S1)
+ return *S2;
+ if (!S2)
+ return *S1;
+ return utils::join(*S1, *S2, LoanSetFactory);
});
return Lattice(JoinedOrigins);
}
@@ -1169,96 +1178,145 @@ class LoanPropagationAnalysis
};
// ========================================================================= //
-// Expired Loans Analysis
+// Live Origins Analysis
// ========================================================================= //
-/// The dataflow lattice for tracking the set of expired loans.
-struct ExpiredLattice {
- /// Map from an expired `LoanID` to the `ExpireFact` that made it expire.
- ExpiredLoanMap Expired;
+/// Information about why an origin is live at a program point.
+struct LivenessInfo {
+ // TODO: Doc.
+ const UseFact *CausingUseFact;
+ // TODO: Doc.
+ Confidence ConfidenceLevel;
+
+ LivenessInfo() : CausingUseFact(nullptr), ConfidenceLevel(Confidence::None) {}
+ LivenessInfo(const UseFact *UF, Confidence C)
+ : CausingUseFact(UF), ConfidenceLevel(C) {}
- ExpiredLattice() : Expired(nullptr) {};
- explicit ExpiredLattice(ExpiredLoanMap M) : Expired(M) {}
+ bool operator==(const LivenessInfo &Other) const {
+ return CausingUseFact == Other.CausingUseFact &&
+ ConfidenceLevel == Other.ConfidenceLevel;
+ }
+ bool operator!=(const LivenessInfo &Other) const { return !(*this == Other); }
+
+ void Profile(llvm::FoldingSetNodeID &IDBuilder) const {
+ IDBuilder.AddPointer(CausingUseFact);
+ IDBuilder.Add(ConfidenceLevel);
+ }
+};
+
+using LivenessMap = llvm::ImmutableMap<OriginID, LivenessInfo>;
- bool operator==(const ExpiredLattice &Other) const {
- return Expired == Other.Expired;
+/// The dataflow lattice for origin liveness analysis.
+/// It tracks which origins are live, why they're live (which UseFact),
+/// and the confidence level of that liveness.
+struct LivenessLattice {
+ LivenessMap LiveOrigins;
+
+ LivenessLattice() : LiveOrigins(nullptr) {};
+
+ explicit LivenessLattice(LivenessMap L) : LiveOrigins(L) {}
+
+ bool operator==(const LivenessLattice &Other) const {
+ return LiveOrigins == Other.LiveOrigins;
}
- bool operator!=(const ExpiredLattice &Other) const {
+
+ bool operator!=(const LivenessLattice &Other) const {
return !(*this == Other);
}
void dump(llvm::raw_ostream &OS) const {
- OS << "ExpiredLattice State:\n";
- if (Expired.isEmpty())
+ OS << "LivenessLattice State:\n";
+ if (LiveOrigins.isEmpty())
OS << " <empty>\n";
- for (const auto &[ID, _] : Expired)
- OS << " Loan " << ID << " is expired\n";
+ for (const auto &Entry : LiveOrigins) {
+ OriginID OID = Entry.first;
+ const LivenessInfo &Info = Entry.second;
+ OS << " Origin " << OID << " is ";
+ switch (Info.ConfidenceLevel) {
+ case Confidence::Definite:
+ OS << "definitely";
+ break;
+ case Confidence::Maybe:
+ OS << "maybe";
+ break;
+ case Confidence::None:
+ llvm_unreachable("liveness condidence should not be none.");
+ }
+ OS << " live at this point\n";
+ }
}
};
-/// The analysis that tracks which loans have expired.
-class ExpiredLoansAnalysis
- : public DataflowAnalysis<ExpiredLoansAnalysis, ExpiredLattice,
- Direction::Forward> {
-
- ExpiredLoanMap::Factory &Factory;
+/// The analysis that tracks which origins are live, with granular information
+/// about the causing use fact and confidence level. This is a backward
+/// analysis.
+class LiveOriginAnalysis
+ : public DataflowAnalysis<LiveOriginAnalysis, LivenessLattice,
+ Direction::Backward> {
+ FactManager &FactMgr;
+ LivenessMap::Factory &Factory;
public:
- ExpiredLoansAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &Factory)
- : DataflowAnalysis(C, AC, F), Factory(Factory.ExpiredLoanMapFactory) {}
+ LiveOriginAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
+ LivenessMap::Factory &SF)
+ : DataflowAnalysis(C, AC, F), FactMgr(F), Factory(SF) {}
+ using DataflowAnalysis<LiveOriginAnalysis, Lattice,
+ Direction::Backward>::transfer;
- using Base::transfer;
-
- StringRef getAnalysisName() const { return "ExpiredLoans"; }
+ StringRef getAnalysisName() const { return "LiveOrigins"; }
Lattice getInitialState() { return Lattice(Factory.getEmptyMap()); }
- /// Merges two lattices by taking the union of the two expired loans.
- Lattice join(Lattice L1, Lattice L2) {
- return Lattice(
- utils::join(L1.Expired, L2.Expired, Factory,
- // Take the last expiry fact to make this hermetic.
- [](const ExpireFact *F1, const ExpireFact *F2) {
- return F1->getExpiryLoc() > F2->getExpiryLoc() ? F1 : F2;
- }));
- }
-
- Lattice transfer(Lattice In, const ExpireFact &F) {
- return Lattice(Factory.add(In.Expired, F.getLoanID(), &F));
- }
-
- // Removes the loan from the set of expired loans.
- //
- // When a loan is re-issued (e.g., in a loop), it is no longer considered
- // expired. A loan can be in the expired set at the point of issue due to
- // the dataflow state from a previous loop iteration being propagated along
- // a backedge in the CFG.
- //
- // Note: This has a subtle false-negative though where a loan from previous
- // iteration is not overwritten by a reissue. This needs careful tracking
- // of loans "across iterations" which can be considered for future
- // enhancements.
- //
- // void foo(int safe) {
- // int* p = &safe;
- // int* q = &safe;
- // while (condition()) {
- // int x = 1;
- // p = &x; // A loan to 'x' is issued to 'p' in every iteration.
- // if (condition()) {
- // q = p;
- // }
- // (void)*p; // OK — 'p' points to 'x' from new iteration.
- // (void)*q; // UaF - 'q' still points to 'x' from previous iteration
- // // which is now destroyed.
- // }
- // }
- Lattice transfer(Lattice In, const IssueFact &F) {
- return Lattice(Factory.remove(In.Expired, F.getLoanID()));
+ /// Merges two lattices by combining liveness information.
+ /// When the same origin has different confidence levels, we take the lower
+ /// one.
+ Lattice join(Lattice L1, Lattice L2) const {
+ LivenessMap Merged = L1.LiveOrigins;
+ auto CombineConfidence = [](Confidence C1, Confidence C2) -> Confidence {
+ if (C1 == Confidence::Definite && C2 == Confidence::Definite)
+ return Confidence::Definite;
+ return Confidence::Maybe;
+ };
+ auto CombineUseFact = [](const LivenessInfo &A,
+ const LivenessInfo &B) -> const UseFact * {
+ return A.ConfidenceLevel >= B.ConfidenceLevel ? A.CausingUseFact
+ : B.CausingUseFact;
+ };
+ return Lattice(utils::join(
+ L1.LiveOrigins, L2.LiveOrigins, Factory,
+ [&](const LivenessInfo *L1, const LivenessInfo *L2) -> LivenessInfo {
+ assert((L1 || L2) && "unexpectedly merging 2 empty sets");
+ if (!L1)
+ return LivenessInfo(L2->CausingUseFact, Confidence::Maybe);
+ if (!L2)
+ return LivenessInfo(L1->CausingUseFact, Confidence::Maybe);
+ return LivenessInfo(
+ CombineUseFact(*L1, *L2),
+ CombineConfidence(L1->ConfidenceLevel, L2->ConfidenceLevel));
+ }));
+ }
+
+ /// TODO:Document.
+ Lattice transfer(Lattice In, const UseFact &UF) {
+ OriginID OID = UF.getUsedOrigin(FactMgr.getOriginMgr());
+ // Write kills liveness.
+ if (UF.isWritten())
+ return Lattice(Factory.remove(In.LiveOrigins, OID));
+ // Read makes origin live with definite confidence (dominates this point).
+ LivenessInfo Info(&UF, Confidence::Definite);
+ return Lattice(Factory.add(In.LiveOrigins, OID, Info));
+ }
+
+ /// Issuing a new loan to an origin kills its liveness.
+ Lattice transfer(Lattice In, const IssueFact &IF) {
+ return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID()));
}
- ExpiredLoanMap getExpiredLoans(ProgramPoint P) { return getState(P).Expired; }
+ Lattice transfer(Lattice In, const KillOriginFact &KF) {
+ return Lattice(Factory.remove(In.LiveOrigins, KF.getOriginID()));
+ }
+
+ LivenessMap getLiveOrigins(ProgramPoint P) { return getState(P).LiveOrigins; }
};
// ========================================================================= //
@@ -1276,84 +1334,49 @@ class LifetimeChecker {
private:
llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap;
LoanPropagationAnalysis &LoanPropagation;
- ExpiredLoansAnalysis &ExpiredLoans;
+ LiveOriginAnalysis &LiveOrigins;
FactManager &FactMgr;
AnalysisDeclContext &ADC;
LifetimeSafetyReporter *Reporter;
public:
- LifetimeChecker(LoanPropagationAnalysis &LPA, ExpiredLoansAnalysis &ELA,
+ LifetimeChecker(LoanPropagationAnalysis &LPA, LiveOriginAnalysis &LOA,
FactManager &FM, AnalysisDeclContext &ADC,
LifetimeSafetyReporter *Reporter)
- : LoanPropagation(LPA), ExpiredLoans(ELA), FactMgr(FM), ADC(ADC),
+ : LoanPropagation(LPA), LiveOrigins(LOA), FactMgr(FM), ADC(ADC),
Reporter(Reporter) {}
void run() {
llvm::TimeTraceScope TimeProfile("LifetimeChecker");
for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
for (const Fact *F : FactMgr.getFacts(B))
- if (const auto *UF = F->getAs<UseFact>())
- checkUse(UF);
+ if (const auto *EF = F->getAs<ExpireFact>())
+ checkExpiry(EF);
issuePendingWarnings();
}
- /// Checks for use-after-free errors for a given use of an Origin.
- ///
- /// This method is called for each 'UseFact' identified in the control flow
- /// graph. It determines if the loans held by the used origin have expired
- /// at the point of use.
- void checkUse(const UseFact *UF) {
- if (UF->isWritten())
- return;
- OriginID O = UF->getUsedOrigin(FactMgr.getOriginMgr());
-
- // Get the set of loans that the origin might hold at this program point.
- LoanSet HeldLoans = LoanPropagation.getLoans(O, UF);
-
- // Get the set of all loans that have expired at this program point.
- ExpiredLoanMap AllExpiredLoans = ExpiredLoans.getExpiredLoans(UF);
-
- // If the pointer holds no loans or no loans have expired, there's nothing
- // to check.
- if (HeldLoans.isEmpty() || AllExpiredLoans.isEmpty())
- return;
-
- // Identify loans that which have expired but are held by the pointer. Using
- // them is a use-after-free.
- llvm::SmallVector<LoanID> DefaultedLoans;
- // A definite UaF error occurs if all loans the origin might hold have
- // expired.
- bool IsDefiniteError = true;
- for (LoanID L : HeldLoans) {
- if (AllExpiredLoans.contains(L))
- DefaultedLoans.push_back(L);
- else
- // If at least one loan is not expired, this use is not a definite UaF.
- IsDefiniteError = false;
- }
- // If there are no defaulted loans, the use is safe.
- if (DefaultedLoans.empty())
- return;
-
- // Determine the confidence level of the error (definite or maybe).
- Confidence CurrentConfidence =
- IsDefiniteError ? Confidence::Definite : Confidence::Maybe;
-
- // For each expired loan, create a pending warning.
- for (LoanID DefaultedLoan : DefaultedLoans) {
- // If we already have a warning for this loan with a higher or equal
- // confidence, skip this one.
- if (FinalWarningsMap.count(DefaultedLoan) &&
- CurrentConfidence <= FinalWarningsMap[DefaultedLoan].ConfidenceLevel)
+ void checkExpiry(const ExpireFact *EF) {
+ LoanID ExpiredLoan = EF->getLoanID();
+ LivenessMap Origins = LiveOrigins.getLiveOrigins(EF);
+ Confidence CurConfidence = Confidence::None;
+ const UseFact *BadUse = nullptr;
+ for (auto &[OID, Info] : Origins) {
+ LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF);
+ if (!HeldLoans.contains(ExpiredLoan))
continue;
-
- auto *EF = AllExpiredLoans.lookup(DefaultedLoan);
- assert(EF && "Could not find ExpireFact for an expired loan.");
-
- FinalWarningsMap[DefaultedLoan] = {/*ExpiryLoc=*/(*EF)->getExpiryLoc(),
- /*UseExpr=*/UF->getUseExpr(),
- /*ConfidenceLevel=*/CurrentConfidence};
+ // Loan is defaulted.
+ if (CurConfidence < Info.ConfidenceLevel) {
+ CurConfidence = Info.ConfidenceLevel;
+ BadUse = Info.CausingUseFact;
+ }
...
[truncated]
|
|
@llvm/pr-subscribers-clang-analysis Author: Utkarsh Saxena (usx95) ChangesThis PR replaces the forward
The Fixes: #156959 More details describing the design flaw in using Patch is 50.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159991.diff 4 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
index 512cb76cd6349..2cc3fb3d69eb4 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
@@ -55,6 +55,7 @@ class Fact;
class FactManager;
class LoanPropagationAnalysis;
class ExpiredLoansAnalysis;
+class LiveOriginAnalysis;
struct LifetimeFactory;
/// A generic, type-safe wrapper for an ID, distinguished by its `Tag` type.
@@ -89,6 +90,7 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, OriginID ID) {
// TODO(opt): Consider using a bitset to represent the set of loans.
using LoanSet = llvm::ImmutableSet<LoanID>;
using OriginSet = llvm::ImmutableSet<OriginID>;
+using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
/// A `ProgramPoint` identifies a location in the CFG by pointing to a specific
/// `Fact`. identified by a lifetime-related event (`Fact`).
@@ -110,8 +112,9 @@ class LifetimeSafetyAnalysis {
/// Returns the set of loans an origin holds at a specific program point.
LoanSet getLoansAtPoint(OriginID OID, ProgramPoint PP) const;
- /// Returns the set of loans that have expired at a specific program point.
- std::vector<LoanID> getExpiredLoansAtPoint(ProgramPoint PP) const;
+ /// TODO:Document.
+ std::vector<std::pair<OriginID, Confidence>>
+ getLiveOriginsAtPoint(ProgramPoint PP) const;
/// Finds the OriginID for a given declaration.
/// Returns a null optional if not found.
@@ -138,7 +141,7 @@ class LifetimeSafetyAnalysis {
std::unique_ptr<LifetimeFactory> Factory;
std::unique_ptr<FactManager> FactMgr;
std::unique_ptr<LoanPropagationAnalysis> LoanPropagation;
- std::unique_ptr<ExpiredLoansAnalysis> ExpiredLoans;
+ std::unique_ptr<LiveOriginAnalysis> LiveOrigins;
};
} // namespace internal
} // namespace clang::lifetimes
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index a90aa72797e2f..7faf97da60427 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -21,6 +21,7 @@
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
#include <cstdint>
#include <memory>
@@ -901,19 +902,26 @@ class DataflowAnalysis {
llvm::SmallBitVector Visited(Cfg.getNumBlockIDs() + 1);
while (const CFGBlock *B = W.dequeue()) {
- Lattice StateIn = getInState(B);
+ Lattice StateIn = *getInState(B);
Lattice StateOut = transferBlock(B, StateIn);
OutStates[B] = StateOut;
- Visited.set(B->getBlockID());
for (const CFGBlock *AdjacentB : isForward() ? B->succs() : B->preds()) {
if (!AdjacentB)
continue;
- Lattice OldInState = getInState(AdjacentB);
- Lattice NewInState = D.join(OldInState, StateOut);
+ Lattice OldInState;
+ bool SawFirstTime = false;
+ Lattice NewInState;
+ if (const Lattice *In = getInState(AdjacentB)) {
+ OldInState = *In;
+ NewInState = D.join(OldInState, StateOut);
+ } else {
+ OldInState = D.getInitialState();
+ SawFirstTime = true;
+ NewInState = StateOut;
+ }
// Enqueue the adjacent block if its in-state has changed or if we have
// never visited it.
- if (!Visited.test(AdjacentB->getBlockID()) ||
- NewInState != OldInState) {
+ if (SawFirstTime || NewInState != OldInState) {
InStates[AdjacentB] = NewInState;
W.enqueueBlock(AdjacentB);
}
@@ -924,7 +932,12 @@ class DataflowAnalysis {
protected:
Lattice getState(ProgramPoint P) const { return PerPointStates.lookup(P); }
- Lattice getInState(const CFGBlock *B) const { return InStates.lookup(B); }
+ const Lattice *getInState(const CFGBlock *B) const {
+ auto It = InStates.find(B);
+ if (It != InStates.end())
+ return &It->second;
+ return nullptr;
+ }
Lattice getOutState(const CFGBlock *B) const { return OutStates.lookup(B); }
@@ -1023,22 +1036,26 @@ static bool isSubsetOf(const llvm::ImmutableSet<T> &A,
// instead of the current AVL-tree-based ImmutableMap.
template <typename K, typename V, typename Joiner>
static llvm::ImmutableMap<K, V>
-join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
+join(const llvm::ImmutableMap<K, V> &A, const llvm::ImmutableMap<K, V> &B,
typename llvm::ImmutableMap<K, V>::Factory &F, Joiner JoinValues) {
if (A.getHeight() < B.getHeight())
- std::swap(A, B);
+ return join(B, A, F, JoinValues);
// For each element in B, join it with the corresponding element in A
// (or with an empty value if it doesn't exist in A).
+ llvm::ImmutableMap<K, V> Res = A;
for (const auto &Entry : B) {
const K &Key = Entry.first;
const V &ValB = Entry.second;
- if (const V *ValA = A.lookup(Key))
- A = F.add(A, Key, JoinValues(*ValA, ValB));
- else
- A = F.add(A, Key, ValB);
+ Res = F.add(Res, Key, JoinValues(A.lookup(Key), &ValB));
}
- return A;
+ for (const auto &Entry : A) {
+ const K &Key = Entry.first;
+ const V &ValA = Entry.second;
+ if (!B.contains(Key))
+ Res = F.add(Res, Key, JoinValues(&ValA, nullptr));
+ }
+ return Res;
}
} // namespace utils
@@ -1046,19 +1063,6 @@ join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
// Loan Propagation Analysis
// ========================================================================= //
-using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
-using ExpiredLoanMap = llvm::ImmutableMap<LoanID, const ExpireFact *>;
-
-/// An object to hold the factories for immutable collections, ensuring
-/// that all created states share the same underlying memory management.
-struct LifetimeFactory {
- llvm::BumpPtrAllocator Allocator;
- OriginLoanMap::Factory OriginMapFactory{Allocator, /*canonicalize=*/false};
- LoanSet::Factory LoanSetFactory{Allocator, /*canonicalize=*/false};
- ExpiredLoanMap::Factory ExpiredLoanMapFactory{Allocator,
- /*canonicalize=*/false};
-};
-
/// Represents the dataflow lattice for loan propagation.
///
/// This lattice tracks which loans each origin may hold at a given program
@@ -1102,10 +1106,10 @@ class LoanPropagationAnalysis
public:
LoanPropagationAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &LFactory)
- : DataflowAnalysis(C, AC, F),
- OriginLoanMapFactory(LFactory.OriginMapFactory),
- LoanSetFactory(LFactory.LoanSetFactory) {}
+ OriginLoanMap::Factory &OriginLoanMapFactory,
+ LoanSet::Factory &LoanSetFactory)
+ : DataflowAnalysis(C, AC, F), OriginLoanMapFactory(OriginLoanMapFactory),
+ LoanSetFactory(LoanSetFactory) {}
using Base::transfer;
@@ -1118,8 +1122,13 @@ class LoanPropagationAnalysis
Lattice join(Lattice A, Lattice B) {
OriginLoanMap JoinedOrigins =
utils::join(A.Origins, B.Origins, OriginLoanMapFactory,
- [&](LoanSet S1, LoanSet S2) {
- return utils::join(S1, S2, LoanSetFactory);
+ [&](const LoanSet *S1, const LoanSet *S2) {
+ assert((S1 || S2) && "unexpectedly merging 2 empty sets");
+ if (!S1)
+ return *S2;
+ if (!S2)
+ return *S1;
+ return utils::join(*S1, *S2, LoanSetFactory);
});
return Lattice(JoinedOrigins);
}
@@ -1169,96 +1178,145 @@ class LoanPropagationAnalysis
};
// ========================================================================= //
-// Expired Loans Analysis
+// Live Origins Analysis
// ========================================================================= //
-/// The dataflow lattice for tracking the set of expired loans.
-struct ExpiredLattice {
- /// Map from an expired `LoanID` to the `ExpireFact` that made it expire.
- ExpiredLoanMap Expired;
+/// Information about why an origin is live at a program point.
+struct LivenessInfo {
+ // TODO: Doc.
+ const UseFact *CausingUseFact;
+ // TODO: Doc.
+ Confidence ConfidenceLevel;
+
+ LivenessInfo() : CausingUseFact(nullptr), ConfidenceLevel(Confidence::None) {}
+ LivenessInfo(const UseFact *UF, Confidence C)
+ : CausingUseFact(UF), ConfidenceLevel(C) {}
- ExpiredLattice() : Expired(nullptr) {};
- explicit ExpiredLattice(ExpiredLoanMap M) : Expired(M) {}
+ bool operator==(const LivenessInfo &Other) const {
+ return CausingUseFact == Other.CausingUseFact &&
+ ConfidenceLevel == Other.ConfidenceLevel;
+ }
+ bool operator!=(const LivenessInfo &Other) const { return !(*this == Other); }
+
+ void Profile(llvm::FoldingSetNodeID &IDBuilder) const {
+ IDBuilder.AddPointer(CausingUseFact);
+ IDBuilder.Add(ConfidenceLevel);
+ }
+};
+
+using LivenessMap = llvm::ImmutableMap<OriginID, LivenessInfo>;
- bool operator==(const ExpiredLattice &Other) const {
- return Expired == Other.Expired;
+/// The dataflow lattice for origin liveness analysis.
+/// It tracks which origins are live, why they're live (which UseFact),
+/// and the confidence level of that liveness.
+struct LivenessLattice {
+ LivenessMap LiveOrigins;
+
+ LivenessLattice() : LiveOrigins(nullptr) {};
+
+ explicit LivenessLattice(LivenessMap L) : LiveOrigins(L) {}
+
+ bool operator==(const LivenessLattice &Other) const {
+ return LiveOrigins == Other.LiveOrigins;
}
- bool operator!=(const ExpiredLattice &Other) const {
+
+ bool operator!=(const LivenessLattice &Other) const {
return !(*this == Other);
}
void dump(llvm::raw_ostream &OS) const {
- OS << "ExpiredLattice State:\n";
- if (Expired.isEmpty())
+ OS << "LivenessLattice State:\n";
+ if (LiveOrigins.isEmpty())
OS << " <empty>\n";
- for (const auto &[ID, _] : Expired)
- OS << " Loan " << ID << " is expired\n";
+ for (const auto &Entry : LiveOrigins) {
+ OriginID OID = Entry.first;
+ const LivenessInfo &Info = Entry.second;
+ OS << " Origin " << OID << " is ";
+ switch (Info.ConfidenceLevel) {
+ case Confidence::Definite:
+ OS << "definitely";
+ break;
+ case Confidence::Maybe:
+ OS << "maybe";
+ break;
+ case Confidence::None:
+ llvm_unreachable("liveness condidence should not be none.");
+ }
+ OS << " live at this point\n";
+ }
}
};
-/// The analysis that tracks which loans have expired.
-class ExpiredLoansAnalysis
- : public DataflowAnalysis<ExpiredLoansAnalysis, ExpiredLattice,
- Direction::Forward> {
-
- ExpiredLoanMap::Factory &Factory;
+/// The analysis that tracks which origins are live, with granular information
+/// about the causing use fact and confidence level. This is a backward
+/// analysis.
+class LiveOriginAnalysis
+ : public DataflowAnalysis<LiveOriginAnalysis, LivenessLattice,
+ Direction::Backward> {
+ FactManager &FactMgr;
+ LivenessMap::Factory &Factory;
public:
- ExpiredLoansAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &Factory)
- : DataflowAnalysis(C, AC, F), Factory(Factory.ExpiredLoanMapFactory) {}
+ LiveOriginAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
+ LivenessMap::Factory &SF)
+ : DataflowAnalysis(C, AC, F), FactMgr(F), Factory(SF) {}
+ using DataflowAnalysis<LiveOriginAnalysis, Lattice,
+ Direction::Backward>::transfer;
- using Base::transfer;
-
- StringRef getAnalysisName() const { return "ExpiredLoans"; }
+ StringRef getAnalysisName() const { return "LiveOrigins"; }
Lattice getInitialState() { return Lattice(Factory.getEmptyMap()); }
- /// Merges two lattices by taking the union of the two expired loans.
- Lattice join(Lattice L1, Lattice L2) {
- return Lattice(
- utils::join(L1.Expired, L2.Expired, Factory,
- // Take the last expiry fact to make this hermetic.
- [](const ExpireFact *F1, const ExpireFact *F2) {
- return F1->getExpiryLoc() > F2->getExpiryLoc() ? F1 : F2;
- }));
- }
-
- Lattice transfer(Lattice In, const ExpireFact &F) {
- return Lattice(Factory.add(In.Expired, F.getLoanID(), &F));
- }
-
- // Removes the loan from the set of expired loans.
- //
- // When a loan is re-issued (e.g., in a loop), it is no longer considered
- // expired. A loan can be in the expired set at the point of issue due to
- // the dataflow state from a previous loop iteration being propagated along
- // a backedge in the CFG.
- //
- // Note: This has a subtle false-negative though where a loan from previous
- // iteration is not overwritten by a reissue. This needs careful tracking
- // of loans "across iterations" which can be considered for future
- // enhancements.
- //
- // void foo(int safe) {
- // int* p = &safe;
- // int* q = &safe;
- // while (condition()) {
- // int x = 1;
- // p = &x; // A loan to 'x' is issued to 'p' in every iteration.
- // if (condition()) {
- // q = p;
- // }
- // (void)*p; // OK — 'p' points to 'x' from new iteration.
- // (void)*q; // UaF - 'q' still points to 'x' from previous iteration
- // // which is now destroyed.
- // }
- // }
- Lattice transfer(Lattice In, const IssueFact &F) {
- return Lattice(Factory.remove(In.Expired, F.getLoanID()));
+ /// Merges two lattices by combining liveness information.
+ /// When the same origin has different confidence levels, we take the lower
+ /// one.
+ Lattice join(Lattice L1, Lattice L2) const {
+ LivenessMap Merged = L1.LiveOrigins;
+ auto CombineConfidence = [](Confidence C1, Confidence C2) -> Confidence {
+ if (C1 == Confidence::Definite && C2 == Confidence::Definite)
+ return Confidence::Definite;
+ return Confidence::Maybe;
+ };
+ auto CombineUseFact = [](const LivenessInfo &A,
+ const LivenessInfo &B) -> const UseFact * {
+ return A.ConfidenceLevel >= B.ConfidenceLevel ? A.CausingUseFact
+ : B.CausingUseFact;
+ };
+ return Lattice(utils::join(
+ L1.LiveOrigins, L2.LiveOrigins, Factory,
+ [&](const LivenessInfo *L1, const LivenessInfo *L2) -> LivenessInfo {
+ assert((L1 || L2) && "unexpectedly merging 2 empty sets");
+ if (!L1)
+ return LivenessInfo(L2->CausingUseFact, Confidence::Maybe);
+ if (!L2)
+ return LivenessInfo(L1->CausingUseFact, Confidence::Maybe);
+ return LivenessInfo(
+ CombineUseFact(*L1, *L2),
+ CombineConfidence(L1->ConfidenceLevel, L2->ConfidenceLevel));
+ }));
+ }
+
+ /// TODO:Document.
+ Lattice transfer(Lattice In, const UseFact &UF) {
+ OriginID OID = UF.getUsedOrigin(FactMgr.getOriginMgr());
+ // Write kills liveness.
+ if (UF.isWritten())
+ return Lattice(Factory.remove(In.LiveOrigins, OID));
+ // Read makes origin live with definite confidence (dominates this point).
+ LivenessInfo Info(&UF, Confidence::Definite);
+ return Lattice(Factory.add(In.LiveOrigins, OID, Info));
+ }
+
+ /// Issuing a new loan to an origin kills its liveness.
+ Lattice transfer(Lattice In, const IssueFact &IF) {
+ return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID()));
}
- ExpiredLoanMap getExpiredLoans(ProgramPoint P) { return getState(P).Expired; }
+ Lattice transfer(Lattice In, const KillOriginFact &KF) {
+ return Lattice(Factory.remove(In.LiveOrigins, KF.getOriginID()));
+ }
+
+ LivenessMap getLiveOrigins(ProgramPoint P) { return getState(P).LiveOrigins; }
};
// ========================================================================= //
@@ -1276,84 +1334,49 @@ class LifetimeChecker {
private:
llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap;
LoanPropagationAnalysis &LoanPropagation;
- ExpiredLoansAnalysis &ExpiredLoans;
+ LiveOriginAnalysis &LiveOrigins;
FactManager &FactMgr;
AnalysisDeclContext &ADC;
LifetimeSafetyReporter *Reporter;
public:
- LifetimeChecker(LoanPropagationAnalysis &LPA, ExpiredLoansAnalysis &ELA,
+ LifetimeChecker(LoanPropagationAnalysis &LPA, LiveOriginAnalysis &LOA,
FactManager &FM, AnalysisDeclContext &ADC,
LifetimeSafetyReporter *Reporter)
- : LoanPropagation(LPA), ExpiredLoans(ELA), FactMgr(FM), ADC(ADC),
+ : LoanPropagation(LPA), LiveOrigins(LOA), FactMgr(FM), ADC(ADC),
Reporter(Reporter) {}
void run() {
llvm::TimeTraceScope TimeProfile("LifetimeChecker");
for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
for (const Fact *F : FactMgr.getFacts(B))
- if (const auto *UF = F->getAs<UseFact>())
- checkUse(UF);
+ if (const auto *EF = F->getAs<ExpireFact>())
+ checkExpiry(EF);
issuePendingWarnings();
}
- /// Checks for use-after-free errors for a given use of an Origin.
- ///
- /// This method is called for each 'UseFact' identified in the control flow
- /// graph. It determines if the loans held by the used origin have expired
- /// at the point of use.
- void checkUse(const UseFact *UF) {
- if (UF->isWritten())
- return;
- OriginID O = UF->getUsedOrigin(FactMgr.getOriginMgr());
-
- // Get the set of loans that the origin might hold at this program point.
- LoanSet HeldLoans = LoanPropagation.getLoans(O, UF);
-
- // Get the set of all loans that have expired at this program point.
- ExpiredLoanMap AllExpiredLoans = ExpiredLoans.getExpiredLoans(UF);
-
- // If the pointer holds no loans or no loans have expired, there's nothing
- // to check.
- if (HeldLoans.isEmpty() || AllExpiredLoans.isEmpty())
- return;
-
- // Identify loans that which have expired but are held by the pointer. Using
- // them is a use-after-free.
- llvm::SmallVector<LoanID> DefaultedLoans;
- // A definite UaF error occurs if all loans the origin might hold have
- // expired.
- bool IsDefiniteError = true;
- for (LoanID L : HeldLoans) {
- if (AllExpiredLoans.contains(L))
- DefaultedLoans.push_back(L);
- else
- // If at least one loan is not expired, this use is not a definite UaF.
- IsDefiniteError = false;
- }
- // If there are no defaulted loans, the use is safe.
- if (DefaultedLoans.empty())
- return;
-
- // Determine the confidence level of the error (definite or maybe).
- Confidence CurrentConfidence =
- IsDefiniteError ? Confidence::Definite : Confidence::Maybe;
-
- // For each expired loan, create a pending warning.
- for (LoanID DefaultedLoan : DefaultedLoans) {
- // If we already have a warning for this loan with a higher or equal
- // confidence, skip this one.
- if (FinalWarningsMap.count(DefaultedLoan) &&
- CurrentConfidence <= FinalWarningsMap[DefaultedLoan].ConfidenceLevel)
+ void checkExpiry(const ExpireFact *EF) {
+ LoanID ExpiredLoan = EF->getLoanID();
+ LivenessMap Origins = LiveOrigins.getLiveOrigins(EF);
+ Confidence CurConfidence = Confidence::None;
+ const UseFact *BadUse = nullptr;
+ for (auto &[OID, Info] : Origins) {
+ LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF);
+ if (!HeldLoans.contains(ExpiredLoan))
continue;
-
- auto *EF = AllExpiredLoans.lookup(DefaultedLoan);
- assert(EF && "Could not find ExpireFact for an expired loan.");
-
- FinalWarningsMap[DefaultedLoan] = {/*ExpiryLoc=*/(*EF)->getExpiryLoc(),
- /*UseExpr=*/UF->getUseExpr(),
- /*ConfidenceLevel=*/CurrentConfidence};
+ // Loan is defaulted.
+ if (CurConfidence < Info.ConfidenceLevel) {
+ CurConfidence = Info.ConfidenceLevel;
+ BadUse = Info.CausingUseFact;
+ }
...
[truncated]
|
|
@llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) ChangesThis PR replaces the forward
The Fixes: #156959 More details describing the design flaw in using Patch is 50.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159991.diff 4 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
index 512cb76cd6349..2cc3fb3d69eb4 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety.h
@@ -55,6 +55,7 @@ class Fact;
class FactManager;
class LoanPropagationAnalysis;
class ExpiredLoansAnalysis;
+class LiveOriginAnalysis;
struct LifetimeFactory;
/// A generic, type-safe wrapper for an ID, distinguished by its `Tag` type.
@@ -89,6 +90,7 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, OriginID ID) {
// TODO(opt): Consider using a bitset to represent the set of loans.
using LoanSet = llvm::ImmutableSet<LoanID>;
using OriginSet = llvm::ImmutableSet<OriginID>;
+using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
/// A `ProgramPoint` identifies a location in the CFG by pointing to a specific
/// `Fact`. identified by a lifetime-related event (`Fact`).
@@ -110,8 +112,9 @@ class LifetimeSafetyAnalysis {
/// Returns the set of loans an origin holds at a specific program point.
LoanSet getLoansAtPoint(OriginID OID, ProgramPoint PP) const;
- /// Returns the set of loans that have expired at a specific program point.
- std::vector<LoanID> getExpiredLoansAtPoint(ProgramPoint PP) const;
+ /// TODO:Document.
+ std::vector<std::pair<OriginID, Confidence>>
+ getLiveOriginsAtPoint(ProgramPoint PP) const;
/// Finds the OriginID for a given declaration.
/// Returns a null optional if not found.
@@ -138,7 +141,7 @@ class LifetimeSafetyAnalysis {
std::unique_ptr<LifetimeFactory> Factory;
std::unique_ptr<FactManager> FactMgr;
std::unique_ptr<LoanPropagationAnalysis> LoanPropagation;
- std::unique_ptr<ExpiredLoansAnalysis> ExpiredLoans;
+ std::unique_ptr<LiveOriginAnalysis> LiveOrigins;
};
} // namespace internal
} // namespace clang::lifetimes
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index a90aa72797e2f..7faf97da60427 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -21,6 +21,7 @@
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
#include <cstdint>
#include <memory>
@@ -901,19 +902,26 @@ class DataflowAnalysis {
llvm::SmallBitVector Visited(Cfg.getNumBlockIDs() + 1);
while (const CFGBlock *B = W.dequeue()) {
- Lattice StateIn = getInState(B);
+ Lattice StateIn = *getInState(B);
Lattice StateOut = transferBlock(B, StateIn);
OutStates[B] = StateOut;
- Visited.set(B->getBlockID());
for (const CFGBlock *AdjacentB : isForward() ? B->succs() : B->preds()) {
if (!AdjacentB)
continue;
- Lattice OldInState = getInState(AdjacentB);
- Lattice NewInState = D.join(OldInState, StateOut);
+ Lattice OldInState;
+ bool SawFirstTime = false;
+ Lattice NewInState;
+ if (const Lattice *In = getInState(AdjacentB)) {
+ OldInState = *In;
+ NewInState = D.join(OldInState, StateOut);
+ } else {
+ OldInState = D.getInitialState();
+ SawFirstTime = true;
+ NewInState = StateOut;
+ }
// Enqueue the adjacent block if its in-state has changed or if we have
// never visited it.
- if (!Visited.test(AdjacentB->getBlockID()) ||
- NewInState != OldInState) {
+ if (SawFirstTime || NewInState != OldInState) {
InStates[AdjacentB] = NewInState;
W.enqueueBlock(AdjacentB);
}
@@ -924,7 +932,12 @@ class DataflowAnalysis {
protected:
Lattice getState(ProgramPoint P) const { return PerPointStates.lookup(P); }
- Lattice getInState(const CFGBlock *B) const { return InStates.lookup(B); }
+ const Lattice *getInState(const CFGBlock *B) const {
+ auto It = InStates.find(B);
+ if (It != InStates.end())
+ return &It->second;
+ return nullptr;
+ }
Lattice getOutState(const CFGBlock *B) const { return OutStates.lookup(B); }
@@ -1023,22 +1036,26 @@ static bool isSubsetOf(const llvm::ImmutableSet<T> &A,
// instead of the current AVL-tree-based ImmutableMap.
template <typename K, typename V, typename Joiner>
static llvm::ImmutableMap<K, V>
-join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
+join(const llvm::ImmutableMap<K, V> &A, const llvm::ImmutableMap<K, V> &B,
typename llvm::ImmutableMap<K, V>::Factory &F, Joiner JoinValues) {
if (A.getHeight() < B.getHeight())
- std::swap(A, B);
+ return join(B, A, F, JoinValues);
// For each element in B, join it with the corresponding element in A
// (or with an empty value if it doesn't exist in A).
+ llvm::ImmutableMap<K, V> Res = A;
for (const auto &Entry : B) {
const K &Key = Entry.first;
const V &ValB = Entry.second;
- if (const V *ValA = A.lookup(Key))
- A = F.add(A, Key, JoinValues(*ValA, ValB));
- else
- A = F.add(A, Key, ValB);
+ Res = F.add(Res, Key, JoinValues(A.lookup(Key), &ValB));
}
- return A;
+ for (const auto &Entry : A) {
+ const K &Key = Entry.first;
+ const V &ValA = Entry.second;
+ if (!B.contains(Key))
+ Res = F.add(Res, Key, JoinValues(&ValA, nullptr));
+ }
+ return Res;
}
} // namespace utils
@@ -1046,19 +1063,6 @@ join(llvm::ImmutableMap<K, V> A, llvm::ImmutableMap<K, V> B,
// Loan Propagation Analysis
// ========================================================================= //
-using OriginLoanMap = llvm::ImmutableMap<OriginID, LoanSet>;
-using ExpiredLoanMap = llvm::ImmutableMap<LoanID, const ExpireFact *>;
-
-/// An object to hold the factories for immutable collections, ensuring
-/// that all created states share the same underlying memory management.
-struct LifetimeFactory {
- llvm::BumpPtrAllocator Allocator;
- OriginLoanMap::Factory OriginMapFactory{Allocator, /*canonicalize=*/false};
- LoanSet::Factory LoanSetFactory{Allocator, /*canonicalize=*/false};
- ExpiredLoanMap::Factory ExpiredLoanMapFactory{Allocator,
- /*canonicalize=*/false};
-};
-
/// Represents the dataflow lattice for loan propagation.
///
/// This lattice tracks which loans each origin may hold at a given program
@@ -1102,10 +1106,10 @@ class LoanPropagationAnalysis
public:
LoanPropagationAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &LFactory)
- : DataflowAnalysis(C, AC, F),
- OriginLoanMapFactory(LFactory.OriginMapFactory),
- LoanSetFactory(LFactory.LoanSetFactory) {}
+ OriginLoanMap::Factory &OriginLoanMapFactory,
+ LoanSet::Factory &LoanSetFactory)
+ : DataflowAnalysis(C, AC, F), OriginLoanMapFactory(OriginLoanMapFactory),
+ LoanSetFactory(LoanSetFactory) {}
using Base::transfer;
@@ -1118,8 +1122,13 @@ class LoanPropagationAnalysis
Lattice join(Lattice A, Lattice B) {
OriginLoanMap JoinedOrigins =
utils::join(A.Origins, B.Origins, OriginLoanMapFactory,
- [&](LoanSet S1, LoanSet S2) {
- return utils::join(S1, S2, LoanSetFactory);
+ [&](const LoanSet *S1, const LoanSet *S2) {
+ assert((S1 || S2) && "unexpectedly merging 2 empty sets");
+ if (!S1)
+ return *S2;
+ if (!S2)
+ return *S1;
+ return utils::join(*S1, *S2, LoanSetFactory);
});
return Lattice(JoinedOrigins);
}
@@ -1169,96 +1178,145 @@ class LoanPropagationAnalysis
};
// ========================================================================= //
-// Expired Loans Analysis
+// Live Origins Analysis
// ========================================================================= //
-/// The dataflow lattice for tracking the set of expired loans.
-struct ExpiredLattice {
- /// Map from an expired `LoanID` to the `ExpireFact` that made it expire.
- ExpiredLoanMap Expired;
+/// Information about why an origin is live at a program point.
+struct LivenessInfo {
+ // TODO: Doc.
+ const UseFact *CausingUseFact;
+ // TODO: Doc.
+ Confidence ConfidenceLevel;
+
+ LivenessInfo() : CausingUseFact(nullptr), ConfidenceLevel(Confidence::None) {}
+ LivenessInfo(const UseFact *UF, Confidence C)
+ : CausingUseFact(UF), ConfidenceLevel(C) {}
- ExpiredLattice() : Expired(nullptr) {};
- explicit ExpiredLattice(ExpiredLoanMap M) : Expired(M) {}
+ bool operator==(const LivenessInfo &Other) const {
+ return CausingUseFact == Other.CausingUseFact &&
+ ConfidenceLevel == Other.ConfidenceLevel;
+ }
+ bool operator!=(const LivenessInfo &Other) const { return !(*this == Other); }
+
+ void Profile(llvm::FoldingSetNodeID &IDBuilder) const {
+ IDBuilder.AddPointer(CausingUseFact);
+ IDBuilder.Add(ConfidenceLevel);
+ }
+};
+
+using LivenessMap = llvm::ImmutableMap<OriginID, LivenessInfo>;
- bool operator==(const ExpiredLattice &Other) const {
- return Expired == Other.Expired;
+/// The dataflow lattice for origin liveness analysis.
+/// It tracks which origins are live, why they're live (which UseFact),
+/// and the confidence level of that liveness.
+struct LivenessLattice {
+ LivenessMap LiveOrigins;
+
+ LivenessLattice() : LiveOrigins(nullptr) {};
+
+ explicit LivenessLattice(LivenessMap L) : LiveOrigins(L) {}
+
+ bool operator==(const LivenessLattice &Other) const {
+ return LiveOrigins == Other.LiveOrigins;
}
- bool operator!=(const ExpiredLattice &Other) const {
+
+ bool operator!=(const LivenessLattice &Other) const {
return !(*this == Other);
}
void dump(llvm::raw_ostream &OS) const {
- OS << "ExpiredLattice State:\n";
- if (Expired.isEmpty())
+ OS << "LivenessLattice State:\n";
+ if (LiveOrigins.isEmpty())
OS << " <empty>\n";
- for (const auto &[ID, _] : Expired)
- OS << " Loan " << ID << " is expired\n";
+ for (const auto &Entry : LiveOrigins) {
+ OriginID OID = Entry.first;
+ const LivenessInfo &Info = Entry.second;
+ OS << " Origin " << OID << " is ";
+ switch (Info.ConfidenceLevel) {
+ case Confidence::Definite:
+ OS << "definitely";
+ break;
+ case Confidence::Maybe:
+ OS << "maybe";
+ break;
+ case Confidence::None:
+ llvm_unreachable("liveness condidence should not be none.");
+ }
+ OS << " live at this point\n";
+ }
}
};
-/// The analysis that tracks which loans have expired.
-class ExpiredLoansAnalysis
- : public DataflowAnalysis<ExpiredLoansAnalysis, ExpiredLattice,
- Direction::Forward> {
-
- ExpiredLoanMap::Factory &Factory;
+/// The analysis that tracks which origins are live, with granular information
+/// about the causing use fact and confidence level. This is a backward
+/// analysis.
+class LiveOriginAnalysis
+ : public DataflowAnalysis<LiveOriginAnalysis, LivenessLattice,
+ Direction::Backward> {
+ FactManager &FactMgr;
+ LivenessMap::Factory &Factory;
public:
- ExpiredLoansAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
- LifetimeFactory &Factory)
- : DataflowAnalysis(C, AC, F), Factory(Factory.ExpiredLoanMapFactory) {}
+ LiveOriginAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F,
+ LivenessMap::Factory &SF)
+ : DataflowAnalysis(C, AC, F), FactMgr(F), Factory(SF) {}
+ using DataflowAnalysis<LiveOriginAnalysis, Lattice,
+ Direction::Backward>::transfer;
- using Base::transfer;
-
- StringRef getAnalysisName() const { return "ExpiredLoans"; }
+ StringRef getAnalysisName() const { return "LiveOrigins"; }
Lattice getInitialState() { return Lattice(Factory.getEmptyMap()); }
- /// Merges two lattices by taking the union of the two expired loans.
- Lattice join(Lattice L1, Lattice L2) {
- return Lattice(
- utils::join(L1.Expired, L2.Expired, Factory,
- // Take the last expiry fact to make this hermetic.
- [](const ExpireFact *F1, const ExpireFact *F2) {
- return F1->getExpiryLoc() > F2->getExpiryLoc() ? F1 : F2;
- }));
- }
-
- Lattice transfer(Lattice In, const ExpireFact &F) {
- return Lattice(Factory.add(In.Expired, F.getLoanID(), &F));
- }
-
- // Removes the loan from the set of expired loans.
- //
- // When a loan is re-issued (e.g., in a loop), it is no longer considered
- // expired. A loan can be in the expired set at the point of issue due to
- // the dataflow state from a previous loop iteration being propagated along
- // a backedge in the CFG.
- //
- // Note: This has a subtle false-negative though where a loan from previous
- // iteration is not overwritten by a reissue. This needs careful tracking
- // of loans "across iterations" which can be considered for future
- // enhancements.
- //
- // void foo(int safe) {
- // int* p = &safe;
- // int* q = &safe;
- // while (condition()) {
- // int x = 1;
- // p = &x; // A loan to 'x' is issued to 'p' in every iteration.
- // if (condition()) {
- // q = p;
- // }
- // (void)*p; // OK — 'p' points to 'x' from new iteration.
- // (void)*q; // UaF - 'q' still points to 'x' from previous iteration
- // // which is now destroyed.
- // }
- // }
- Lattice transfer(Lattice In, const IssueFact &F) {
- return Lattice(Factory.remove(In.Expired, F.getLoanID()));
+ /// Merges two lattices by combining liveness information.
+ /// When the same origin has different confidence levels, we take the lower
+ /// one.
+ Lattice join(Lattice L1, Lattice L2) const {
+ LivenessMap Merged = L1.LiveOrigins;
+ auto CombineConfidence = [](Confidence C1, Confidence C2) -> Confidence {
+ if (C1 == Confidence::Definite && C2 == Confidence::Definite)
+ return Confidence::Definite;
+ return Confidence::Maybe;
+ };
+ auto CombineUseFact = [](const LivenessInfo &A,
+ const LivenessInfo &B) -> const UseFact * {
+ return A.ConfidenceLevel >= B.ConfidenceLevel ? A.CausingUseFact
+ : B.CausingUseFact;
+ };
+ return Lattice(utils::join(
+ L1.LiveOrigins, L2.LiveOrigins, Factory,
+ [&](const LivenessInfo *L1, const LivenessInfo *L2) -> LivenessInfo {
+ assert((L1 || L2) && "unexpectedly merging 2 empty sets");
+ if (!L1)
+ return LivenessInfo(L2->CausingUseFact, Confidence::Maybe);
+ if (!L2)
+ return LivenessInfo(L1->CausingUseFact, Confidence::Maybe);
+ return LivenessInfo(
+ CombineUseFact(*L1, *L2),
+ CombineConfidence(L1->ConfidenceLevel, L2->ConfidenceLevel));
+ }));
+ }
+
+ /// TODO:Document.
+ Lattice transfer(Lattice In, const UseFact &UF) {
+ OriginID OID = UF.getUsedOrigin(FactMgr.getOriginMgr());
+ // Write kills liveness.
+ if (UF.isWritten())
+ return Lattice(Factory.remove(In.LiveOrigins, OID));
+ // Read makes origin live with definite confidence (dominates this point).
+ LivenessInfo Info(&UF, Confidence::Definite);
+ return Lattice(Factory.add(In.LiveOrigins, OID, Info));
+ }
+
+ /// Issuing a new loan to an origin kills its liveness.
+ Lattice transfer(Lattice In, const IssueFact &IF) {
+ return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID()));
}
- ExpiredLoanMap getExpiredLoans(ProgramPoint P) { return getState(P).Expired; }
+ Lattice transfer(Lattice In, const KillOriginFact &KF) {
+ return Lattice(Factory.remove(In.LiveOrigins, KF.getOriginID()));
+ }
+
+ LivenessMap getLiveOrigins(ProgramPoint P) { return getState(P).LiveOrigins; }
};
// ========================================================================= //
@@ -1276,84 +1334,49 @@ class LifetimeChecker {
private:
llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap;
LoanPropagationAnalysis &LoanPropagation;
- ExpiredLoansAnalysis &ExpiredLoans;
+ LiveOriginAnalysis &LiveOrigins;
FactManager &FactMgr;
AnalysisDeclContext &ADC;
LifetimeSafetyReporter *Reporter;
public:
- LifetimeChecker(LoanPropagationAnalysis &LPA, ExpiredLoansAnalysis &ELA,
+ LifetimeChecker(LoanPropagationAnalysis &LPA, LiveOriginAnalysis &LOA,
FactManager &FM, AnalysisDeclContext &ADC,
LifetimeSafetyReporter *Reporter)
- : LoanPropagation(LPA), ExpiredLoans(ELA), FactMgr(FM), ADC(ADC),
+ : LoanPropagation(LPA), LiveOrigins(LOA), FactMgr(FM), ADC(ADC),
Reporter(Reporter) {}
void run() {
llvm::TimeTraceScope TimeProfile("LifetimeChecker");
for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
for (const Fact *F : FactMgr.getFacts(B))
- if (const auto *UF = F->getAs<UseFact>())
- checkUse(UF);
+ if (const auto *EF = F->getAs<ExpireFact>())
+ checkExpiry(EF);
issuePendingWarnings();
}
- /// Checks for use-after-free errors for a given use of an Origin.
- ///
- /// This method is called for each 'UseFact' identified in the control flow
- /// graph. It determines if the loans held by the used origin have expired
- /// at the point of use.
- void checkUse(const UseFact *UF) {
- if (UF->isWritten())
- return;
- OriginID O = UF->getUsedOrigin(FactMgr.getOriginMgr());
-
- // Get the set of loans that the origin might hold at this program point.
- LoanSet HeldLoans = LoanPropagation.getLoans(O, UF);
-
- // Get the set of all loans that have expired at this program point.
- ExpiredLoanMap AllExpiredLoans = ExpiredLoans.getExpiredLoans(UF);
-
- // If the pointer holds no loans or no loans have expired, there's nothing
- // to check.
- if (HeldLoans.isEmpty() || AllExpiredLoans.isEmpty())
- return;
-
- // Identify loans that which have expired but are held by the pointer. Using
- // them is a use-after-free.
- llvm::SmallVector<LoanID> DefaultedLoans;
- // A definite UaF error occurs if all loans the origin might hold have
- // expired.
- bool IsDefiniteError = true;
- for (LoanID L : HeldLoans) {
- if (AllExpiredLoans.contains(L))
- DefaultedLoans.push_back(L);
- else
- // If at least one loan is not expired, this use is not a definite UaF.
- IsDefiniteError = false;
- }
- // If there are no defaulted loans, the use is safe.
- if (DefaultedLoans.empty())
- return;
-
- // Determine the confidence level of the error (definite or maybe).
- Confidence CurrentConfidence =
- IsDefiniteError ? Confidence::Definite : Confidence::Maybe;
-
- // For each expired loan, create a pending warning.
- for (LoanID DefaultedLoan : DefaultedLoans) {
- // If we already have a warning for this loan with a higher or equal
- // confidence, skip this one.
- if (FinalWarningsMap.count(DefaultedLoan) &&
- CurrentConfidence <= FinalWarningsMap[DefaultedLoan].ConfidenceLevel)
+ void checkExpiry(const ExpireFact *EF) {
+ LoanID ExpiredLoan = EF->getLoanID();
+ LivenessMap Origins = LiveOrigins.getLiveOrigins(EF);
+ Confidence CurConfidence = Confidence::None;
+ const UseFact *BadUse = nullptr;
+ for (auto &[OID, Info] : Origins) {
+ LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF);
+ if (!HeldLoans.contains(ExpiredLoan))
continue;
-
- auto *EF = AllExpiredLoans.lookup(DefaultedLoan);
- assert(EF && "Could not find ExpireFact for an expired loan.");
-
- FinalWarningsMap[DefaultedLoan] = {/*ExpiryLoc=*/(*EF)->getExpiryLoc(),
- /*UseExpr=*/UF->getUseExpr(),
- /*ConfidenceLevel=*/CurrentConfidence};
+ // Loan is defaulted.
+ if (CurConfidence < Info.ConfidenceLevel) {
+ CurConfidence = Info.ConfidenceLevel;
+ BadUse = Info.CausingUseFact;
+ }
...
[truncated]
|
fc4c118 to
0c1eae5
Compare
4e63e10 to
271e174
Compare
0c1eae5 to
f49485d
Compare
caac182 to
082d8a2
Compare
f49485d to
37c0343
Compare
082d8a2 to
1c92d1c
Compare
1fea50f to
5685696
Compare
5685696 to
c8da262
Compare
c8da262 to
5a72d7f
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
5a72d7f to
8a7ec9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, thanks!
8a7ec9f to
6b742c4
Compare
382322b to
335b2b5
Compare
6b742c4 to
0e7c173
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the substantial delay in reviewing. I've left an initial set of comments specifically on the LiveOriginAnalysis. Overall, the analysis looks good, my comments largely relate to terminology. However, I'm concerned with the framing -- given my understanding of origins, I'm surprised at the description of origin "liveness". I would sooner associate liveness with a declaration (or DeclValue in AST terms) or abstract value.
I think there are multiple ways to conceive of origins, but none of them map (in my understanding) to an abstraction for which liveness is a meaningful description. So, if we're going to do that, I think you need more explanation, at least in the PR description but possibly in the code itself, of the concepts involved. I'm afraid we keep increasing the complexity of the meaning of origin and I'm not sure it will bear the weight. So, that makes it doubly important to be sure these kinds of things are fully explained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the delay! not too much to add and otherwise LG
0e7c173 to
f239d90
Compare
f239d90 to
b7fe309
Compare

This PR replaces the forward
ExpiredLoansAnalysiswith a backwardLiveOriginAnalysisthat tracks which origins are live at each program point, along with confidence levels (Definite or Maybe). The new approach:The
LifetimeCheckernow checks for use-after-free by examining if an origin is live when a loan expires, rather than checking if a loan is expired when an origin is used.More details describing the design flaw in using
ExpiredLoansis mentioned in #156959 (comment)Fixes: #156959
(With this, we can build LLVM with no false-positives 🎉 )
Benchmark report
Lifetime Analysis Performance Report
Test Case: Pointer Cycle in Loop
Timing Results:
Complexity Analysis:
Test Case: CFG Merges
Timing Results:
Complexity Analysis:
Test Case: Deeply Nested Loops
Timing Results:
Complexity Analysis:
Test Case: Switch Fan-out
Timing Results:
Complexity Analysis: