[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 25 15:11:10 PDT 2019


Szelethus added a comment.

In D66721#1644514 <https://reviews.llvm.org/D66721#1644514>, @NoQ wrote:

> No-no, you're disabling the checkers here but you should be silencing them. This will be crashing because modeling is disabled.
>
> I also recommend checker options, even if they apply to multiple checkers (@Szelethus mentioned that package options are a thing, you might use these if you don't want to make multiple options).


Just to be clear, I object to nothing as long as it is an off-by-default developer-only feature.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp:66
+      if (BO->isBitwiseOrShiftOp() && !Opts.CheckBitwise)
+        Opts.SilencedCheckersAndPackages.push_back("core.uninitialized.Branch");
+
----------------
This is one of the reasons why I believe `AnalyzerOptions` should be immutable once analysis begins. Say that you'd like to list the checkers currently enabled with `-analyzer-list-enabled-checkers`, and that list is compiled around the time `CheckerRegistry` is active (which is a very brief period during the initialization of `CheckerManager`). Wouldn't it be super infuriating to learn that this lislt may change *during* analysis?

I feel somewhat the same way about this, this should either be moved to `shouldRegister*()` or be handled outside the analyzer.



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

https://reviews.llvm.org/D66721





More information about the cfe-commits mailing list