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,
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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
