address comments and make expired loans hermetic

Author: usx95

clang/lib/Analysis/LifetimeSafety.cpp

@@ -901,8 +901,8 @@ struct ExpiredLattice {
     OS << "ExpiredLattice State:\n";
     if (Expired.isEmpty())
       OS << "  <empty>\n";
-    for (const auto &ID_ : Expired)
-      OS << "  Loan " << ID_.first << " is expired\n";
+    for (const auto &[ID, _] : Expired)
+      OS << "  Loan " << ID << " is expired\n";
   }
 };
 
@@ -928,8 +928,10 @@ class ExpiredLoansAnalysis
   Lattice join(Lattice L1, Lattice L2) {
     return Lattice(
         utils::join(L1.Expired, L2.Expired, Factory,
-                    // Take any ExpireFact to join the values.
-                    [](const ExpireFact *F, const ExpireFact *) { return F; }));
+                    // Take the last expiry fact to make this hermetic.
+                    [](const ExpireFact *F1, const ExpireFact *F2) {
+                      return F1->getExpiryLoc() > F2->getExpiryLoc() ? F1 : F2;
+                    }));
   }
 
   Lattice transfer(Lattice In, const ExpireFact &F) {
@@ -975,10 +977,9 @@ class ExpiredLoansAnalysis
 
 /// Struct to store the complete context for a potential lifetime violation.
 struct PendingWarning {
-  const Expr *IssueExpr;    // Where the loan was originally issued.
   SourceLocation ExpiryLoc; // Where the loan expired.
   const Expr *UseExpr;      // Where the origin holding this loan was used.
-  Confidence Level;
+  Confidence ConfidenceLevel;
 };
 
 class LifetimeChecker {
@@ -1052,28 +1053,26 @@ class LifetimeChecker {
       // If we already have a warning for this loan with a higher or equal
       // confidence, skip this one.
       if (FinalWarningsMap.count(DefaultedLoan) &&
-          CurrentConfidence <= FinalWarningsMap[DefaultedLoan].Level)
+          CurrentConfidence <= FinalWarningsMap[DefaultedLoan].ConfidenceLevel)
         continue;
 
-      const Loan &L = FactMgr.getLoanMgr().getLoan(DefaultedLoan);
       auto *EF = AllExpiredLoans.lookup(DefaultedLoan);
       assert(EF && "Could not find ExpireFact for an expired loan.");
 
-      const Expr *IssueExpr = L.IssueExpr;
-      SourceLocation ExpiryLoc = dyn_cast<ExpireFact>(*EF)->getExpiryLoc();
-
-      FinalWarningsMap[DefaultedLoan] = {IssueExpr, ExpiryLoc, UF->getUseExpr(),
-                                         CurrentConfidence};
+      FinalWarningsMap[DefaultedLoan] = {/*ExpiryLoc=*/(*EF)->getExpiryLoc(),
+                                         /*UseExpr=*/UF->getUseExpr(),
+                                         /*ConfidenceLevel=*/CurrentConfidence};
     }
   }
 
   void issuePendingWarnings() {
     if (!Reporter)
       return;
-    for (const auto &pair : FinalWarningsMap) {
-      const PendingWarning &PW = pair.second;
-      Reporter->reportUseAfterFree(PW.IssueExpr, PW.UseExpr, PW.ExpiryLoc,
-                                   PW.Level);
+    for (const auto &[LID, Warning] : FinalWarningsMap) {
+      const Loan &L = FactMgr.getLoanMgr().getLoan(LID);
+      const Expr *IssueExpr = L.IssueExpr;
+      Reporter->reportUseAfterFree(IssueExpr, Warning.UseExpr,
+                                   Warning.ExpiryLoc, Warning.ConfidenceLevel);
     }
   }
 };

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

@@ -182,6 +182,41 @@ void potential_loop_with_break(bool cond) {
   (void)*p;     // expected-note {{later used here}}
 }
 
+void potential_multiple_expiry_of_same_loan(bool cond) {
+  // Choose the last expiry location for the loan.
+  MyObj safe;
+  MyObj* p = &safe;
+  for (int i = 0; i < 10; ++i) {
+    MyObj unsafe;
+    if (cond) {
+      p = &unsafe; // expected-warning {{may not live long enough}}
+      break;
+    }
+  }               // expected-note {{destroyed here}} 
+  (void)*p;       // expected-note {{later used here}}
+
+  p = &safe;
+  for (int i = 0; i < 10; ++i) {
+    MyObj unsafe;
+    if (cond) {
+      p = &unsafe;    // expected-warning {{may not live long enough}}
+      if (cond)
+        break;
+    }
+  }                   // expected-note {{destroyed here}}
+  (void)*p;           // expected-note {{later used here}}
+
+  p = &safe;
+  for (int i = 0; i < 10; ++i) {
+    if (cond) {
+      MyObj unsafe2;
+      p = &unsafe2;   // expected-warning {{may not live long enough}}
+      break;          // expected-note {{destroyed here}}
+    }
+  }
+  (void)*p;           // expected-note {{later used here}}
+}
+
 void potential_switch(int mode) {
   MyObj safe;
   MyObj* p = &safe;
@@ -236,18 +271,3 @@ void no_error_if_dangle_then_rescue() {
   p = &safe;    // p is "rescued" before use.
   (void)*p;     // This is safe.
 }
-
-// MyObj some_name(bool condition, MyObj x) {
-//   MyObj* p = &x;
-//   MyObj* q = &x;
-//   if (condition)
-//   {
-//     MyObj y{20};
-//     MyObj * abcd = &y;
-//     p = abcd;
-//     q = abcd;
-//   }
-//   MyObj a = *p;
-//   MyObj b = *q;
-//   return a + b;
-// }