[LifetimeSafety] Track gsl::Pointer types

Author: usx95

clang/lib/Analysis/LifetimeSafety.cpp

@@ -433,6 +433,31 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
 
   void VisitDeclRefExpr(const DeclRefExpr *DRE) { handleUse(DRE); }
 
+  void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
+    if (!isGslPointerType(CCE->getType()))
+      return;
+
+    if (CCE->getNumArgs() > 0 && hasOrigin(CCE->getArg(0)->getType()))
+      // This is a propagation.
+      addAssignOriginFact(*CCE, *CCE->getArg(0));
+    else
+      // This could be a new borrow.
+      checkForBorrows(CCE, CCE->getConstructor(),
+                      {CCE->getArgs(), CCE->getNumArgs()});
+  }
+
+  void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+    if (!isGslPointerType(MCE->getImplicitObjectArgument()->getType()))
+      return;
+    // Specifically for conversion operators, like `std::string_view p = a;`
+    if (isa<CXXConversionDecl>(MCE->getCalleeDecl())) {
+      // The argument is the implicit object itself.
+      checkForBorrows(MCE, MCE->getMethodDecl(),
+                      {MCE->getImplicitObjectArgument()});
+    }
+    // Note: A more general VisitCallExpr could also be used here.
+  }
+
   void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
     /// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
     /// pointers can use the same type of loan.
@@ -492,10 +517,27 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
   void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE) {
     // Check if this is a test point marker. If so, we are done with this
     // expression.
-    if (VisitTestPoint(FCE))
+    if (handleTestPoint(FCE))
       return;
-    // Visit as normal otherwise.
-    Base::VisitCXXFunctionalCastExpr(FCE);
+    if (isGslPointerType(FCE->getType()))
+      addAssignOriginFact(*FCE, *FCE->getSubExpr());
+  }
+
+  void VisitInitListExpr(const InitListExpr *ILE) {
+    if (!hasOrigin(ILE->getType()))
+      return;
+    // For list initialization with a single element, like `View{...}`, the
+    // origin of the list itself is the origin of its single element.
+    if (ILE->getNumInits() == 1)
+      addAssignOriginFact(*ILE, *ILE->getInit(0));
+  }
+
+  void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) {
+    if (!hasOrigin(MTE->getType()))
+      return;
+    // A temporary object's origin is the same as the origin of the
+    // expression that initializes it.
+    addAssignOriginFact(*MTE, *MTE->getSubExpr());
   }
 
   void handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
@@ -521,8 +563,75 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
   }
 
 private:
+  static bool isGslPointerType(QualType QT) {
+    if (const auto *RD = QT->getAsCXXRecordDecl()) {
+      // We need to check the template definition for specializations.
+      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+        return CTSD->getSpecializedTemplate()
+            ->getTemplatedDecl()
+            ->hasAttr<PointerAttr>();
+      return RD->hasAttr<PointerAttr>();
+    }
+    return false;
+  }
+
   // Check if a type has an origin.
