tomasz-kaminski-sonarsource updated this revision to Diff 464684.
tomasz-kaminski-sonarsource added a comment.
Applied suggested comment updates.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134947/new/
https://reviews.llvm.org/D134947
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
clang/test/Analysis/trivial-copy-struct.cpp
Index: clang/test/Analysis/trivial-copy-struct.cpp
===================================================================
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -49,11 +49,53 @@
if (c.value == 42)
return;
clang_analyzer_value(c.value);
- // expected-warning@-1 {{32s:{ [-2147483648, 2147483647] }}}
- // The symbol was garbage collected too early, hence we lose the constraints.
+ // expected-warning@-1 {{32s:{ [-2147483648, 41], [43, 2147483647] }}}
+ // Before symbol was garbage collected too early, and we lost the constraints.
if (c.value != 42)
return;
- // Dead code should be unreachable
- clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ clang_analyzer_warnIfReached(); // no-warning: Dead code.
+};
+
+void ptr1(List* n) {
+ List* n2 = new List(*n); // ctor
+ if (!n->next) {
+ if (n2->next) {
+ clang_analyzer_warnIfReached(); // unreachable
+ }
+ }
+ delete n2;
+}
+
+void ptr2(List* n) {
+ List* n2 = new List(); // ctor
+ *n2 = *n; // assignment
+ if (!n->next) {
+ if (n2->next) {
+ clang_analyzer_warnIfReached(); // unreachable
+ }
+ }
+ delete n2;
+}
+
+struct Wrapper {
+ List head;
+ int count;
+};
+
+void nestedLazyCompoundVal(List* n) {
+ Wrapper* w = 0;
+ {
+ Wrapper lw;
+ lw.head = *n;
+ w = new Wrapper(lw);
+ }
+ if (!n->next) {
+ if (w->head.next) {
+ // FIXME: Unreachable, w->head is a copy of *n, therefore
+ // w->head.next and n->next are equal
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+ }
+ }
+ delete w;
}
Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -411,10 +411,14 @@
}
void SymbolReaper::markLive(const MemRegion *region) {
- RegionRoots.insert(region->getBaseRegion());
+ LiveRegionRoots.insert(region->getBaseRegion());
markElementIndicesLive(region);
}
+void SymbolReaper::markLazilyCopied(const clang::ento::MemRegion *region) {
+ LazilyCopiedRegionRoots.insert(region->getBaseRegion());
+}
+
void SymbolReaper::markElementIndicesLive(const MemRegion *region) {
for (auto SR = dyn_cast<SubRegion>(region); SR;
SR = dyn_cast<SubRegion>(SR->getSuperRegion())) {
@@ -437,8 +441,7 @@
// is not used later in the path, we can diagnose a leak of a value within
// that field earlier than, say, the variable that contains the field dies.
MR = MR->getBaseRegion();
-
- if (RegionRoots.count(MR))
+ if (LiveRegionRoots.count(MR))
return true;
if (const auto *SR = dyn_cast<SymbolicRegion>(MR))
@@ -454,6 +457,15 @@
return isa<AllocaRegion, CXXThisRegion, MemSpaceRegion, CodeTextRegion>(MR);
}
+bool SymbolReaper::isLazilyCopiedRegion(const MemRegion *MR) const {
+ // TODO: See comment in isLiveRegion.
+ return LazilyCopiedRegionRoots.count(MR->getBaseRegion());
+}
+
+bool SymbolReaper::isReadableRegion(const MemRegion *MR) {
+ return isLiveRegion(MR) || isLazilyCopiedRegion(MR);
+}
+
bool SymbolReaper::isLive(SymbolRef sym) {
if (TheLiving.count(sym)) {
markDependentsLive(sym);
@@ -464,7 +476,7 @@
switch (sym->getKind()) {
case SymExpr::SymbolRegionValueKind:
- KnownLive = isLiveRegion(cast<SymbolRegionValue>(sym)->getRegion());
+ KnownLive = isReadableRegion(cast<SymbolRegionValue>(sym)->getRegion());
break;
case SymExpr::SymbolConjuredKind:
KnownLive = false;
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2838,6 +2838,10 @@
// 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);
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -582,7 +582,12 @@
SymbolMapTy TheLiving;
SymbolSetTy MetadataInUse;
- RegionSetTy RegionRoots;
+ RegionSetTy LiveRegionRoots;
+ // The lazily copied regions are locations for which a program
+ // can access the value stored at that location, but not its address.
+ // These regions are constructed as a set of regions referred to by
+ // lazyCompoundVal.
+ RegionSetTy LazilyCopiedRegionRoots;
const StackFrameContext *LCtx;
const Stmt *Loc;
@@ -628,8 +633,8 @@
using region_iterator = RegionSetTy::const_iterator;
- region_iterator region_begin() const { return RegionRoots.begin(); }
- region_iterator region_end() const { return RegionRoots.end(); }
+ region_iterator region_begin() const { return LiveRegionRoots.begin(); }
+ region_iterator region_end() const { return LiveRegionRoots.end(); }
/// Returns whether or not a symbol has been confirmed dead.
///
@@ -640,6 +645,7 @@
}
void markLive(const MemRegion *region);
+ void markLazilyCopied(const MemRegion *region);
void markElementIndicesLive(const MemRegion *region);
/// Set to the value of the symbolic store after
@@ -647,6 +653,12 @@
void setReapedStore(StoreRef st) { reapedStore = st; }
private:
+ bool isLazilyCopiedRegion(const MemRegion *region) const;
+ // A readable region is a region that live or lazily copied.
+ // Any symbols that refer to values in regions are alive if the region
+ // is readable.
+ bool isReadableRegion(const MemRegion *region);
+
/// Mark the symbols dependent on the input symbol as live.
void markDependentsLive(SymbolRef sym);
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
