[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added a reviewer: NoQ.
Herald added subscribers: gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.

Follow up for a problem described in https://reviews.llvm.org/D57230

I think the ClusterAnalysis is fundamentally flawed at this point and it does not support non-base regions properly. The reason we could get away with this so far is that it is very rare to have non-base regions there.

I made an attempt to fix this and I successfully fixes the false positive described by Artem in https://reviews.llvm.org/D57230 but it introduced some other problems.
Nevertheless, I wanted to share the work in progress solution to get some feedback about the directions.


Repository:
  rC Clang

https://reviews.llvm.org/D58121

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp


Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -654,7 +654,6 @@
 template <typename DERIVED>
 class ClusterAnalysis  {
 protected:
-  typedef llvm::DenseMap<const MemRegion *, const ClusterBindings *> ClusterMap;
   typedef const MemRegion * WorkListElement;
   typedef SmallVector<WorkListElement, 10> WorkList;
 
@@ -726,7 +725,8 @@
       WorkListElement E = WL.pop_back_val();
       const MemRegion *BaseR = E;
 
-      static_cast<DERIVED*>(this)->VisitCluster(BaseR, getCluster(BaseR));
+      static_cast<DERIVED *>(this)->VisitCluster(
+          BaseR, getCluster(BaseR->getBaseRegion()));
     }
   }
 
@@ -984,7 +984,7 @@
   bool doNotInvalidateSuperRegion = ITraits.hasTrait(
       R, RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
   const MemRegion *BaseR = doNotInvalidateSuperRegion ? R : R->getBaseRegion();
-  return AddToWorkList(WorkListElement(BaseR), getCluster(BaseR));
+  return AddToWorkList(WorkListElement(BaseR), getCluster(R->getBaseRegion()));
 }
 
 void InvalidateRegionsWorker::VisitBinding(SVal V) {
@@ -1020,13 +1020,23 @@
                        RegionAndSymbolInvalidationTraits::TK_PreserveContents);
 
   if (C) {
-    for (ClusterBindings::iterator I = C->begin(), E = C->end(); I != E; ++I)
-      VisitBinding(I.getData());
+    const MemRegion *realBase = baseR->getBaseRegion();
+    if (realBase == baseR) {
+      for (ClusterBindings::iterator I = C->begin(), E = C->end(); I != E; ++I)
+        VisitBinding(I.getData());
+    } else {
+      if (const SVal *Direct =
+              C->lookup(BindingKey::Make(baseR, BindingKey::Direct)))
+        VisitBinding(*Direct);
+      else if (const SVal *Default =
+                   C->lookup(BindingKey::Make(baseR, BindingKey::Default)))
+        VisitBinding(*Default);
+    }
 
     // Invalidate regions contents.
     if (!PreserveRegionsContents)
       B = B.remove(baseR);
-  }
+  } 
 
   if (const auto *TO = dyn_cast<TypedValueRegion>(baseR)) {
     if (const auto *RD = TO->getValueType()->getAsCXXRecordDecl()) {
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -314,7 +314,7 @@
           }
         }
       }
-      // todo: factor this out + handle the lower level const pointers.
+      // TODO: factor this out + handle the lower level const pointers.
       if (PreserveArgs.count(Idx))
         ETraits.setTrait(
             UseBaseRegion ? MR->getBaseRegion() : MR,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58121.186462.patch
Type: text/x-patch
Size: 2714 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190212/aa3a599b/attachment.bin>