[LifetimeSafety] Track gsl::Pointer types

Author: usx95

clang/lib/Analysis/LifetimeSafety.cpp

@@ -478,6 +478,25 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
     }
   }
 
+  void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
+    if (isGslPointerType(CCE->getType())) {
+      handleGSLPointerConstruction(CCE);
+      return;
+    }
+  }
+
+  void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+    // Specifically for conversion operators,
+    // like `std::string_view p = std::string{};`
+    if (isGslPointerType(MCE->getType()) &&
+        isa<CXXConversionDecl>(MCE->getCalleeDecl())) {
+      // The argument is the implicit object itself.
+      handleFunctionCall(MCE, MCE->getMethodDecl(),
+                         {MCE->getImplicitObjectArgument()});
+    }
+    // FIXME: 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.
@@ -530,8 +549,27 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
   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;
+    if (isGslPointerType(FCE->getType()))
+      addAssignOriginFact(*FCE, *FCE->getSubExpr());
+  }
+
+  void VisitInitListExpr(const InitListExpr *ILE) {
+    if (!hasOrigin(ILE))
+      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))
+      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) {
@@ -557,10 +595,21 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
   }
 
 private:
-  static bool isPointerType(QualType QT) {
-    return QT->isPointerOrReferenceType();
+  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;
   }
 
+  static bool isPointerType(QualType QT) {
+    return QT->isPointerOrReferenceType() || isGslPointerType(QT);
+  }
   // Check if a type has an origin.
   static bool hasOrigin(const Expr *E) {
     return E->isGLValue() || isPointerType(E->getType());
@@ -570,6 +619,41 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
     return isPointerType(VD->getType());
   }
 
+  void handleGSLPointerConstruction(const CXXConstructExpr *CCE) {
+    assert(isGslPointerType(CCE->getType()));
+    if (CCE->getNumArgs() != 1)
+      return;
+    if (hasOrigin(CCE->getArg(0)))
+      addAssignOriginFact(*CCE, *CCE->getArg(0));
+    else
+      // This could be a new borrow.
+      handleFunctionCall(CCE, CCE->getConstructor(),
+                         {CCE->getArgs(), CCE->getNumArgs()});
+  }
+
+  /// Checks if a call-like expression creates a borrow by passing a value to a
+  /// reference parameter, creating an IssueFact if it does.
+  void handleFunctionCall(const Expr *Call, const FunctionDecl *FD,
+                          ArrayRef<const Expr *> Args) {
+    if (!FD)
+      return;
+    // TODO: Handle more than one arguments.
+    for (unsigned I = 0; I <= 0 /*Args.size()*/; ++I) {
+      const Expr *ArgExpr = Args[I];
+
+      // Propagate origins for CXX this.
+      if (FD->isCXXClassMember() && I == 0) {
+        addAssignOriginFact(*Call, *ArgExpr);
+        continue;
+      }
+      // The parameter is a pointer, reference, or gsl::Pointer.
+      // This is a borrow. We propagate the origin from the argument expression
+      // at the call site to the parameter declaration in the callee.
+      if (hasOrigin(ArgExpr))
+        addAssignOriginFact(*Call, *ArgExpr);
+    }
+  }
+
   /// 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.
@@ -593,7 +677,7 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
 
   /// 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;
 
@@ -641,6 +725,8 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
   }
 
   void markUseAsWrite(const DeclRefExpr *DRE) {
+    if (!isPointerType(DRE->getType()))
+      return;
     assert(UseFacts.contains(DRE));
     UseFacts[DRE]->markAsWritten();
   }

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.
+}