[Openmp-commits] [PATCH] D134947: [analyzer] Fix liveness of Symbols for values in regions reffered by LazyCompoundVal

[Artem Dergachev via Phabricator via Openmp-commits]

NoQ added a comment.

Wow thanks!!

Yeah this matches my understanding of the problem. I still encourage you to test it on real-world code before committing, and carefully investigate every change in diagnostics, because symbol reaper is very convoluted and filled with insane cornercases.



================
Comment at: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp:461
+bool SymbolReaper::isLazilyCopiedRegion(const MemRegion *MR) const {
+  // TODO: See comment in isLiveRegion.
+  return LazilyCopiedRegionRoots.count(MR->getBaseRegion());
----------------
tomasz-kaminski-sonarsource wrote:
> martong wrote:
> > Just out of curiosity, do you have plans to tackle this todo sometime?
> We do not plan to takle it in near future.
Could you add a negative/FIXME test for it?

At a glance I suspect that this TODO is significantly less important for `isLiveRegion()` than it is for your new function, so I encourage you to explore the possibility of dropping `getBaseRegion()`, even if just a little bit and doesn't have to be in this patch.

If a smaller subregion is truly live, any value inside of the base region can theoretically be accessed through safe pointer arithmetic. It's very difficult to prove that it can't be accessed anymore. Every pointer escape will be a potential access.

In your case, however, if the superregion is neither live nor lazily copied, the information outside of the lazily copied subregion is truly lost, there's literally nothing the program can do to recover it.


================
Comment at: clang/test/Analysis/trivial-copy-struct.cpp:98
+      // w->head.next and n->next are equal
+      clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+    }
----------------
martong wrote:
> 
Do you know what's causing this to not work? Is this a regression or just never worked?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134947/new/

https://reviews.llvm.org/D134947