Implement vector deleting destructors

[rnk]

Bugzilla Link	19398
Version	unspecified
OS	Windows NT
Blocks	llvm/llvm-project#12849
CC	@majnemer,@timurrrr

Extended Description

MSVC supports an extension that allows users to delete an array of polymorphic objects where the dynamic type doesn't match the static type of the array.

Here's an example of MSVC doing this where we can't:

$ cat t.cpp

extern "C" int printf(const char *, ...);
struct A {
  A() : a(42) {}
  virtual ~A() { printf("a: %d\n", a); }
  int a;
};
struct B : A {
  B() : b(13) {}
  ~B() { printf("b: %d\n", b); }
  int b;
};
void foo(A *a) { delete[] a; }
int main() {
  B *b = new B[2];
  foo(b);
}

$ cl -nologo t.cpp && ./t.exe
t.cpp
b: 13
a: 42
b: 13
a: 42

$ clang-cl t.cpp && ./t.exe
a: 16699704
a: 42

They use "vector deleting destructors" to do this special array deletion, and those are what go in the vftable. We currently put the scalar deleting dtor there instead.

[rnk]

What was the rationale to take a different approach?

Vector deleting destructors are a little less performance-optimized, aren't they? And they are not needed in most of the cases...

They cost one conditional branch per virtual deletion. We're already doing a virtual delete, I don't think one branch will matter.

We need a vector deleting destructor in order to be ABI compatible with MSVC when MSVC wants to delete an array of types with a virtual destructor, even when it's not a polymorphic delete.

Consider:

struct A {
  virtual ~A(); // assume clang compiled this
};
void f(A *p) {
  delete[] p; // will call deleting dtor through vftable
}

Clang itself will currently never call this virtual array deleting dtor, but we could teach it do as a code size optimization.

[timurrrr]

What was the rationale to take a different approach?

Vector deleting destructors are a little less performance-optimized, aren't they? And they are not needed in most of the cases...

[rnk]

David and I were chatting about this the other day. We decided that the best way to do this is to forget we ever heard about scalar deleting destructors and just have one destructor for array and scalar deletion.

[timurrrr]

There was a patch for that http://reviews.llvm.org/D824 Very likely needs rebasing.

[tahonermann]

@rnk, If I'm not mistaken, this issue is still valid though at least some support for vector deleting destructors does appear to be implemented per the comments here and here. Various FIXME comments remain though as can be seen here and here.

Do you have a sense of what needs to be done to complete support for vector deleting destructors? Particularly with regard to ABI compatibility with MSVC (according to this article, it seems that additional virtual function table slots are allocated for them)? Do you know if support for vector constructor and destructor iterator helpers are also needed (everything I know about these comes from reading this article).

Within Intel, we have a case where Clang and MSVC are apparently generating incompatible code or virtual function tables in cases that involve vector deleting destructors. Unfortunately, I don't have a good example I can share yet that illustrates the problem.

[rnk]

I think the simplest way we could get full compatibility would come at a code size and performance cost, which some users may not appreciate, since this is after all a non-conforming extension.

I think it's correct to simply add more logic to the deleting destructor body here to check the implicit flags parameter as we do here and emit an if/else diamond around a single destructor call for non-arrays and a for loop using the array cookie value to handle arrays. Then, change the mangler to always call our deleting destructor vector deleting destructors, because they all are. I believe that is sufficient to be compatible, but it's suboptimal, and probably the wrong tradeoff for most users.

Going beyond that, you can try to optimize for the case where no array deletions of a type occur (the common case). If any array deletions of the type are observed in the TU, you emit the vector deleting destructor as described above. If not, you emit the scalar deleting destructor (no conditional branch, no for loop). You also emit a weak alias from vector deleting dtor (?_E) to scalar deleting dtor (?_G). The vtable entry should always refer to the vector deleting dtor (?_G). With Clang's incremental IR emission, you can can speculatively emit the scalar deleting dtor the first time it is required, and then when delete [] (T*) is encountered, delete the scalar deleting dtor and replace the vector deleting alias with the full vector deleting dtor.

A major downside to weak aliases is that they will block optimizations like devirtualization and inlining. Most of them will not look through a weak alias, since it can be overridden in another TU. Perhaps you could model it as a weak_odr alias to indicate that it's legal to optimize "as if" the weak alias cannot be overridden, but this is pretty sketchy and you'd need to run it by optimizer people.

Going further beyond that, I believe the array ctor/dtor iterators are another optional size saving optimization, but I think that's separable. I think they mainly reduce overhead when exception cleanups are enabled (/EHsc).

[rnk]

@jyknight what do you think would be the best way to model the vector deleting destructor weak alias in LLVM IR?

[tahonermann]

Thank you, @rnk! With regard to:

The vtable entry should always refer to the vector deleting dtor (?_G)

I think you meant ?_E there, yes?

[rnk]

I think you meant ?_E there, yes?

Yeah, sorry.

Regarding ABI compatibility concerns, just thinking about it, I can come up with some examples where mixing a Clang vtables with MSVC delete[] invocation will run into issues. Consider something like this:

