[LifetimeSafety] Associate origins to all l-valued expressions #156896
+269
−169
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 4, 2025
usx95
changed the title
usx95
added
the
clang:temporal-safety
Graphite App
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) ChangesThis patch refactors the C++ lifetime safety analysis to implement a more consistent model for tracking borrows. The central idea is to make loan creation a consequence of referencing a variable, while making loan propagation dependent on the type's semantics. This change introduces a more uniform model for tracking borrows from non-pointer types:
Future Work: Multi-Origin Model Patch is 36.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156896.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index dbbf7f3cc14b1..1b5837ff0004d 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -50,6 +50,11 @@ struct Loan {
Loan(LoanID id, AccessPath path, const Expr *IssueExpr)
: ID(id), Path(path), IssueExpr(IssueExpr) {}
+
+ void dump(llvm::raw_ostream &OS) const {
+ OS << ID << " (Path: ";
+ OS << Path.D->getNameAsString() << ")";
+ }
};
/// An Origin is a symbolic identifier that represents the set of possible
@@ -120,17 +125,19 @@ class OriginManager {
// TODO: Mark this method as const once we remove the call to getOrCreate.
OriginID get(const Expr &E) {
- // Origin of DeclRefExpr is that of the declaration it refers to.
+ auto It = ExprToOriginID.find(&E);
+ if (It != ExprToOriginID.end())
+ return It->second;
+ // If the expression itself has no specific origin, and it's a reference
+ // to a declaration, its origin is that of the declaration it refers to.
+ // For pointer types, where we don't pre-emptively create an origin for the
+ // DeclRefExpr itself.
if (const auto *DRE = dyn_cast<DeclRefExpr>(&E))
return get(*DRE->getDecl());
- auto It = ExprToOriginID.find(&E);
// TODO: This should be an assert(It != ExprToOriginID.end()). The current
// implementation falls back to getOrCreate to avoid crashing on
// yet-unhandled pointer expressions, creating an empty origin for them.
- if (It == ExprToOriginID.end())
- return getOrCreate(E);
-
- return It->second;
+ return getOrCreate(E);
}
OriginID get(const ValueDecl &D) {
@@ -149,10 +156,6 @@ class OriginManager {
if (It != ExprToOriginID.end())
return It->second;
- if (const auto *DRE = dyn_cast<DeclRefExpr>(&E)) {
- // Origin of DeclRefExpr is that of the declaration it refers to.
- return getOrCreate(*DRE->getDecl());
- }
OriginID NewID = getNextOriginID();
addOrigin(NewID, E);
ExprToOriginID[&E] = NewID;
@@ -235,7 +238,8 @@ class Fact {
return nullptr;
}
- virtual void dump(llvm::raw_ostream &OS, const OriginManager &) const {
+ virtual void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &) const {
OS << "Fact (Kind: " << static_cast<int>(K) << ")\n";
}
};
@@ -250,8 +254,11 @@ class IssueFact : public Fact {
IssueFact(LoanID LID, OriginID OID) : Fact(Kind::Issue), LID(LID), OID(OID) {}
LoanID getLoanID() const { return LID; }
OriginID getOriginID() const { return OID; }
- void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
- OS << "Issue (LoanID: " << getLoanID() << ", ToOrigin: ";
+ void dump(llvm::raw_ostream &OS, const LoanManager &LM,
+ const OriginManager &OM) const override {
+ OS << "Issue (";
+ LM.getLoan(getLoanID()).dump(OS);
+ OS << ", ToOrigin: ";
OM.dump(getOriginID(), OS);
OS << ")\n";
}
@@ -270,8 +277,11 @@ class ExpireFact : public Fact {
LoanID getLoanID() const { return LID; }
SourceLocation getExpiryLoc() const { return ExpiryLoc; }
- void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
- OS << "Expire (LoanID: " << getLoanID() << ")\n";
+ void dump(llvm::raw_ostream &OS, const LoanManager &LM,
+ const OriginManager &) const override {
+ OS << "Expire (";
+ LM.getLoan(getLoanID()).dump(OS);
+ OS << ")\n";
}
};
@@ -288,7 +298,8 @@ class AssignOriginFact : public Fact {
: Fact(Kind::AssignOrigin), OIDDest(OIDDest), OIDSrc(OIDSrc) {}
OriginID getDestOriginID() const { return OIDDest; }
OriginID getSrcOriginID() const { return OIDSrc; }
- void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const override {
OS << "AssignOrigin (Dest: ";
OM.dump(getDestOriginID(), OS);
OS << ", Src: ";
@@ -307,7 +318,8 @@ class ReturnOfOriginFact : public Fact {
ReturnOfOriginFact(OriginID OID) : Fact(Kind::ReturnOfOrigin), OID(OID) {}
OriginID getReturnedOriginID() const { return OID; }
- void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const override {
OS << "ReturnOfOrigin (";
OM.dump(getReturnedOriginID(), OS);
OS << ")\n";
@@ -333,10 +345,11 @@ class UseFact : public Fact {
void markAsWritten() { IsWritten = true; }
bool isWritten() const { return IsWritten; }
- void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const override {
OS << "Use (";
OM.dump(getUsedOrigin(OM), OS);
- OS << " " << (isWritten() ? "Write" : "Read") << ")\n";
+ OS << ", " << (isWritten() ? "Write" : "Read") << ")\n";
}
};
@@ -353,7 +366,8 @@ class TestPointFact : public Fact {
StringRef getAnnotation() const { return Annotation; }
- void dump(llvm::raw_ostream &OS, const OriginManager &) const override {
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &) const override {
OS << "TestPoint (Annotation: \"" << getAnnotation() << "\")\n";
}
};
@@ -392,7 +406,7 @@ class FactManager {
if (It != BlockToFactsMap.end()) {
for (const Fact *F : It->second) {
llvm::dbgs() << " ";
- F->dump(llvm::dbgs(), OriginMgr);
+ F->dump(llvm::dbgs(), LoanMgr, OriginMgr);
}
}
llvm::dbgs() << " End of Block\n";
@@ -438,12 +452,31 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
void VisitDeclStmt(const DeclStmt *DS) {
for (const Decl *D : DS->decls())
if (const auto *VD = dyn_cast<VarDecl>(D))
- if (hasOrigin(VD->getType()))
+ if (hasOrigin(VD))
if (const Expr *InitExpr = VD->getInit())
addAssignOriginFact(*VD, *InitExpr);
}
- void VisitDeclRefExpr(const DeclRefExpr *DRE) { handleUse(DRE); }
+ void VisitDeclRefExpr(const DeclRefExpr *DRE) {
+ handleUse(DRE);
+ // For non-pointer/non-view types, a reference to the variable's storage
+ // is a borrow. We create a loan for it.
+ // For pointer/view types, we stick to the existing model for now and do
+ // not create an extra origin for the l-value expression itself.
+
+ // FIXME: A loan to `DeclRefExpr` for a pointer or view type can be
+ // ambiguous. It can refer to the variable's storage (as an l-value) or its
+ // value (as an r-value, which is a pointer). The current single-origin
+ // model cannot distinguish between a loan to the variable itself and a loan
+ // to what it points to. A multi-origin model would be required for this.
+ if (!isPointerType(DRE->getType())) {
+ if (const Loan *L = createLoan(DRE)) {
+ OriginID ExprOID = FactMgr.getOriginMgr().getOrCreate(*DRE);
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<IssueFact>(L->ID, ExprOID));
+ }
+ }
+ }
void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
/// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
@@ -452,38 +485,31 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
}
void VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
- if (!hasOrigin(ICE->getType()))
+ if (!hasOrigin(ICE))
return;
// An ImplicitCastExpr node itself gets an origin, which flows from the
// origin of its sub-expression (after stripping its own parens/casts).
- // TODO: Consider if this is actually useful in practice. Alternatively, we
- // could directly use the sub-expression's OriginID instead of creating a
- // new one.
addAssignOriginFact(*ICE, *ICE->getSubExpr());
}
void VisitUnaryOperator(const UnaryOperator *UO) {
if (UO->getOpcode() == UO_AddrOf) {
const Expr *SubExpr = UO->getSubExpr();
- if (const auto *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
- if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
- // Check if it's a local variable.
- if (VD->hasLocalStorage()) {
- OriginID OID = FactMgr.getOriginMgr().getOrCreate(*UO);
- AccessPath AddrOfLocalVarPath(VD);
- const Loan &L =
- FactMgr.getLoanMgr().addLoan(AddrOfLocalVarPath, UO);
- CurrentBlockFacts.push_back(
- FactMgr.createFact<IssueFact>(L.ID, OID));
- }
- }
- }
+ // Taking address of a pointer-type expression is not yet supported and
+ // will be supported in multi-origin model.
+ if (isPointerType(SubExpr->getType()))
+ return;
+ // The origin of an address-of expression (e.g., &x) is the origin of
+ // its sub-expression (x). This fact will cause the dataflow analysis
+ // to propagate any loans held by the sub-expression's origin to the
+ // origin of this UnaryOperator expression.
+ addAssignOriginFact(*UO, *SubExpr);
}
}
void VisitReturnStmt(const ReturnStmt *RS) {
if (const Expr *RetExpr = RS->getRetValue()) {
- if (hasOrigin(RetExpr->getType())) {
+ if (hasOrigin(RetExpr)) {
OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr);
CurrentBlockFacts.push_back(
FactMgr.createFact<ReturnOfOriginFact>(OID));
@@ -506,20 +532,6 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
// expression.
if (VisitTestPoint(FCE))
return;
- // Visit as normal otherwise.
- Base::VisitCXXFunctionalCastExpr(FCE);
- }
-
-private:
- // Check if a type has an origin.
- bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
-
- template <typename Destination, typename Source>
- void addAssignOriginFact(const Destination &D, const Source &S) {
- OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D);
- OriginID SrcOID = FactMgr.getOriginMgr().get(S);
- CurrentBlockFacts.push_back(
- FactMgr.createFact<AssignOriginFact>(DestOID, SrcOID));
}
void handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
@@ -544,6 +556,41 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
}
}
+private:
+ static bool isPointerType(QualType QT) {
+ return QT->isPointerOrReferenceType();
+ }
+
+ // Check if a type has an origin.
+ static bool hasOrigin(const Expr *E) {
+ return E->isGLValue() || isPointerType(E->getType());
+ }
+
+ static bool hasOrigin(const VarDecl *VD) {
+ return isPointerType(VD->getType());
+ }
+
+ /// Creates a loan for the storage path of a given declaration reference.
+ /// This function should be called whenever a DeclRefExpr represents a borrow.
+ /// \param DRE The declaration reference expression that initiates the borrow.
+ /// \return The new Loan on success, nullptr otherwise.
+ const Loan *createLoan(const DeclRefExpr *DRE) {
+ if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) {
+ AccessPath Path(VD);
+ // The loan is created at the location of the DeclRefExpr.
+ return &FactMgr.getLoanMgr().addLoan(Path, DRE);
+ }
+ return nullptr;
+ }
+
+ template <typename Destination, typename Source>
+ void addAssignOriginFact(const Destination &D, const Source &S) {
+ OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D);
+ OriginID SrcOID = FactMgr.getOriginMgr().get(S);
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<AssignOriginFact>(DestOID, SrcOID));
+ }
+
/// Checks if the expression is a `void("__lifetime_test_point_...")` cast.
/// If so, creates a `TestPointFact` and returns true.
bool VisitTestPoint(const CXXFunctionalCastExpr *FCE) {
@@ -566,17 +613,18 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
}
void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr) {
+ if (!hasOrigin(LHSExpr))
+ return;
// Find the underlying variable declaration for the left-hand side.
if (const auto *DRE_LHS =
dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenImpCasts())) {
markUseAsWrite(DRE_LHS);
if (const auto *VD_LHS = dyn_cast<ValueDecl>(DRE_LHS->getDecl()))
- if (hasOrigin(LHSExpr->getType()))
- // We are interested in assignments like `ptr1 = ptr2` or `ptr = &var`
- // LHS must be a pointer/reference type that can be an origin.
- // RHS must also represent an origin (either another pointer/ref or an
- // address-of).
- addAssignOriginFact(*VD_LHS, *RHSExpr);
+ // We are interested in assignments like `ptr1 = ptr2` or `ptr = &var`.
+ // LHS must be a pointer/reference type that can be an origin. RHS must
+ // also represent an origin (either another pointer/ref or an
+ // address-of).
+ addAssignOriginFact(*VD_LHS, *RHSExpr);
}
}
@@ -584,7 +632,7 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
// use-after-free unless it is being written to (e.g. on the left-hand side
// of an assignment).
void handleUse(const DeclRefExpr *DRE) {
- if (hasOrigin(DRE->getType())) {
+ if (isPointerType(DRE->getType())) {
UseFact *UF = FactMgr.createFact<UseFact>(DRE);
CurrentBlockFacts.push_back(UF);
assert(!UseFacts.contains(DRE));
diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
index 11437d024b693..7dac27506fb6b 100644
--- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
@@ -10,45 +10,55 @@ struct MyObj {
// CHECK-LABEL: Function: return_local_addr
MyObj* return_local_addr() {
MyObj x {10};
- MyObj* p = &x;
// CHECK: Block B{{[0-9]+}}:
-// CHECK: Issue (LoanID: [[L_X:[0-9]+]], ToOrigin: [[O_ADDR_X:[0-9]+]] (Expr: UnaryOperator))
+// CHECK: Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: [[O_DRE_X:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_ADDR_X:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_X]] (Expr: DeclRefExpr))
+ MyObj* p = &x;
// CHECK: AssignOrigin (Dest: [[O_P:[0-9]+]] (Decl: p), Src: [[O_ADDR_X]] (Expr: UnaryOperator))
return p;
+// CHECK: Use ([[O_P]] (Decl: p), Read)
// CHECK: AssignOrigin (Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_P]] (Decl: p))
// CHECK: ReturnOfOrigin ([[O_RET_VAL]] (Expr: ImplicitCastExpr))
-// CHECK: Expire (LoanID: [[L_X]])
+// CHECK: Expire ([[L_X]] (Path: x))
}
// Pointer Assignment and Return
// CHECK-LABEL: Function: assign_and_return_local_addr
-// CHECK-NEXT: Block B{{[0-9]+}}:
MyObj* assign_and_return_local_addr() {
MyObj y{20};
+// CHECK: Block B{{[0-9]+}}:
+// CHECK: Issue ([[L_Y:[0-9]+]] (Path: y), ToOrigin: [[O_DRE_Y:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_Y]] (Expr: DeclRefExpr))
MyObj* ptr1 = &y;
-// CHECK: Issue (LoanID: [[L_Y:[0-9]+]], ToOrigin: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator))
-// CHECK: AssignOrigin (Dest: [[O_PTR1:[0-9]+]] (Decl: ptr1), Src: [[O_ADDR_Y]] (Expr: UnaryOperator))
+// CHECK: AssignOrigin (Dest: [[O_PTR1:[0-9]+]] (Decl: ptr1), Src: [[O_ADDR_Y]] (Expr: UnaryOperator))
MyObj* ptr2 = ptr1;
-// CHECK: AssignOrigin (Dest: [[O_PTR1_RVAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR1]] (Decl: ptr1))
-// CHECK: AssignOrigin (Dest: [[O_PTR2:[0-9]+]] (Decl: ptr2), Src: [[O_PTR1_RVAL]] (Expr: ImplicitCastExpr))
+// CHECK: Use ([[O_PTR1]] (Decl: ptr1), Read)
+// CHECK: AssignOrigin (Dest: [[O_PTR1_RVAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR1]] (Decl: ptr1))
+// CHECK: AssignOrigin (Dest: [[O_PTR2:[0-9]+]] (Decl: ptr2), Src: [[O_PTR1_RVAL]] (Expr: ImplicitCastExpr))
ptr2 = ptr1;
-// CHECK: AssignOrigin (Dest: [[O_PTR1_RVAL_2:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR1]] (Decl: ptr1))
-// CHECK: AssignOrigin (Dest: [[O_PTR2]] (Decl: ptr2), Src: [[O_PTR1_RVAL_2]] (Expr: ImplicitCastExpr))
+// CHECK: Use ([[O_PTR1]] (Decl: ptr1), Read)
+// CHECK: AssignOrigin (Dest: [[O_PTR1_RVAL_2:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR1]] (Decl: ptr1))
+// CHECK: Use ({{[0-9]+}} (Decl: ptr2), Write)
+// CHECK: AssignOrigin (Dest: [[O_PTR2]] (Decl: ptr2), Src: [[O_PTR1_RVAL_2]] (Expr: ImplicitCastExpr))
ptr2 = ptr2; // Self assignment.
-// CHECK: AssignOrigin (Dest: [[O_PTR2_RVAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR2]] (Decl: ptr2))
-// CHECK: AssignOrigin (Dest: [[O_PTR2]] (Decl: ptr2), Src: [[O_PTR2_RVAL]] (Expr: ImplicitCastExpr))
+// CHECK: Use ([[O_PTR2]] (Decl: ptr2), Read)
+// CHECK: AssignOrigin (Dest: [[O_PTR2_RVAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR2]] (Decl: ptr2))
+// CHECK: Use ([[O_PTR2]] (Decl: ptr2), Write)
+// CHECK: AssignOrigin (Dest: [[O_PTR2]] (Decl: ptr2), Src: [[O_PTR2_RVAL]] (Expr: ImplicitCastExpr))
return ptr2;
-// CHECK: AssignOrigin (Dest: [[O_PTR2_RVAL_2:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR2]] (Decl: ptr2))
-// CHECK: ReturnOfOrigin ([[O_PTR2_RVAL_2]] (Expr: ImplicitCastExpr))
-// CHECK: Expire (LoanID: [[L_Y]])
+// CHECK: Use ([[O_PTR2]] (Decl: ptr2), Read)
+// CHECK: AssignOrigin (Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR2]] (Decl: ptr2))
+// CHECK: ReturnOfOrigin ([[O_RET_VAL]] (Expr: ImplicitCastExpr))
+// CHECK: Expire ([[L_Y]] (Path: y))
}
// Return of Non-Pointer Type
// CHECK-LABEL: Function: return_int_val
-// CHECK-NEXT: Block B{{[0-9]+}}:
int return_int_val() {
int x = 10;
+// CHECK: Block B{{[0-9]+}}:
+// CHECK: Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr))
return x;
}
// CHECK-NEXT: End of Block
@@ -56,25 +66,27 @@ int return_int_val() {
// Loan Expiration (Automatic Variable, C++)
// CHECK-LABEL: Function: loan_expires_cpp
-// CHECK-NEXT: Block B{{[0-9]+}}:
void loan_expires_cpp() {
MyObj obj{1};
+// CHECK: Block B{{[0-9]+}}:
+// CHECK: Issue ([[L_OBJ:[0-9]+]] (Path: obj), ToOrigin: [[O_DRE_OBJ:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_ADDR_OBJ:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_OBJ]] (Expr: DeclRefExpr))
MyObj* pObj = &obj;
-// CHECK: Issue (LoanID: [[L_OBJ:[0-9]+]], ToOrigin: [[O_ADDR_OBJ:[0-9]+]] (Expr: UnaryOperator))
-// CHECK: AssignOrigin (Dest: [[O_POBJ:[0-9]+]] (Decl: pObj), Src: [[O_ADDR_OBJ]] (Expr: UnaryOperator))
-// CHECK: Expire (LoanID: [[L_OBJ]])
+// CHECK: AssignOrigin (Dest: {{[0-9]+}} (Decl: pObj), Src: [[O_ADDR_OBJ]] (Expr: UnaryOperator))
+// CHECK: Expire ([[L_OBJ]] (Path: obj))
}
// FIXME: No expire for Trivial Destructors
// CHECK-LABEL: Function: loan_expires_trivial
-// CHECK-NEXT: Block B{{[0-9]+}}:
void loan_expires_trivial() {
int trivial_obj = 1;
+// CHECK: Block B{{[0-9]+}}:
+// CHECK: Issue ([[L_TRIVIAL_OBJ:[0-9]+]] (Path: trivial_obj), ToOrigin: [[O_DRE_TRIVIAL:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_ADDR_TRIVIAL_OBJ:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_TRIVIAL]] (Expr: DeclRefExpr))
int* pTrivialObj = &trivial_obj;
-// CHECK: Issue (LoanID: [[L_TRIVIAL_OBJ:[0-9]+]], ToOrigin: [[O_ADDR_TRIVIAL_OBJ:[0-9]+]] (Expr: UnaryOperator))
-// CHECK: AssignOrigin (Dest: [[O_PTOBJ:[0-9]+]] (Decl: pTrivialObj), Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator))
-// CHECK-NOT: Expire (LoanID: [[L_TRIVIAL_OBJ]])
+// CHECK: AssignOrigin (Dest: {{[0-9]+}} (Decl: pTrivialObj), Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator))
+// CHECK-NOT: Expire
// CHECK-NEXT: End of Block
// FIXME: Add check for Expire once trivial destructors are handled for expiration.
}
@@ -86,16 +98,22 @@ void conditional(bool condition) {
int* p = nullptr;
if (condition)
+// CHECK: Block B{{[0-9]+}}:
+// CHECK: Issue ([[L_A:[0-9]+]] (Path: a), ToOrigin: [[O_DRE_A:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_A]] (Expr: DeclRefExpr))
+// CHECK: AssignOrigin (Dest: [[O_P:[0-9]+]] (Decl: p), Src: [[O_ADDR_A]] (Expr: UnaryOperator))
p = &a;
-// CHECK: Issue (LoanID: [[L_A:[0-9]+]], ToOrigin: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator))
-// CHECK: AssignOrigin (Dest: [[O_P:[0-9]+]] (Decl: p), Src: [[O_AD...
[truncated]
|
Xazax-hun
reviewed
Sep 4, 2025
| // For pointer/view types, we stick to the existing model for now and do | ||
| // not create an extra origin for the l-value expression itself. | ||
|
|
||
| // FIXME: A loan to `DeclRefExpr` for a pointer or view type can be |
There was a problem hiding this comment.
Couldn't we disambiguate based on the value category? I'd expect us to only create loans here when the DRE is not an r-value.
There was a problem hiding this comment.
Ah, never mind. The DeclRefExpr itself is always an lvalue. We just have an LValueToRValue conversion on top.
There was a problem hiding this comment.
We could potentially optimize by ignoring DREs under such conversions but I am fine not doing that for now.
There was a problem hiding this comment.
Yes.
This is to not create loans to pointer types/view types which will need multi-origin modelling.
std::string_view s;
std::string_view t = s; // RHS is essentially a reference to view needing two origins (Only the inner origin makes it to the LHS t)
There was a problem hiding this comment.
That being said, I think there is no ambiguity here. The DeclRefExpr is always referring to the lvalue. And we should always have the LValueToRValue conversion when we refer to the value. Handling that conversion correctly can help us never be ambiguous.
There was a problem hiding this comment.
Except for opaque function calls. We need to rely on annotations for those.
There was a problem hiding this comment.
I agree. An LValueToRValue would be responsible for propagating the correct (inner) origin when we have multiple origins for l-valued pointer-type expressions.
// FIXME: A loan to
DeclRefExprfor a pointer or view type can be ambiguous
Avoided mentioning the ambiguity here. Changed to
TODO: A single origin for a `DeclRefExpr` for a pointer or view type
is not sufficient to model the different levels of indirection.
Xazax-hun
approved these changes
Sep 4, 2025
usx95
force-pushed
the
users/usx95/09-04-all-lvalues-have-origin
branch
from
2f0c4c1 to
1a5fa17
Compare
Base automatically changed from
users/usx95/08-19-identify_declrefexpr_as_a_use_of_an_origin
to
main
September 5, 2025 20:17
usx95
force-pushed
the
users/usx95/09-04-all-lvalues-have-origin
branch
from
1a5fa17 to
fdbae53
Compare
usx95
force-pushed
the
users/usx95/09-04-all-lvalues-have-origin
branch
from
fdbae53 to
f7906c2
Compare
|
Since we discussed this patch briefly f2f and no major concerns were raised, I will land this for now @ymand . Happy to address review comments post commit. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/30920 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch refactors the C++ lifetime safety analysis to implement a more consistent model for tracking borrows. The central idea is to make loan creation a consequence of referencing a variable, while making loan propagation dependent on the type's semantics.
This change introduces a more uniform model for tracking borrows from non-pointer types:
Centralised Loan Creation: A Loan is now created for every
DeclRefExprthat refers to a non-pointer type (e.g.,std::string,int). This correctly models that any use of an gl-value is a borrow of its storage, replacing the previous heuristic-based loan creation.The address-of operator (&) no longer creates loans. Instead, it propagates the origin (and thus the loans) of its sub-expression. This is guarded to exclude expressions that are already pointer types, deferring the complexity of pointers-to-pointers.
Future Work: Multi-Origin Model
This patch deliberately defers support for creating loans on references to pointer-type expressions (e.g.,
&my_pointer). The current single-origin model is unable to distinguish between a loan to the pointer variable itself (its storage) and a loan to the object it points to. The future plan is to move to a multi-origin model where a type has a "list of origins" governed by its level of indirection, which will allow the analysis to track these distinct lifetimes separately. Once this more advanced model is in place, the restriction can be lifted, and allDeclRefExprnodes, regardless of type, can uniformly create a loan, making the analysis consistent.