[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie marked 5 inline comments as done.
Discookie added a comment.

Ran the checker on a couple larger projects, but no real-world reports found so far (same as DeleteWithNonVirtualDtor). Can't tell if it's an engine limitation or just the bug being uncommon in general.



================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:762-763
+def CXXArrayDeleteChecker : Checker<"ArrayDelete">,
+  HelpText<"Reports destructions of arrays of polymorphic objects that are"
+           "destructed as their base class.">,
+  Documentation<HasDocumentation>;
----------------
steakhal wrote:
> I thin these strings are concatenated like in C, thus it's gonna have "aredestructed" joined.
Whoops, fixed.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:199-201
+  // FIXME: This way of getting base types does not support reference types.
+  QualType SourceType = CastE->getSubExpr()->getType()->getPointeeType();
+  QualType TargetType = CastE->getType()->getPointeeType();
----------------
steakhal wrote:
> What is the problem with this?
> I thought `getPointeeType()` works for ReferenceTypes.
Apparently not, because references aren't ReferenceTypes but qualified Types. I could add support for it in a future commit, but I'd think casting and deleting array-references wrongly is even less common than deleting array-pointers.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:218-219
+
+  OS << "Casting from `" << SourceType.getAsString() << "` to `"
+     << TargetType.getAsString() << "` here";
+
----------------
steakhal wrote:
> We use single apostrophes for quoting names in CSA.
Got it, thanks!


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

https://reviews.llvm.org/D158156