[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.

In D57230#1372275 <https://reviews.llvm.org/D57230#1372275>, @NoQ wrote:

> Could you share reproducible examples for these, probably in the form of FIXME tests? Given that they are "regressions", they are easy to creduce down to a small repro by using the test "there is still a change in behavior on this file".


I think the most common cause of false positives is infeasible paths. Do you have success reducing false positives using creduce? My problem usually is that we cannot tell if a reduction rendered a false positive into a true positive.



================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:320-321
+        ETraits.setTrait(
+            UseBaseRegion ? MR->getBaseRegion() : MR,
+            RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+    }
----------------
NoQ wrote:
> I suspect that the trait for non-base `MR` would never be read. The only place where this trait is accessed is in RegionStore.cpp where it asks whether the trait is applied to a cluster base, which is always a base region.
I see some test failures when I always used the base region. I suspect the reason is that `InvalidateRegionsWorker::AddToWorkList` will add the region itself instead of the base region when `TK_DoNotInvalidateSuperRegion` is set. So if we only set the `TK_PreserveContents` trait for the base region `InvalidateRegionsWorker::VisitCluster` will not see the `TK_PreserveContents` trait. 

In fact, the naming of regions in the those functions are very confusing. Even though the formal paramter is called `baseR`, my suspicion is that, we might visit non-base regions (due to the  `TK_DoNotInvalidateSuperRegion`  trait). 


Repository:
  rC Clang

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

https://reviews.llvm.org/D57230