[PATCH] [ms-cxxabi] Get closer to emitting the correct set of destructors in each TU (PR12784).
Peter Collingbourne
peter at pcc.me.uk
Fri Apr 26 14:06:15 PDT 2013
On Fri, Apr 26, 2013 at 06:45:04PM +0400, Timur Iskhodzhanov wrote:
> Hi Peter,
>
> lib/AST/MicrosoftMangle.cpp
> @@ -551,17 +551,18 @@ void
> MicrosoftCXXNameMangler::manglePostfix(const DeclContext *DC,
> void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
> switch (T) {
> case Dtor_VectorDeleting:
> Out << "?_E";
> return;
> case Dtor_Deleting:
> Out << "?_G";
> return;
> case Dtor_Base:
> Out << "?1";
> return;
> case Dtor_Complete:
> Out << "?_D";
> return;
> }
> --
> Why do you think Complete is "?_D" and Base is "?1" ?
> I thought "?1" stands for the "usual" destructor (complete) whilst
> "?_D" stands for vbase destructor?
The term "vbase destructor" is somewhat confusing -- it is in fact most
similar to the complete destructor in the Itanium ABI. Please consider the
following TU:
------------------------------------------------------------------------
struct A {
~A();
};
struct B : virtual A {
~B();
};
void f(B *b) {
b->~B();
}
------------------------------------------------------------------------
cl.exe's function f will (indirectly) call '?_DB' which in turn calls
the base destructors '?1B' and '?1A'.
> See test below.
>
> test/CodeGenCXX/microsoft-abi-static-initializers.cpp
> @@ -11,7 +11,7 @@ struct S {
> // CHECK: ret void
>
> // CHECK: define internal void @"__dtor_\01?s@@3US@@A"() [[NUW]] {
> -// CHECK: call x86_thiscallcc void @"\01??1S@@QAE at XZ"
> +// CHECK: call x86_thiscallcc void @"\01??_DS@@QAE at XZ"
> // CHECK: ret void
> ---
> I believe this is wrong.
> CL does not emit a "??_DS@@" destructor on this file and it calls
> "??1S@@..." when "s" is to be destroyed.
This would appear to be an optimisation similar to the one we perform
when we know that the object we are generating a destructor for does not
have virtual bases, in which case we know that the base destructor can
be used directly (in Itanium we emit an alias, cl.exe just calls it).
> lib/AST/VTableBuilder.cpp
> @@ -1951,7 +1951,7 @@ void VTableBuilder::dumpLayout(raw_ostream& Out) {
> if (IsComplete)
> Out << "() [complete]";
> else if (isMicrosoftABI())
> - Out << "() [scalar deleting]";
> + Out << "() [vector deleting]";
> else
> Out << "() [deleting]";
> ---
> Hm, why do you think we should put a vector deleting dtor in the vftable?
This is an ABI requirement -- cl.exe will emit a call of the vector
deleting dtor in the vftable when it compiles a 'delete[]' of pointer
to class type.
> Please look at http://llvm.org/bugs/show_bug.cgi?id=15058#c4 - if we
> don't emit the deleting dtor definition, the linker error happens for
> a scalar deleting dtor.
That's probably because of the external weak alias from the vector
to the scalar deleting dtor.
> I think the "scalar" dtor deletes one object whilst the "vector" dtor
> deletes multiple objects (e.g. when used with "delete []").
Not necessarily. The scalar/vector deleting dtors decide their
behaviour based on the second parameter, an integer in which the
2 lowest bits are significant. The lower bit decides whether to
deallocate memory, and the higher bit decides whether to treat the
operand like an array allocated with 'new []'. The only difference
between the scalar and vector deleting dtors is that the vector dtor
assumes that the higher bit is never set, so it can omit the code to
perform array deletion.
> So I don't think we should change what's printed in the
> VTableBuilder::dumpLayout.
>
> The vector and scalar deleting dtors are usually aliases, right?
> Are you sure the scalar dtor is an alias for a vector dtor, not vice versa?
Yes. The alias is in place so that a translation unit can replace
the scalar deleting dtor for dynamic class type T with its vector
deleting dtor if it sees an allocation of the form 'new T[]'.
> lib/CodeGen/MicrosoftCXXABI.cpp
> @@ -328,17 +331,18 @@ RValue
> MicrosoftCXXABI::EmitVirtualDestructorCall(CodeGenFunction &CGF,
> ...
> ASTContext &Context = CGF.getContext();
> llvm::Value *ImplicitParam
> - = llvm::ConstantInt::get(llvm::IntegerType::getInt1Ty(CGF.getLLVMContext()),
> - DtorType == Dtor_Deleting);
> + = llvm::ConstantInt::get(llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
> + DtorType == Dtor_Deleting);
> ---
> Are you sure there's a 32-bit argument? I'm sure I put Int1/bool on
> purpose because I saw zero extension somewhere. Is there any case
> where something other than 0/1 is passed as an argument to
> scalar/vector deleting dtor?
Yes, a 'delete[]' emitted by cl.exe will pass an argument of 3.
> lib/CodeGen/CGCXX.cpp
> @@ -230,15 +239,17 @@ CodeGenModule::GetAddrOfCXXConstructor(const
> CXXConstructorDecl *ctor,
> }
>
> void CodeGenModule::EmitCXXDestructors(const CXXDestructorDecl *D) {
> ...
> + if (getTarget().getCXXABI().hasDestructorVariants()) {
> ---
> Hm, is there any C++ ABI which doesn't have destructor variants? :)
I should probably change this code. I guess it would in
principle be possible to design an ABI where the choice between
base/complete/deleting dtors is made based on a parameter.
> 2013/4/26 Peter Collingbourne <peter at pcc.me.uk>:
> > Hi Timur,
> >
> > Patch attached -- please review.
> >
> > The main missing part of this patch is vector deleting
> > destructors, which we don't need ourselves but may be
> > required by MSVC TUs.
> Do you have an example when we really need them?
>
> I think they are only needed for polymorphic "operator delete[]",
> which stackoverflow says to be an undefined behavior:
> http://stackoverflow.com/questions/6171814/why-is-it-undefined-behavior-to-delete-an-array-of-derived-objects-via-a-base
>
> If that's just "nice to have", I'd personally like to defer that until
> we have a justification to use them and real-world example of code
> using it. Otherwise we don't even know if we implement it correctly :)
I think we have to emit vector deleting dtors to maintain ABI compatibility
with cl.exe, as otherwise 'delete[]' in a cl.exe compiled TU may break. For
example, consider these two TUs:
Compiled with Clang:
------------------------------------------------------------------------
struct S {
virtual ~S();
};
void f(S *s);
int main(void) {
S *s = new S[2];
f(s);
}
------------------------------------------------------------------------
Compiled with cl.exe:
------------------------------------------------------------------------
struct S {
virtual ~S();
};
void f(S *s) {
delete[] s;
}
------------------------------------------------------------------------
As mentioned above, 'delete[] s' will call the deleting dtor in the
vftable. Unless we emit a vector deleting dtor as a result of seeing
'new S[2]' in our TU, the scalar deleting dtor will be called, and
only one instance of S will be deleted (and the wrong memory address
will be passed to 'operator delete[]()').
Thanks,
--
Peter
More information about the cfe-commits
mailing list