[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:199
+    } else {
+      const auto *TrackingExpr = getFirstArgExpr(Call);
+      C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr](
----------------
xazax.hun wrote:
> I am a bit confused. As far as I understand we ALWAYS add the note tag below, regardless of the first argument being null.
> I think I do understand that we only trigger this note tag when there is a null dereference but consider the code below:
> 
> ```
> void f(int *p) {
>   std::unique_ptr<int> u(p); // p is not always null here.
>   if (!p) {
>     *u; // p is always null here
>   }
> }
> ```
> 
> Do we ant to output the message that the smart pointer is constructed using a null value? While this is technically true, there is a later event in the path that uncovers the fact that `p` is null.
> 
> @NoQ what do you think? I do not have any strong feelings about this, I only wonder how users would react to a bug report like that.
Indeed, nice!! We can only proclaim the pointer to be null if it's known to be null in the current state. I.e., if previous steps of the report indicated that it's null. If it's discovered to be null later, we cannot call it null yet. I think we should check the current state and say "is constructed using a null value" if it's already null or simply "is constructed" if it's not yet known to be null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600