From 957ddbb9f04f55cabd8496e8d74eb35ee4d29105 Mon Sep 17 00:00:00 2001
From: Chris Lalancette <clalancette@gmail.com>
Date: Mon, 29 Apr 2024 04:05:34 -0400
Subject: [PATCH] Fix gcc warnings when building with optimizations. (#672)

* Fix gcc warnings when building with optimizations.

When building the allocator_tutorial_pmr demo with -O2,
gcc is throwing an error saying that new and delete are
mismatched.  This is something of a misnomer, however;
the real problem is that the global new override we
have in that demo is actually implemented incorrectly.

In particular, the documentation at
https://en.cppreference.com/w/cpp/memory/new/operator_new
very clearly specifies that operator new either has to
return a valid pointer, or throw an exception on error.
Our version wasn't throwing the exception, so change it
to throw std::bad_alloc if std::malloc fails.

While we are in here, also fix another small possible
is where std::malloc could return nullptr on a zero-sized
object, thus throwing an exception it shouldn't.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

* Always inline the new and delete operators.

That's because gcc 13 has a bug where it can sometimes
inline one or the other, and then it detects that they
mismatch.  For gcc and clang, just force them to always
be inline in this demo.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

* Switch to NOINLINE instead.

Both clang and MSVC don't like inlining these, so instead
ensure that they are *not* inlined.  This also works
because the problem is when new is inlined but not delete
(or vice-versa).  As long as they are both not inlined,
this should fix the warning.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
---
 .../src/topics/allocator_tutorial_pmr.cpp     | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/demo_nodes_cpp/src/topics/allocator_tutorial_pmr.cpp b/demo_nodes_cpp/src/topics/allocator_tutorial_pmr.cpp
index 0d8bad71f..000e901d3 100644
--- a/demo_nodes_cpp/src/topics/allocator_tutorial_pmr.cpp
+++ b/demo_nodes_cpp/src/topics/allocator_tutorial_pmr.cpp
@@ -16,6 +16,7 @@
 #include <list>
 #include <memory>
 #include <memory_resource>
+#include <stdexcept>
 #include <string>
 #include <utility>
 
@@ -64,15 +65,34 @@ static bool is_running = false;
 static uint32_t global_runtime_allocs = 0;
 static uint32_t global_runtime_deallocs = 0;
 
-void * operator new(std::size_t size)
+// Due to GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103993, we
+// always inline the overridden new and delete operators.
+
+#if defined(__GNUC__) || defined(__clang__)
+#define NOINLINE __attribute__((noinline))
+#else
+#define NOINLINE
+#endif
+
+NOINLINE void * operator new(std::size_t size)
 {
+  if (size == 0) {
+    ++size;
+  }
+
   if (is_running) {
     global_runtime_allocs++;
   }
-  return std::malloc(size);
+
+  void * ptr = std::malloc(size);
+  if (ptr != nullptr) {
+    return ptr;
+  }
+
+  throw std::bad_alloc{};
 }
 
-void operator delete(void * ptr, size_t size) noexcept
+NOINLINE void operator delete(void * ptr, size_t size) noexcept
 {
   (void)size;
   if (ptr != nullptr) {
@@ -83,7 +103,7 @@ void operator delete(void * ptr, size_t size) noexcept
   }
 }
 
-void operator delete(void * ptr) noexcept
+NOINLINE void operator delete(void * ptr) noexcept
 {
   if (ptr != nullptr) {
     if (is_running) {