[LifetimeSafety] Detect expiry of loans to trivially destructed types (llvm#168855)

Handling Trivially Destructed Types

This PR uses `AddLifetime` to handle expiry of loans to trivially
destructed types.

Example:
```cpp
int * trivial_uar(){
    int *ptr;
    int x = 1;
    ptr = &x;
    return ptr;
}
```

The CFG created now has an Expire Fact for trivially destructed types:
```
Function: trivial_uar
  Block B2:
  End of Block
  Block B1:
    Issue (0 (Path: x), ToOrigin: 0 (Expr: DeclRefExpr))
    OriginFlow (Dest: 1 (Expr: UnaryOperator), Src: 0 (Expr: DeclRefExpr))
    Use (2 (Decl: ptr), Write)
    OriginFlow (Dest: 2 (Decl: ptr), Src: 1 (Expr: UnaryOperator))
    Use (2 (Decl: ptr), Read)
    OriginFlow (Dest: 3 (Expr: ImplicitCastExpr), Src: 2 (Decl: ptr))
    Expire (0 (Path: x))
    OriginEscapes (3 (Expr: ImplicitCastExpr))
  End of Block
  Block B0:
  End of Block
```

This Expire Fact issues UAR and UAF warnings.

Fixes llvm#162862

Author: kashika0112

clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h

@@ -50,7 +50,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
   void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE);
 
 private:
-  void handleDestructor(const CFGAutomaticObjDtor &DtorOpt);
+  void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds);
 
   void handleGSLPointerConstruction(const CXXConstructExpr *CCE);
 

clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp

@@ -63,9 +63,9 @@ void FactsGenerator::run() {
       const CFGElement &Element = Block->Elements[I];
       if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
         Visit(CS->getStmt());
-      else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
-                   Element.getAs<CFGAutomaticObjDtor>())
-        handleDestructor(*DtorOpt);
+      else if (std::optional<CFGLifetimeEnds> LifetimeEnds =
+                   Element.getAs<CFGLifetimeEnds>())
+        handleLifetimeEnds(*LifetimeEnds);
     }
     CurrentBlockFacts.append(EscapesInCurrentBlock.begin(),
                              EscapesInCurrentBlock.end());
@@ -229,25 +229,19 @@ void FactsGenerator::VisitMaterializeTemporaryExpr(
   killAndFlowOrigin(*MTE, *MTE->getSubExpr());
 }
 
-void FactsGenerator::handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
-  /// TODO: Also handle trivial destructors (e.g., for `int`
-  /// variables) which will never have a CFGAutomaticObjDtor node.
+void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) {
   /// TODO: Handle loans to temporaries.
-  /// TODO: Consider using clang::CFG::BuildOptions::AddLifetime to reuse the
-  /// lifetime ends.
-  const VarDecl *DestructedVD = DtorOpt.getVarDecl();
-  if (!DestructedVD)
+  const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl();
+  if (!LifetimeEndsVD)
     return;
   // Iterate through all loans to see if any expire.
-  /// TODO(opt): Do better than a linear search to find loans associated with
-  /// 'DestructedVD'.
-  for (const Loan &L : FactMgr.getLoanMgr().getLoans()) {
-    const AccessPath &LoanPath = L.Path;
+  for (const auto &Loan : FactMgr.getLoanMgr().getLoans()) {
+    const AccessPath &LoanPath = Loan.Path;
     // Check if the loan is for a stack variable and if that variable
     // is the one being destructed.
-    if (LoanPath.D == DestructedVD)
+    if (LoanPath.D == LifetimeEndsVD)
       CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
-          L.ID, DtorOpt.getTriggerStmt()->getEndLoc()));
+          Loan.ID, LifetimeEnds.getTriggerStmt()->getEndLoc()));
   }
 }
 

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2992,6 +2992,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
 
   bool EnableLifetimeSafetyAnalysis = S.getLangOpts().EnableLifetimeSafety;
 
+  if (EnableLifetimeSafetyAnalysis)
+    AC.getCFGBuildOptions().AddLifetime = true;
+
   // Force that certain expressions appear as CFGElements in the CFG.  This
   // is used to speed up various analyses.
   // FIXME: This isn't the right factoring.  This is here for initial

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

@@ -59,6 +59,7 @@ 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))
+// CHECK:   Expire ([[L_X:[0-9]+]] (Path: x))
   return x;
 }
 // CHECK-NEXT: End of Block
