Why, really, deleting an incomplete type is undefined behaviour?

abigagli picture abigagli · Mar 25, 2010 · Viewed 9.2k times · Source

Consider this classic example used to explain what not to do with forward declarations:

//in Handle.h file
class Body;

class Handle
{
   public:
      Handle();
      ~Handle() {delete impl_;}
   //....
   private:
      Body *impl_;
};

//---------------------------------------
//in Handle.cpp file

#include "Handle.h"

class Body 
{
  //Non-trivial destructor here
    public:
       ~Body () {//Do a lot of things...}
};

Handle::Handle () : impl_(new Body) {}

//---------------------------------------
//in Handle_user.cpp client code:

#include "Handle.h"

//... in some function... 
{
    Handle handleObj;

    //Do smtg with handleObj...

    //handleObj now reaches end-of-life, and BUM: Undefined behaviour
} 

I understand from the standard that this case is headed towards UB since Body's destructor is non trivial. What I'm trying to understand is really the root cause of this.

I mean, the problem seems to be "triggered" by the fact that Handle's dtor is inline, and so the compiler does something like the following "inline expansion" (almost pseudo-code here).

inline Handle::~Handle()
{
     impl_->~Body();
     operator delete (impl_);
}

In all translation units (only Handle_user.cpp in this case) where a Handle instance gets to be destroyed, right? I just can't understand this: ok, when generating the above inline expansion the compiler doesn't have a full definition of the Body class, but why cannot it simply have the linker resolve for the impl_->~Body() thing and so have it call the Body's destructor function that's actually defined in its implementation file?

In other words: I understand that at the point of Handle destruction the compiler doesn't even know if a (non-trivial) destructor exists or not for Body, but why can't it do as it always does, that is leave a "placeholder" for the linker to fill in, and eventually have a linker "unresolved external" if that function is really not available?

Am I missing something big here (and in that case sorry for the stupid question)? If that's not the case, I'm just curious to understand the rationale behind this.

Answer

Steve Jessop picture Steve Jessop · Mar 25, 2010

To combine several answers and add my own, without a class definition the calling code doesn't know:

  • whether the class has a declared destructor, or if the default destructor is to be used, and if so whether the default destructor is trivial,
  • whether the destructor is accessible to the calling code,
  • what base classes exist and have destructors,
  • whether the destructor is virtual. Virtual function calls in effect use a different calling convention from non-virtual ones. The compiler can't just "emit the code to call ~Body", and leave the linker to work out the details later,
  • (this just in, thanks GMan) whether delete is overloaded for the class.

You can't call any member function on an incomplete type for some or all of those reasons (plus another that doesn't apply to destructors - you wouldn't know the parameters or return type). A destructor is no different. So I'm not sure what you mean when you say "why can't it do as it always does?".

As you already know, the solution is to define the destructor of Handle in the TU which has the definition of Body, same place as you define every other member function of Handle which calls functions or uses data members of Body. Then at the point where delete impl_; is compiled, all the information is available to emit the code for that call.

Note that the standard actually says, 5.3.5/5:

if the object being deleted has incomplete class type at the point of deletion and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined.

I presume this is so that you can delete an incomplete POD type, same as you could free it in C. g++ gives you a pretty stern warning if you try it, though.