[PATCH] D31982: [analyzer] Improve suppression for inlined defensive checks when operator& is involved.

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 13 09:23:52 PDT 2017


dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Yay! This is going to fix some really confusing false positives.LGTM with Gabor's comments addressed.

One note. I am not a big fan of using clang_analyzer_explain() in tests for functionality. It is great for debugging, but for testing it exposes too much of the implementation details of the region hierarchy and store. Exposing this kind of state is fine in tests specific to the store/region store, but in my opinion it should be another clang_analyzer_ entry point and should be limited to regions.

Similarly we should have a clang_analyzer_ entry point for testing symbols but that doesn't expose the guts of MemRegions. And (maybe?) another one for SymExprs.



================
Comment at: test/Analysis/explain-svals.cpp:103
+  C *c = 0;
+  // FIXME: we need to be explaining '40' rather than '0' here; not explainer bug.
+  clang_analyzer_explain(&c->y); // expected-warning-re{{{{^concrete memory address '0'$}}}}
----------------
It would be good to mention that this is a modeling bug and point to where it could be fixed. Otherwise the FIXME is not super helpful to future maintainers.




https://reviews.llvm.org/D31982





More information about the cfe-commits mailing list