diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ b/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); }; diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2838,6 +2838,10 @@ // Is it a LazyCompoundVal? All referenced regions are live as well. if (Optional LCS = V.getAs()) { + // 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); diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp --- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ b/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(region); SR; SR = dyn_cast(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(MR)) @@ -454,6 +457,15 @@ return isa(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(sym)->getRegion()); + KnownLive = isReadableRegion(cast(sym)->getRegion()); break; case SymExpr::SymbolConjuredKind: KnownLive = false; diff --git a/clang/test/Analysis/trivial-copy-struct.cpp b/clang/test/Analysis/trivial-copy-struct.cpp --- a/clang/test/Analysis/trivial-copy-struct.cpp +++ b/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; }