summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h3
-rw-r--r--clang/lib/StaticAnalyzer/Core/RegionStore.cpp60
-rw-r--r--clang/test/Analysis/malloc.c18
-rw-r--r--clang/test/Analysis/symbol-reaper.c25
4 files changed, 63 insertions, 43 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 2121b5bc04f..85eb4b865ab 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -773,9 +773,6 @@ class SymbolicRegion : public SubRegion {
assert(s->getType()->isAnyPointerType() ||
s->getType()->isReferenceType() ||
s->getType()->isBlockPointerType());
-
- // populateWorklistFromSymbol() relies on this assertion, and needs to be
- // updated if more cases are introduced.
assert(isa<UnknownSpaceRegion>(sreg) || isa<HeapSpaceRegion>(sreg));
}
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 679d95b7c58..3323bb97a71 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2386,10 +2386,7 @@ RegionStoreManager::bindAggregate(RegionBindingsConstRef B,
namespace {
class RemoveDeadBindingsWorker
: public ClusterAnalysis<RemoveDeadBindingsWorker> {
- using ChildrenListTy = SmallVector<const SymbolDerived *, 4>;
- using MapParentsToDerivedTy = llvm::DenseMap<SymbolRef, ChildrenListTy>;
-
- MapParentsToDerivedTy ParentsToDerived;
+ SmallVector<const SymbolicRegion *, 12> Postponed;
SymbolReaper &SymReaper;
const StackFrameContext *CurrentLCtx;
@@ -2410,10 +2407,8 @@ public:
bool AddToWorkList(const MemRegion *R);
+ bool UpdatePostponed();
void VisitBinding(SVal V);
-
-private:
- void populateWorklistFromSymbol(SymbolRef s);
};
}
@@ -2433,11 +2428,10 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR,
}
if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR)) {
- if (SymReaper.isLive(SR->getSymbol())) {
+ if (SymReaper.isLive(SR->getSymbol()))
AddToWorkList(SR, &C);
- } else if (const auto *SD = dyn_cast<SymbolDerived>(SR->getSymbol())) {
- ParentsToDerived[SD->getParentSymbol()].push_back(SD);
- }
+ else
+ Postponed.push_back(SR);
return;
}
@@ -2450,7 +2444,7 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR,
// CXXThisRegion in the current or parent location context is live.
if (const CXXThisRegion *TR = dyn_cast<CXXThisRegion>(baseR)) {
const auto *StackReg =
- cast<StackArgumentsSpaceRegion>(TR->getSuperRegion());
+ cast<StackArgumentsSpaceRegion>(TR->getSuperRegion());
const StackFrameContext *RegCtx = StackReg->getStackFrame();
if (CurrentLCtx &&
(RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx)))
@@ -2494,15 +2488,6 @@ void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
// If V is a region, then add it to the worklist.
if (const MemRegion *R = V.getAsRegion()) {
AddToWorkList(R);
-
- if (const auto *TVR = dyn_cast<TypedValueRegion>(R)) {
- DefinedOrUnknownSVal RVS =
- RM.getSValBuilder().getRegionValueSymbolVal(TVR);
- if (const MemRegion *SR = RVS.getAsRegion()) {
- AddToWorkList(SR);
- }
- }
-
SymReaper.markLive(R);
// All regions captured by a block are also live.
@@ -2516,30 +2501,25 @@ void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
// Update the set of live symbols.
- for (auto SI = V.symbol_begin(), SE = V.symbol_end(); SI != SE; ++SI) {
- populateWorklistFromSymbol(*SI);
-
- for (const auto *SD : ParentsToDerived[*SI])
- populateWorklistFromSymbol(SD);
-
+ for (auto SI = V.symbol_begin(), SE = V.symbol_end(); SI!=SE; ++SI)
SymReaper.markLive(*SI);
- }
}
-void RemoveDeadBindingsWorker::populateWorklistFromSymbol(SymbolRef S) {
- if (const auto *SD = dyn_cast<SymbolData>(S)) {
- if (Loc::isLocType(SD->getType()) && !SymReaper.isLive(SD)) {
- const SymbolicRegion *SR = RM.getRegionManager().getSymbolicRegion(SD);
+bool RemoveDeadBindingsWorker::UpdatePostponed() {
+ // See if any postponed SymbolicRegions are actually live now, after
+ // having done a scan.
+ bool Changed = false;
- if (B.contains(SR))
- AddToWorkList(SR);
-
- const SymbolicRegion *SHR =
- RM.getRegionManager().getSymbolicHeapRegion(SD);
- if (B.contains(SHR))
- AddToWorkList(SHR);
+ for (auto I = Postponed.begin(), E = Postponed.end(); I != E; ++I) {
+ if (const SymbolicRegion *SR = *I) {
+ if (SymReaper.isLive(SR->getSymbol())) {
+ Changed |= AddToWorkList(SR);
+ *I = nullptr;
+ }
}
}
+
+ return Changed;
}
StoreRef RegionStoreManager::removeDeadBindings(Store store,
@@ -2555,7 +2535,7 @@ StoreRef RegionStoreManager::removeDeadBindings(Store store,
W.AddToWorkList(*I);
}
- W.RunWorkList();
+ do W.RunWorkList(); while (W.UpdatePostponed());
// We have now scanned the store, marking reachable regions and symbols
// as live. We now remove all the regions that are dead from the store
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index d4a44c57624..cbc21b492e5 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1794,6 +1794,24 @@ void testNoCrashOnOffendingParameter() {
allocateSomeMemory(offendingParameter, &ptr);
} // expected-warning {{Potential leak of memory pointed to by 'ptr'}}
+
+// Test a false positive caused by a bug in liveness analysis.
+struct A {
+ int *buf;
+};
+struct B {
+ struct A *a;
+};
+void livenessBugRealloc(struct A *a) {
+ a->buf = realloc(a->buf, sizeof(int)); // no-warning
+}
+void testLivenessBug(struct B *in_b) {
+ struct B *b = in_b;
+ livenessBugRealloc(b->a);
+ ((void) 0); // An attempt to trick liveness analysis.
+ livenessBugRealloc(b->a);
+}
+
// ----------------------------------------------------------------------------
// False negatives.
diff --git a/clang/test/Analysis/symbol-reaper.c b/clang/test/Analysis/symbol-reaper.c
index ef8ff18a2d8..62bb5d6c6b3 100644
--- a/clang/test/Analysis/symbol-reaper.c
+++ b/clang/test/Analysis/symbol-reaper.c
@@ -133,3 +133,28 @@ void test_zombie_referenced_only_through_field_in_store_value() {
clang_analyzer_warnOnDeadSymbol((int) s);
int *x = &s->field;
} // expected-warning{{SYMBOL DEAD}}
+
+void double_dereference_of_implicit_value_aux1(int *p) {
+ *p = 0;
+}
+
+void double_dereference_of_implicit_value_aux2(int *p) {
+ if (*p != 0)
+ clang_analyzer_warnIfReached(); // no-warning
+}
+
+void test_double_dereference_of_implicit_value(int **x) {
+ clang_analyzer_warnOnDeadSymbol(**x);
+ int **y = x;
+ {
+ double_dereference_of_implicit_value_aux1(*y);
+ // Give time for symbol reaping to happen.
+ ((void)0);
+ // The symbol for **y was cleaned up from the Store at this point,
+ // even though it was not perceived as dead when asked explicitly.
+ // For that reason the SYMBOL DEAD warning never appeared at this point.
+ double_dereference_of_implicit_value_aux2(*y);
+ }
+ // The symbol is generally reaped here regardless.
+ ((void)0); // expected-warning{{SYMBOL DEAD}}
+}
OpenPOWER on IntegriCloud