Author: Tomasz Kamiński Date: 2022-10-19T16:06:32+02:00 New Revision: 6194229c6287fa5d8a9a30aa489bcbb8a9754ae0
URL: https://github.com/llvm/llvm-project/commit/6194229c6287fa5d8a9a30aa489bcbb8a9754ae0 DIFF: https://github.com/llvm/llvm-project/commit/6194229c6287fa5d8a9a30aa489bcbb8a9754ae0.diff LOG: [analyzer] Make directly bounded LazyCompoundVal as lazily copied Previously, `LazyCompoundVal` bindings to subregions referred by `LazyCopoundVals`, were not marked as //lazily copied//. This change returns `LazyCompoundVals` from `getInterestingValues()`, so their regions can be marked as //lazily copied// in `RemoveDeadBindingsWorker::VisitBinding()`. Depends on D134947 Authored by: Tomasz Kamiński <tomasz.kamińs...@sonarsource.com> Reviewed By: martong Differential Revision: https://reviews.llvm.org/D135136 Added: Modified: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/trivial-copy-struct.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 7e4e00454e96..8af351f64246 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -608,6 +608,10 @@ class RegionStoreManager : public StoreManager { /// The precise value of "interesting" is determined for the purposes of /// RegionStore's internal analysis. It must always contain all regions and /// symbols, but may omit constants and other kinds of SVal. + /// + /// In contrast to compound values, LazyCompoundVals are also added + /// to the 'interesting values' list in addition to the child interesting + /// values. const SValListTy &getInterestingValues(nonloc::LazyCompoundVal LCV); //===------------------------------------------------------------------===// @@ -1032,12 +1036,11 @@ void InvalidateRegionsWorker::VisitBinding(SVal V) { if (Optional<nonloc::LazyCompoundVal> LCS = V.getAs<nonloc::LazyCompoundVal>()) { - const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS); - - for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(), - E = Vals.end(); - I != E; ++I) - VisitBinding(*I); + // `getInterestingValues()` returns SVals contained within LazyCompoundVals, + // so there is no need to visit them. + for (SVal V : RM.getInterestingValues(*LCS)) + if (!isa<nonloc::LazyCompoundVal>(V)) + VisitBinding(V); return; } @@ -1290,15 +1293,10 @@ void RegionStoreManager::populateWorkList(InvalidateRegionsWorker &W, if (Optional<nonloc::LazyCompoundVal> LCS = V.getAs<nonloc::LazyCompoundVal>()) { - const SValListTy &Vals = getInterestingValues(*LCS); - - for (SValListTy::const_iterator I = Vals.begin(), - E = Vals.end(); I != E; ++I) { - // Note: the last argument is false here because these are - // non-top-level regions. - if (const MemRegion *R = (*I).getAsRegion()) + for (SVal S : getInterestingValues(*LCS)) + if (const MemRegion *R = S.getAsRegion()) W.AddToWorkList(R); - } + continue; } @@ -2283,11 +2281,9 @@ RegionStoreManager::getInterestingValues(nonloc::LazyCompoundVal LCV) { if (V.isUnknownOrUndef() || V.isConstant()) continue; - if (Optional<nonloc::LazyCompoundVal> InnerLCV = - V.getAs<nonloc::LazyCompoundVal>()) { + if (auto InnerLCV = V.getAs<nonloc::LazyCompoundVal>()) { const SValListTy &InnerList = getInterestingValues(*InnerLCV); List.insert(List.end(), InnerList.begin(), InnerList.end()); - continue; } List.push_back(V); @@ -2835,20 +2831,17 @@ void RemoveDeadBindingsWorker::VisitCluster(const MemRegion *baseR, } void RemoveDeadBindingsWorker::VisitBinding(SVal V) { - // 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); - - for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(), - E = Vals.end(); - I != E; ++I) - VisitBinding(*I); + // Is it a LazyCompoundVal? All referenced regions are live as well. + // The LazyCompoundVal itself is not live but should be readable. + if (auto LCS = V.getAs<nonloc::LazyCompoundVal>()) { + SymReaper.markLazilyCopied(LCS->getRegion()); + + for (SVal V : RM.getInterestingValues(*LCS)) { + if (auto DepLCS = V.getAs<nonloc::LazyCompoundVal>()) + SymReaper.markLazilyCopied(DepLCS->getRegion()); + else + VisitBinding(V); + } return; } diff --git a/clang/test/Analysis/trivial-copy-struct.cpp b/clang/test/Analysis/trivial-copy-struct.cpp index b1f4cb3fbfdb..b2da777541ab 100644 --- a/clang/test/Analysis/trivial-copy-struct.cpp +++ b/clang/test/Analysis/trivial-copy-struct.cpp @@ -92,9 +92,9 @@ void nestedLazyCompoundVal(List* n) { } if (!n->next) { if (w->head.next) { - // FIXME: Unreachable, w->head is a copy of *n, therefore + // Unreachable, w->head is a copy of *n, therefore // w->head.next and n->next are equal - clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning: unreachable } } delete w; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
