Merge pull request #20465 from nicolasnoble/poly-delete

Polymorphic delete support

Makefile

@@ -355,7 +355,7 @@ LDFLAGS += -framework CoreFoundation
 endif
 CFLAGS += -g
 CPPFLAGS += -g -Wall -Wextra -DOSATOMIC_USE_INLINED=1 -Ithird_party/upb -Isrc/core/ext/upb-generated
-COREFLAGS += -fno-rtti -fno-exceptions
+COREFLAGS += -fno-exceptions
 LDFLAGS += -g
 
 CPPFLAGS += $(CPPFLAGS_$(CONFIG))
@@ -7925,7 +7925,7 @@ PUBLIC_HEADERS_C += \
 LIBBORINGSSL_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(LIBBORINGSSL_SRC))))
 
 $(LIBBORINGSSL_OBJS): CPPFLAGS += -Ithird_party/boringssl/include -fvisibility=hidden -DOPENSSL_NO_ASM -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
-$(LIBBORINGSSL_OBJS): CXXFLAGS += -fno-rtti -fno-exceptions
+$(LIBBORINGSSL_OBJS): CXXFLAGS += -fno-exceptions
 $(LIBBORINGSSL_OBJS): CFLAGS += -g
 
 $(LIBDIR)/$(CONFIG)/libboringssl.a: $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP)  $(LIBBORINGSSL_OBJS) 
@@ -7955,7 +7955,7 @@ PUBLIC_HEADERS_CXX += \
 LIBBORINGSSL_TEST_UTIL_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(LIBBORINGSSL_TEST_UTIL_SRC))))
 
 $(LIBBORINGSSL_TEST_UTIL_OBJS): CPPFLAGS += -Ithird_party/boringssl/include -fvisibility=hidden -DOPENSSL_NO_ASM -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
-$(LIBBORINGSSL_TEST_UTIL_OBJS): CXXFLAGS += -fno-rtti -fno-exceptions
+$(LIBBORINGSSL_TEST_UTIL_OBJS): CXXFLAGS += -fno-exceptions
 $(LIBBORINGSSL_TEST_UTIL_OBJS): CFLAGS += -g
 
 ifeq ($(NO_PROTOBUF),true)
