[PATCH] D108753: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:281-288
 /// Tells if the callee is one of the builtin new/delete operators, including
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
 static bool isStandardNewDelete(const CallEvent &Call) {
   if (!Call.getDecl() || !isa<FunctionDecl>(Call.getDecl()))
     return false;
   return isStandardNewDelete(cast<FunctionDecl>(Call.getDecl()));
----------------
These serve to identify new/delete operators. I've done a lot of work to make this better, but I don't remember why `isFreeingCall` doesn't call these.


================
Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:23
 
+// TODO: AST analysis of sink would reveal that it doesn't indent to free the
+// allocated memory, but in this instance, its also the only function with
----------------
martong wrote:
> intent
> 
> BTW, why is this TODO here? It is obvious that `sink` does not have a `delete`, so we don't expect any warning. Or am I missing something?
Well, its debatable, I suppose, but in this case, `sink` is noteworthy, as no other function had the ability to take care of this memory.

https://lists.llvm.org/pipermail/cfe-dev/2021-June/068469.html
> Kristof's case is interesting in a different manner. If taken literally, it doesn't satisfy our criteria of "there exists another execution path on which it did actually do ${Something}". We probably shouldn't emit a note every time the function simply accepts the value of interest by pointer, because this doesn't necessarily prove the intent to delete the pointer; there are a million other reasons to accept a pointer. However, Kristof's case does still deserve a note because sink() is the *only* function that had a chance to delete the pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108753