@@ -77,7 +78,6 @@ void loan_expires_cpp() {
 }
 
 
-// FIXME: No expire for Trivial Destructors
 // CHECK-LABEL: Function: loan_expires_trivial
 void loan_expires_trivial() {
   int trivial_obj = 1;
@@ -86,11 +86,29 @@ void loan_expires_trivial() {
 // CHECK:   OriginFlow (Dest: [[O_ADDR_TRIVIAL_OBJ:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_TRIVIAL]] (Expr: DeclRefExpr))
   int* pTrivialObj = &trivial_obj;
 // CHECK:   OriginFlow (Dest: {{[0-9]+}} (Decl: pTrivialObj), Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator))
-// CHECK-NOT: Expire
+// CHECK:   Expire ([[L_TRIVIAL_OBJ:[0-9]+]] (Path: trivial_obj))
 // CHECK-NEXT: End of Block
-  // FIXME: Add check for Expire once trivial destructors are handled for expiration.
 }
 
+// Trivial Destructors
+// CHECK-LABEL: Function: return_int_pointer
+int* return_int_pointer() {
+  int* ptr;
+// CHECK: Block B{{[0-9]+}}:
+  int x = 1;
+// CHECK:   Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: [[O_DRE_X:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK:   OriginFlow (Dest: [[O_ADDR_X:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_X]] (Expr: DeclRefExpr))
+  ptr = &x;
+// CHECK:   Use ([[O_PTR:[0-9]+]] (Decl: ptr), Write)
+// CHECK:   OriginFlow (Dest: [[O_PTR]] (Decl: ptr), Src: [[O_ADDR_X]] (Expr: UnaryOperator))
+// CHECK:   Use ([[O_PTR]] (Decl: ptr), Read)
+// CHECK:   OriginFlow (Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR]] (Decl: ptr))
+// CHECK:   Expire ([[L_X]] (Path: x))
+// CHECK:   OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr))
+  return ptr;
+}
+// CHECK-NEXT: End of Block
+
 // CHECK-LABEL: Function: conditional
 void conditional(bool condition) {
   int a = 5;

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

@@ -12,6 +12,10 @@ struct [[gsl::Pointer()]] View {
   void use() const;
 };
 
+class TriviallyDestructedClass {
+  View a, b;
+};
+
 //===----------------------------------------------------------------------===//
 // Basic Definite Use-After-Free (-W...permissive)
 // These are cases where the pointer is guaranteed to be dangling at the use site.
@@ -396,6 +400,24 @@ void loan_from_previous_iteration(MyObj safe, bool condition) {
   }             // expected-note {{destroyed here}}
 }
 
+void trivial_int_uaf() {
+  int * a;
+  {
+      int b = 1;
+      a = &b;  // expected-warning {{object whose reference is captured does not live long enough}}
+  }            // expected-note {{destroyed here}}
+  (void)*a;    // expected-note {{later used here}}
+}
+
+void trivial_class_uaf() {
+  TriviallyDestructedClass* ptr;
+  {
+      TriviallyDestructedClass s;
+      ptr = &s; // expected-warning {{object whose reference is captured does not live long enough}}
+  }             // expected-note {{destroyed here}}
+  (void)ptr;    // expected-note {{later used here}}
+}
+
 //===----------------------------------------------------------------------===//
 // Basic Definite Use-After-Return (Return-Stack-Address) (-W...permissive)
 // These are cases where the pointer is guaranteed to be dangling at the use site.
@@ -493,6 +515,43 @@ MyObj& reference_return_of_local() {
                     // expected-note@-1 {{returned here}}
 }
 
+int* trivial_int_uar() {
+  int *a;
+  int b = 1;
+  a = &b;          // expected-warning {{address of stack memory is returned later}}
+  return a;        // expected-note {{returned here}}
+}
+
+TriviallyDestructedClass* trivial_class_uar () {
+  TriviallyDestructedClass *ptr;
+  TriviallyDestructedClass s;
+  ptr = &s;       // expected-warning {{address of stack memory is returned later}}
+  return ptr;     // expected-note {{returned here}}
+}
+
+// FIXME: No lifetime warning for this as no expire facts are generated for parameters
+const int& return_parameter(int a) { 
+  return a; 
+}
+
+// FIXME: No lifetime warning for this as no expire facts are generated for parameters
+int* return_pointer_to_parameter(int a) {
+    return &a;
+}
+
+const int& return_reference_to_parameter(int a)
+{
+    const int &b = a; 
+    return b;         // expected-warning {{address of stack memory is returned later}}
+                      // expected-note@-1 {{returned here}}
+}
+
+const int& get_ref_to_local() {
+    int a = 42;
+    return a;         // expected-warning {{address of stack memory is returned later}}
+                      // expected-note@-1 {{returned here}}
+}
+
 //===----------------------------------------------------------------------===//
 // Use-After-Scope & Use-After-Return (Return-Stack-Address) Combined
 // These are cases where the diagnostic kind is determined by location
@@ -688,7 +747,8 @@ void lifetimebound_partial_safety(bool cond) {
   v.use();                // expected-note {{later used here}}
 }
 
-// FIXME: Creating reference from lifetimebound call doesn't propagate loans.
+// FIXME: Warning should be on the 'GetObject' call, not the assignment to 'ptr'. 
+// The loan from the lifetimebound argument is not propagated to the call expression itself.
 const MyObj& GetObject(View v [[clang::lifetimebound]]);
 void lifetimebound_return_reference() {
   View v;
@@ -697,9 +757,9 @@ void lifetimebound_return_reference() {
     MyObj obj;
     View temp_v = obj;
     const MyObj& ref = GetObject(temp_v);
-    ptr = &ref;
-  }
-  (void)*ptr;
+    ptr = &ref;           // expected-warning {{object whose reference is captured does not live long enough}}
+  }                       // expected-note {{destroyed here}}
+  (void)*ptr;             // expected-note {{later used here}}
 }
 
 // FIXME: No warning for non gsl::Pointer types. Origin tracking is only supported for pointer types.

