Skip to content

Commit d3e43bb

Browse files
committed
[cxx-interop] Fix IRGen for C++ types that use tail padding of their bases
In C++, a field of a derived class might be placed into the tail padding of a base class. Swift was not handling this case correctly, causing an asserts-disabled compiler to run out of RAM, and an asserts-enabled compiler to fail with an assertion. Fixes this IRGen assertion: ``` Assertion failed: (offset >= NextOffset && "adding fields out of order"), function addField, file GenStruct.cpp, line 1509. ``` rdar://138764929
1 parent 108ad6f commit d3e43bb

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

lib/IRGen/GenStruct.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1423,7 +1423,8 @@ class ClangRecordLowering {
14231423
auto sfi = swiftProperties.begin(), sfe = swiftProperties.end();
14241424
// When collecting fields from the base subobjects, we do not have corresponding swift
14251425
// stored properties.
1426-
if (decl != ClangDecl)
1426+
bool isBaseSubobject = decl != ClangDecl;
1427+
if (isBaseSubobject)
14271428
sfi = swiftProperties.end();
14281429

14291430
while (cfi != cfe) {
@@ -1475,10 +1476,14 @@ class ClangRecordLowering {
14751476

14761477
assert(sfi == sfe && "more Swift fields than there were Clang fields?");
14771478

1478-
Size objectTotalStride = Size(layout.getSize().getQuantity());
1479-
// We never take advantage of tail padding, because that would prevent
1480-
// us from passing the address of the object off to C, which is a pretty
1481-
// likely scenario for imported C types.
1479+
auto objectSize = isBaseSubobject ? layout.getDataSize() : layout.getSize();
1480+
Size objectTotalStride = Size(objectSize.getQuantity());
1481+
// Unless this is a base subobject of a C++ type, we do not take advantage
1482+
// of tail padding, because that would prevent us from passing the address
1483+
// of the object off to C, which is a pretty likely scenario for imported C
1484+
// types.
1485+
// In C++, fields of a derived class might get placed into tail padding of a
1486+
// base class, in which case we should not add extra padding here.
14821487
assert(NextOffset <= SubobjectAdjustment + objectTotalStride);
14831488
assert(SpareBits.size() <= SubobjectAdjustment.getValueInBits() +
14841489
objectTotalStride.getValueInBits());

test/Interop/Cxx/class/inheritance/Inputs/fields.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,17 @@ struct InheritFromStructsWithVirtualMethod: HasOneFieldWithVirtualMethod, HasTwo
118118
int d;
119119
virtual ~InheritFromStructsWithVirtualMethod() = default;
120120
};
121+
122+
// MARK: Types that pack their fields into tail padding of a base class.
123+
124+
struct BaseAlign8 {
125+
long long field8 = 123;
126+
}; // sizeof=8, dsize=8, align=8
127+
128+
struct DerivedHasTailPadding : public BaseAlign8 {
129+
int field4 = 456;
130+
}; // sizeof=16, dsize=12, align=8
131+
132+
struct DerivedUsesBaseTailPadding : public DerivedHasTailPadding {
133+
short field2 = 789;
134+
}; // sizeof=16, dsize=14, align=8

test/Interop/Cxx/class/inheritance/fields.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,11 @@ FieldsTestSuite.test("Structs with virtual methods") {
117117
expectEqual(derived.d, 42)
118118
}
119119

120+
FieldsTestSuite.test("Field in tail padding of base class") {
121+
let usesBaseTailPadding = DerivedUsesBaseTailPadding()
122+
expectEqual(usesBaseTailPadding.field2, 789)
123+
expectEqual(usesBaseTailPadding.field4, 456)
124+
expectEqual(usesBaseTailPadding.field8, 123)
125+
}
126+
120127
runAllTests()

0 commit comments

Comments
 (0)