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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 25 18:10:52 PST 2019


NoQ added a comment.

Thanks! A surprise, to be sure :) I'll try to test this on my set of projects as well :)

> Most of the extra results are not false positive due to the less invalidation but other reasons, so we could focus on those problems instead of them being hidden.

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".



================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:320-321
+        ETraits.setTrait(
+            UseBaseRegion ? MR->getBaseRegion() : MR,
+            RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+    }
----------------
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.


================
Comment at: test/Analysis/call-invalidation.cpp:146
   useAnything(&(s1.y));
-  clang_analyzer_eval(s1.x == 1); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
 }
----------------
Let's leave at least one positive check around, eg. demonstrate that invalidation does happen for `s1.y` here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57230





More information about the cfe-commits mailing list