Skip to content

Commit f94bbfe

Browse files
authored
[Clang][CodeGen] Fix CanSkipVTablePointerInitialization for dynamic classes with a trivial anonymous union (#84651)
Hit this when trying upgrade an old project of mine. I couldn't find a corresponding existing issue for this when spelunking the open issues here on github. Thankfully I can work-around it today with the `[[clang::no_destroy]]` attribute for my use case. However it should still be properly fixed. ### Issue and History ### https://godbolt.org/z/EYnhce8MK for reference. All subsequent text below refers to the example in the godbolt above. Anonymous unions never have their destructor invoked automatically. Therefore we can skip vtable initialization of the destructor of a dynamic class if that destructor effectively does no work. This worked previously as the following check would be hit and return true for the trivial anonymous union, https://github.com/llvm/llvm-project/blob/release/18.x/clang/lib/CodeGen/CGClass.cpp#L1348, resulting in the code skipping vtable initialization. This was broken here 982bbf4 in relation to comments made on this review here https://reviews.llvm.org/D10508. ### Fixes ### The check the code is doing is correct however the return value is inverted. We want to return true here since a field with anonymous union never has its destructor invoked and thus effectively has a trivial destructor body from the perspective of requiring vtable init in the parent dynamic class. Also added some extra missing unit tests to test for this use case and a couple others.
1 parent bee3377 commit f94bbfe

File tree

2 files changed

+63
-1
lines changed

2 files changed

+63
-1
lines changed

clang/lib/CodeGen/CGClass.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ FieldHasTrivialDestructorBody(ASTContext &Context,
14041404

14051405
// The destructor for an implicit anonymous union member is never invoked.
14061406
if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
1407-
return false;
1407+
return true;
14081408

14091409
return HasTrivialDestructorBody(Context, FieldClassDecl, FieldClassDecl);
14101410
}

clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,3 +198,65 @@ struct C : virtual B {
198198
C::~C() {}
199199

200200
}
201+
202+
namespace Test10 {
203+
204+
// Check that we don't initialize the vtable pointer in A::~A(), since the class has an anonymous union which
205+
// never has its destructor invoked.
206+
struct A {
207+
virtual void f();
208+
~A();
209+
210+
union
211+
{
212+
int i;
213+
unsigned u;
214+
};
215+
};
216+
217+
// CHECK-LABEL: define{{.*}} void @_ZN6Test101AD2Ev
218+
// CHECK-NOT: store ptr getelementptr inbounds ({ [3 x ptr] }, ptr @_ZTVN6Test101AE, i32 0, inrange i32 0, i32 2), ptr
219+
A::~A() {
220+
}
221+
222+
}
223+
224+
namespace Test11 {
225+
226+
// Check that we don't initialize the vtable pointer in A::~A(), even if the base class has a non trivial destructor.
227+
struct Field {
228+
~Field();
229+
};
230+
231+
struct A : public Field {
232+
virtual void f();
233+
~A();
234+
};
235+
236+
// CHECK-LABEL: define{{.*}} void @_ZN6Test111AD2Ev
237+
// CHECK-NOT: store ptr getelementptr inbounds ({ [3 x ptr] }, ptr @_ZTVN6Test111AE, i32 0, inrange i32 0, i32 2), ptr
238+
A::~A() {
239+
}
240+
241+
}
242+
243+
namespace Test12 {
244+
245+
// Check that we don't initialize the vtable pointer in A::~A(), since the class has an anonymous struct with trivial fields.
246+
struct A {
247+
virtual void f();
248+
~A();
249+
250+
struct
251+
{
252+
int i;
253+
unsigned u;
254+
};
255+
};
256+
257+
// CHECK-LABEL: define{{.*}} void @_ZN6Test121AD2Ev
258+
// CHECK-NOT: store ptr getelementptr inbounds ({ [3 x ptr] }, ptr @_ZTVN6Test121AE, i32 0, inrange i32 0, i32 2), ptr
259+
A::~A() {
260+
}
261+
262+
}

0 commit comments

Comments
 (0)