[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 22 17:44:48 PDT 2018


NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, baloghadamsoftware.

Fixes https://bugs.llvm.org/show_bug.cgi?id=39348

Under `-fsized-deallocation`, the new test case in `NewDelete-sized-deallocation.cpp` produces a call to operator delete with an explicitly passed size argument. MallocChecker didn't expect that and identified this as a custom operator delete, which lead to a leak false positive on extremely simple code.

Identify custom operators by source locations instead of guessing by the number of parameters. This sounds much more correct.

Additionally, it exposes a regression in `NewDelete-intersections.mm`'s `testStandardPlacementNewAfterDelete()` test, where the diagnostic is delayed from before the call of placement new into the code of placement new in the header. This happens because the check for pass-into-function-after-free for placement arguments is located in `checkNewAllocator()`, which happens after the allocator is inlined, which is too late. Move this use-after-free check into `checkPreCall` instead, where it works automagically because the guard that prevents it from working is useless and can be removed as well.

I didn't bother looking at `NewDelete-custom.cpp` because they only manifest under the non-default-and-never-planned-to-be-default-anymore option `-analyzer-config c++-allocator-inlining=false`. I believe that this flag can be removed entirely. I think we've previously been thinking that these are false negatives in the new mode that are potentially undesired. Did anybody actually end up using it? If a more aggressive behavior towards custom allocators is desired, would it make sense to re-implement it directly, without relying on side effects of broken operator new modeling (?)


Repository:
  rC Clang

https://reviews.llvm.org/D53543

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/NewDelete-custom.cpp
  test/Analysis/NewDelete-sized-deallocation.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53543.170520.patch
Type: text/x-patch
Size: 6277 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181023/929b0c13/attachment.bin>


More information about the cfe-commits mailing list