Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not require complete object or deleting destructor symbols for abstract classes #10

Open
zygoloid opened this issue Feb 25, 2017 · 7 comments
Labels
ABIv2 recommendation The issue proposes a better but incompatible ABI that may be well-suited for a new target.

Comments

@zygoloid
Copy link
Contributor

zygoloid commented Feb 25, 2017

Given

struct A { virtual ~A() = 0; }; A::~A() {}

we seem to require three symbols to be emitted: _ZN1AD0Ev, _ZN1AD1Ev, and _ZN1AD2Ev (or at least, Clang and GCC both emit all three). Of these, only _ZN1AD2Ev can ever be referenced; the deleting destructor and complete object destructor are not entered into the vtable, and a complete object of type A can never be destroyed directly.

We should not require an implementation to emit these extra symbols. Note in particular that code must be emitted for the operator delete call for D0, which needs whole program analysis (-ffunction-sections, LTO, etc) to remove.

The same holds regardless of whether the destructor is virtual (but if not, then there's at least only the complete object destructor symbol to worry about, which can always be an alias to the base subobject destructor symbol).

@zygoloid
Copy link
Contributor Author

zygoloid commented Feb 25, 2017

Looks like this will need to wait for v2. For this testcase:

struct X { virtual ~X(); };
struct A : virtual X { A(); virtual void f() = 0; ~A(); };
struct Y : A { void f() {} } y;

... Clang emits a reference to _ZN1AD0Ev and to _ZN1AD1Ev when forming the construction vtable for A in Y, so ceasing to emit these symbols would break ABI in practice.

However, we can emit them as symbols with value 0 or something similar (that's what GCC puts in the construction vtable today). __cxa_pure_virtual is almost a good fit here, but in practice the diagnostic it would produce for the case of, say,

A::A() { delete this; }

... doesn't really explain the problem.

@zygoloid
Copy link
Contributor Author

See also http://wg21.link/cwg1658, which means that it is no longer correct in general for emission of an abstract class's destructor to emit references to vbase destructors (for instance, a vbase destructor in a class template doesn't necessarily have a point of instantiation in the same translation unit).

@rjmccall rjmccall added the ABIv2 recommendation The issue proposes a better but incompatible ABI that may be well-suited for a new target. label Feb 27, 2017
@rjmccall
Copy link
Collaborator

I agree with everything you have to say here. Tagging so that we can easily find this when we start adding v2-recommendation sections to the ABI.

@zygoloid
Copy link
Contributor Author

So... it turns out that this is valid:

struct B : A {};
void f(B *p) { p->A::~A(); }

... and ends up calling the complete object destructor for A. But that seems like a language bug to me.

@rjmccall
Copy link
Collaborator

It's also necessarily UB, right? So we're not actually obliged to implement it by calling the complete object destructor; we could just call the base constructor (or much worse, of course).

@zygoloid
Copy link
Contributor Author

It probably should be UB; I don't think the current language rules say that it is, but we should presumably fix that too :)

@rjmccall
Copy link
Collaborator

Well, we can get started on implementing a runtime global dictionary of object kinds by address if the committee insists. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABIv2 recommendation The issue proposes a better but incompatible ABI that may be well-suited for a new target.
Projects
None yet
Development

No branches or pull requests

2 participants