[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
----------------
xazax.hun wrote:
> NoQ wrote:
> > This is never necessary, just call all the methods directly on `QT` - it has an overloaded operator `->()` for this.
> `getPointeeOrArrayElementType` returns a `Type *`, so I cannot really continue to use `QualType` and I am not interested in the qualifiers at all.
Whoops ^.^"


================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+    ++PtrToHandleLevel;
----------------
xazax.hun wrote:
> NoQ wrote:
> > Ok, so what's the deal with arrays here? If the function receives an array of handles, do you ultimately plan to return multiple symbols - one for each element of the array?
> > 
> > Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?
> I do not have a real plan with arrays just yet. Creating eagerly a symbol for all the elements might look a bit wasteful but also users are probably not expected to have large arrays of handles? Probably I should remove that code for now.
> 
> I would expect non-Fuchsia checkers to introduce their own `getHandleSymbol` logic. Does that make sense? Maybe I should rename this to `getFuchsiaHandleSymbol`. 
But, i mean, why do you even consider arrays in this code? Why not simply `QT = QT->getPointeeType()`?


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

https://reviews.llvm.org/D70470