[llvm-branch-commits] [clang] [LifetimeSafety] Mark all DeclRefExpr as usages of the corresp. origin (PR #154316)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

Author: Utkarsh Saxena (usx95)

<details>
<summary>Changes</summary>

Instead of identifying various forms of pointer usage (like dereferencing, member access, or function calls) individually, this new approach simplifies the logic by treating all `DeclRefExpr`s as uses of their underlying origin

When a `DeclRefExpr` appears on the left-hand side of an assignment, the corresponding `UseFact` is marked as a "write" operation. These write operations are then exempted from use-after-free checks.

---
Full diff: https://github.com/llvm/llvm-project/pull/154316.diff


2 Files Affected:

- (modified) clang/lib/Analysis/LifetimeSafety.cpp (+55-20) 
- (modified) clang/test/Sema/warn-lifetime-safety-dataflow.cpp (+26) 


``````````diff
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index 9397c530a9af2..00efa4c5e548f 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -317,6 +317,7 @@ class ReturnOfOriginFact : public Fact {
 class UseFact : public Fact {
   OriginID UsedOrigin;
   const Expr *UseExpr;
+  bool IsWritten = false;
 
 public:
   static bool classof(const Fact *F) { return F->getKind() == Kind::Use; }
@@ -326,11 +327,13 @@ class UseFact : public Fact {
 
   OriginID getUsedOrigin() const { return UsedOrigin; }
   const Expr *getUseExpr() const { return UseExpr; }
+  void markAsWritten() { IsWritten = true; }
+  bool isWritten() const { return IsWritten; }
 
   void dump(llvm::raw_ostream &OS, const OriginManager &OM) const override {
     OS << "Use (";
     OM.dump(getUsedOrigin(), OS);
-    OS << ")\n";
+    OS << " " << (isWritten() ? "Write" : "Read") << ")\n";
   }
 };
 
@@ -428,6 +431,8 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
             addAssignOriginFact(*VD, *InitExpr);
   }
 
+  void VisitDeclRefExpr(const DeclRefExpr *DRE) { handleUse(DRE); }
+
   void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
     /// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
     /// pointers can use the same type of loan.
@@ -461,10 +466,6 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
           }
         }
       }
-    } else if (UO->getOpcode() == UO_Deref) {
-      // This is a pointer use, like '*p'.
-      OriginID OID = FactMgr.getOriginMgr().get(*UO->getSubExpr());
-      CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>(OID, UO));
     }
   }
 
@@ -479,20 +480,13 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
   }
 
   void VisitBinaryOperator(const BinaryOperator *BO) {
-    if (BO->isAssignmentOp()) {
-      const Expr *LHSExpr = BO->getLHS();
-      const Expr *RHSExpr = BO->getRHS();
-
-      // 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).
-      if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr))
-        if (const auto *VD_LHS =
-                dyn_cast<ValueDecl>(DRE_LHS->getDecl()->getCanonicalDecl());
-            VD_LHS && hasOrigin(VD_LHS->getType()))
-          addAssignOriginFact(*VD_LHS, *RHSExpr);
-    }
+    if (BO->isAssignmentOp())
+      handleAssignment(BO->getLHS(), BO->getRHS());
+  }
+
+  void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
+    if (OCE->isAssignmentOp() && OCE->getNumArgs() == 2)
+      handleAssignment(OCE->getArg(0), OCE->getArg(1));
   }
 
   void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE) {
@@ -559,9 +553,49 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
     return false;
   }
 
+  void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr) {
+    // 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(VD_LHS->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);
+    }
+  }
+
+  // A DeclRefExpr is a use of the referenced decl. It is checked for
+  // use-after-free unless it is being written to (e.g. on the left-hand side
+  // of an assignment).
+  void handleUse(const DeclRefExpr *DRE) {
+    const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl());
+    if (VD && hasOrigin(VD->getType())) {
+      OriginID OID = FactMgr.getOriginMgr().get(*VD);
+      UseFact *UF = FactMgr.createFact<UseFact>(OID, DRE);
+      CurrentBlockFacts.push_back(UF);
+      assert(!UseFacts.contains(DRE));
+      UseFacts[DRE] = UF;
+    }
+  }
+
+  void markUseAsWrite(const DeclRefExpr *DRE) {
+    assert(UseFacts.contains(DRE));
+    UseFacts[DRE]->markAsWritten();
+  }
+
   FactManager &FactMgr;
   const CFGBlock *CurrentBlock = nullptr;
   llvm::SmallVector<Fact *> CurrentBlockFacts;
+  // To distinguish between reads and writes for use-after-free checks, this map
+  // stores the `UseFact` for each `DeclRefExpr`. We initially identify all
+  // `DeclRefExpr`s as "read" uses. When an assignment is processed, the use
+  // corresponding to the left-hand side is updated to be a "write", thereby
+  // exempting it from the check.
+  llvm::DenseMap<const DeclRefExpr *, UseFact *> UseFacts;
 };
 
 class FactGenerator : public RecursiveASTVisitor<FactGenerator> {
@@ -1076,7 +1110,8 @@ class LifetimeChecker {
   /// 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();
 
     // Get the set of loans that the origin might hold at this program point.
diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
index bcde9adf25ca5..5660a8ace4a09 100644
--- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
@@ -293,3 +293,29 @@ void pointer_indirection() {
   int *q = *pp;
 // CHECK:   AssignOrigin (Dest: {{[0-9]+}} (Decl: q), Src: {{[0-9]+}} (Expr: ImplicitCastExpr))
 }
+
+// CHECK-LABEL: Function: test_use_facts
+void usePointer(MyObj*);
+void test_use_facts() {
+  // CHECK: Block B{{[0-9]+}}:
+  MyObj x;
+  MyObj *p;
+  p = &x;
+  // CHECK:   Use ([[O_P:[0-9]+]] (Decl: p) Write)
+  (void)*p;
+  // CHECK:   Use ([[O_P]] (Decl: p) Read)
+  usePointer(p);
+  // CHECK:   Use ([[O_P]] (Decl: p) Read)
+  p->id = 1;
+  // CHECK:   Use ([[O_P]] (Decl: p) Read)
+
+
+  MyObj* q;
+  q = p;
+  // CHECK:   Use ([[O_P]] (Decl: p) Read)
+  // CHECK:   Use ([[O_Q:[0-9]+]] (Decl: q) Write)
+  usePointer(q);
+  // CHECK:   Use ([[O_Q]] (Decl: q) Read)
+  q->id = 2;
+  // CHECK:   Use ([[O_Q]] (Decl: q) Read)
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/154316