[cfe-dev] is delete on abstract with non-virtal ever safe?

[Richard Smith]

On Thu, Oct 3, 2013 at 2:43 AM, emmanuel.attia
<emmanuel.attia at philips.com>wrote:

> Hi,
>
> Sorry to dig out an old topic, but there is one case where delete on
> abstract with non-virtual is safe.
> If you make a component model for plugin managment and you don't want to
> enforce any compiler / compiler settings / runtime settings, you have to
> delegate the delete operator to make sure deletion happen in the same
> module
> as allocation.
>
> Here is a small example of this: (suppose main.cpp and toto.cpp are in
> different module, for instance main.exe and toto.dll).
>
>
> // common.h (visible in main.cpp and toto.cpp)
> struct my_destroy_interface
> {
>     virtual void Destroy() = 0;
>
>     inline void operator delete (void * ptr) {
>         if (ptr != NULL) static_cast<my_destroy_interface
> *>(ptr)->Destroy();
>

This has undefined behavior. You're calling a virtual function on an object
whose lifetime has ended (the destructor has already been called).

    }
> };
>
> my_destroy_interface * createToto();
>
> // toto.cpp
> struct my_destroy_default_impl : public my_destroy_interface
> {
> protected:
>     virtual ~my_destroy_default_impl()
>     {
>         printf("~my_destroy_default_impl()\n");
>     }
>
>     virtual void Destroy()
>     {
>         ::delete this;
>     }
> };
>
> struct Toto : public my_destroy_default_impl
> {
> protected:
>     ~Toto()
>     {
>         printf("~Toto()\n");
>     }
> };
>
> my_destroy_interface * createToto()
> {
>     return new Toto;
> }
>
> // main.cpp
> int main(int argc, char* argv[])
> {
>     delete createToto();
>

This has undefined behavior (you're using 'delete', the static and dynamic
types don't match, and your base class does not have a virtual destructor).

        return 0;
> }
>
> Result:
> ~Toto()
> ~my_destroy_default_impl()
>
>
> This is perfectly valid


Nope. See above. Or try giving my_destroy_interface a user-declared
(non-virtual) destructor, and watch as it either gets called twice or your
program starts to die due to a pure virtual function call (depending on
implementation details).

but gets a warning:
> warning : delete called on 'my_destroy_interface' that is abstract but has
> non-virtual destructor [-Wdelete-non-virtual-dtor]
>
> If I would have put the virtual destructor in the interface definition,
> "delete createToto()" would have called ~Toto(), but also "::delete this".
>
> Usually this is not an issue because in this kind of component model, the
> practice is to call ->Destroy() (more oftently called Release() ) directly.
>
> Using the delete operator allows to combine this kind of objects with smart
> pointer classes, without the smart pointer classes having any knowledge
> that
> they handle objects from another "plugin".
>
> An alternative to this without warning would be to:
> * add an empty virtual destructor to the interface.
> * call ::operator delete(this) instead of ::delete this in
> my_destroy_default_impl::Destroy to trigger only memory freeing and not
> destructor calling.
> Both method are valid but not having to add a empty virtual destructor to
> each interface is nice (plus interface are not supposed to have implemented
> functions)
>
> One could live with ignoring this warning, but I guess when the operator
> delete is custom, it is safe not to emit this warning.
>

No, the warning is always correct.


> Best regards,
>
> Emmanuel
>
>
>
> --
> View this message in context:
> http://clang-developers.42468.n3.nabble.com/is-delete-on-abstract-with-non-virtal-ever-safe-tp4025653p4034847.html
> Sent from the Clang Developers mailing list archive at Nabble.com.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131003/22d9a33a/attachment.html>