// t.h
struct Foo {
  Foo();
  virtual ~Foo();
  int x;
};
//struct Bar : Foo { int c; };
#ifdef __clang__
// clang TU, provide a vftable with scalar deleting dtor, link it first to prevail
Foo::Foo() {}
Foo::~Foo() {}
#else
// MSVC TU
void deleteit(Foo *p) { delete[] p; } // this will call through the vftable slot, passing 3
int main() {
  Foo *p = new Foo[4]();
  deleteit(p);
}
#endif

https://godbolt.org/z/6z6Ks6cKE

Clang only emits the scalar deleting dtor, and doesn't test flag bit 2 to handle the array deletion case.

MSVC calls through the vftable destructor slot and passes the flags 0x3 to indicate that this is an array, and it should be deleted:

        mov     rax, QWORD PTR [rcx]
        mov     edx, 3 ; flags
        add     rsp, 40                             ; 00000028H
        rex_jmp QWORD PTR [rax] ; tail call

It also provides the vector deleting dtor in this TU, which would make this work if MSVC compiled the first TU.

[llvmbot]

@llvm/issue-subscribers-c-1

Author: Reid Kleckner (rnk)

| | | | --- | --- | | Bugzilla Link | [19398](https://llvm.org/bz19398) | | Version | unspecified | | OS | Windows NT | | Blocks | llvm/llvm-project#12849 | | CC | @majnemer,@timurrrr | ## Extended Description MSVC supports an extension that allows users to delete an array of polymorphic objects where the dynamic type doesn't match the static type of the array. Here's an example of MSVC doing this where we can't: ```console $ cat t.cpp ``` ```cpp extern "C" int printf(const char *, ...); struct A { A() : a(42) {} virtual ~A() { printf("a: %d\n", a); } int a; }; struct B : A { B() : b(13) {} ~B() { printf("b: %d\n", b); } int b; }; void foo(A *a) { delete[] a; } int main() { B *b = new B[2]; foo(b); } ``` ```console $ cl -nologo t.cpp && ./t.exe t.cpp b: 13 a: 42 b: 13 a: 42 $ clang-cl t.cpp && ./t.exe a: 16699704 a: 42 ``` They use "vector deleting destructors" to do this special array deletion, and those are what go in the vftable. We currently put the scalar deleting dtor there instead.

[tahonermann]

I've been investigating the conditions under which MSVC does and does not emit a vector deleting destructor. Avoiding link errors due to missing definitions will presumably require emitting a definition in a superset of the scenarios in which MSVC does.

For a virtual destructor with a definition, a call to new[] (but not delete[]) appears to be necessary for the definition of a vector deleting destructor to be emitted. The following symbols are emitted for the following scenarios. https://godbolt.org/z/fx6MvKPqY (I found it helpful to clear the "Library functions" and "Directives" filters in Compiler Explorer to see this clearly).

• No new[] or delete[] calls:
  • scalar deleting destructor definition.
  • extern vector deleting destructor reference.
• Calls to new[] but not delete[]:
  • vector deleting destructor definition.
• Calls to delete[] but not new[]:
  • If the destructor is inline:
  • No scalar or vector deleting destructors are referenced or defined.
  • Otherwise:
  • scalar deleting destructor definition.
  • extern vector deleting destructor reference.

This makes sense because, in order for a call to delete[] to be well-formed, there must have been a call to new[] somewhere in the program.

If the destructor is defined, but not virtual, then a vector deleting destructor is emitted due to use of either new[] or delete[]. https://godbolt.org/z/r1sjK664o.

• No new[] or delete[] calls:
  • No scalar or vector deleting destructors are referenced or defined.
• Calls to new[] but not delete[]:
  • vector deleting destructor definition.
• Calls to delete[] but not new[]:
  • vector deleting destructor definition.

This makes sense because, without a reference to the vector deleting destructor in the virtual function table, there is no assurance that a definition will be provided by some other translation unit.

If the destructor is not defined in the translation unit, then the following symbols are emitted for each scenario (This behavior does not appear to be dependent on emit of a virtual function table). https://godbolt.org/z/dWb3j46Eo.

• No new[] or delete[] calls:
  • No scalar or vector deleting destructors are referenced or defined.
• Calls to new[] but not delete[]:
  • vector deleting destructor definition (with no opportunity to inline the call to the destructor).
• Calls to delete[] but not new[]:
  • If the destructor is virtual:
  • No scalar or vector deleting destructors are referenced or defined.
  • Otherwise:
  • vector deleting destructor definition (with no opportunity to inline the call to the destructor).

I suspect that some of the behavior described above falls out from whether a virtual function table definition is emitted in the translation unit (which appears to be tied to the presence of a destructor definition; perhaps virtual destructors are used as the key function for emit of a virtual function table in the MS ABI?).

The scalar/vector deleting destructors appear to always be emitted as COMDAT.

[tahonermann]

In addition to the last comment, if the destructor is virtual and declared __declspec(dllexport), then the vector deleting destructor is emitted as a defined symbol if the destructor is defined in the translation unit.