all-lvalues-have-origin

Author: usx95

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.
+
+    // TODO: A single origin for a `DeclRefExpr` for a pointer or view type is
+    // not sufficient to model the different levels of indirection. The current
+    // single-origin model cannot distinguish between a loan to the variable's
+    // storage 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,25 +613,26 @@ 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);
     }
   }
 
-  // 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).
+  // A DeclRefExpr will be treated as a use of the referenced decl. It will be
+  // checked for use-after-free unless it is later marked as 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));