-  bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
+  static bool hasOrigin(QualType QT) {
+    if (QT->isFunctionPointerType())
+      return false;
+    return QT->isPointerOrReferenceType() || isGslPointerType(QT);
+  }
+
+  /// Checks if a call-like expression creates a borrow by passing a local
+  /// value to a reference parameter, creating an IssueFact if it does.
+  void checkForBorrows(const Expr *Call, const FunctionDecl *FD,
+                       ArrayRef<const Expr *> Args) {
+    if (!FD)
+      return;
+
+    for (unsigned I = 0; I < Args.size(); ++I) {
+      if (I >= FD->getNumParams())
+        break;
+
+      const ParmVarDecl *Param = FD->getParamDecl(I);
+      const Expr *Arg = Args[I];
+
+      // This is the core condition for a new borrow: a value type (no origin)
+      // is passed to a reference parameter.
+      if (Param->getType()->isReferenceType() && !hasOrigin(Arg->getType())) {
+        if (const Loan *L = createLoanFrom(Arg, Call)) {
+          OriginID OID = FactMgr.getOriginMgr().getOrCreate(*Call);
+          CurrentBlockFacts.push_back(
+              FactMgr.createFact<IssueFact>(L->ID, OID));
+          // For view creation, we assume the first borrow is the significant
+          // one.
+          return;
+        }
+      }
+    }
+  }
+
+  /// Attempts to create a loan by analyzing the source expression of a borrow.
+  /// This method is the single point for creating loans, allowing for future
+  /// expansion to handle temporaries, field members, etc.
+  /// \param SourceExpr The expression representing the object being borrowed
+  /// from.
+  /// \param IssueExpr The expression that triggers the borrow (e.g., a
+  /// constructor call).
+  /// \return The new Loan on success, nullptr on failure.
+  const Loan *createLoanFrom(const Expr *SourceExpr, const Expr *IssueExpr) {
+    // For now, we only handle direct borrows from local variables.
+    // In the future, this can be extended to handle MaterializeTemporaryExpr,
+    // etc.
+    if (const auto *DRE =
+            dyn_cast<DeclRefExpr>(SourceExpr->IgnoreParenImpCasts())) {
+      if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) {
+        AccessPath Path(VD);
+        return &FactMgr.getLoanMgr().addLoan(Path, IssueExpr);
+      }
+    }
+    return nullptr;
+  }
 
   template <typename Destination, typename Source>
   void addAssignOriginFact(const Destination &D, const Source &S) {
@@ -534,7 +643,7 @@ class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
 
   /// Checks if the expression is a `void("__lifetime_test_point_...")` cast.
   /// If so, creates a `TestPointFact` and returns true.
-  bool VisitTestPoint(const CXXFunctionalCastExpr *FCE) {
+  bool handleTestPoint(const CXXFunctionalCastExpr *FCE) {
     if (!FCE->getType()->isVoidType())
       return false;
 
@@ -612,10 +721,13 @@ class FactGenerator : public RecursiveASTVisitor<FactGenerator> {
     for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) {
       FactGeneratorBlockRAII BlockGenerator(FG, Block);
       for (const CFGElement &Element : *Block) {
-        if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
+        if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) {
+          DEBUG_WITH_TYPE("PrintCFG", llvm::dbgs() << " ================== \n");
+          DEBUG_WITH_TYPE("PrintCFG", CS->dump());
+          DEBUG_WITH_TYPE("PrintCFG", CS->getStmt()->dumpColor());
           TraverseStmt(const_cast<Stmt *>(CS->getStmt()));
-        else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
-                     Element.getAs<CFGAutomaticObjDtor>())
+        } else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
+                       Element.getAs<CFGAutomaticObjDtor>())
           FG.handleDestructor(*DtorOpt);
       }
     }

clang/test/Sema/warn-lifetime-safety.cpp

@@ -6,6 +6,12 @@ struct MyObj {
   MyObj operator+(MyObj);
 };
 
+struct [[gsl::Pointer()]] View {
+  View(const MyObj&); // Borrows from MyObj
+  View();
+  void use() const;
+};
+
 //===----------------------------------------------------------------------===//
 // Basic Definite Use-After-Free (-W...permissive)
 // These are cases where the pointer is guaranteed to be dangling at the use site.
@@ -20,12 +26,31 @@ void definite_simple_case() {
   (void)*p;     // expected-note {{later used here}}
 }
 