clang/unittests/Analysis/LifetimeSafetyTest.cpp

@@ -59,6 +59,7 @@ class LifetimeTestRunner {
     BuildOptions.setAllAlwaysAdd();
     BuildOptions.AddImplicitDtors = true;
     BuildOptions.AddTemporaryDtors = true;
+    BuildOptions.AddLifetime = true;
 
     // Run the main analysis.
     Analysis = std::make_unique<LifetimeSafetyAnalysis>(*AnalysisCtx, nullptr);
@@ -1307,6 +1308,42 @@ TEST_F(LifetimeAnalysisTest, LivenessOutsideLoop) {
   EXPECT_THAT(Origins({"p"}), MaybeLiveAt("p1"));
 }
 
+TEST_F(LifetimeAnalysisTest, TrivialDestructorsUAF) {
+  SetupTest(R"(
+    void target() {
+      int *ptr;
+      {
+          int s = 1;
+          ptr = &s;
+      }
+      POINT(p1);    
+      (void)*ptr;
+    }
+  )");
+  EXPECT_THAT(Origin("ptr"), HasLoansTo({"s"}, "p1"));
+  EXPECT_THAT(Origins({"ptr"}), MustBeLiveAt("p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, TrivialClassDestructorsUAF) {
+  SetupTest(R"(
+    class S {
+      View a, b;
+    };
+
+    void target() {
+      S* ptr;
+      {
+          S s;
+          ptr = &s;
+      }
+      POINT(p1);
+      (void)ptr;
+    }
+  )");
+  EXPECT_THAT(Origin("ptr"), HasLoansTo({"s"}, "p1"));
+  EXPECT_THAT(Origins({"ptr"}), MustBeLiveAt("p1"));
+}
+
 TEST_F(LifetimeAnalysisTest, SimpleReturnStackAddress) {
   SetupTest(R"(
     MyObj* target() {
@@ -1506,5 +1543,34 @@ TEST_F(LifetimeAnalysisTest, ReturnBeforeUseAfterScope) {
   EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1"));
 }
 
+TEST_F(LifetimeAnalysisTest, TrivialDestructorsUAR) {
+  SetupTest(R"(
+    int* target() {
+      int s = 10;
+      int* p = &s;
+      POINT(p1);
+      return p;
+    }
+  )");
+  EXPECT_THAT("s", HasLiveLoanAtExpiry("p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, TrivialClassDestructorsUAR) {
+  SetupTest(R"(
+    class S {
+      View a, b;
+    };
+
+    S* target() {
+      S *ptr;
+      S s;
+      ptr = &s;
+      POINT(p1);
+      return ptr;
+    }
+  )");
+  EXPECT_THAT("s", HasLiveLoanAtExpiry("p1"));
+}
+
 } // anonymous namespace
 } // namespace clang::lifetimes::internal