@@ -20118,7 +20118,7 @@ $(BINDIR)/$(CONFIG)/boringssl_ssl_test: $(PROTOBUF_DEP) $(BORINGSSL_SSL_TEST_OBJ
 endif
 
 $(BORINGSSL_SSL_TEST_OBJS): CPPFLAGS += -Ithird_party/boringssl/include -fvisibility=hidden -DOPENSSL_NO_ASM -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
-$(BORINGSSL_SSL_TEST_OBJS): CXXFLAGS += -fno-rtti -fno-exceptions
+$(BORINGSSL_SSL_TEST_OBJS): CXXFLAGS += -fno-exceptions
 $(BORINGSSL_SSL_TEST_OBJS): CFLAGS += -g
 $(OBJDIR)/$(CONFIG)/third_party/boringssl/crypto/test/gtest_main.o:  $(LIBDIR)/$(CONFIG)/libboringssl_test_util.a $(LIBDIR)/$(CONFIG)/libboringssl.a
 
@@ -20210,7 +20210,7 @@ $(BINDIR)/$(CONFIG)/boringssl_crypto_test: $(PROTOBUF_DEP) $(BORINGSSL_CRYPTO_TE
 endif
 
 $(BORINGSSL_CRYPTO_TEST_OBJS): CPPFLAGS += -Ithird_party/boringssl/include -fvisibility=hidden -DOPENSSL_NO_ASM -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
-$(BORINGSSL_CRYPTO_TEST_OBJS): CXXFLAGS += -fno-rtti -fno-exceptions
+$(BORINGSSL_CRYPTO_TEST_OBJS): CXXFLAGS += -fno-exceptions
 $(BORINGSSL_CRYPTO_TEST_OBJS): CFLAGS += -g
 $(OBJDIR)/$(CONFIG)/src/boringssl/crypto_test_data.o:  $(LIBDIR)/$(CONFIG)/libboringssl_test_util.a $(LIBDIR)/$(CONFIG)/libboringssl.a
 

build.yaml

@@ -6165,10 +6165,10 @@ defaults:
     CFLAGS: -g
     CPPFLAGS: -Ithird_party/boringssl/include -fvisibility=hidden -DOPENSSL_NO_ASM
       -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
-    CXXFLAGS: -fno-rtti -fno-exceptions
+    CXXFLAGS: -fno-exceptions
   global:
     CFLAGS: -g
-    COREFLAGS: -fno-rtti -fno-exceptions
+    COREFLAGS: -fno-exceptions
     CPPFLAGS: -g -Wall -Wextra -DOSATOMIC_USE_INLINED=1 -Ithird_party/upb -Isrc/core/ext/upb-generated
     LDFLAGS: -g
   zlib:

src/core/lib/gprpp/memory.h

@@ -48,19 +48,32 @@
 
 namespace grpc_core {
 
-// Alternative to new, since we cannot use it (for fear of libstdc++)
+// Alternative to new, to ensure memory allocation being wrapped to gpr_malloc
 template <typename T, typename... Args>
 inline T* New(Args&&... args) {
   void* p = gpr_malloc(sizeof(T));
   return new (p) T(std::forward<Args>(args)...);
 }
 
-// Alternative to delete, since we cannot use it (for fear of libstdc++)
+// Gets the base pointer of any class, in case of multiple inheritence.
+// Used by Delete and friends.
+template <typename T, bool isPolymorphic>
+struct BasePointerGetter {
+  static void* get(T* p) { return p; }
+};
+
+template <typename T>
+struct BasePointerGetter<T, true> {
+  static void* get(T* p) { return dynamic_cast<void*>(p); }
+};
+
+// Alternative to delete, to ensure memory allocation being wrapped to gpr_free
 template <typename T>
 inline void Delete(T* p) {
   if (p == nullptr) return;
+  void* basePtr = BasePointerGetter<T, std::is_polymorphic<T>::value>::get(p);
   p->~T();
-  gpr_free(p);
+  gpr_free(basePtr);
 }
 
 class DefaultDelete {
@@ -68,12 +81,13 @@ class DefaultDelete {
   template <typename T>
   void operator()(T* p) {
     // Delete() checks whether the value is null, but std::unique_ptr<> is
-    // gauranteed not to call the deleter if the pointer is nullptr
+    // guaranteed not to call the deleter if the pointer is nullptr
     // (i.e., it already does this check for us), and we don't want to
     // do the check twice.  So, instead of calling Delete() here, we
     // manually call the object's dtor and free it.
+    void* basePtr = BasePointerGetter<T, std::is_polymorphic<T>::value>::get(p);
     p->~T();
-    gpr_free(p);
+    gpr_free(basePtr);
   }
 };
 

test/core/gprpp/memory_test.cc

@@ -29,6 +29,21 @@ struct Foo {
   int b;
 };
 
+struct Base1 {
+  int a;
+  virtual ~Base1() {}
+};
+
+struct Base2 {
+  int b;
+  virtual ~Base2() {}
+};
+
+struct Compound : public Base1, Base2 {
+  int c;
+  virtual ~Compound() {}
+};
+
 TEST(MemoryTest, NewDeleteTest) { Delete(New<int>()); }
 
 TEST(MemoryTest, NewDeleteWithArgTest) {
@@ -64,6 +79,12 @@ TEST(MemoryTest, UniquePtrWithCustomDeleter) {
   EXPECT_EQ(1, n);
 }
 
+TEST(MemoryTest, MultipleInheritence) {
+  Base2* p = New<Compound>();
+  EXPECT_NE(p, dynamic_cast<void*>(p));
+  Delete(p);
+}
+
 }  // namespace testing
 }  // namespace grpc_core