[PATCH] D81315: [analyzer] Warning for default constructed unique pointer dereferences

Mon Jun 29 09:10:19 PDT 2020

Szelethus added inline comments.

================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:577-583
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "CheckSmartPtrDereference",
+                  "Enable check for SmartPtr null dereferences",
+                  "false",
+                  InAlpha>,
+  ]>,
----------------
NoQ wrote:
> Szelethus wrote:
> > This goes against D81750 -- Sorry for not bringing this up earlier, but you can't emit diagnostics from a checker that is hidden :/
> > 
> > The proper solution would be to create a non-hidden checker, and emit a diagnostic under its name. You can take inspiration from `MallocChecker`, `NullabilityChecker`, and a variety of other sound in the stack of this patch: D77845
> Aha, ok, that's what i meant at the end of D81315#inline-760088. @Szelethus insists, so let's turn ourselves into a separate checker right away!
> 
> In fact, let's *not* take the inspiration from poor-man's solutions in `MallocChecker` etc., but instead let's try a full-on modular approach:
> - Make a separate .cpp file and a separate Checker class for emitting diagnostics.
>   - Remove the checker option and instead add a separate checker entry in Checkers.td.
> - Subscribe the new checker on PreCall only and move all the bug reporting logic there.
> - Keep all the modeling logic in the old checker (it's called SmartPtr//Modeling// for a reason!).
> - Put common functionality into a header file shared between the two checkers.
>   - In particular, the GDM trait should be only accessed through such getter.
>     - Take some inspiration from `Taint.h`.
Sounds great! And of course, say the word if any help is needed! :)

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

https://reviews.llvm.org/D81315