From 4f26fad1892c8bb18bbec841594c7f283555da13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Joly?= Date: Sat, 1 Jun 2024 01:44:01 +0200 Subject: [PATCH] Modify rule S1265: Add support for sized delete (M23_358) and globally improve the rule (CPP-5138 CPP-1313 ) --- rules/S1265/cfamily/metadata.json | 3 +- rules/S1265/cfamily/rule.adoc | 63 ++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/rules/S1265/cfamily/metadata.json b/rules/S1265/cfamily/metadata.json index ef793bcaad5..4f8782652ef 100644 --- a/rules/S1265/cfamily/metadata.json +++ b/rules/S1265/cfamily/metadata.json @@ -14,7 +14,8 @@ }, "tags": [ "cppcoreguidelines", - "cert" + "cert", + "based-on-misra" ], "extra": { "replacementRules": [ diff --git a/rules/S1265/cfamily/rule.adoc b/rules/S1265/cfamily/rule.adoc index 3b16344f017..66641c2e964 100644 --- a/rules/S1265/cfamily/rule.adoc +++ b/rules/S1265/cfamily/rule.adoc @@ -1,26 +1,32 @@ == Why is this an issue? -The `operator new` allocates memory for objects, and the `operator delete` frees the memory allocated by the matching `operator new`. When a class needs to customize memory allocation, it can override the `operator new` to use a custom memory allocation strategy and override the `operator delete` accordingly. +The `operator new` allocates memory for objects, and the `operator delete` frees the memory allocated by the matching `operator new`. It is possible to customize these operations, either for a specific class (by overloading these operators in the class) or globally (by defining them in the global namespace, they will replace the default version). -If the `operator delete` is not overridden alongside the `operator new`, the program will call its default implementation, which may not be suitable for the custom memory allocation strategy used by the overridden `operator new`. +If the `operator delete` is not overloaded alongside the `operator new`, the program will call its default implementation, which may not be suitable for the custom memory allocation strategy used by the overloaded `operator new`. For instance, if the `operator new` draws memory from a preallocated buffer instead of allocating memory, the `operator delete` should not call the `free` function to release the memory. Reciprocally, if the `operator new` allocate memory with `malloc`, the `operator delete` must call `free`. -On the other hand, if the `operator delete` is overridden without overriding the `operator new`, it is suspicious because it may not correctly release the memory allocated by the default `operator new`. +On the other hand, if the `operator delete` is overloaded without overloading the `operator new`, it is suspicious because it may not correctly release the memory allocated by the default `operator new`. By defining the `operator delete` along with the `operator new`, the memory is deallocated in a way consistent with the custom allocation strategy used by the `operator new`. +Up to this point, we mentioned `operator new` and `operator delete`, but it is a simplification. There are many different forms of https://en.cppreference.com/w/cpp/memory/new/operator_new[`operator new`] and https://en.cppreference.com/w/cpp/memory/new/operator_delete[`operator delete`] (for a single object or an array, with an extra alignment parameter... see the links for a full list), and the consistency between new and delete should be enforced for each form. + +For instance, if `++void * operator new[]( std::size_t count, std::align_val_t al );++` is defined (for arrays, with extra alignment), then `++void operator delete[]( void* ptr, std::align_val_t al ) noexcept;++` should be defined too. + +Additionally, since {cpp}17, it is possible to define a version of the delete operator with an additional size argument, alongside the unsized version of `operator delete`. When overloading these operators in a class, defining both a sized and an unsized version of operator delete is useless, since the unsized version will always be preferred. However, for free replacement, it is necessary to specify both versions since the language does not specify which version will be called. + === What is the potential impact? -Overriding only one of the two operators may result in your program consuming more and more memory over time, eventually leading to a crash. +Overloading only one of the two operators may result in your program consuming more and more memory over time, eventually leading to a crash. Deallocating memory that was not allocated with the corresponding strategy results in undefined behavior. == How to fix it -Each override of the `operator new` should have a matching overridden `operator delete` and vice versa. +Each overload of the `operator new` should have a matching overload `operator delete` and vice versa. Since {cpp}17, within a class, define only a sized or an unsized version of `operator delete`, but as a free function, define both. -=== Code examples +=== Example with overloaded operators in a class Imagine a custom allocator `MyAllocator`: @@ -48,7 +54,7 @@ public: ---- class MyClass { public: - // Noncompliant: there is no matching override of the delete operator + // Noncompliant: there is no matching overload of the delete operator void* operator new(size_t size) { return allocator.allocate(size); } @@ -60,7 +66,7 @@ private: void f() { MyClass* obj = new MyClass(); delete obj; // It will call the default implementation of the operator delete - // and this latter will not call the MyAllocator::deallocate function to correctly release the memory + // which will not call the MyAllocator::deallocate function to correctly release the memory } ---- @@ -74,7 +80,7 @@ public: return allocator.allocate(size); } - void operator delete(void* p) { + void operator delete(void* p) noexcept { allocator.deallocate(p); } @@ -88,6 +94,43 @@ void f() { } ---- +=== Example with replacement of global allocators + +In the following example, the intent is to replace allocation functions with system-specific variants. + +==== Noncompliant code example + +[source,cpp,diff-id=2,diff-type=noncompliant] +---- +// Noncompliant: The sized version of delete is not replaced + +void* operator new(std::size_t count) { + return SystemSpecificNew(count); +} + +void operator delete(void* ptr ) noexcept { + return SystemSpecificDelete(ptr); +} +---- +==== Compliant solution + +[source,cpp,diff-id=2,diff-type=compliant] +---- +void* operator new(std::size_t count) { + return SystemSpecificNew(count); +} + +void operator delete(void* ptr) noexcept { + return SystemSpecificDelete(ptr); +} + +// Compliant: Even if it does exactly the same as the unsized version, this sized +// version of delete replaces the default-provided one that probably deallocates +// memory in a different and incompatible way. +void operator delete(void* ptr, std::size_t) noexcept { + return SystemSpecificDelete(ptr); +} +---- == Resources === Documentation @@ -102,6 +145,8 @@ void f() { === External coding guidelines * {cpp} Core Guidelines - https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#r15-always-overload-matched-allocationdeallocation-pairs[R.15: Always overload matched allocation/deallocation pairs] +* MISRA {cpp}:2023, 21.6.4 - If a project defines either a sized or unsized version of a global +operator delete, then both shall be defined === Related rules