[PATCH] D13071: [PATCH] New checker for mismatched operator new/operator delete definitions

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: klimek, alexfh.
aaron.ballman added a subscriber: cfe-commits.

When defining a free store function, it is almost invariably a bug to fail to declare the corresponding free store function. For instance, implementing operator new() but failing to implement operator delete(). This new checker catches instances where the free store functions are mismatched, while still allowing for common code patterns involving deleted functions or placement forms of functions. Specifically, this will not warn on implicit free store functions, placement forms, deleted functions (such as defining a placement new and a deleted operator delete(void*)), and private functions (for code that predates C++11).

This patch corresponds to: https://www.securecoding.cert.org/confluence/display/cplusplus/DCL54-CPP.+Overload+allocation+and+deallocation+functions+as+a+pair+in+the+same+scope

I ran this checker over Clang and LLVM and it did find two issues, one of which I have fixed. YAMLParser.h had a protected operator delete() with a trivial definition that I converted to be a deleted function in r248320. This was a borderline false positive -- the code was correct because the body of operator delete() was trivial. However, defining the function as deleted is cleaner since accidental deletion from within Node or one of its subclasses will now be diagnosed instead of silently accepted and doing nothing. The other is a true positive as best I can tell. The MDNode class in Metadata.h defines a placement new operator, a free store operator delete(), and two placement delete functions. The free store operator delete() has no matching free store operator new() (including when looking in super classes), but the implementation does not appear to trigger undefined behavior -- it just seems like a really confused design (which is mitigated by the fact that the free store delete is protected). Also, one of the placement delete functions is not required by std, as the comments suggest, and should be removed.

~Aaron

http://reviews.llvm.org/D13071

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/NewDeleteOverloadsCheck.cpp
  clang-tidy/misc/NewDeleteOverloadsCheck.h
  test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp
  test/clang-tidy/misc-new-delete-overloads.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13071.35414.patch
Type: text/x-patch
Size: 13887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150922/b3e501c8/attachment-0001.bin>