Skip to content

Commit 75838d6

Browse files
author
Gabor Horvath
committed
[cxx-interop] Fix a crash with [[no_unique_address]]
Swift does not support storing fields in the padding of the previous fields just yet, so let's not import fields like that from C++. Represent them as opaque blobs instead. Fixes #80764
1 parent ff42f2e commit 75838d6

File tree

4 files changed

+52
-2
lines changed

4 files changed

+52
-2
lines changed

Diff for: lib/ClangImporter/ImportDecl.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "clang/AST/DeclCXX.h"
6262
#include "clang/AST/DeclObjCCommon.h"
6363
#include "clang/AST/PrettyPrinter.h"
64+
#include "clang/AST/RecordLayout.h"
6465
#include "clang/AST/Type.h"
6566
#include "clang/Basic/Specifiers.h"
6667
#include "clang/Basic/TargetInfo.h"
@@ -4341,6 +4342,18 @@ namespace {
43414342
}
43424343

43434344
Decl *VisitFieldDecl(const clang::FieldDecl *decl) {
4345+
if (decl->hasAttr<clang::NoUniqueAddressAttr>()) {
4346+
if (const auto *rd = decl->getType()->getAsRecordDecl()) {
4347+
// Clang can store the next field in the padding of this one. Swift
4348+
// does not support this yet so let's not import the field and
4349+
// represent it with an opaque blob in codegen.
4350+
const auto &fieldLayout =
4351+
decl->getASTContext().getASTRecordLayout(rd);
4352+
if (!decl->isZeroSize(decl->getASTContext()) &&
4353+
fieldLayout.getDataSize() != fieldLayout.getSize())
4354+
return nullptr;
4355+
}
4356+
}
43444357
// Fields are imported as variables.
43454358
std::optional<ImportedName> correctSwiftName;
43464359
ImportedName importedName;

Diff for: lib/IRGen/GenStruct.cpp

+14-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "GenStruct.h"
1818

19+
#include "IRGen.h"
1920
#include "swift/AST/ClangModuleLoader.h"
2021
#include "swift/AST/ConformanceLookup.h"
2122
#include "swift/AST/Decl.h"
@@ -42,6 +43,7 @@
4243
#include "llvm/ADT/STLExtras.h"
4344
#include "llvm/IR/DerivedTypes.h"
4445
#include "llvm/IR/Function.h"
46+
#include "llvm/Support/Error.h"
4547
#include <iterator>
4648

4749
#include "GenDecl.h"
@@ -1459,6 +1461,14 @@ class ClangRecordLowering {
14591461
unsigned fieldOffset = layout.getFieldOffset(clangField->getFieldIndex());
14601462
assert(!clangField->isBitField());
14611463
Size offset( SubobjectAdjustment.getValue() + fieldOffset / 8);
1464+
std::optional<Size> dataSize;
1465+
if (clangField->hasAttr<clang::NoUniqueAddressAttr>()) {
1466+
if (const auto *rd = clangField->getType()->getAsRecordDecl()) {
1467+
// Clang can store the next field in the padding of this one.
1468+
const auto &fieldLayout = ClangContext.getASTRecordLayout(rd);
1469+
dataSize = Size(fieldLayout.getDataSize().getQuantity());
1470+
}
1471+
}
14621472

14631473
// If we have a Swift import of this type, use our lowered information.
14641474
if (swiftField) {
@@ -1471,7 +1481,8 @@ class ClangRecordLowering {
14711481

14721482
// Otherwise, add it as an opaque blob.
14731483
auto fieldSize = isZeroSized ? clang::CharUnits::Zero() : ClangContext.getTypeSizeInChars(clangField->getType());
1474-
return addOpaqueField(offset, Size(fieldSize.getQuantity()));
1484+
return addOpaqueField(offset,
1485+
dataSize.value_or(Size(fieldSize.getQuantity())));
14751486
}
14761487

14771488
/// Add opaque storage for bitfields spanning the given range of bits.
@@ -1512,7 +1523,8 @@ class ClangRecordLowering {
15121523
}
15131524

15141525
/// Add information to track a value field at the current offset.
1515-
void addFieldInfo(VarDecl *swiftField, const FixedTypeInfo &fieldType, bool isZeroSized) {
1526+
void addFieldInfo(VarDecl *swiftField, const FixedTypeInfo &fieldType,
1527+
bool isZeroSized) {
15161528
bool isLoadableField = isa<LoadableTypeInfo>(fieldType);
15171529
unsigned explosionSize = 0;
15181530
if (isLoadableField)

Diff for: test/Interop/Cxx/class/Inputs/member-variables.h

+11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
22
#define TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
33

4+
#include <cstddef>
5+
#include <optional>
6+
47
class MyClass {
58
public:
69
const int const_member = 23;
@@ -24,6 +27,14 @@ struct HasZeroSizedField {
2427
void set_c(short c) { this->c = c; }
2528
};
2629

30+
struct ReuseFieldPadding {
31+
[[no_unique_address]] std::optional<int> a;
32+
char c;
33+
char get_c() const { return c; }
34+
void set_c(char c) { this->c = c; }
35+
int offset() const { return offsetof(ReuseFieldPadding, c); }
36+
};
37+
2738
inline int takesZeroSizedInCpp(HasZeroSizedField x) {
2839
return x.a;
2940
}

Diff for: test/Interop/Cxx/class/zero-sized-field.swift

+14
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,18 @@ FieldsTestSuite.test("Zero sized field") {
2424
expectEqual(s.b.getNum(), 42)
2525
}
2626

27+
FieldsTestSuite.test("Field padding reused") {
28+
var s = ReuseFieldPadding()
29+
s.c = 5
30+
expectEqual(Int(s.offset()), MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
31+
expectEqual(s.c, 5)
32+
expectEqual(s.get_c(), 5)
33+
s.set_c(6)
34+
expectEqual(s.c, 6)
35+
expectEqual(s.get_c(), 6)
36+
var s2 = s
37+
expectEqual(s2.c, 6)
38+
expectEqual(s2.get_c(), 6)
39+
}
40+
2741
runAllTests()

0 commit comments

Comments
 (0)