+void definite_simple_case_gsl() {
+  View v;
+  {
+    MyObj s;
+    v = s;      // expected-warning {{object whose reference is captured does not live long enough}}
+  }             // expected-note {{destroyed here}}
+  v.use();      // expected-note {{later used here}}
+}
+
 void no_use_no_error() {
   MyObj* p;
   {
     MyObj s;
     p = &s;
   }
+  // 'p' is dangling here, but since it is never used, no warning is issued.
+}
+
+void no_use_no_error_gsl() {
+  View v;
+  {
+    MyObj s;
+    v = s;
+  }
+  // 'v' is dangling here, but since it is never used, no warning is issued.
 }
 
 void definite_pointer_chain() {
@@ -39,6 +64,16 @@ void definite_pointer_chain() {
   (void)*q;     // expected-note {{later used here}}
 }
 
+void definite_propagation_gsl() {
+  View v1, v2;
+  {
+    MyObj s;
+    v1 = s;     // expected-warning {{object whose reference is captured does not live long enough}}
+    v2 = v1;
+  }             // expected-note {{destroyed here}}
+  v2.use();     // expected-note {{later used here}}
+}
+
 void definite_multiple_uses_one_warning() {
   MyObj* p;
   {
@@ -78,6 +113,19 @@ void definite_single_pointer_multiple_loans(bool cond) {
   (void)*p;     // expected-note 2  {{later used here}}
 }
 
+void definite_single_pointer_multiple_loans_gsl(bool cond) {
+  View v;
+  if (cond){
+    MyObj s;
+    v = s;      // expected-warning {{object whose reference is captured does not live long enough}}
+  }             // expected-note {{destroyed here}}
+  else {
+    MyObj t;
+    v = t;      // expected-warning {{object whose reference is captured does not live long enough}}
+  }             // expected-note {{destroyed here}}
+  v.use();      // expected-note 2 {{later used here}}
+}
+
 
 //===----------------------------------------------------------------------===//
 // Potential (Maybe) Use-After-Free (-W...strict)
@@ -94,18 +142,14 @@ void potential_if_branch(bool cond) {
   (void)*p;     // expected-note {{later used here}}
 }
 
-// If all paths lead to a dangle, it becomes a definite error.
-void potential_becomes_definite(bool cond) {
-  MyObj* p;
+void potential_if_branch_gsl(bool cond) {
+  MyObj safe;
+  View v = safe;
   if (cond) {
-    MyObj temp1;
-    p = &temp1; // expected-warning {{does not live long enough}}
-  }             // expected-note {{destroyed here}}
-  else {      
-    MyObj temp2;
-    p = &temp2; // expected-warning {{does not live long enough}}
+    MyObj temp;
+    v = temp;   // expected-warning {{object whose reference is captured may not live long enough}}
   }             // expected-note {{destroyed here}}
-  (void)*p;     // expected-note 2 {{later used here}}
+  v.use();      // expected-note {{later used here}}
 }
 
 void definite_potential_together(bool cond) {
@@ -159,6 +203,16 @@ void potential_for_loop_use_after_loop_body(MyObj safe) {
   (void)*p;     // expected-note {{later used here}}
 }
 
+void potential_for_loop_gsl() {
+  MyObj safe;
+  View v = safe;
+  for (int i = 0; i < 1; ++i) {
+    MyObj s;
+    v = s;      // expected-warning {{object whose reference is captured may not live long enough}}
+  }             // expected-note {{destroyed here}}
+  v.use();      // expected-note {{later used here}}
+}
+
 void potential_for_loop_use_before_loop_body(MyObj safe) {
   MyObj* p = &safe;
   for (int i = 0; i < 1; ++i) {
@@ -182,6 +236,19 @@ void potential_loop_with_break(bool cond) {
   (void)*p;     // expected-note {{later used here}}
 }
 
+void potential_loop_with_break_gsl(bool cond) {
+  MyObj safe;
+  View v = safe;
+  for (int i = 0; i < 10; ++i) {
+    if (cond) {
+      MyObj temp;
+      v = temp;   // expected-warning {{object whose reference is captured may not live long enough}}
+      break;      // expected-note {{destroyed here}}
+    }
+  }
+  v.use();      // expected-note {{later used here}}
+}
+
 void potential_multiple_expiry_of_same_loan(bool cond) {
   // Choose the last expiry location for the loan.
   MyObj safe;
@@ -258,6 +325,28 @@ void definite_switch(int mode) {
   (void)*p;     // expected-note 3 {{later used here}}
 }
 
+void definite_switch_gsl(int mode) {
+  View v;
+  switch (mode) {
+  case 1: {
+    MyObj temp1;
+    v = temp1;  // expected-warning {{object whose reference is captured does not live long enough}}
+    break;      // expected-note {{destroyed here}}
+  }
+  case 2: {
+    MyObj temp2;
+    v = temp2;  // expected-warning {{object whose reference is captured does not live long enough}}
+    break;      // expected-note {{destroyed here}}
+  }
+  default: {
+    MyObj temp3;
+    v = temp3;  // expected-warning {{object whose reference is captured does not live long enough}}
+    break;      // expected-note {{destroyed here}}
+  }
+  }
+  v.use();      // expected-note 3 {{later used here}}
+}
+
 //===----------------------------------------------------------------------===//
 // No-Error Cases
 //===----------------------------------------------------------------------===//
@@ -271,3 +360,14 @@ void no_error_if_dangle_then_rescue() {
   p = &safe;    // p is "rescued" before use.
   (void)*p;     // This is safe.
 }
+
+void no_error_if_dangle_then_rescue_gsl() {
+  MyObj safe;
+  View v;
+  {
+    MyObj temp;
+    v = temp;  // 'v' is temporarily dangling.
+  }
+  v = safe;    // 'v' is "rescued" before use by reassigning to a valid object.
+  v.use();     // This is safe.
+}