[clang] 6194229 - [analyzer] Make directly bounded LazyCompoundVal as lazily copied

[Tomasz Kamiński via cfe-commits]

Author: Tomasz Kamiński
Date: 2022-10-19T16:06:32+02:00
New Revision: 6194229c6287fa5d8a9a30aa489bcbb8a9754ae0

URL: https://github.com/llvm/llvm-project/commit/6194229c6287fa5d8a9a30aa489bcbb8a9754ae0
DIFF: https://github.com/llvm/llvm-project/commit/6194229c6287fa5d8a9a30aa489bcbb8a9754ae0.diff

LOG: [analyzer] Make directly bounded LazyCompoundVal as lazily copied

Previously, `LazyCompoundVal` bindings to subregions referred by
`LazyCopoundVals`, were not marked as //lazily copied//.

This change returns `LazyCompoundVals` from `getInterestingValues()`,
so their regions can be marked as //lazily copied// in `RemoveDeadBindingsWorker::VisitBinding()`.

Depends on D134947

Authored by: Tomasz Kamiński <tomasz.kamiński at sonarsource.com>

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D135136

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/RegionStore.cpp
    clang/test/Analysis/trivial-copy-struct.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 7e4e00454e96..8af351f64246 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -608,6 +608,10 @@ class RegionStoreManager : public StoreManager {
   /// The precise value of "interesting" is determined for the purposes of
   /// RegionStore's internal analysis. It must always contain all regions and
   /// symbols, but may omit constants and other kinds of SVal.
+  ///
+  /// In contrast to compound values, LazyCompoundVals are also added
+  /// to the 'interesting values' list in addition to the child interesting
+  /// values.
   const SValListTy &getInterestingValues(nonloc::LazyCompoundVal LCV);
 
   //===------------------------------------------------------------------===//
@@ -1032,12 +1036,11 @@ void InvalidateRegionsWorker::VisitBinding(SVal V) {
   if (Optional<nonloc::LazyCompoundVal> LCS =
           V.getAs<nonloc::LazyCompoundVal>()) {
 
-    const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-    for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-                                                        E = Vals.end();
-         I != E; ++I)
-      VisitBinding(*I);
+    // `getInterestingValues()` returns SVals contained within LazyCompoundVals,
+    // so there is no need to visit them.
+    for (SVal V : RM.getInterestingValues(*LCS))
+      if (!isa<nonloc::LazyCompoundVal>(V))
+        VisitBinding(V);
 
     return;
   }
@@ -1290,15 +1293,10 @@ void RegionStoreManager::populateWorkList(InvalidateRegionsWorker &W,
     if (Optional<nonloc::LazyCompoundVal> LCS =
         V.getAs<nonloc::LazyCompoundVal>()) {
 
-      const SValListTy &Vals = getInterestingValues(*LCS);
-
-      for (SValListTy::const_iterator I = Vals.begin(),
-                                      E = Vals.end(); I != E; ++I) {
-        // Note: the last argument is false here because these are
-        // non-top-level regions.
-        if (const MemRegion *R = (*I).getAsRegion())
+      for (SVal S : getInterestingValues(*LCS))
+        if (const MemRegion *R = S.getAsRegion())
           W.AddToWorkList(R);
-      }
+
       continue;
     }
 
@@ -2283,11 +2281,9 @@ RegionStoreManager::getInterestingValues(nonloc::LazyCompoundVal LCV) {
     if (V.isUnknownOrUndef() || V.isConstant())
       continue;
 
-    if (Optional<nonloc::LazyCompoundVal> InnerLCV =
-            V.getAs<nonloc::LazyCompoundVal>()) {
+    if (auto InnerLCV = V.getAs<nonloc::LazyCompoundVal>()) {
       const SValListTy &InnerList = getInterestingValues(*InnerLCV);
       List.insert(List.end(), InnerList.begin(), InnerList.end());
-      continue;
     }
 
     List.push_back(V);
@@ -2835,20 +2831,17 @@ void RemoveDeadBindingsWorker::VisitCluster(const MemRegion *baseR,
 }
 
 void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
-  // Is it a LazyCompoundVal?  All referenced regions are live as well.
-  if (Optional<nonloc::LazyCompoundVal> LCS =
-          V.getAs<nonloc::LazyCompoundVal>()) {
-    // TODO: Make regions referred to by `lazyCompoundVals` that are bound to
-    // subregions of the `LCS.getRegion()` also lazily copied.
-    if (const MemRegion *R = LCS->getRegion())
-      SymReaper.markLazilyCopied(R);
-
-    const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
-
-    for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
-                                                        E = Vals.end();
-         I != E; ++I)
-      VisitBinding(*I);
+  // Is it a LazyCompoundVal? All referenced regions are live as well.
+  // The LazyCompoundVal itself is not live but should be readable.
+  if (auto LCS = V.getAs<nonloc::LazyCompoundVal>()) {
+    SymReaper.markLazilyCopied(LCS->getRegion());
+
+    for (SVal V : RM.getInterestingValues(*LCS)) {
+      if (auto DepLCS = V.getAs<nonloc::LazyCompoundVal>())
+        SymReaper.markLazilyCopied(DepLCS->getRegion());
+      else
+        VisitBinding(V);
+    }
 
     return;
   }

diff  --git a/clang/test/Analysis/trivial-copy-struct.cpp b/clang/test/Analysis/trivial-copy-struct.cpp
index b1f4cb3fbfdb..b2da777541ab 100644
--- a/clang/test/Analysis/trivial-copy-struct.cpp
+++ b/clang/test/Analysis/trivial-copy-struct.cpp
@@ -92,9 +92,9 @@ void nestedLazyCompoundVal(List* n) {
   }
   if (!n->next) {
     if (w->head.next) {
-      // FIXME: Unreachable, w->head is a copy of *n, therefore
+      // Unreachable, w->head is a copy of *n, therefore
       // w->head.next and n->next are equal
-      clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+      clang_analyzer_warnIfReached(); // no-warning: unreachable
     }
   }
   delete w;